diff --git a/loopd/daemon.go b/loopd/daemon.go index 3ac9c93c..51428ce5 100644 --- a/loopd/daemon.go +++ b/loopd/daemon.go @@ -700,6 +700,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error { Store: staticAddressLoopInStore, WalletKit: d.lnd.WalletKit, ChainNotifier: d.lnd.ChainNotifier, + TxOutChecker: loopin.NewLndTxOutChecker(d.lnd.Client), NotificationManager: notificationManager, ChainParams: d.lnd.ChainParams, Signer: d.lnd.Signer, diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index cf21fc5f..9be268d3 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2/schnorr/musig2" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/chain" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/loop" @@ -343,7 +344,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, // cancelSwapInvoice best-effort cancels the current swap invoice using a // detached timeout-limited context. func (f *FSM) cancelSwapInvoice() { - if f.loopIn.SwapInvoice == "" { + if f.loopIn.SwapHash == (lntypes.Hash{}) { return } @@ -359,6 +360,68 @@ func (f *FSM) cancelSwapInvoice() { } } +// handleInvoiceUpdate applies the monitor state's invoice-update semantics and +// reports whether the update produced a terminal event. +func (f *FSM) handleInvoiceUpdate(update lndclient.InvoiceUpdate) ( + fsm.EventType, bool) { + + switch update.State { + case invoices.ContractOpen: + return fsm.NoOp, false + + case invoices.ContractAccepted: + return fsm.NoOp, false + + case invoices.ContractSettled: + f.Debugf("received off-chain payment update %v", update.State) + return OnPaymentReceived, true + + case invoices.ContractCanceled: + // If the invoice was canceled we only log here since we still need + // to monitor until the htlc timed out. + log.Warnf("invoice for swap hash %v canceled", f.loopIn.SwapHash) + return fsm.NoOp, false + + default: + err := fmt.Errorf("unexpected invoice state %v for swap hash %v "+ + "canceled", update.State, f.loopIn.SwapHash) + return f.HandleError(err), true + } +} + +// originalDepositOutpointUnavailable checks the original selected deposit +// outpoints against the chain backend's UTXO view. +func (f *FSM) originalDepositOutpointUnavailable(ctx context.Context) ( + bool, error) { + + if f.cfg.TxOutChecker == nil { + return false, nil + } + + const includeMempool = true + for _, outpointStr := range f.loopIn.DepositOutpoints { + outpoint, err := wire.NewOutPointFromString(outpointStr) + if err != nil { + return false, fmt.Errorf("invalid deposit outpoint %q: %w", + outpointStr, err) + } + + txOut, err := f.cfg.TxOutChecker.GetTxOut( + ctx, *outpoint, includeMempool, + ) + if err != nil { + return false, fmt.Errorf("unable to get txout %v: %w", + outpoint, err) + } + + if txOut == nil { + return true, nil + } + } + + return false, nil +} + // SignHtlcTxAction is called if the htlc was initialized and the server // provided the necessary information to construct the htlc tx. We sign the htlc // tx and send the signatures to the server. @@ -367,6 +430,18 @@ func (f *FSM) SignHtlcTxAction(ctx context.Context, var err error + outpointUnavailable, err := f.originalDepositOutpointUnavailable(ctx) + if err != nil { + return f.HandleError(err) + } + if outpointUnavailable { + err = errors.New("original deposit outpoint no longer available") + f.Warnf("%v, canceling swap invoice", err) + f.cancelSwapInvoice() + + return f.HandleError(err) + } + f.loopIn.AddressParams, err = f.cfg.AddressManager.GetStaticAddressParameters(ctx) @@ -736,32 +811,22 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, return f.HandleError(err) - case update := <-invoiceUpdateChan: - switch update.State { - case invoices.ContractOpen: - case invoices.ContractAccepted: - case invoices.ContractSettled: - f.Debugf("received off-chain payment update "+ - "%v", update.State) - - return OnPaymentReceived - - case invoices.ContractCanceled: - // If the invoice was canceled we only log here - // since we still need to monitor until the htlc - // timed out. - log.Warnf("invoice for swap hash %v canceled", - f.loopIn.SwapHash) - - default: - err = fmt.Errorf("unexpected invoice state %v "+ - "for swap hash %v canceled", - update.State, f.loopIn.SwapHash) - - return f.HandleError(err) + case update, ok := <-invoiceUpdateChan: + if !ok { + invoiceUpdateChan = nil + continue + } + + if event, done := f.handleInvoiceUpdate(update); done { + return event + } + + case err, ok := <-invoiceErrChan: + if !ok { + invoiceErrChan = nil + continue } - case err = <-invoiceErrChan: f.Errorf("invoice subscription error: %v", err) case <-ctx.Done(): diff --git a/staticaddr/loopin/actions_test.go b/staticaddr/loopin/actions_test.go index ee935de8..d3d2b979 100644 --- a/staticaddr/loopin/actions_test.go +++ b/staticaddr/loopin/actions_test.go @@ -54,10 +54,10 @@ func TestMonitorInvoiceAndHtlcTxReRegistersOnConfErr(t *testing.T) { loopIn.SetState(MonitorInvoiceAndHtlcTx) // Seed the mock invoice store so LookupInvoice succeeds. - mockLnd.Invoices[swapHash] = &lndclient.Invoice{ + mockLnd.SetInvoice(&lndclient.Invoice{ Hash: swapHash, State: invoices.ContractOpen, - } + }) cfg := &Config{ AddressManager: &mockAddressManager{ @@ -270,6 +270,133 @@ func testValidateLoopInContract(_ int32, _ int32) error { return nil } +// TestOriginalDepositOutpointUnavailableRequiresMissingTxOut verifies that a +// present txout does not trigger the RBF cancellation path. +func TestOriginalDepositOutpointUnavailableRequiresMissingTxOut(t *testing.T) { + originalOutpoint := wire.OutPoint{ + Hash: chainhash.Hash{1}, + Index: 0, + } + + txOutChecker := &testTxOutChecker{ + txOut: &wire.TxOut{Value: 10_000}, + } + f := &FSM{ + cfg: &Config{ + TxOutChecker: txOutChecker, + }, + loopIn: &StaticAddressLoopIn{ + DepositOutpoints: []string{originalOutpoint.String()}, + }, + } + + unavailable, err := f.originalDepositOutpointUnavailable(t.Context()) + require.NoError(t, err) + require.False(t, unavailable) + require.Equal(t, []wire.OutPoint{originalOutpoint}, txOutChecker.outpoints) + require.Equal(t, []bool{true}, txOutChecker.includeMempool) +} + +// TestSignHtlcTxActionCancelsWhenOriginalOutpointUnavailable verifies that a +// pending loop-in is canceled before HTLC signing if GetTxOut with mempool +// awareness reports that one of the originally selected outpoints is gone. +func TestSignHtlcTxActionCancelsWhenOriginalOutpointUnavailable(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + + swapHash := lntypes.Hash{9, 8, 7} + originalOutpoint := wire.OutPoint{ + Hash: chainhash.Hash{1}, + Index: 0, + } + + loopIn := &StaticAddressLoopIn{ + SwapHash: swapHash, + DepositOutpoints: []string{originalOutpoint.String()}, + } + + txOutChecker := &testTxOutChecker{} + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: &script.Parameters{ + ProtocolVersion: version.ProtocolVersion_V0, + }, + }, + InvoicesClient: mockLnd.LndServices.Invoices, + TxOutChecker: txOutChecker, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + event := f.SignHtlcTxAction(ctx, nil) + require.Equal(t, fsm.OnError, event) + require.ErrorContains( + t, f.LastActionError, "original deposit outpoint no longer available", + ) + + select { + case hash := <-mockLnd.FailInvoiceChannel: + require.Equal(t, swapHash, hash) + case <-ctx.Done(): + t.Fatalf("invoice was not canceled: %v", ctx.Err()) + } + + require.Equal(t, []wire.OutPoint{originalOutpoint}, txOutChecker.outpoints) + require.Equal(t, []bool{true}, txOutChecker.includeMempool) +} + +// TestSignHtlcTxActionDoesNotCancelOnTxOutLookupError verifies that lookup +// failures are treated as errors, but do not cancel the invoice. The invoice is +// only canceled when GetTxOut explicitly returns nil for an original outpoint. +func TestSignHtlcTxActionDoesNotCancelOnTxOutLookupError(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + + swapHash := lntypes.Hash{9, 8, 6} + originalOutpoint := wire.OutPoint{ + Hash: chainhash.Hash{3}, + Index: 0, + } + + loopIn := &StaticAddressLoopIn{ + SwapHash: swapHash, + DepositOutpoints: []string{originalOutpoint.String()}, + } + + txOutChecker := &testTxOutChecker{ + err: errors.New("backend unavailable"), + } + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: &script.Parameters{ + ProtocolVersion: version.ProtocolVersion_V0, + }, + }, + InvoicesClient: mockLnd.LndServices.Invoices, + TxOutChecker: txOutChecker, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + event := f.SignHtlcTxAction(ctx, nil) + require.Equal(t, fsm.OnError, event) + require.ErrorContains( + t, f.LastActionError, "unable to get txout", + ) + + select { + case hash := <-mockLnd.FailInvoiceChannel: + t.Fatalf("invoice should not have been canceled: %x", hash) + default: + } +} + // TestInitHtlcActionCancelsInvoiceOnServerError verifies that an invoice // created before a server-side rejection is canceled immediately. func TestInitHtlcActionCancelsInvoiceOnServerError(t *testing.T) { @@ -546,6 +673,24 @@ func (r *recordingDepositManager) TransitionDeposits(_ context.Context, return r.err } +type testTxOutChecker struct { + txOut *wire.TxOut + err error + + outpoints []wire.OutPoint + includeMempool []bool +} + +// GetTxOut records lookup parameters and returns the configured result. +func (t *testTxOutChecker) GetTxOut(_ context.Context, + outpoint wire.OutPoint, includeMempool bool) (*wire.TxOut, error) { + + t.outpoints = append(t.outpoints, outpoint) + t.includeMempool = append(t.includeMempool, includeMempool) + + return t.txOut, t.err +} + // initHtlcTestServer lets InitHtlcAction tests inject a deterministic server // response without standing up the full gRPC client. type initHtlcTestServer struct { diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index f4c13309..e2c8c615 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -79,6 +79,10 @@ type Config struct { // blocks. ChainNotifier lndclient.ChainNotifierClient + // TxOutChecker checks whether selected deposit outpoints are still + // available before we sign an HTLC transaction for them. + TxOutChecker TxOutChecker + // Signer is the signer client that is used to sign transactions. Signer lndclient.SignerClient diff --git a/test/lightning_client_mock.go b/test/lightning_client_mock.go index 27f3fada..350fb59a 100644 --- a/test/lightning_client_mock.go +++ b/test/lightning_client_mock.go @@ -155,7 +155,9 @@ func (h *mockLightningClient) LookupInvoice(_ context.Context, return nil, fmt.Errorf("invoice: %x not found", hash) } - return inv, nil + invoiceCopy := *inv + + return &invoiceCopy, nil } // ListTransactions returns all known transactions of the backing lnd node. diff --git a/test/lnd_services_mock.go b/test/lnd_services_mock.go index 63aa34c2..a41a3dbc 100644 --- a/test/lnd_services_mock.go +++ b/test/lnd_services_mock.go @@ -225,6 +225,15 @@ func (s *LndMockServices) AddTx(tx *wire.MsgTx) { s.lock.Unlock() } +// SetInvoice stores a copy of the given invoice in the mock invoice store. +func (s *LndMockServices) SetInvoice(invoice *lndclient.Invoice) { + s.lock.Lock() + defer s.lock.Unlock() + + invoiceCopy := *invoice + s.Invoices[invoice.Hash] = &invoiceCopy +} + // IsDone checks whether all channels have been fully emptied. If not this may // indicate unexpected behaviour of the code under test. func (s *LndMockServices) IsDone() error {