Skip to content

Commit 880694c

Browse files
hieblmihieblmi
committed
staticaddr: harden and test signMusig2Tx
1 parent 078399d commit 880694c

File tree

2 files changed

+367
-0
lines changed

2 files changed

+367
-0
lines changed

‎staticaddr/withdraw/manager.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,24 @@ func (m *Manager) signMusig2Tx(ctx context.Context,
763763
// Create our digest.
764764
var sigHash [32]byte
765765

766+
if len(sigInfo) != len(depositsToIdx) {
767+
return nil, fmt.Errorf("unexpected number of partial " +
768+
"signatures from server")
769+
}
770+
771+
for txIndex, input := range tx.TxIn {
772+
outpoint := input.PreviousOutPoint.String()
773+
if i, ok := depositsToIdx[outpoint]; ok {
774+
if i != txIndex {
775+
return nil, fmt.Errorf("deposit index maps " +
776+
"wrong tx index")
777+
}
778+
continue
779+
}
780+
781+
return nil, fmt.Errorf("tx outpoint not in deposit index map")
782+
}
783+
766784
// We'll now add the nonce to our session and sign the tx.
767785
for deposit, sigAndNonce := range sigInfo {
768786
session, ok := sessions[deposit]

‎staticaddr/withdraw/manager_test.go‎

Lines changed: 349 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
package withdraw
22

33
import (
4+
"context"
45
"testing"
56

7+
"github.com/btcsuite/btcd/txscript"
8+
"github.com/btcsuite/btcd/wire"
9+
"github.com/lightninglabs/loop/swapserverrpc"
10+
"github.com/lightninglabs/loop/test"
11+
"github.com/lightningnetwork/lnd/input"
612
"github.com/stretchr/testify/require"
713
)
814

@@ -19,3 +25,346 @@ func TestNewManagerHeightValidation(t *testing.T) {
1925
require.NoError(t, err)
2026
require.NotNil(t, manager)
2127
}
28+
29+
// TestSignMusig2Tx_MissingSigningInfo tests that signMusig2Tx should error
30+
// when sigInfo is missing an entry for one of the deposits.
31+
//
32+
// This test documents expected behavior. The function should validate that
33+
// len(sigInfo) == len(sessions) and all sessions have corresponding sigInfo
34+
// entries before attempting to sign, returning an error if validation fails.
35+
func TestSignMusig2Tx_MissingSigningInfo(t *testing.T) {
36+
t.Parallel()
37+
38+
// Create a dummy transaction with two inputs.
39+
tx := wire.NewMsgTx(2)
40+
tx.AddTxIn(&wire.TxIn{
41+
PreviousOutPoint: wire.OutPoint{
42+
Hash: [32]byte{1},
43+
Index: 0,
44+
},
45+
})
46+
tx.AddTxIn(&wire.TxIn{
47+
PreviousOutPoint: wire.OutPoint{
48+
Hash: [32]byte{2},
49+
Index: 0,
50+
},
51+
})
52+
53+
// Add a dummy output with a simple pkScript.
54+
pkScript := []byte{
55+
0x51, 0x20, // OP_1 OP_PUSHBYTES_32
56+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
57+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
58+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
59+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
60+
}
61+
62+
tx.AddTxOut(&wire.TxOut{
63+
Value: 10000,
64+
PkScript: pkScript,
65+
})
66+
67+
// Create deposit keys for both inputs.
68+
deposit1Key := "0100000000000000000000000000000000000000000000000000000000000000:0"
69+
deposit2Key := "0200000000000000000000000000000000000000000000000000000000000000:0"
70+
71+
// Create sessions for both deposits.
72+
sessions := map[string]*input.MuSig2SessionInfo{
73+
deposit1Key: {
74+
SessionID: [32]byte{1},
75+
},
76+
deposit2Key: {
77+
SessionID: [32]byte{2},
78+
},
79+
}
80+
81+
// Create sigInfo with only one entry (missing deposit2).
82+
sigInfo := map[string]*swapserverrpc.ServerPsbtWithdrawSigningInfo{
83+
deposit1Key: {
84+
Nonce: make([]byte, 66),
85+
Sig: make([]byte, 64),
86+
},
87+
}
88+
89+
// Create depositsToIdx mapping both deposits.
90+
depositsToIdx := map[string]int{
91+
deposit1Key: 0,
92+
deposit2Key: 1,
93+
}
94+
95+
// Create prevOutFetcher.
96+
prevOuts := map[wire.OutPoint]*wire.TxOut{
97+
tx.TxIn[0].PreviousOutPoint: {
98+
Value: 5000,
99+
PkScript: pkScript,
100+
},
101+
tx.TxIn[1].PreviousOutPoint: {
102+
Value: 5000,
103+
PkScript: pkScript,
104+
},
105+
}
106+
prevOutFetcher := txscript.NewMultiPrevOutFetcher(prevOuts)
107+
108+
// Create a mock signer.
109+
lnd := test.NewMockLnd()
110+
signer := lnd.Signer
111+
112+
// Create a minimal manager.
113+
m := &Manager{
114+
cfg: &ManagerConfig{
115+
Signer: signer,
116+
},
117+
}
118+
119+
// Call signMusig2Tx - it should error because sigInfo is missing
120+
// an entry for deposit2.
121+
//
122+
// The function should validate that:
123+
// 1. len(sigInfo) == len(sessions) == len(tx.TxIn)
124+
// 2. All keys in sessions exist in sigInfo
125+
// 3. No partial signing is allowed
126+
//
127+
// This test verifies that the function errors when sigInfo is
128+
// incomplete, preventing a partially signed transaction.
129+
ctx := context.Background()
130+
_, err := m.signMusig2Tx(
131+
ctx, prevOutFetcher, signer, tx, sessions, sigInfo,
132+
depositsToIdx,
133+
)
134+
135+
// Expect an error. The function should validate that sigInfo has
136+
// entries for all sessions before attempting to sign.
137+
require.ErrorContains(t, err, "unexpected number of partial "+
138+
"signatures from server")
139+
}
140+
141+
// TestSignMusig2Tx_MismatchedIndex tests that signMusig2Tx should error when
142+
// sigInfo has all expected keys but one maps to the wrong index in depositsToIdx.
143+
//
144+
// This test documents expected behavior. The function should validate that
145+
// each deposit maps to a unique index in [0, len(tx.TxIn)-1] before signing,
146+
// returning an error if indices conflict or are out of bounds.
147+
func TestSignMusig2Tx_MismatchedIndex(t *testing.T) {
148+
t.Parallel()
149+
150+
// Create a dummy transaction with two inputs.
151+
tx := wire.NewMsgTx(2)
152+
tx.AddTxIn(&wire.TxIn{
153+
PreviousOutPoint: wire.OutPoint{
154+
Hash: [32]byte{1},
155+
Index: 0,
156+
},
157+
})
158+
tx.AddTxIn(&wire.TxIn{
159+
PreviousOutPoint: wire.OutPoint{
160+
Hash: [32]byte{2},
161+
Index: 0,
162+
},
163+
})
164+
165+
// Add a dummy output with a simple pkScript.
166+
pkScript := []byte{
167+
0x51, 0x20, // OP_1 OP_PUSHBYTES_32
168+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
169+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
170+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
171+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
172+
}
173+
174+
tx.AddTxOut(&wire.TxOut{
175+
Value: 10000,
176+
PkScript: pkScript,
177+
})
178+
179+
// Create deposit keys for both inputs.
180+
deposit1Key := "0000000000000000000000000000000000000000000000000000000000000001:0"
181+
deposit2Key := "0000000000000000000000000000000000000000000000000000000000000002:0"
182+
183+
// Create sessions for both deposits.
184+
sessions := map[string]*input.MuSig2SessionInfo{
185+
deposit1Key: {
186+
SessionID: [32]byte{1},
187+
},
188+
deposit2Key: {
189+
SessionID: [32]byte{2},
190+
},
191+
}
192+
193+
// Create sigInfo with both entries.
194+
sigInfo := map[string]*swapserverrpc.ServerPsbtWithdrawSigningInfo{
195+
deposit1Key: {
196+
Nonce: make([]byte, 66),
197+
Sig: make([]byte, 64),
198+
},
199+
deposit2Key: {
200+
Nonce: make([]byte, 66),
201+
Sig: make([]byte, 64),
202+
},
203+
}
204+
205+
// Create depositsToIdx with WRONG mapping for deposit2.
206+
// deposit2 should map to index 1, but we map it to 0.
207+
depositsToIdx := map[string]int{
208+
deposit1Key: 0,
209+
deposit2Key: 0, // Wrong! Should be 1.
210+
}
211+
212+
// Create prevOutFetcher.
213+
prevOuts := map[wire.OutPoint]*wire.TxOut{
214+
tx.TxIn[0].PreviousOutPoint: {
215+
Value: 5000,
216+
PkScript: pkScript,
217+
},
218+
tx.TxIn[1].PreviousOutPoint: {
219+
Value: 5000,
220+
PkScript: pkScript,
221+
},
222+
}
223+
prevOutFetcher := txscript.NewMultiPrevOutFetcher(prevOuts)
224+
225+
// Create a mock signer.
226+
lnd := test.NewMockLnd()
227+
signer := lnd.Signer
228+
229+
// Create a minimal manager.
230+
m := &Manager{
231+
cfg: &ManagerConfig{
232+
Signer: signer,
233+
},
234+
}
235+
236+
// Call signMusig2Tx - it should error because depositsToIdx has
237+
// a mismatched index for deposit2.
238+
//
239+
// The function should validate that:
240+
// 1. Each deposit in depositsToIdx maps to a unique index
241+
// 2. All indices from 0 to len(tx.TxIn)-1 are covered exactly once
242+
// 3. No index is used twice (which would cause signature overwrites)
243+
//
244+
// In this case, both deposit1 and deposit2 map to index 0, which
245+
// is invalid. The second deposit would overwrite the witness of
246+
// the first input, resulting in an invalid transaction.
247+
ctx := context.Background()
248+
_, err := m.signMusig2Tx(
249+
ctx, prevOutFetcher, signer, tx, sessions, sigInfo,
250+
depositsToIdx,
251+
)
252+
253+
// Expect an error. The function should validate index uniqueness.
254+
require.ErrorContains(t, err, "deposit index maps wrong tx index")
255+
}
256+
257+
// TestSignMusig2Tx_MissingOutpointInDepositMap tests that signMusig2Tx errors
258+
// when a transaction input's outpoint is not present in depositsToIdx map.
259+
//
260+
// This test validates that the function checks all transaction inputs have
261+
// corresponding entries in the depositsToIdx map before signing.
262+
func TestSignMusig2Tx_MissingOutpointInDepositMap(t *testing.T) {
263+
t.Parallel()
264+
265+
// Create a dummy transaction with two inputs.
266+
tx := wire.NewMsgTx(2)
267+
tx.AddTxIn(&wire.TxIn{
268+
PreviousOutPoint: wire.OutPoint{
269+
Hash: [32]byte{1},
270+
Index: 0,
271+
},
272+
})
273+
tx.AddTxIn(&wire.TxIn{
274+
PreviousOutPoint: wire.OutPoint{
275+
Hash: [32]byte{2},
276+
Index: 0,
277+
},
278+
})
279+
280+
// Add a dummy output with a simple pkScript.
281+
pkScript := []byte{
282+
0x51, 0x20, // OP_1 OP_PUSHBYTES_32
283+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
284+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
285+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
286+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
287+
}
288+
289+
tx.AddTxOut(&wire.TxOut{
290+
Value: 10000,
291+
PkScript: pkScript,
292+
})
293+
294+
// Create deposit keys for both inputs.
295+
deposit1Key := "0100000000000000000000000000000000000000000000000000000000000000:0"
296+
deposit2Key := "0200000000000000000000000000000000000000000000000000000000000000:0"
297+
298+
// Create sessions for both deposits.
299+
sessions := map[string]*input.MuSig2SessionInfo{
300+
deposit1Key: {
301+
SessionID: [32]byte{1},
302+
},
303+
deposit2Key: {
304+
SessionID: [32]byte{2},
305+
},
306+
}
307+
308+
// Create sigInfo with both entries.
309+
sigInfo := map[string]*swapserverrpc.ServerPsbtWithdrawSigningInfo{
310+
deposit1Key: {
311+
Nonce: make([]byte, 66),
312+
Sig: make([]byte, 64),
313+
},
314+
deposit2Key: {
315+
Nonce: make([]byte, 66),
316+
Sig: make([]byte, 64),
317+
},
318+
}
319+
320+
// Create a third deposit key that doesn't correspond to any tx input.
321+
deposit3Key := "0300000000000000000000000000000000000000000000000000000000000000:0"
322+
323+
// Create depositsToIdx with deposit1 at correct index, but deposit3
324+
// (which doesn't exist as a tx input) instead of deposit2.
325+
// This means when the function iterates through tx.TxIn, it will find
326+
// deposit1 in the map, but deposit2 (the actual second input) won't
327+
// be found.
328+
depositsToIdx := map[string]int{
329+
deposit1Key: 0,
330+
deposit3Key: 1, // Wrong key - this isn't a real tx input
331+
}
332+
333+
// Create prevOutFetcher.
334+
prevOuts := map[wire.OutPoint]*wire.TxOut{
335+
tx.TxIn[0].PreviousOutPoint: {
336+
Value: 5000,
337+
PkScript: pkScript,
338+
},
339+
tx.TxIn[1].PreviousOutPoint: {
340+
Value: 5000,
341+
PkScript: pkScript,
342+
},
343+
}
344+
prevOutFetcher := txscript.NewMultiPrevOutFetcher(prevOuts)
345+
346+
// Create a mock signer.
347+
lnd := test.NewMockLnd()
348+
signer := lnd.Signer
349+
350+
// Create a minimal manager.
351+
m := &Manager{
352+
cfg: &ManagerConfig{
353+
Signer: signer,
354+
},
355+
}
356+
357+
// Call signMusig2Tx - it should error because the second transaction
358+
// input's outpoint is not in depositsToIdx map.
359+
//
360+
// The function should validate that every transaction input has a
361+
// corresponding entry in depositsToIdx before attempting to sign.
362+
ctx := context.Background()
363+
_, err := m.signMusig2Tx(
364+
ctx, prevOutFetcher, signer, tx, sessions, sigInfo,
365+
depositsToIdx,
366+
)
367+
368+
// Expect an error indicating the missing outpoint.
369+
require.ErrorContains(t, err, "tx outpoint not in deposit index map")
370+
}

0 commit comments

Comments
 (0)