Merge pull request #1059 from hieblmi/reregister

staticaddr: re-register on htlc tx conf error

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:

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
+}