From 89f2dcff8b2293a8e8bee83f3164cbabee483fee Mon Sep 17 00:00:00 2001 From: Chanda Chewe <82529756+chandachewe10@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:57:28 +0000 Subject: [PATCH 1/2] loopin: fix dead error variable and silent invoice cancel in setStateAbandoned The err variable is always nil when the final fmt.Errorf is reached (a non-nil err exits earlier), producing a trailing ', ' in the error string visible to callers and in logs. Additionally, CancelInvoice errors were silently swallowed with a bare '_' assignment. The timeout path in the same file correctly checks for ErrInvoiceAlreadySettled; this commit makes the abandon path consistent: ignore already-settled invoices and log any other unexpected error so operators can diagnose issues without failing the abandon itself. Co-authored-by: Chanda Chewe --- loopin.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/loopin.go b/loopin.go index ae6802c8..06d63629 100644 --- a/loopin.go +++ b/loopin.go @@ -1174,13 +1174,17 @@ func (s *loopInSwap) setStateAbandoned(ctx context.Context) error { return err } - // If the invoice is already settled or canceled, this is a nop. - _ = s.lnd.Invoices.CancelInvoice(ctx, s.hash) + // Cancel the invoice so the server can no longer settle it. If the + // invoice is already settled we ignore the error, matching the + // behaviour of the timeout path. Any other unexpected error is logged + // but does not prevent the abandon from completing. + err = s.lnd.Invoices.CancelInvoice(ctx, s.hash) + if err != nil && err != invpkg.ErrInvoiceAlreadySettled { + s.log.Warnf("Failed to cancel invoice for abandoned swap: %v", + err) + } - return fmt.Errorf("swap hash "+ - "abandoned by client, "+ - "swap ID: %v, %v", - s.hash, err) + return fmt.Errorf("swap hash abandoned by client, swap ID: %v", s.hash) } // persistAndAnnounceState updates the swap state on disk and sends out an From 8716a517224e1aaf53d9d27d9f7358e9de980fc4 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Fri, 17 Jul 2026 15:36:02 -0500 Subject: [PATCH 2/2] loopin: classify already-settled invoice RPC errors lnd returns ErrInvoiceAlreadySettled from its RPC handler as an uncoded gRPC error. On the client it is reconstructed as an Unknown status, so comparing it directly with the Go sentinel never matches. This made the abandon path log a warning for an expected condition. The older timeout path had the same comparison and could return the RPC error instead of finishing normal timeout processing. Add a shared classifier that accepts the local sentinel and the exact gRPC status representation, and use it in both cancellation paths. Add coverage for the classifier and timeout handling. --- loopin.go | 25 ++++++++- loopin_test.go | 138 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 2 deletions(-) diff --git a/loopin.go b/loopin.go index 06d63629..c047aefc 100644 --- a/loopin.go +++ b/loopin.go @@ -27,6 +27,8 @@ import ( "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var ( @@ -58,6 +60,25 @@ var ( ErrSwapFinalized = errors.New("swap is in a final state") ) +// isInvoiceAlreadySettledError reports whether err indicates that an invoice +// cancellation failed because the invoice was already settled. If lnd returns +// the sentinel from an RPC handler, gRPC transports it as an Unknown status +// with the sentinel's error text. +func isInvoiceAlreadySettledError(err error) bool { + if err == nil { + return false + } + + if errors.Is(err, invpkg.ErrInvoiceAlreadySettled) { + return true + } + + rpcStatus, ok := status.FromError(err) + return ok && + rpcStatus.Code() == codes.Unknown && + rpcStatus.Message() == invpkg.ErrInvoiceAlreadySettled.Error() +} + // loopInSwap contains all the in-memory state related to a pending loop in // swap. type loopInSwap struct { @@ -1087,7 +1108,7 @@ func (s *loopInSwap) processHtlcSpend(ctx context.Context, // already settled. This means that the server didn't succeed in // sweeping the htlc after paying the invoice. err := s.lnd.Invoices.CancelInvoice(ctx, s.hash) - if err != nil && err != invpkg.ErrInvoiceAlreadySettled { + if err != nil && !isInvoiceAlreadySettledError(err) { return err } } @@ -1179,7 +1200,7 @@ func (s *loopInSwap) setStateAbandoned(ctx context.Context) error { // behaviour of the timeout path. Any other unexpected error is logged // but does not prevent the abandon from completing. err = s.lnd.Invoices.CancelInvoice(ctx, s.hash) - if err != nil && err != invpkg.ErrInvoiceAlreadySettled { + if err != nil && !isInvoiceAlreadySettledError(err) { s.log.Warnf("Failed to cancel invoice for abandoned swap: %v", err) } diff --git a/loopin_test.go b/loopin_test.go index aa9fe9a0..93d019da 100644 --- a/loopin_test.go +++ b/loopin_test.go @@ -20,6 +20,8 @@ import ( "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/zpay32" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var ( @@ -44,6 +46,142 @@ type probeInvoicesMock struct { cancelCtxErr chan error } +// cancelErrorInvoicesMock is an InvoicesClient that returns a configured +// cancellation error. +type cancelErrorInvoicesMock struct { + lndclient.InvoicesClient + + err error +} + +// CancelInvoice returns the cancellation error configured on the mock. +func (c *cancelErrorInvoicesMock) CancelInvoice(context.Context, + lntypes.Hash) error { + + return c.err +} + +// TestIsInvoiceAlreadySettledError verifies the local and gRPC error forms +// recognized by the already-settled classifier. +func TestIsInvoiceAlreadySettledError(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + err error + expected bool + }{ + { + name: "sentinel", + err: invpkg.ErrInvoiceAlreadySettled, + expected: true, + }, + { + name: "wrapped sentinel", + err: fmt.Errorf( + "cancel invoice: %w", + invpkg.ErrInvoiceAlreadySettled, + ), + expected: true, + }, + { + name: "grpc representation", + err: status.Error( + codes.Unknown, + invpkg.ErrInvoiceAlreadySettled.Error(), + ), + expected: true, + }, + { + name: "different grpc status", + err: status.Error( + codes.FailedPrecondition, + invpkg.ErrInvoiceAlreadySettled.Error(), + ), + }, + { + name: "different error", + err: fmt.Errorf("cancel invoice failed"), + }, + { + name: "nil", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal( + t, testCase.expected, + isInvoiceAlreadySettledError(testCase.err), + ) + }) + } +} + +// TestProcessHtlcSpendIgnoresGRPCAlreadySettled verifies that the timeout path +// completes normally when invoice cancellation reports an already-settled +// invoice. +func TestProcessHtlcSpendIgnoresGRPCAlreadySettled(t *testing.T) { + defer test.Guard(t)() + + // Initialize a loop-in so the test uses the same contract and HTLC + // scripts as the production flow. + testCtx := newLoopInTestContext(t) + cfg := newSwapConfig( + &testCtx.lnd.LndServices, testCtx.store, testCtx.server, nil, + clock.NewTestClock(time.Unix(123, 0)), + ) + + initResult, err := newLoopInSwap( + context.Background(), cfg, 600, &testLoopInRequest, + ) + require.NoError(t, err) + testCtx.store.AssertLoopInStored() + + // Return the gRPC status observed when lnd transports its + // ErrInvoiceAlreadySettled sentinel across the RPC boundary. + cfg.lnd.Invoices = &cancelErrorInvoicesMock{ + err: status.Error( + codes.Unknown, + invpkg.ErrInvoiceAlreadySettled.Error(), + ), + } + + // Confirmation processing normally selects the HTLC version that was + // found on chain. Select the initialized version explicitly because this + // test calls processHtlcSpend directly. + if initResult.swap.htlcP2TR != nil { + initResult.swap.htlc = initResult.swap.htlcP2TR + } else { + initResult.swap.htlc = initResult.swap.htlcP2WSH + } + require.NotNil(t, initResult.swap.htlc) + + // Construct a timeout spend so processHtlcSpend takes the invoice + // cancellation branch. + timeoutWitness, err := initResult.swap.htlc.GenTimeoutWitness([]byte{1}) + require.NoError(t, err) + + timeoutTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + Witness: timeoutWitness, + }, + }, + } + + // The already-settled status must be suppressed while the swap still + // transitions to its terminal timeout state. + err = initResult.swap.processHtlcSpend( + context.Background(), &chainntnfs.SpendDetail{ + SpendingTx: timeoutTx, + SpenderInputIndex: 0, + }, 100, + ) + require.NoError(t, err) + require.Equal(t, loopdb.StateFailTimeout, initResult.swap.state) +} + // SubscribeSingleInvoice returns the mock's preconfigured channels. func (p *probeInvoicesMock) SubscribeSingleInvoice(_ context.Context, _ lntypes.Hash) (<-chan lndclient.InvoiceUpdate, <-chan error, error) {