diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index 9286485b1..837a6eb87 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -572,7 +572,21 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, htlcConfirmed = true case err = <-htlcErrConfChan: - f.Errorf("htlc tx conf chan error: %v", err) + f.Errorf("htlc tx conf chan error, re-registering: "+ + "%v", err) + + // A previous confirmation may no longer be valid if the + // subscription failed, so reset and wait for a fresh one. + htlcConfirmed = false + + // Re-register for htlc confirmation. + htlcConfChan, htlcErrConfChan, err = registerHtlcConf() + if err != nil { + err = fmt.Errorf("unable to re-register for "+ + "htlc tx confirmation: %w", err) + + return f.HandleError(err) + } case <-reorgChan: // A reorg happened. We invalidate a previous htlc @@ -582,8 +596,10 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, htlcConfChan, htlcErrConfChan, err = registerHtlcConf() if err != nil { - f.Errorf("unable to monitor htlc tx "+ + err = fmt.Errorf("unable to monitor htlc tx "+ "confirmation: %v", err) + + return f.HandleError(err) } case <-deadlineChan: diff --git a/staticaddr/loopin/actions_test.go b/staticaddr/loopin/actions_test.go new file mode 100644 index 000000000..646e9898b --- /dev/null +++ b/staticaddr/loopin/actions_test.go @@ -0,0 +1,182 @@ +package loopin + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/lightninglabs/lndclient" + "github.com/lightninglabs/loop/fsm" + "github.com/lightninglabs/loop/staticaddr/address" + "github.com/lightninglabs/loop/staticaddr/deposit" + "github.com/lightninglabs/loop/staticaddr/script" + "github.com/lightninglabs/loop/staticaddr/version" + "github.com/lightninglabs/loop/test" + "github.com/lightningnetwork/lnd/invoices" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/stretchr/testify/require" +) + +// TestMonitorInvoiceAndHtlcTxReRegistersOnConfErr ensures that an error from +// the HTLC confirmation subscription triggers a re-registration. Without the +// regression fix, only the initial registration would be performed and the +// test would time out waiting for the second one. +func TestMonitorInvoiceAndHtlcTxReRegistersOnConfErr(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + + clientKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + serverKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + swapHash := lntypes.Hash{1, 2, 3} + + loopIn := &StaticAddressLoopIn{ + SwapHash: swapHash, + HtlcCltvExpiry: 2_000, + InitiationHeight: uint32(mockLnd.Height), + InitiationTime: time.Now(), + ProtocolVersion: version.ProtocolVersion_V0, + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + PaymentTimeoutSeconds: 3_600, + } + loopIn.SetState(MonitorInvoiceAndHtlcTx) + + // Seed the mock invoice store so LookupInvoice succeeds. + mockLnd.Invoices[swapHash] = &lndclient.Invoice{ + Hash: swapHash, + State: invoices.ContractOpen, + } + + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: &address.Parameters{ + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + ProtocolVersion: version.ProtocolVersion_V0, + }, + }, + ChainNotifier: mockLnd.ChainNotifier, + DepositManager: &noopDepositManager{}, + InvoicesClient: mockLnd.LndServices.Invoices, + LndClient: mockLnd.Client, + ChainParams: mockLnd.ChainParams, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + resultChan := make(chan fsm.EventType, 1) + go func() { + resultChan <- f.MonitorInvoiceAndHtlcTxAction(ctx, nil) + }() + + // Capture the invoice subscription the action registers so we can feed + // an update later and let the action exit. + var invSub *test.SingleInvoiceSubscription + select { + case invSub = <-mockLnd.SingleInvoiceSubcribeChannel: + case <-ctx.Done(): + t.Fatalf("invoice subscription not registered: %v", ctx.Err()) + } + + // The first confirmation registration should happen immediately. + var firstReg *test.ConfRegistration + select { + case firstReg = <-mockLnd.RegisterConfChannel: + case <-ctx.Done(): + t.Fatalf("htlc conf registration not received: %v", ctx.Err()) + } + + // Force the confirmation stream to error so the FSM re-registers. + firstReg.ErrChan <- errors.New("test htlc conf error") + + // FSM registers again, otherwise it would time out. + var secondReg *test.ConfRegistration + select { + case secondReg = <-mockLnd.RegisterConfChannel: + case <-ctx.Done(): + t.Fatalf("htlc conf was not re-registered: %v", ctx.Err()) + } + + require.NotEqual(t, firstReg, secondReg) + + // Settle the invoice to let the action exit. + invSub.Update <- lndclient.InvoiceUpdate{ + Invoice: lndclient.Invoice{ + Hash: swapHash, + State: invoices.ContractSettled, + }, + } + + select { + case event := <-resultChan: + require.Equal(t, OnPaymentReceived, event) + case <-ctx.Done(): + t.Fatalf("fsm did not return: %v", ctx.Err()) + } +} + +// mockAddressManager is a minimal AddressManager implementation used by the +// test FSM setup. +type mockAddressManager struct { + params *address.Parameters +} + +// GetStaticAddressParameters returns the configured address parameters. +func (m *mockAddressManager) GetStaticAddressParameters(_ context.Context) ( + *address.Parameters, error) { + + return m.params, nil +} + +// GetStaticAddress is unused for this test and returns nil. +func (m *mockAddressManager) GetStaticAddress(_ context.Context) ( + *script.StaticAddress, error) { + + return nil, nil +} + +// noopDepositManager is a stub DepositManager used to satisfy FSM config. +type noopDepositManager struct{} + +// GetAllDeposits implements DepositManager with a no-op. +func (n *noopDepositManager) GetAllDeposits(_ context.Context) ( + []*deposit.Deposit, error) { + + return nil, nil +} + +// AllStringOutpointsActiveDeposits implements DepositManager with a no-op. +func (n *noopDepositManager) AllStringOutpointsActiveDeposits( + _ []string, _ fsm.StateType) ([]*deposit.Deposit, bool) { + + return nil, false +} + +// TransitionDeposits implements DepositManager with a no-op. +func (n *noopDepositManager) TransitionDeposits(context.Context, + []*deposit.Deposit, fsm.EventType, fsm.StateType) error { + + return nil +} + +// DepositsForOutpoints implements DepositManager with a no-op. +func (n *noopDepositManager) DepositsForOutpoints(context.Context, []string, + bool) ([]*deposit.Deposit, error) { + + return nil, nil +} + +// GetActiveDepositsInState implements DepositManager with a no-op. +func (n *noopDepositManager) GetActiveDepositsInState(fsm.StateType) ( + []*deposit.Deposit, error) { + + return nil, nil +}