From eb53d49195d47702b9b69e6e93aba208fe7e5880 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 30 Oct 2026 12:23:07 +0200 Subject: [PATCH 1/4] staticaddr/loopin: sweep the htlc tx that spent the deposits The client co-signs a standard, a high and an extremely high fee variant of the static loop-in htlc tx, and the server may publish any of them. The timeout sweep always rebuilt the standard variant, so after a backup variant confirmed it spent an output that doesn't exist. Only the standard fee rate is persisted, so the variant can't be rebuilt. Take the htlc tx from the chain instead: wait for the spend of the first deposit, accept the tx only if it spends exactly the swap's deposits and pays the htlc script once, and sweep that output with its actual value. Inputs are matched as a set, since recovery can reorder deposits. The lookup runs before the fee estimate and is bounded by the new HtlcTxLookupTimeout (30 minutes by default). lnd reports missing inputs and mempool conflicts as an already spent output, which the sweep counted as published without a txid. Now it only does so if a confirmed tx spends the htlc output and pays the sweep address, and it monitors that tx. --- staticaddr/loopin/actions.go | 151 +++- staticaddr/loopin/htlc_timeout_sweep_test.go | 795 +++++++++++++++++++ staticaddr/loopin/loopin.go | 132 ++- staticaddr/loopin/loopin_test.go | 3 +- staticaddr/loopin/manager.go | 7 + 5 files changed, 1042 insertions(+), 46 deletions(-) create mode 100644 staticaddr/loopin/htlc_timeout_sweep_test.go diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index 09aead3fb..323660edb 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -1518,10 +1518,18 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, } } -// htlcTimeoutSweepRetryDelay is the delay between retries when publishing the -// htlc timeout sweep transaction fails. +// htlcTimeoutSweepRetryDelay is the delay between retries when looking up the +// htlc tx or creating or publishing the htlc timeout sweep transaction fails. const htlcTimeoutSweepRetryDelay = time.Hour +// defaultHtlcTxLookupTimeout is the default maximum time to wait for a spend +// when looking up the htlc tx to sweep, or the tx that already spent the htlc +// output. The wait ends as soon as a confirmed spend is found, which is right +// away when the spend is already confirmed. The timeout only matters while +// the spend is unconfirmed, for example an earlier sweep of ours, or never +// confirms. +const defaultHtlcTxLookupTimeout = 30 * time.Minute + // SweepHtlcTimeoutAction is called if the server published the htlc tx without // paying the invoice. We wait for the timeout path to open up and sweep the // funds back to us. @@ -1534,6 +1542,13 @@ func (f *FSM) SweepHtlcTimeoutAction(ctx context.Context, return OnHtlcTimeoutSweepPublished } + // Keep the current state on shutdown so recovery resumes the + // sweep after restart. + if ctx.Err() != nil { + return fsm.NoOp + } + + f.LastActionError = err f.Errorf("unable to create and publish htlc timeout sweep "+ "tx: %v, retrying in %v", err, htlcTimeoutSweepRetryDelay) @@ -1673,6 +1688,13 @@ func (f *FSM) unlockDeposits(ctx context.Context) error { // createAndPublishHtlcTimeoutSweepTx creates and publishes the htlc timeout // sweep transaction. func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { + // Look up the htlc tx first. This can take a while, so the fee rate + // and the block height are only fetched once it is known. + htlcTx, err := f.fetchHtlcTx(ctx) + if err != nil { + return err + } + // Get a fee rate. feeRate, err := f.cfg.WalletKit.EstimateFeeRate(ctx, defaultConfTarget) if err != nil { @@ -1686,9 +1708,8 @@ func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { // Create htlc timeout transaction. timeoutTx, err := f.loopIn.createHtlcSweepTx( - ctx, f.cfg.Signer, f.loopIn.HtlcTimeoutSweepAddress, feeRate, - f.cfg.ChainParams, getInfo.BlockHeight, - f.cfg.MaxStaticAddrHtlcFeePercentage, + ctx, f.cfg.Signer, htlcTx, f.loopIn.HtlcTimeoutSweepAddress, + feeRate, f.cfg.ChainParams, getInfo.BlockHeight, ) if err != nil { return fmt.Errorf("unable to create htlc timeout sweep tx: %w", @@ -1710,17 +1731,129 @@ func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { f.LastActionError = err return err } - } else { - f.Debugf("published htlc timeout sweep with txid: %v", - timeoutTx.TxHash()) - hash := timeoutTx.TxHash() + // lnd also reports missing inputs and mempool conflicts this + // way. With the invoice canceled, only our sweeps can spend the + // htlc output, so monitor the confirmed spender. + spendingTx, spendErr := f.htlcSpendingTx( + ctx, timeoutTx.TxIn[0].PreviousOutPoint, + ) + if spendErr != nil { + return fmt.Errorf("%v: %w, and no confirmed spend of the "+ + "htlc output was found: %w", txLabel, err, spendErr) + } + + // The monitor only sees txs paying the sweep address. + hash := spendingTx.TxHash() + paysSweepAddr, _ := input.FindScriptOutputIndex( + spendingTx, timeoutTx.TxOut[0].PkScript, + ) + if !paysSweepAddr { + return fmt.Errorf("%v: htlc output spent by tx %v, which "+ + "doesn't pay the sweep address", txLabel, hash) + } + + f.Infof("htlc output already spent by sweep tx %v", hash) f.loopIn.HtlcTimeoutSweepTxHash = &hash + + return nil } + f.Debugf("published htlc timeout sweep with txid: %v", + timeoutTx.TxHash()) + + hash := timeoutTx.TxHash() + f.loopIn.HtlcTimeoutSweepTxHash = &hash + return nil } +// htlcSpendingTx waits for the spend of the given htlc output and returns the +// spending tx. +func (f *FSM) htlcSpendingTx(ctx context.Context, + htlcOutpoint wire.OutPoint) (*wire.MsgTx, error) { + + htlc, err := f.loopIn.getHtlc(f.cfg.ChainParams) + if err != nil { + return nil, err + } + + return f.waitForSpend( + ctx, htlcOutpoint, htlc.PkScript, "htlc output", "sweep tx", + ) +} + +// fetchHtlcTx waits for the spend of the swap's first deposit and returns the +// spending htlc tx. The server may publish any of the htlc tx variants the +// client co-signed, and only the standard fee rate is persisted, so the htlc +// tx that is swept is taken from the chain instead of being rebuilt. +func (f *FSM) fetchHtlcTx(ctx context.Context) (*wire.MsgTx, error) { + outpoints := f.loopIn.Outpoints() + if len(outpoints) == 0 { + return nil, errors.New("loop-in has no deposits") + } + + if f.loopIn.AddressParams == nil { + return nil, errors.New("loop-in has no static address " + + "parameters") + } + + return f.waitForSpend( + ctx, outpoints[0], f.loopIn.AddressParams.PkScript, "deposit", + "htlc tx", + ) +} + +// waitForSpend returns the tx that spends the outpoint, waiting at most the +// htlc tx lookup timeout. The labels are for logs and errors. +func (f *FSM) waitForSpend(ctx context.Context, outpoint wire.OutPoint, + pkScript []byte, description, spender string) (*wire.MsgTx, error) { + + lookupTimeout := f.cfg.HtlcTxLookupTimeout + if lookupTimeout == 0 { + lookupTimeout = defaultHtlcTxLookupTimeout + } + + spendCtx, cancel := context.WithTimeout(ctx, lookupTimeout) + defer cancel() + + spendChan, errChan, err := f.cfg.ChainNotifier.RegisterSpendNtfn( + spendCtx, &outpoint, pkScript, int32(f.loopIn.InitiationHeight), + ) + if err != nil { + return nil, fmt.Errorf("unable to register for the spend of "+ + "%s %v: %w", description, outpoint, err) + } + + f.Infof("waiting for the %s that spends %s %v, height hint %v", + spender, description, outpoint, f.loopIn.InitiationHeight) + + select { + case spend := <-spendChan: + if spend == nil || spend.SpendingTx == nil { + return nil, fmt.Errorf("spend of %s %v has no spending "+ + "tx", description, outpoint) + } + + f.Infof("%s %v spent by %s %v", description, outpoint, spender, + spend.SpendingTx.TxHash()) + + return spend.SpendingTx, nil + + case err := <-errChan: + return nil, fmt.Errorf("unable to watch the spend of %s %v: %w", + description, outpoint, err) + + case <-spendCtx.Done(): + if ctx.Err() != nil { + return nil, ctx.Err() + } + + return nil, fmt.Errorf("%s %v was not spent within %v", + description, outpoint, lookupTimeout) + } +} + // toNonces converts a byte slice to a 66 byte slice. func toNonces(nonces [][]byte) ([][musig2.PubNonceSize]byte, error) { res := make([][musig2.PubNonceSize]byte, 0, len(nonces)) diff --git a/staticaddr/loopin/htlc_timeout_sweep_test.go b/staticaddr/loopin/htlc_timeout_sweep_test.go new file mode 100644 index 000000000..9f38d561c --- /dev/null +++ b/staticaddr/loopin/htlc_timeout_sweep_test.go @@ -0,0 +1,795 @@ +package loopin + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/lndclient" + "github.com/lightninglabs/loop/fsm" + "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/chainntnfs" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" +) + +const ( + testStandardHtlcFeeRate = chainfee.SatPerKWeight(200) + testHighHtlcFeeRate = chainfee.SatPerKWeight(1_000) + testExtremeHtlcFeeRate = chainfee.SatPerKWeight(4_000) + testMaxHtlcFeePct = 0.2 + testMaxHtlcBackupFeePct = 0.5 +) + +// recordingSweepSigner records the sign descriptors of the htlc timeout sweep +// and returns dummy signatures. +type recordingSweepSigner struct { + lndclient.SignerClient + + descs chan *lndclient.SignDescriptor +} + +// SignOutputRaw records the sign descriptors and returns dummy 64-byte +// signatures. +func (s *recordingSweepSigner) SignOutputRaw(_ context.Context, _ *wire.MsgTx, + descs []*lndclient.SignDescriptor, _ []*wire.TxOut) ([][]byte, error) { + + sigs := make([][]byte, len(descs)) + for i, desc := range descs { + sigs[i] = make([]byte, 64) + + select { + case s.descs <- desc: + default: + } + } + + return sigs, nil +} + +// newHtlcVariantTestLoopIn returns a loop-in over two deposits with a change +// output and the three htlc tx variants the client co-signs for it. +func newHtlcVariantTestLoopIn(t *testing.T, + mockLnd *test.LndMockServices) (*StaticAddressLoopIn, + map[chainfee.SatPerKWeight]*wire.MsgTx) { + + t.Helper() + + clientKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + serverKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + staticAddr, err := newStaticAddress( + clientKey.PubKey(), serverKey.PubKey(), 4032, + ) + require.NoError(t, err) + + pkScript, err := staticAddr.StaticAddressScript() + require.NoError(t, err) + + deposits := []*deposit.Deposit{ + { + Hash: chainhash.Hash{0x11}, + Index: 1, + Value: btcutil.Amount(600_000), + }, + { + Hash: chainhash.Hash{0x22}, + Index: 0, + Value: btcutil.Amount(400_000), + }, + } + + sweepAddr, err := btcutil.NewAddressTaproot( + make([]byte, 32), mockLnd.ChainParams, + ) + require.NoError(t, err) + + loopIn := &StaticAddressLoopIn{ + SwapHash: lntypes.Hash{7, 7, 7}, + HtlcCltvExpiry: mockLnd.Height + 10, + InitiationHeight: uint32(mockLnd.Height), + InitiationTime: time.Now(), + ProtocolVersion: version.ProtocolVersion_V0, + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + Deposits: deposits, + SelectedAmount: btcutil.Amount(800_000), + AddressParams: &script.Parameters{ + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + PkScript: pkScript, + Expiry: 4032, + ProtocolVersion: version.ProtocolVersion_V0, + }, + HtlcTxFeeRate: testStandardHtlcFeeRate, + HtlcTxHighFeeRate: testHighHtlcFeeRate, + HtlcTxExtremelyHighFeeRate: testExtremeHtlcFeeRate, + HtlcTimeoutSweepAddress: sweepAddr, + PaymentTimeoutSeconds: 3_600, + } + + variants := make(map[chainfee.SatPerKWeight]*wire.MsgTx, 3) + for _, rate := range []chainfee.SatPerKWeight{ + testStandardHtlcFeeRate, testHighHtlcFeeRate, + testExtremeHtlcFeeRate, + } { + maxFeePct := testMaxHtlcBackupFeePct + if rate == testStandardHtlcFeeRate { + maxFeePct = testMaxHtlcFeePct + } + + htlcTx, err := loopIn.createHtlcTx( + mockLnd.ChainParams, rate, maxFeePct, + ) + require.NoError(t, err) + + variants[rate] = htlcTx + } + + return loopIn, variants +} + +// TestHtlcTimeoutSweepSpendsPublishedVariant verifies that the htlc timeout +// sweep spends the htlc output of the htlc tx variant that spent the deposits +// on chain. The client co-signs a standard, a high fee and an extremely high +// fee variant and the server may publish any of them. Only the standard fee +// rate is persisted, so the sweep must not depend on the backup fee rates +// either. +func TestHtlcTimeoutSweepSpendsPublishedVariant(t *testing.T) { + tests := []struct { + name string + rate chainfee.SatPerKWeight + }{ + { + name: "standard fee variant", + rate: testStandardHtlcFeeRate, + }, + { + name: "high fee variant", + rate: testHighHtlcFeeRate, + }, + { + name: "extremely high fee variant", + rate: testExtremeHtlcFeeRate, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + testHtlcTimeoutSweepSpendsVariant(t, tc.rate) + }) + } +} + +// testHtlcTimeoutSweepSpendsVariant publishes the htlc timeout sweep of a +// recovered loop-in after the htlc tx variant with the given fee rate spent +// the deposits. +func testHtlcTimeoutSweepSpendsVariant(t *testing.T, + publishedRate chainfee.SatPerKWeight) { + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + // The three variants are distinct transactions. + require.NotEqual( + t, variants[testStandardHtlcFeeRate].TxHash(), + variants[testHighHtlcFeeRate].TxHash(), + ) + require.NotEqual( + t, variants[testHighHtlcFeeRate].TxHash(), + variants[testExtremeHtlcFeeRate].TxHash(), + ) + + publishedTx := variants[publishedRate] + + // Simulate a restart. The backup fee rates are not persisted, so a + // recovered loop-in only knows the standard fee rate. + loopIn.HtlcTxHighFeeRate = 0 + loopIn.HtlcTxExtremelyHighFeeRate = 0 + + signer := &recordingSweepSigner{ + descs: make(chan *lndclient.SignDescriptor, 1), + } + + f := newHtlcSweepTestFSM(t, ctx, mockLnd, loopIn, signer) + + resultChan := make(chan fsm.EventType, 1) + go func() { + resultChan <- f.SweepHtlcTimeoutAction(ctx, nil) + }() + + // The sweep looks up the tx that spent the first deposit. + spendReg := receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + require.Equal(t, loopIn.AddressParams.PkScript, spendReg.PkScript) + require.Equal(t, int32(loopIn.InitiationHeight), spendReg.HeightHint) + + deliverDepositSpend(t, ctx, spendReg, publishedTx) + + var sweepTx *wire.MsgTx + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not published: %v", ctx.Err()) + } + + select { + case event := <-resultChan: + require.Equal(t, OnHtlcTimeoutSweepPublished, event) + + case <-ctx.Done(): + t.Fatalf("sweep action did not finish: %v", ctx.Err()) + } + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + // The htlc output is at index 0 and the change output at index 1. + require.Len(t, publishedTx.TxOut, 2) + require.Equal(t, htlc.PkScript, publishedTx.TxOut[0].PkScript) + + require.Len(t, sweepTx.TxIn, 1) + require.Equal(t, wire.OutPoint{ + Hash: publishedTx.TxHash(), + Index: 0, + }, sweepTx.TxIn[0].PreviousOutPoint) + + sweepHash := sweepTx.TxHash() + require.Equal(t, &sweepHash, loopIn.HtlcTimeoutSweepTxHash) + + // The sweep is signed for the value of the published htlc output. + select { + case desc := <-signer.descs: + require.Equal(t, publishedTx.TxOut[0].Value, desc.Output.Value) + require.Equal(t, htlc.PkScript, desc.Output.PkScript) + + default: + t.Fatalf("htlc timeout sweep was not signed") + } + require.Less(t, sweepTx.TxOut[0].Value, publishedTx.TxOut[0].Value) +} + +// TestHtlcOutput verifies that only a tx that spends exactly the swap's +// deposits and pays the htlc once is accepted as htlc tx. +func TestHtlcOutput(t *testing.T) { + t.Parallel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + tests := []struct { + name string + modify func(tx *wire.MsgTx) *wire.MsgTx + htlcIndex uint32 + err string + }{ + { + name: "published variant", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + return tx + }, + }, + { + name: "deposits in a different order", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[0], tx.TxIn[1] = tx.TxIn[1], tx.TxIn[0] + return tx + }, + }, + { + name: "htlc output after the change output", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + out := tx.TxOut + out[0], out[1] = out[1], out[0] + + return tx + }, + htlcIndex: 1, + }, + { + name: "no tx", + modify: func(*wire.MsgTx) *wire.MsgTx { + return nil + }, + err: "no htlc tx provided", + }, + { + name: "missing deposit", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn = tx.TxIn[:1] + return tx + }, + err: "expected the 2 deposits", + }, + { + name: "same deposit twice", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[1].PreviousOutPoint = + tx.TxIn[0].PreviousOutPoint + + return tx + }, + err: "not a remaining deposit", + }, + { + name: "foreign input", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[1].PreviousOutPoint.Index++ + return tx + }, + err: "not a remaining deposit", + }, + { + name: "no htlc output", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut = tx.TxOut[1:] + return tx + }, + err: "has no htlc output", + }, + { + name: "two htlc outputs", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut[1].PkScript = htlc.PkScript + return tx + }, + err: "more than one htlc output", + }, + { + name: "htlc output without value", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut[0].Value = 0 + return tx + }, + err: "without value", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + htlcTx := tc.modify( + variants[testHighHtlcFeeRate].Copy(), + ) + + outpoint, txOut, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + return + } + + require.NoError(t, err) + require.Equal(t, wire.OutPoint{ + Hash: htlcTx.TxHash(), + Index: tc.htlcIndex, + }, outpoint) + require.Equal(t, htlcTx.TxOut[tc.htlcIndex], txOut) + }) + } +} + +// newHtlcSweepTestFSM returns a loop-in FSM that can publish the htlc timeout +// sweep of the given loop-in against the mock lnd. +func newHtlcSweepTestFSM(t *testing.T, ctx context.Context, + mockLnd *test.LndMockServices, loopIn *StaticAddressLoopIn, + signer lndclient.SignerClient) *FSM { + + t.Helper() + + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: loopIn.AddressParams, + }, + ChainNotifier: mockLnd.ChainNotifier, + DepositManager: &noopDepositManager{}, + LndClient: mockLnd.Client, + ChainParams: mockLnd.ChainParams, + WalletKit: mockLnd.WalletKit, + Signer: signer, + MaxStaticAddrHtlcFeePercentage: testMaxHtlcFeePct, + MaxStaticAddrHtlcBackupFeePercentage: testMaxHtlcBackupFeePct, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + return f +} + +// receiveDepositSpendReg waits for the spend registration of the first +// deposit of the loop-in. +func receiveDepositSpendReg(t *testing.T, ctx context.Context, + mockLnd *test.LndMockServices, + loopIn *StaticAddressLoopIn) *test.SpendRegistration { + + t.Helper() + + select { + case spendReg := <-mockLnd.RegisterSpendChannel: + require.Equal( + t, loopIn.Deposits[0].OutPoint, *spendReg.Outpoint, + ) + + return spendReg + + case <-ctx.Done(): + t.Fatalf("deposit spend registration not received: %v", + ctx.Err()) + + return nil + } +} + +// deliverDepositSpend reports the given tx as the spend of the registered +// deposit. +func deliverDepositSpend(t *testing.T, ctx context.Context, + spendReg *test.SpendRegistration, spendingTx *wire.MsgTx) { + + t.Helper() + + select { + case spendReg.SpendChannel <- &chainntnfs.SpendDetail{ + SpentOutPoint: spendReg.Outpoint, + SpendingTx: spendingTx, + }: + + case <-ctx.Done(): + t.Fatalf("unable to deliver deposit spend: %v", ctx.Err()) + } +} + +// TestHtlcTimeoutSweepFeeRateAfterLookup verifies that the sweep fee rate is +// estimated after the htlc tx was found, not before waiting for it. +func TestHtlcTimeoutSweepFeeRateAfterLookup(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + const ( + staleRate = chainfee.SatPerKWeight(253) + freshRate = chainfee.SatPerKWeight(5_000) + ) + + mockLnd := test.NewMockLnd() + mockLnd.SetFeeEstimate(defaultConfTarget, staleRate) + + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + f := newHtlcSweepTestFSM(t, ctx, mockLnd, loopIn, &noopSigner{}) + + resultChan := make(chan fsm.EventType, 1) + go func() { + resultChan <- f.SweepHtlcTimeoutAction(ctx, nil) + }() + + spendReg := receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + + // Fees rise while the sweep waits for the htlc tx. + mockLnd.SetFeeEstimate(defaultConfTarget, freshRate) + deliverDepositSpend(t, ctx, spendReg, htlcTx) + + var sweepTx *wire.MsgTx + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not published: %v", ctx.Err()) + } + require.Equal(t, OnHtlcTimeoutSweepPublished, <-resultChan) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + var weightEstimator input.TxWeightEstimator + weightEstimator.AddP2TROutput() + require.NoError(t, htlc.AddSuccessToEstimator(&weightEstimator)) + + fee := htlcTx.TxOut[0].Value - sweepTx.TxOut[0].Value + require.Equal( + t, int64(freshRate.FeeForWeight(weightEstimator.Weight())), + fee, + ) +} + +// TestFetchHtlcTxFailures verifies that the htlc tx lookup fails the sweep +// attempt instead of waiting forever or sweeping without an htlc tx. +func TestFetchHtlcTxFailures(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + lookupTimeout time.Duration + deliver func(ctx context.Context, shutdown func(), + spendReg *test.SpendRegistration) bool + err string + errorIs error + }{ + { + name: "notifier error", + deliver: func(ctx context.Context, _ func(), + spendReg *test.SpendRegistration) bool { + + select { + case spendReg.ErrChan <- errors.New("closed"): + return true + + case <-ctx.Done(): + return false + } + }, + err: "closed", + }, + { + name: "spend without tx", + deliver: func(ctx context.Context, _ func(), + spendReg *test.SpendRegistration) bool { + + spend := &chainntnfs.SpendDetail{} + select { + case spendReg.SpendChannel <- spend: + return true + + case <-ctx.Done(): + return false + } + }, + err: "has no spending tx", + }, + { + name: "deposit not spent", + lookupTimeout: 100 * time.Millisecond, + deliver: func(context.Context, func(), + *test.SpendRegistration) bool { + + return true + }, + err: "was not spent within", + }, + { + name: "shutdown", + deliver: func(_ context.Context, shutdown func(), + _ *test.SpendRegistration) bool { + + shutdown() + return true + }, + errorIs: context.Canceled, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, _ := newHtlcVariantTestLoopIn(t, mockLnd) + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = tc.lookupTimeout + + lookupCtx, shutdown := context.WithCancel(ctx) + defer shutdown() + + errChan := make(chan error, 1) + go func() { + _, err := f.fetchHtlcTx(lookupCtx) + errChan <- err + }() + + spendReg := receiveDepositSpendReg( + t, ctx, mockLnd, loopIn, + ) + require.True(t, tc.deliver(ctx, shutdown, spendReg)) + + select { + case err := <-errChan: + if tc.errorIs != nil { + require.ErrorIs(t, err, tc.errorIs) + return + } + + require.ErrorContains(t, err, tc.err) + + case <-ctx.Done(): + t.Fatalf("htlc tx lookup did not fail: %v", + ctx.Err()) + } + }) + } +} + +// TestHtlcTimeoutSweepOutputAlreadySpent verifies that an "output already +// spent" publish error only counts as a published sweep if the htlc output is +// spent by a tx that pays the sweep address, and that this tx is the one that +// is monitored. +func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + + // spent is set if a tx spends the htlc output. + spent bool + + // paysSweepAddress is set if that tx pays the sweep address. + paysSweepAddress bool + + err string + }{ + { + name: "spent by an earlier sweep", + spent: true, + paysSweepAddress: true, + }, + { + name: "spent by a tx paying elsewhere", + spent: true, + err: "doesn't pay the sweep address", + }, + { + name: "not spent", + err: "no confirmed spend of the htlc output", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + mockLnd.PublishHandler = func(context.Context, + *wire.MsgTx, string) error { + + return lnwallet.ErrDoubleSpend + } + + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = time.Second + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + htlcOutpoint, _, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + require.NoError(t, err) + + errChan := make(chan error, 1) + go func() { + errChan <- f.createAndPublishHtlcTimeoutSweepTx(ctx) + }() + + deliverDepositSpend( + t, ctx, receiveDepositSpendReg( + t, ctx, mockLnd, loopIn, + ), htlcTx, + ) + + // The sweep checks whether the htlc output is spent. + var spendReg *test.SpendRegistration + select { + case spendReg = <-mockLnd.RegisterSpendChannel: + case <-ctx.Done(): + t.Fatalf("htlc output spend registration not "+ + "received: %v", ctx.Err()) + } + require.Equal(t, htlcOutpoint, *spendReg.Outpoint) + require.Equal(t, htlc.PkScript, spendReg.PkScript) + + // An earlier sweep with another fee, or a tx paying + // elsewhere, spent the output. + sweepPkScript, err := txscript.PayToAddrScript( + loopIn.HtlcTimeoutSweepAddress, + ) + require.NoError(t, err) + pkScript := sweepPkScript + if !tc.paysSweepAddress { + pkScript = []byte{0x00, 0x14, 0x01} + } + + spendingTx := wire.NewMsgTx(2) + spendingTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: htlcOutpoint, + }) + spendingTx.AddTxOut(&wire.TxOut{ + Value: 1_000, + PkScript: pkScript, + }) + spendingTx.LockTime = 1 + if tc.spent { + select { + case spendReg.SpendChannel <- &chainntnfs.SpendDetail{ + SpentOutPoint: spendReg.Outpoint, + SpendingTx: spendingTx, + }: + + case <-ctx.Done(): + t.Fatalf("unable to deliver htlc output "+ + "spend: %v", ctx.Err()) + } + } + + select { + case err = <-errChan: + case <-ctx.Done(): + t.Fatalf("sweep attempt did not return: %v", + ctx.Err()) + } + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Nil(t, loopIn.HtlcTimeoutSweepTxHash) + + return + } + + require.NoError(t, err) + require.Equal( + t, spendingTx.TxHash(), + *loopIn.HtlcTimeoutSweepTxHash, + ) + }) + } +} + +// TestSweepHtlcTimeoutActionReportsLookupError checks that a failed htlc tx +// lookup is reported in LastActionError before the action waits to retry. +func TestSweepHtlcTimeoutActionReportsLookupError(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + mockLnd := test.NewMockLnd() + f := &FSM{ + StateMachine: &fsm.StateMachine{}, + cfg: &Config{ + LndClient: mockLnd.Client, + WalletKit: mockLnd.WalletKit, + }, + loopIn: &StaticAddressLoopIn{}, + } + + // The lookup fails right away because the swap has no deposits. Stop + // the action while it waits to retry. + stop := time.AfterFunc(100*time.Millisecond, cancel) + defer stop.Stop() + + event := f.SweepHtlcTimeoutAction(ctx, nil) + require.Equal(t, fsm.NoOp, event) + require.ErrorContains(t, f.LastActionError, "loop-in has no deposits") +} diff --git a/staticaddr/loopin/loopin.go b/staticaddr/loopin/loopin.go index ba2217f59..e9038d534 100644 --- a/staticaddr/loopin/loopin.go +++ b/staticaddr/loopin/loopin.go @@ -321,8 +321,7 @@ func (l *StaticAddressLoopIn) createHtlcTx(chainParams *chaincfg.Params, return nil, err } - // Create the sweep output. NOTE: The HTLC output must be added at - // index 0. createHtlcSweepTx relies on this layout invariant. + // Create the sweep output. The HTLC output is added at index 0. sweepOutput := &wire.TxOut{ Value: int64(swapAmt - fee), PkScript: pkscript, @@ -366,12 +365,95 @@ func (l *StaticAddressLoopIn) htlcWeight(hasChange bool) lntypes.WeightUnit { return weightEstimator.Weight() } +// htlcOutput returns the outpoint and the output of the htlc in the given htlc +// tx. The client co-signs a standard, a high fee and an extremely high fee +// variant of the htlc tx, and the server may publish any of them. The timeout +// sweep therefore has to spend the htlc output of the tx that actually spent +// the deposits instead of rebuilding one of the variants. The tx is only +// accepted if it spends exactly the deposits of this swap and has a single +// output to the given htlc pkScript. The input order is not checked because +// recovery can load the deposits in a different order than they were signed +// in, and only the co-signed htlc tx both spends the deposits and pays to the +// htlc pkScript. +func (l *StaticAddressLoopIn) htlcOutput(htlcTx *wire.MsgTx, + htlcPkScript []byte) (wire.OutPoint, *wire.TxOut, error) { + + if htlcTx == nil { + return wire.OutPoint{}, nil, errors.New("no htlc tx provided") + } + + outpoints := l.Outpoints() + if len(outpoints) == 0 { + return wire.OutPoint{}, nil, errors.New("loop-in has no " + + "deposits") + } + + txHash := htlcTx.TxHash() + if len(htlcTx.TxIn) != len(outpoints) { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v spends %d "+ + "inputs, expected the %d deposits of the swap", txHash, + len(htlcTx.TxIn), len(outpoints)) + } + + unspent := make(map[wire.OutPoint]struct{}, len(outpoints)) + for _, outpoint := range outpoints { + unspent[outpoint] = struct{}{} + } + + for i, txIn := range htlcTx.TxIn { + if _, ok := unspent[txIn.PreviousOutPoint]; !ok { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v input "+ + "%d spends %v, which is not a remaining "+ + "deposit of the swap", txHash, i, + txIn.PreviousOutPoint) + } + + delete(unspent, txIn.PreviousOutPoint) + } + + var ( + htlcIndex = -1 + htlcOut *wire.TxOut + ) + for i, txOut := range htlcTx.TxOut { + if !bytes.Equal(txOut.PkScript, htlcPkScript) { + continue + } + + if htlcOut != nil { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has "+ + "more than one htlc output", txHash) + } + + htlcIndex = i + htlcOut = txOut + } + + if htlcOut == nil { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has no htlc "+ + "output", txHash) + } + + if htlcOut.Value <= 0 { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has an htlc "+ + "output without value", txHash) + } + + htlcOutpoint := wire.OutPoint{ + Hash: txHash, + Index: uint32(htlcIndex), + } + + return htlcOutpoint, htlcOut, nil +} + // createHtlcSweepTx creates the htlc sweep transaction for the timeout path of -// the loop-in htlc. +// the loop-in htlc. The htlcTx is the htlc tx variant that spent the deposits +// on chain. func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, - signer lndclient.SignerClient, sweepAddress btcutil.Address, - feeRate chainfee.SatPerKWeight, network *chaincfg.Params, - blockHeight uint32, maxFeePercentage float64) (*wire.MsgTx, error) { + signer lndclient.SignerClient, htlcTx *wire.MsgTx, + sweepAddress btcutil.Address, feeRate chainfee.SatPerKWeight, + network *chaincfg.Params, blockHeight uint32) (*wire.MsgTx, error) { if network == nil { return nil, errors.New("no network provided") @@ -382,6 +464,11 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, return nil, err } + htlcOutpoint, htlcOut, err := l.htlcOutput(htlcTx, htlc.PkScript) + if err != nil { + return nil, err + } + // Create the sweep transaction. sweepTx := wire.NewMsgTx(2) sweepTx.LockTime = blockHeight @@ -394,36 +481,11 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, return nil, err } - htlcTx, err := l.createHtlcTx( - network, l.HtlcTxFeeRate, maxFeePercentage, - ) - if err != nil { - return nil, err - } - - // The HTLC output is always at index 0 (createHtlcTx adds it first). - // If there is a change output, it is at index 1. Verify this invariant - // so we fail fast if createHtlcTx's layout ever changes. - const htlcInputIndex = uint32(0) - if len(htlcTx.TxOut) == 2 { - if bytes.Equal( - htlcTx.TxOut[0].PkScript, l.AddressParams.PkScript, - ) { - - return nil, fmt.Errorf("htlc tx output layout " + - "invariant violated: expected HTLC output " + - "at index 0, got change output") - } - } - // Add the htlc input. sweepTx.AddTxIn(&wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: htlcTx.TxHash(), - Index: htlcInputIndex, - }, - SignatureScript: htlc.SigScript, - Sequence: htlc.SuccessSequence(), + PreviousOutPoint: htlcOutpoint, + SignatureScript: htlc.SigScript, + Sequence: htlc.SuccessSequence(), }) // Add the sweep output. @@ -434,7 +496,7 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, fee := feeRate.FeeForWeight(weightEstimator.Weight()) - htlcOutValue := htlcTx.TxOut[htlcInputIndex].Value + htlcOutValue := htlcOut.Value output := &wire.TxOut{ Value: htlcOutValue - int64(fee), PkScript: sweepPkScript, diff --git a/staticaddr/loopin/loopin_test.go b/staticaddr/loopin/loopin_test.go index 8b0892e66..8f4b588cc 100644 --- a/staticaddr/loopin/loopin_test.go +++ b/staticaddr/loopin/loopin_test.go @@ -130,9 +130,8 @@ func TestCreateHtlcSweepTxSweepValue(t *testing.T) { // Call createHtlcSweepTx and verify that the sweep output is derived // from the HTLC value, not the change. sweepTx, err := loopIn.createHtlcSweepTx( - t.Context(), signer, sweepAddr, feeRate, + t.Context(), signer, htlcTx, sweepAddr, feeRate, network, uint32(loopIn.HtlcCltvExpiry)+1, - maxFeePercentage, ) require.NoError(t, err) require.Len(t, sweepTx.TxOut, 1) diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index 3cb3d6502..b9b24272a 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -125,6 +125,13 @@ type Config struct { // MaxStaticAddrHtlcFeePercentage since it serves the server as backup // transaction in case of fee spikes. MaxStaticAddrHtlcBackupFeePercentage float64 + + // HtlcTxLookupTimeout is the maximum time to wait for a confirmed spend + // when looking up the htlc tx to sweep, or the tx that already spent + // the htlc output. A timeout fails the sweep attempt so that it is + // retried and reported like any other failure. Zero means + // defaultHtlcTxLookupTimeout. + HtlcTxLookupTimeout time.Duration } // newSwapRequest is used to send a loop-in request to the manager main loop. From 540ef9350a55190f686ca7c243718e12e3686446 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 30 Oct 2026 12:23:36 +0200 Subject: [PATCH 2/4] staticaddr/loopin: reject htlc timeout sweeps below dust The timeout sweep now spends whichever htlc tx variant confirmed. The backup variants leave a smaller htlc output, so a high sweep fee rate can leave a dust or negative sweep output. Such a sweep was signed and only rejected by the backend with an unclear error on every retry. Check the sweep output against the dust limit before signing and return an error that names the htlc output, its value and the fee. --- staticaddr/loopin/htlc_timeout_sweep_test.go | 67 ++++++++++++++++++++ staticaddr/loopin/loopin.go | 10 +++ 2 files changed, 77 insertions(+) diff --git a/staticaddr/loopin/htlc_timeout_sweep_test.go b/staticaddr/loopin/htlc_timeout_sweep_test.go index 9f38d561c..d4eb636c7 100644 --- a/staticaddr/loopin/htlc_timeout_sweep_test.go +++ b/staticaddr/loopin/htlc_timeout_sweep_test.go @@ -390,6 +390,73 @@ func TestHtlcOutput(t *testing.T) { } } +// TestCreateHtlcSweepTxRejectsDust verifies that the sweep is only signed if +// the fee leaves an output at or above the dust limit. +func TestCreateHtlcSweepTxRejectsDust(t *testing.T) { + t.Parallel() + + const feeRate = chainfee.SatPerKWeight(50_000) + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + var weightEstimator input.TxWeightEstimator + weightEstimator.AddP2TROutput() + require.NoError(t, htlc.AddSuccessToEstimator(&weightEstimator)) + + fee := int64(feeRate.FeeForWeight(weightEstimator.Weight())) + dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + + tests := []struct { + name string + htlcValue int64 + err string + }{ + { + name: "output at dust limit", + htlcValue: fee + dustLimit, + }, + { + name: "output below dust limit", + htlcValue: fee + dustLimit - 1, + err: "too small to pay the sweep fee", + }, + { + name: "fee above htlc value", + htlcValue: fee - 1, + err: "too small to pay the sweep fee", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + htlcTx := variants[testExtremeHtlcFeeRate].Copy() + htlcTx.TxOut[0].Value = tc.htlcValue + + sweepTx, err := loopIn.createHtlcSweepTx( + t.Context(), &noopSigner{}, htlcTx, + loopIn.HtlcTimeoutSweepAddress, feeRate, + mockLnd.ChainParams, + uint32(loopIn.HtlcCltvExpiry), + ) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + return + } + + require.NoError(t, err) + require.Equal( + t, dustLimit, sweepTx.TxOut[0].Value, + ) + }) + } +} + // newHtlcSweepTestFSM returns a loop-in FSM that can publish the htlc timeout // sweep of the given loop-in against the mock lnd. func newHtlcSweepTestFSM(t *testing.T, ctx context.Context, diff --git a/staticaddr/loopin/loopin.go b/staticaddr/loopin/loopin.go index e9038d534..eafd6b35f 100644 --- a/staticaddr/loopin/loopin.go +++ b/staticaddr/loopin/loopin.go @@ -26,6 +26,7 @@ import ( "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/zpay32" @@ -496,7 +497,16 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, fee := feeRate.FeeForWeight(weightEstimator.Weight()) + // The backup htlc tx variants leave a smaller htlc output, so make + // sure the sweep fee still leaves a spendable output. htlcOutValue := htlcOut.Value + dustLimit := lnwallet.DustLimitForSize(input.P2TRSize) + if htlcOutValue-int64(fee) < int64(dustLimit) { + return nil, fmt.Errorf("htlc output %v of %v is too small to "+ + "pay the sweep fee %v at fee rate %v", htlcOutpoint, + btcutil.Amount(htlcOutValue), fee, feeRate) + } + output := &wire.TxOut{ Value: htlcOutValue - int64(fee), PkScript: sweepPkScript, From 177c7b749bde64a5f18b29ff95037c2ae11e1d45 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 30 Oct 2026 13:06:01 +0200 Subject: [PATCH 3/4] docs: note static loop-in htlc timeout sweep fix The timeout sweep of a static address loop-in could not sweep a confirmed backup fee variant of the htlc tx. Note the fix and the new dust check so that users with affected swaps know to upgrade. --- docs/release-notes/release-notes-next.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-next.md b/docs/release-notes/release-notes-next.md index 8ea78eedd..27a5c4e9e 100644 --- a/docs/release-notes/release-notes-next.md +++ b/docs/release-notes/release-notes-next.md @@ -15,6 +15,12 @@ already using, and a failed lock no longer resets deposits that a withdrawal or a channel open has taken. +* The static address Loop In timeout sweep now spends the exact HTLC output + confirmed on chain, including server-published backup fee variants. + Previously it always targeted the standard fee variant, so a confirmed high + fee or extremely high fee HTLC transaction could not be swept back after the + HTLC timed out. Sweeps that would leave a dust output are no longer signed. + #### Maintenance #### Contributors (Alphabetical Order) From 3d4e29afdd222d36d8b41e3df7315f8ea62dc794 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 12 Nov 2026 08:35:39 +0200 Subject: [PATCH 4/4] staticaddr/loopin: find missed htlc spends in the wallet If lnd's historical spend scan fails, lnd logs the error and keeps the scan marked as pending. Registering for the spend again doesn't start a new scan, so the timeout sweep never learns about the htlc tx, or about an earlier sweep of ours, until lnd restarts. See lightningnetwork/lnd#11234. When the spend lookup times out, look for a confirmed wallet tx that spends the exact outpoint, starting at the htlc tx search height. The deposits are wallet outputs and our sweeps pay to the wallet, so lnd lists those spends. The tx found goes through the same checks as one from the notifier. If the wallet has no match, the sweep keeps retrying as before. --- staticaddr/loopin/actions.go | 52 ++++- staticaddr/loopin/actions_test.go | 29 +++ staticaddr/loopin/htlc_timeout_sweep_test.go | 199 +++++++++++++++++-- staticaddr/loopin/interface.go | 6 + staticaddr/loopin/manager.go | 14 +- staticaddr/loopin/txout_checker.go | 28 +++ staticaddr/loopin/txout_checker_test.go | 87 ++++++++ 7 files changed, 379 insertions(+), 36 deletions(-) diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index 323660edb..6c478c075 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -1522,12 +1522,12 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, // htlc tx or creating or publishing the htlc timeout sweep transaction fails. const htlcTimeoutSweepRetryDelay = time.Hour -// defaultHtlcTxLookupTimeout is the default maximum time to wait for a spend -// when looking up the htlc tx to sweep, or the tx that already spent the htlc -// output. The wait ends as soon as a confirmed spend is found, which is right -// away when the spend is already confirmed. The timeout only matters while -// the spend is unconfirmed, for example an earlier sweep of ours, or never -// confirms. +// defaultHtlcTxLookupTimeout is the default maximum time to wait for lnd to +// report a spend when looking up the htlc tx to sweep, or the tx that already +// spent the htlc output. The wait ends as soon as a confirmed spend is +// reported, which is right away when the spend is already confirmed. After the +// timeout, lnd's wallet is checked for a confirmed spend that lnd didn't +// report. const defaultHtlcTxLookupTimeout = 30 * time.Minute // SweepHtlcTimeoutAction is called if the server published the htlc tx without @@ -1804,8 +1804,9 @@ func (f *FSM) fetchHtlcTx(ctx context.Context) (*wire.MsgTx, error) { ) } -// waitForSpend returns the tx that spends the outpoint, waiting at most the -// htlc tx lookup timeout. The labels are for logs and errors. +// waitForSpend returns the confirmed tx that spends the outpoint. If lnd +// doesn't report the spend within the htlc tx lookup timeout, lnd's wallet is +// checked before giving up. The labels are for logs and errors. func (f *FSM) waitForSpend(ctx context.Context, outpoint wire.OutPoint, pkScript []byte, description, spender string) (*wire.MsgTx, error) { @@ -1849,11 +1850,46 @@ func (f *FSM) waitForSpend(ctx context.Context, outpoint wire.OutPoint, return nil, ctx.Err() } + // If lnd's historical spend scan fails, lnd neither reports + // the error nor starts a new scan when we register again, so a + // past spend is only found after lnd restarts. See + // lightningnetwork/lnd#11234. Check lnd's wallet instead. + tx := f.walletSpendingTx(ctx, outpoint, description) + if tx != nil { + f.Infof("%s %v spent by %s %v, found in the wallet", + description, outpoint, spender, tx.TxHash()) + + return tx, nil + } + return nil, fmt.Errorf("%s %v was not spent within %v", description, outpoint, lookupTimeout) } } +// walletSpendingTx returns a confirmed tx from lnd's wallet that spends the +// outpoint, or nil if the wallet lists none or the lookup fails. +func (f *FSM) walletSpendingTx(ctx context.Context, outpoint wire.OutPoint, + description string) *wire.MsgTx { + + if f.cfg.TxOutChecker == nil { + return nil + } + + tx, err := f.cfg.TxOutChecker.ConfirmedSpendingTx( + ctx, outpoint, + htlcTxSearchStartHeight(f.loopIn.InitiationHeight), + ) + if err != nil { + f.Warnf("unable to look up the spend of %s %v in the "+ + "wallet: %v", description, outpoint, err) + + return nil + } + + return tx +} + // toNonces converts a byte slice to a 66 byte slice. func toNonces(nonces [][]byte) ([][musig2.PubNonceSize]byte, error) { res := make([][musig2.PubNonceSize]byte, 0, len(nonces)) diff --git a/staticaddr/loopin/actions_test.go b/staticaddr/loopin/actions_test.go index 198deac34..1187cba2f 100644 --- a/staticaddr/loopin/actions_test.go +++ b/staticaddr/loopin/actions_test.go @@ -3863,6 +3863,14 @@ type recordingTxOutChecker struct { spendOutpoints [][]wire.OutPoint spendPkScripts [][]byte spendStartHeights []int32 + + // spendingTxs is what ConfirmedSpendingTx returns per outpoint. + spendingTxs map[wire.OutPoint]*wire.MsgTx + + // spendingTxOutpoints and spendingTxStartHeights record what + // ConfirmedSpendingTx was asked about. + spendingTxOutpoints []wire.OutPoint + spendingTxStartHeights []int32 } // GetTxOuts records the request and returns the configured available outputs. @@ -3897,6 +3905,27 @@ func (r *recordingTxOutChecker) SpentByTxPayingTo(_ context.Context, return r.htlcSpent, nil } +// ConfirmedSpendingTx records the outpoint and start height and returns the +// configured spending tx, or an error if the context is done. +func (r *recordingTxOutChecker) ConfirmedSpendingTx(ctx context.Context, + outpoint wire.OutPoint, startHeight int32) (*wire.MsgTx, error) { + + r.spendingTxOutpoints = append(r.spendingTxOutpoints, outpoint) + r.spendingTxStartHeights = append( + r.spendingTxStartHeights, startHeight, + ) + if r.err != nil { + return nil, r.err + } + + // Like the lnd RPC, fail on a done context. + if err := ctx.Err(); err != nil { + return nil, err + } + + return r.spendingTxs[outpoint], nil +} + // initHtlcTestServer lets InitHtlcAction tests inject a deterministic server // response without standing up the full gRPC client. type initHtlcTestServer struct { diff --git a/staticaddr/loopin/htlc_timeout_sweep_test.go b/staticaddr/loopin/htlc_timeout_sweep_test.go index d4eb636c7..9a0ca2e46 100644 --- a/staticaddr/loopin/htlc_timeout_sweep_test.go +++ b/staticaddr/loopin/htlc_timeout_sweep_test.go @@ -33,6 +33,9 @@ const ( testMaxHtlcBackupFeePct = 0.5 ) +// notSpentErr is part of the error of a spend lookup that timed out. +const notSpentErr = "was not spent within" + // recordingSweepSigner records the sign descriptors of the htlc timeout sweep // and returns dummy signatures. type recordingSweepSigner struct { @@ -630,7 +633,7 @@ func TestFetchHtlcTxFailures(t *testing.T) { return true }, - err: "was not spent within", + err: notSpentErr, }, { name: "shutdown", @@ -707,6 +710,10 @@ func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { // paysSweepAddress is set if that tx pays the sweep address. paysSweepAddress bool + // inWallet is set if the spend is only found in lnd's wallet, + // because lnd's spend notification never comes. + inWallet bool + err string }{ { @@ -719,6 +726,20 @@ func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { spent: true, err: "doesn't pay the sweep address", }, + { + name: "earlier sweep only found in the " + + "wallet", + spent: true, + paysSweepAddress: true, + inWallet: true, + }, + { + name: "tx paying elsewhere only found in " + + "the wallet", + spent: true, + inWallet: true, + err: "doesn't pay the sweep address", + }, { name: "not spent", err: "no confirmed spend of the htlc output", @@ -755,6 +776,36 @@ func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { ) require.NoError(t, err) + // An earlier sweep with another fee, or a tx paying + // elsewhere, spent the output. + sweepPkScript, err := txscript.PayToAddrScript( + loopIn.HtlcTimeoutSweepAddress, + ) + require.NoError(t, err) + pkScript := sweepPkScript + if !tc.paysSweepAddress { + pkScript = []byte{0x00, 0x14, 0x01} + } + + spendingTx := wire.NewMsgTx(2) + spendingTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: htlcOutpoint, + }) + spendingTx.AddTxOut(&wire.TxOut{ + Value: 1_000, + PkScript: pkScript, + }) + spendingTx.LockTime = 1 + + if tc.inWallet { + walletTxs := map[wire.OutPoint]*wire.MsgTx{ + htlcOutpoint: spendingTx, + } + f.cfg.TxOutChecker = &recordingTxOutChecker{ + spendingTxs: walletTxs, + } + } + errChan := make(chan error, 1) go func() { errChan <- f.createAndPublishHtlcTimeoutSweepTx(ctx) @@ -777,27 +828,7 @@ func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { require.Equal(t, htlcOutpoint, *spendReg.Outpoint) require.Equal(t, htlc.PkScript, spendReg.PkScript) - // An earlier sweep with another fee, or a tx paying - // elsewhere, spent the output. - sweepPkScript, err := txscript.PayToAddrScript( - loopIn.HtlcTimeoutSweepAddress, - ) - require.NoError(t, err) - pkScript := sweepPkScript - if !tc.paysSweepAddress { - pkScript = []byte{0x00, 0x14, 0x01} - } - - spendingTx := wire.NewMsgTx(2) - spendingTx.AddTxIn(&wire.TxIn{ - PreviousOutPoint: htlcOutpoint, - }) - spendingTx.AddTxOut(&wire.TxOut{ - Value: 1_000, - PkScript: pkScript, - }) - spendingTx.LockTime = 1 - if tc.spent { + if tc.spent && !tc.inWallet { select { case spendReg.SpendChannel <- &chainntnfs.SpendDetail{ SpentOutPoint: spendReg.Outpoint, @@ -860,3 +891,127 @@ func TestSweepHtlcTimeoutActionReportsLookupError(t *testing.T) { require.Equal(t, fsm.NoOp, event) require.ErrorContains(t, f.LastActionError, "loop-in has no deposits") } + +// TestHtlcTimeoutSweepWalletFallback verifies that the sweep takes the htlc tx +// from lnd's wallet when lnd never reports the deposit spend, which happens +// when lnd's historical spend scan fails. +func TestHtlcTimeoutSweepWalletFallback(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + + // inWallet is set if lnd's wallet lists the htlc tx. + inWallet bool + + // walletErr is the error of the wallet lookup. + walletErr error + + err string + }{ + { + name: "htlc tx found in the wallet", + inWallet: true, + }, + { + name: "htlc tx not in the wallet", + err: notSpentErr, + }, + { + name: "wallet lookup fails", + walletErr: errors.New("list transactions failed"), + err: notSpentErr, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + depositOutpoint := loopIn.Deposits[0].OutPoint + + checker := &recordingTxOutChecker{err: tc.walletErr} + if tc.inWallet { + walletTxs := map[wire.OutPoint]*wire.MsgTx{ + depositOutpoint: htlcTx, + } + checker.spendingTxs = walletTxs + } + + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = 100 * time.Millisecond + f.cfg.TxOutChecker = checker + + errChan := make(chan error, 1) + go func() { + err := f.createAndPublishHtlcTimeoutSweepTx(ctx) + errChan <- err + }() + + // lnd accepts the registration but never reports the + // spend. + receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + + // The mock publish blocks until the tx is read. + var sweepTx *wire.MsgTx + if tc.err == "" { + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not "+ + "published: %v", ctx.Err()) + } + } + + var err error + select { + case err = <-errChan: + case <-ctx.Done(): + t.Fatalf("sweep attempt did not return: %v", + ctx.Err()) + } + + require.Equal( + t, []wire.OutPoint{depositOutpoint}, + checker.spendingTxOutpoints, + ) + require.Equal( + t, []int32{htlcTxSearchStartHeight( + loopIn.InitiationHeight, + )}, + checker.spendingTxStartHeights, + ) + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Nil(t, loopIn.HtlcTimeoutSweepTxHash) + + return + } + + require.NoError(t, err) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + htlcOutpoint, _, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + require.NoError(t, err) + require.Len(t, sweepTx.TxIn, 1) + require.Equal( + t, htlcOutpoint, + sweepTx.TxIn[0].PreviousOutPoint, + ) + }) + } +} diff --git a/staticaddr/loopin/interface.go b/staticaddr/loopin/interface.go index 17a119347..c375b441c 100644 --- a/staticaddr/loopin/interface.go +++ b/staticaddr/loopin/interface.go @@ -141,6 +141,12 @@ type TxOutChecker interface { // startHeight are considered. SpentByTxPayingTo(ctx context.Context, outpoints []wire.OutPoint, pkScript []byte, startHeight int32) (bool, error) + + // ConfirmedSpendingTx returns a confirmed wallet transaction that + // spends the outpoint, or nil if there is none. Only transactions + // confirmed at or above startHeight are considered. + ConfirmedSpendingTx(ctx context.Context, outpoint wire.OutPoint, + startHeight int32) (*wire.MsgTx, error) } type NotificationManager interface { diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index b9b24272a..dcd797bbe 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -66,8 +66,9 @@ type Config struct { // TxOutChecker checks the selected deposits against lnd's wallet // transactions. It is used before the client gives the server HTLC - // signatures, and after a risk rejection to find out whether the htlc - // tx spent the deposits. + // signatures, after a risk rejection to find out whether the htlc tx + // spent the deposits, and by the htlc timeout sweep when lnd doesn't + // report a spend. TxOutChecker TxOutChecker // InvoicesClient is used to subscribe to invoice settlements and @@ -126,10 +127,11 @@ type Config struct { // transaction in case of fee spikes. MaxStaticAddrHtlcBackupFeePercentage float64 - // HtlcTxLookupTimeout is the maximum time to wait for a confirmed spend - // when looking up the htlc tx to sweep, or the tx that already spent - // the htlc output. A timeout fails the sweep attempt so that it is - // retried and reported like any other failure. Zero means + // HtlcTxLookupTimeout is the maximum time to wait for lnd to report a + // confirmed spend when looking up the htlc tx to sweep, or the tx that + // already spent the htlc output. After it, lnd's wallet is checked. If + // the wallet lists no spend either, the sweep attempt fails so that it + // is retried and reported like any other failure. Zero means // defaultHtlcTxLookupTimeout. HtlcTxLookupTimeout time.Duration } diff --git a/staticaddr/loopin/txout_checker.go b/staticaddr/loopin/txout_checker.go index 3157fd373..fa7a3993f 100644 --- a/staticaddr/loopin/txout_checker.go +++ b/staticaddr/loopin/txout_checker.go @@ -114,6 +114,34 @@ func (c *lndTxOutChecker) SpentByTxPayingTo(ctx context.Context, return false, nil } +// ConfirmedSpendingTx returns a confirmed transaction from lnd's wallet that +// spends the outpoint, or nil if there is none. The wallet only lists +// transactions that spend wallet outputs or pay to the wallet. Only +// transactions confirmed at or above startHeight are listed, so the lookup +// doesn't decode the whole wallet history. +func (c *lndTxOutChecker) ConfirmedSpendingTx(ctx context.Context, + outpoint wire.OutPoint, startHeight int32) (*wire.MsgTx, error) { + + txs, err := c.client.ListTransactions(ctx, startHeight, -1) + if err != nil { + return nil, err + } + + for _, tx := range txs { + if tx.Tx == nil || tx.Confirmations <= 0 { + continue + } + + for _, txIn := range tx.Tx.TxIn { + if txIn.PreviousOutPoint == outpoint { + return tx.Tx, nil + } + } + } + + return nil, nil +} + // paysTo reports whether the transaction has an output with the pkScript. func paysTo(tx *wire.MsgTx, pkScript []byte) bool { for _, txOut := range tx.TxOut { diff --git a/staticaddr/loopin/txout_checker_test.go b/staticaddr/loopin/txout_checker_test.go index b3cdfe981..e9d6b8312 100644 --- a/staticaddr/loopin/txout_checker_test.go +++ b/staticaddr/loopin/txout_checker_test.go @@ -196,6 +196,93 @@ func TestLndTxOutCheckerSpentByTxPayingTo(t *testing.T) { }) } +// TestLndTxOutCheckerConfirmedSpendingTx checks that only a confirmed wallet +// transaction that spends the exact outpoint is returned. +func TestLndTxOutCheckerConfirmedSpendingTx(t *testing.T) { + outpoint := wire.OutPoint{Hash: [32]byte{1}, Index: 1} + sameHashOutpoint := wire.OutPoint{Hash: [32]byte{1}, Index: 0} + otherOutpoint := wire.OutPoint{Hash: [32]byte{2}, Index: 1} + + const startHeight int32 = 500 + + spendingTx := func(prevs ...wire.OutPoint) *wire.MsgTx { + tx := wire.NewMsgTx(2) + for _, prev := range prevs { + tx.AddTxIn(wire.NewTxIn(&prev, nil, nil)) + } + tx.AddTxOut(wire.NewTxOut(1000, []byte{0x51, 0x20, 0x01})) + + return tx + } + + spend := spendingTx(otherOutpoint, outpoint) + + tests := []struct { + name string + txs []lndclient.Transaction + want *wire.MsgTx + }{{ + name: "confirmed spend in a later input", + txs: []lndclient.Transaction{{ + Tx: spendingTx(otherOutpoint), + Confirmations: 3, + }, { + Tx: spend, + Confirmations: 1, + }}, + want: spend, + }, { + name: "unconfirmed spend", + txs: []lndclient.Transaction{{ + Tx: spendingTx(outpoint), + }}, + }, { + name: "spend of another output of the same tx", + txs: []lndclient.Transaction{{ + Tx: spendingTx(sameHashOutpoint), + Confirmations: 1, + }}, + }, { + name: "no wallet transactions", + }, { + name: "transaction without raw tx", + txs: []lndclient.Transaction{{ + Confirmations: 1, + PreviousOutpoints: []*lnrpc.PreviousOutPoint{{ + Outpoint: outpoint.String(), + }}, + }}, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := &mockTxListLightningClient{txs: tc.txs} + checker := NewLndTxOutChecker(client) + + tx, err := checker.ConfirmedSpendingTx( + t.Context(), outpoint, startHeight, + ) + require.NoError(t, err) + require.Equal(t, tc.want, tx) + require.Equal(t, []txListCall{{ + startHeight: startHeight, + endHeight: -1, + }}, client.calls) + }) + } + + t.Run("returns error", func(t *testing.T) { + expectedErr := errors.New("list transactions failed") + client := &mockTxListLightningClient{err: expectedErr} + checker := NewLndTxOutChecker(client) + + _, err := checker.ConfirmedSpendingTx( + t.Context(), outpoint, startHeight, + ) + require.ErrorIs(t, err, expectedErr) + }) +} + type txListCall struct { startHeight int32 endHeight int32