Skip to content

Commit 4f94125

Browse files
committed
staticaddr: guard channel open and withdraw against unconfirmed deposits
Now that Deposited includes mempool outputs, channel opens and withdrawals must explicitly reject unconfirmed deposits (ConfirmationHeight <= 0) since both operations require confirmed inputs.
1 parent c6dc765 commit 4f94125

5 files changed

Lines changed: 166 additions & 8 deletions

File tree

loopd/swapclient_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1754,7 +1754,7 @@ func (s *swapClientServer) WithdrawDeposits(ctx context.Context,
17541754
return nil, err
17551755
}
17561756

1757-
for _, d := range deposits {
1757+
for _, d := range confirmedDeposits(deposits) {
17581758
outpoints = append(outpoints, d.OutPoint)
17591759
}
17601760

loopd/swapclient_server_deposit_test.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package loopd
22

3-
import "testing"
3+
import (
4+
"testing"
5+
6+
"github.com/lightninglabs/loop/staticaddr/deposit"
7+
)
48

59
// TestDepositBlocksUntilExpiry checks blocks-until-expiry handling for
610
// confirmed and unconfirmed deposits.
@@ -19,3 +23,24 @@ func TestDepositBlocksUntilExpiry(t *testing.T) {
1923
}
2024
})
2125
}
26+
27+
// TestConfirmedDeposits checks that helpers for bulk operations only keep
28+
// deposits that can actually be spent on-chain.
29+
func TestConfirmedDeposits(t *testing.T) {
30+
t.Run("filters unconfirmed", func(t *testing.T) {
31+
deposits := []*deposit.Deposit{
32+
{},
33+
{
34+
ConfirmationHeight: 123,
35+
},
36+
}
37+
38+
filtered := confirmedDeposits(deposits)
39+
if len(filtered) != 1 {
40+
t.Fatalf("expected 1 confirmed deposit, got %d", len(filtered))
41+
}
42+
if filtered[0].ConfirmationHeight != 123 {
43+
t.Fatal("expected confirmed deposit to remain")
44+
}
45+
})
46+
}

staticaddr/openchannel/manager.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,10 @@ func (m *Manager) OpenChannel(ctx context.Context,
310310
return nil, err
311311
}
312312

313+
// Automatic channel funding must ignore mempool deposits because
314+
// they cannot yet be used as funding inputs.
315+
deposits = filterConfirmedDeposits(deposits)
316+
313317
if req.LocalFundingAmount != 0 {
314318
deposits, err = staticutil.SelectDeposits(
315319
deposits, req.LocalFundingAmount,
@@ -325,6 +329,14 @@ func (m *Manager) OpenChannel(ctx context.Context,
325329
}
326330
}
327331

332+
for _, d := range deposits {
333+
// Deposited now includes mempool outputs for static loop-ins, but
334+
// channel opens still require the deposit input to be confirmed.
335+
if d.ConfirmationHeight <= 0 {
336+
return nil, ErrOpeningChannelUnavailableDeposits
337+
}
338+
}
339+
328340
// Pre-check: calculate the channel funding amount and the optional
329341
// change before locking deposits. This ensures the selected deposits
330342
// can cover the funding amount plus fees.
@@ -399,6 +411,22 @@ func (m *Manager) OpenChannel(ctx context.Context,
399411
return nil, err
400412
}
401413

414+
// filterConfirmedDeposits filters the given deposits and returns only those
415+
// that have a positive confirmation height, i.e. deposits that have been
416+
// confirmed on-chain.
417+
func filterConfirmedDeposits(deposits []*deposit.Deposit) []*deposit.Deposit {
418+
confirmed := make([]*deposit.Deposit, 0, len(deposits))
419+
for _, d := range deposits {
420+
if d.ConfirmationHeight <= 0 {
421+
continue
422+
}
423+
424+
confirmed = append(confirmed, d)
425+
}
426+
427+
return confirmed
428+
}
429+
402430
// openChannelPsbt starts an interactive channel open protocol that uses a
403431
// partially signed bitcoin transaction (PSBT) to fund the channel output. The
404432
// protocol involves several steps between the loop client and the server:

staticaddr/openchannel/manager_test.go

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type transitionCall struct {
2929
}
3030

3131
type mockDepositManager struct {
32+
activeDeposits []*deposit.Deposit
3233
openingDeposits []*deposit.Deposit
3334
getErr error
3435
transitionErrs map[fsm.EventType]error
@@ -44,15 +45,19 @@ func (m *mockDepositManager) AllOutpointsActiveDeposits([]wire.OutPoint,
4445
func (m *mockDepositManager) GetActiveDepositsInState(stateFilter fsm.StateType) (
4546
[]*deposit.Deposit, error) {
4647

47-
if stateFilter != deposit.OpeningChannel {
48-
return nil, nil
49-
}
48+
switch stateFilter {
49+
case deposit.Deposited:
50+
return m.activeDeposits, nil
51+
52+
case deposit.OpeningChannel:
53+
if m.getErr != nil {
54+
return nil, m.getErr
55+
}
5056

51-
if m.getErr != nil {
52-
return nil, m.getErr
57+
return m.openingDeposits, nil
5358
}
5459

55-
return m.openingDeposits, nil
60+
return nil, nil
5661
}
5762

5863
func (m *mockDepositManager) TransitionDeposits(_ context.Context,
@@ -464,6 +469,97 @@ func TestOpenChannelDuplicateOutpoints(t *testing.T) {
464469
require.ErrorContains(t, err, "duplicate outpoint")
465470
}
466471

472+
// TestOpenChannelSkipsUnconfirmedAutoSelection verifies that automatic coin
473+
// selection ignores mempool deposits and keeps using confirmed ones.
474+
func TestOpenChannelSkipsUnconfirmedAutoSelection(t *testing.T) {
475+
t.Parallel()
476+
477+
confirmedA := &deposit.Deposit{
478+
OutPoint: testOutPoint(1),
479+
Value: 160_000,
480+
ConfirmationHeight: 10,
481+
}
482+
confirmedB := &deposit.Deposit{
483+
OutPoint: testOutPoint(2),
484+
Value: 140_000,
485+
ConfirmationHeight: 11,
486+
}
487+
unconfirmed := &deposit.Deposit{
488+
OutPoint: testOutPoint(3),
489+
Value: 500_000,
490+
}
491+
492+
depositManager := &mockDepositManager{
493+
activeDeposits: []*deposit.Deposit{
494+
unconfirmed, confirmedA, confirmedB,
495+
},
496+
transitionErrs: map[fsm.EventType]error{
497+
deposit.OnOpeningChannel: errors.New("stop after selection"),
498+
},
499+
}
500+
manager := &Manager{
501+
cfg: &Config{
502+
DepositManager: depositManager,
503+
},
504+
}
505+
506+
req := &lnrpc.OpenChannelRequest{
507+
NodePubkey: make([]byte, 33),
508+
LocalFundingAmount: 100_000,
509+
SatPerVbyte: 10,
510+
}
511+
512+
_, err := manager.OpenChannel(context.Background(), req)
513+
require.ErrorContains(t, err, "stop after selection")
514+
require.Len(t, depositManager.calls, 1)
515+
require.Equal(t, deposit.OnOpeningChannel, depositManager.calls[0].event)
516+
require.NotContains(t, depositManager.calls[0].outpoints, unconfirmed.OutPoint)
517+
}
518+
519+
// TestOpenChannelFundMaxSkipsUnconfirmed verifies that fundmax only locks
520+
// confirmed deposits.
521+
func TestOpenChannelFundMaxSkipsUnconfirmed(t *testing.T) {
522+
t.Parallel()
523+
524+
confirmed := &deposit.Deposit{
525+
OutPoint: testOutPoint(1),
526+
Value: 200_000,
527+
ConfirmationHeight: 10,
528+
}
529+
unconfirmed := &deposit.Deposit{
530+
OutPoint: testOutPoint(2),
531+
Value: 300_000,
532+
}
533+
534+
depositManager := &mockDepositManager{
535+
activeDeposits: []*deposit.Deposit{
536+
unconfirmed, confirmed,
537+
},
538+
transitionErrs: map[fsm.EventType]error{
539+
deposit.OnOpeningChannel: errors.New("stop after selection"),
540+
},
541+
}
542+
manager := &Manager{
543+
cfg: &Config{
544+
DepositManager: depositManager,
545+
},
546+
}
547+
548+
req := &lnrpc.OpenChannelRequest{
549+
NodePubkey: make([]byte, 33),
550+
FundMax: true,
551+
SatPerVbyte: 10,
552+
}
553+
554+
_, err := manager.OpenChannel(context.Background(), req)
555+
require.ErrorContains(t, err, "stop after selection")
556+
require.Len(t, depositManager.calls, 1)
557+
require.Equal(
558+
t, []wire.OutPoint{confirmed.OutPoint},
559+
depositManager.calls[0].outpoints,
560+
)
561+
}
562+
467563
// TestValidateInitialPsbtFlags verifies that request fields incompatible with
468564
// PSBT funding are rejected early, before any deposits are locked.
469565
func TestValidateInitialPsbtFlags(t *testing.T) {

staticaddr/withdraw/manager.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,15 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
381381
}
382382
}
383383

384+
for _, d := range deposits {
385+
// Deposited now includes mempool outputs for static loop-ins, but
386+
// withdrawals still require the deposit input to be confirmed.
387+
if d.ConfirmationHeight <= 0 {
388+
return "", "", fmt.Errorf("can't withdraw, " +
389+
"unconfirmed deposits can't be withdrawn")
390+
}
391+
}
392+
384393
var (
385394
withdrawalAddress btcutil.Address
386395
err error

0 commit comments

Comments
 (0)