staticaddr/loopin: cancel orphan invoice when init fails early

If InitHtlcAction creates the private swap invoice but fails before the loop-in
is stored, the retry path otherwise leaves behind a live orphan invoice.

Cancel that invoice on the early error path with a detached, timeout-limited
context, and reuse the same helper when tearing down the monitor path. This
keeps failed initialization attempts from leaving invoices that no local swap
can complete.

Author: hieblmi

staticaddr/loopin/actions.go

@@ -36,6 +36,8 @@ const (
 	defaultConfTarget = 3
 
 	DefaultPaymentTimeoutSeconds = 60
+
+	defaultInvoiceCleanupTimeout = 5 * time.Second
 )
 
 var (
@@ -57,6 +59,24 @@ var (
 func (f *FSM) InitHtlcAction(ctx context.Context,
 	_ fsm.EventContext) fsm.EventType {
 
+	var event fsm.EventType
+	invoiceNeedsCleanup := false
+	defer func() {
+		// If we created the private invoice but failed before persisting the
+		// swap, cancel it so retries do not accumulate orphan invoices.
+		if !invoiceNeedsCleanup || event != fsm.OnError {
+			return
+		}
+
+		f.cancelSwapInvoice(ctx)
+	}()
+
+	returnError := func(err error) fsm.EventType {
+		event = f.HandleError(err)
+
+		return event
+	}
+
 	// Lock the deposits and transition them to the LoopingIn state.
 	err := f.cfg.DepositManager.TransitionDeposits(
 		ctx, f.loopIn.Deposits, deposit.OnLoopInInitiated,
@@ -65,7 +85,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 	if err != nil {
 		err = fmt.Errorf("unable to loop-in deposits: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 
 	// Calculate the swap invoice amount. The server needs to pay us the
@@ -88,7 +108,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		err = fmt.Errorf("unable to create random swap preimage: %w",
 			err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.loopIn.SwapPreimage = swapPreimage
 	f.loopIn.SwapHash = swapPreimage.Hash()
@@ -100,7 +120,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 	if err != nil {
 		err = fmt.Errorf("unable to derive client htlc key: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.loopIn.ClientPubkey = keyDesc.PubKey
 	f.loopIn.HtlcKeyLocator = keyDesc.KeyLocator
@@ -119,10 +139,14 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 	if err != nil {
 		err = fmt.Errorf("unable to create swap invoice: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.loopIn.SwapInvoice = swapInvoice
 
+	// From here until CreateLoopIn succeeds, any error path would otherwise
+	// leave behind a live invoice with no persisted swap to recover it.
+	invoiceNeedsCleanup = true
+
 	f.loopIn.ProtocolVersion = version.AddressProtocolVersion(
 		version.CurrentRPCProtocolVersion(),
 	)
@@ -149,7 +173,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		err = fmt.Errorf("unable to initiate the loop-in with the "+
 			"server: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 
 	// Pushing empty sigs signals the server that we abandoned the swap
@@ -171,7 +195,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		pushEmptySigs()
 		err = fmt.Errorf("unable to parse server pubkey: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.loopIn.ServerPubkey = serverPubkey
 
@@ -185,7 +209,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		err = fmt.Errorf("server response parameters are outside "+
 			"our allowed range: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 
 	f.loopIn.HtlcCltvExpiry = loopInResp.HtlcExpiry
@@ -194,23 +218,23 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		pushEmptySigs()
 		err = fmt.Errorf("unable to convert server nonces: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.htlcServerNoncesHighFee, err = toNonces(
 		loopInResp.HighFeeHtlcInfo.Nonces,
 	)
 	if err != nil {
 		pushEmptySigs()
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.htlcServerNoncesExtremelyHighFee, err = toNonces(
 		loopInResp.ExtremeFeeHtlcInfo.Nonces,
 	)
 	if err != nil {
 		pushEmptySigs()
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 
 	// We need to defend against the server setting high fees for the htlc
@@ -232,7 +256,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		log.Errorf("server htlc tx fee is higher than the configured "+
 			"allowed maximum: %v > %v", fee, maxHtlcTxFee)
 
-		return f.HandleError(ErrFeeTooHigh)
+		return returnError(ErrFeeTooHigh)
 	}
 	f.loopIn.HtlcTxFeeRate = feeRate
 
@@ -246,7 +270,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 			"configured allowed maximum: %v > %v", fee,
 			maxHtlcTxBackupFee)
 
-		return f.HandleError(ErrFeeTooHigh)
+		return returnError(ErrFeeTooHigh)
 	}
 	f.loopIn.HtlcTxHighFeeRate = highFeeRate
 
@@ -262,7 +286,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 			"configured allowed maximum: %v > %v", fee,
 			maxHtlcTxBackupFee)
 
-		return f.HandleError(ErrFeeTooHigh)
+		return returnError(ErrFeeTooHigh)
 	}
 	f.loopIn.HtlcTxExtremelyHighFeeRate = extremelyHighFeeRate
 
@@ -276,7 +300,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		err = fmt.Errorf("unable to derive htlc timeout sweep "+
 			"address: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 	f.loopIn.HtlcTimeoutSweepAddress = sweepAddress
 
@@ -286,10 +310,31 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		pushEmptySigs()
 		err = fmt.Errorf("unable to store loop-in in db: %w", err)
 
-		return f.HandleError(err)
+		return returnError(err)
 	}
 
-	return OnHtlcInitiated
+	// Once the swap is stored, restart/recovery code owns invoice lifecycle.
+	invoiceNeedsCleanup = false
+
+	event = OnHtlcInitiated
+
+	return event
+}
+
+// cancelSwapInvoice best-effort cancels the current swap invoice using a
+// detached timeout-limited context so cleanup still runs even if the caller's
+// context is already done.
+func (f *FSM) cancelSwapInvoice(ctx context.Context) {
+	cleanupCtx, cancel := context.WithTimeout(
+		context.WithoutCancel(ctx), defaultInvoiceCleanupTimeout,
+	)
+	defer cancel()
+
+	err := f.cfg.InvoicesClient.CancelInvoice(cleanupCtx, f.loopIn.SwapHash)
+	if err != nil {
+		f.Warnf("unable to cancel invoice for swap %v: %v",
+			f.loopIn.SwapHash, err)
+	}
 }
 
 // SignHtlcTxAction is called if the htlc was initialized and the server
@@ -557,11 +602,9 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context,
 		// Cancel the lndclient invoice subscription.
 		cancelInvoiceSubscription()
 
-		err = f.cfg.InvoicesClient.CancelInvoice(ctx, f.loopIn.SwapHash)
-		if err != nil {
-			f.Warnf("unable to cancel invoice "+
-				"for swap hash: %v", err)
-		}
+		// Reuse the same helper as InitHtlcAction so timeout cleanup follows
+		// the same detached-context path as early-init cleanup.
+		f.cancelSwapInvoice(ctx)
 	}
 
 	for {

staticaddr/loopin/actions_test.go

@@ -271,6 +271,124 @@ func testValidateLoopInContract(_ int32, _ int32) error {
 	return nil
 }
 
+// TestInitHtlcActionCancelsInvoiceOnServerError verifies that an invoice
+// created before a server-side rejection is canceled immediately.
+func TestInitHtlcActionCancelsInvoiceOnServerError(t *testing.T) {
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	defer cancel()
+
+	mockLnd := test.NewMockLnd()
+
+	loopIn := &StaticAddressLoopIn{
+		Deposits: []*deposit.Deposit{{
+			Value: 200_000,
+		}},
+		InitiationHeight:      uint32(mockLnd.Height),
+		InitiationTime:        time.Now(),
+		PaymentTimeoutSeconds: DefaultPaymentTimeoutSeconds,
+		ProtocolVersion:       version.ProtocolVersion_V0,
+	}
+
+	cfg := &Config{
+		AddressManager: &mockAddressManager{
+			params: &address.Parameters{
+				ProtocolVersion: version.ProtocolVersion_V0,
+			},
+		},
+		DepositManager: &noopDepositManager{},
+		WalletKit:      mockLnd.WalletKit,
+		LndClient:      mockLnd.Client,
+		InvoicesClient: mockLnd.LndServices.Invoices,
+		Server: &initHtlcTestServer{
+			loopInErr: errors.New("server rejected swap"),
+		},
+	}
+
+	f, err := NewFSM(ctx, loopIn, cfg, false)
+	require.NoError(t, err)
+
+	// The init step should fail and synchronously trigger deferred invoice
+	// cleanup.
+	event := f.InitHtlcAction(ctx, nil)
+	require.Equal(t, fsm.OnError, event)
+
+	select {
+	case hash := <-mockLnd.FailInvoiceChannel:
+		require.Equal(t, loopIn.SwapHash, hash)
+
+	case <-ctx.Done():
+		t.Fatalf("invoice was not canceled: %v", ctx.Err())
+	}
+}
+
+// TestInitHtlcActionCancelsInvoiceOnFeeGuardFailure verifies that the early
+// fee guard also cancels the pre-created invoice before returning an error.
+func TestInitHtlcActionCancelsInvoiceOnFeeGuardFailure(t *testing.T) {
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	defer cancel()
+
+	mockLnd := test.NewMockLnd()
+	serverKey, err := btcec.NewPrivateKey()
+	require.NoError(t, err)
+
+	loopIn := &StaticAddressLoopIn{
+		Deposits: []*deposit.Deposit{{
+			Value: 200_000,
+		}},
+		InitiationHeight:      uint32(mockLnd.Height),
+		InitiationTime:        time.Now(),
+		PaymentTimeoutSeconds: DefaultPaymentTimeoutSeconds,
+		ProtocolVersion:       version.ProtocolVersion_V0,
+	}
+
+	cfg := &Config{
+		AddressManager: &mockAddressManager{
+			params: &address.Parameters{
+				ProtocolVersion: version.ProtocolVersion_V0,
+			},
+		},
+		DepositManager: &noopDepositManager{},
+		WalletKit:      mockLnd.WalletKit,
+		LndClient:      mockLnd.Client,
+		InvoicesClient: mockLnd.LndServices.Invoices,
+		Server: &initHtlcTestServer{
+			loopInResp: &swapserverrpc.ServerStaticAddressLoopInResponse{
+				HtlcServerPubKey: serverKey.PubKey().
+					SerializeCompressed(),
+				HtlcExpiry: mockLnd.Height +
+					DefaultLoopInOnChainCltvDelta,
+				StandardHtlcInfo: &swapserverrpc.ServerHtlcSigningInfo{
+					FeeRate: 1_000_000,
+				},
+				HighFeeHtlcInfo: &swapserverrpc.ServerHtlcSigningInfo{},
+				ExtremeFeeHtlcInfo: &swapserverrpc.
+					ServerHtlcSigningInfo{},
+			},
+		},
+		ValidateLoopInContract: func(int32, int32) error {
+			return nil
+		},
+		MaxStaticAddrHtlcFeePercentage:       0,
+		MaxStaticAddrHtlcBackupFeePercentage: 1,
+	}
+
+	f, err := NewFSM(ctx, loopIn, cfg, false)
+	require.NoError(t, err)
+
+	// The fee guard runs before persistence, so the deferred cleanup must
+	// cancel the invoice on this error path as well.
+	event := f.InitHtlcAction(ctx, nil)
+	require.Equal(t, fsm.OnError, event)
+
+	select {
+	case hash := <-mockLnd.FailInvoiceChannel:
+		require.Equal(t, loopIn.SwapHash, hash)
+
+	case <-ctx.Done():
+		t.Fatalf("invoice was not canceled: %v", ctx.Err())
+	}
+}
+
 // mockAddressManager is a minimal AddressManager implementation used by the
 // test FSM setup.
 type mockAddressManager struct {
@@ -328,3 +446,29 @@ func (n *noopDepositManager) GetActiveDepositsInState(fsm.StateType) (
 
 	return nil, nil
 }
+
+// initHtlcTestServer lets InitHtlcAction tests inject a deterministic server
+// response without standing up the full gRPC client.
+type initHtlcTestServer struct {
+	swapserverrpc.StaticAddressServerClient
+
+	loopInResp *swapserverrpc.ServerStaticAddressLoopInResponse
+	loopInErr  error
+}
+
+// ServerStaticAddressLoopIn returns the canned response configured by the test.
+func (s *initHtlcTestServer) ServerStaticAddressLoopIn(context.Context,
+	*swapserverrpc.ServerStaticAddressLoopInRequest, ...grpc.CallOption,
+) (*swapserverrpc.ServerStaticAddressLoopInResponse, error) {
+
+	return s.loopInResp, s.loopInErr
+}
+
+// PushStaticAddressHtlcSigs accepts the abandonment signal used by error-path
+// tests without adding additional assertions.
+func (s *initHtlcTestServer) PushStaticAddressHtlcSigs(context.Context,
+	*swapserverrpc.PushStaticAddressHtlcSigsRequest, ...grpc.CallOption,
+) (*swapserverrpc.PushStaticAddressHtlcSigsResponse, error) {
+
+	return &swapserverrpc.PushStaticAddressHtlcSigsResponse{}, nil
+}