From 79715a7a62126e439083d6567569515c40c349c1 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Mon, 18 May 2026 20:22:18 -0230 Subject: [PATCH 1/2] sweep: lease wallet UTXOs claimed for fee-paying InputSets When the sweeper processes multiple pending inputs in a single coin-select-lock cycle, each one independently asks the wallet for a UTXO to cover fees. Nothing in that loop communicates "this UTXO is already claimed", so two concurrent sweeps can pick the same one; when the winner publishes and burns it, the loser is left referencing a spent input and can loop on retries without finding a free UTXO if the wallet view is slow to reflect the spend. Fix: lease each chosen wallet UTXO on the wallet under a sweeper- specific lock ID at selection time. Subsequent selections in the same cycle and across the retry window before the sweep tx confirms see a strictly smaller pool. The lease expires after the default duration if the sweep is abandoned, and is naturally consumed when the sweep confirms. --- docs/release-notes/release-notes-0.22.0.md | 10 ++ sweep/interface.go | 15 +++ sweep/mock_test.go | 12 ++ sweep/tx_input_set.go | 32 +++++ sweep/tx_input_set_test.go | 130 +++++++++++++++++++++ sweep/walletsweep.go | 13 +++ 6 files changed, 212 insertions(+) diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index fb31bad5662..5a12db21642 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -26,6 +26,15 @@ [clarifies](https://github.com/lightningnetwork/lnd/issues/10568) the ZMQ port-mismatch warnings so they no longer suggest that the connection failed. +* The sweeper [now leases wallet UTXOs](https://github.com/lightningnetwork/lnd/pull/10816) + it claims for fee-paying inputs, preventing concurrent sweeps within a + single node from contending for the same UTXO. It also no longer + ratchets the starting fee rate on failures that are not fee-related + (e.g. when there are no UTXOs available to cover the fees), so inputs + whose intrinsic budget cannot accommodate a higher rate are not + stranded. This resolves [#7397](https://github.com/lightningnetwork/lnd/issues/7397) + and addresses item 3 of [#8680](https://github.com/lightningnetwork/lnd/issues/8680). + # New Features ## Functional Enhancements @@ -71,3 +80,4 @@ * Boris Nagaev * Erick Cestari +* Jared Tobin diff --git a/sweep/interface.go b/sweep/interface.go index 6c8c2cfad28..acf3aed6b3b 100644 --- a/sweep/interface.go +++ b/sweep/interface.go @@ -1,9 +1,12 @@ package sweep import ( + "time" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" @@ -60,6 +63,18 @@ type Wallet interface { // which could be e.g. btcd, bitcoind, neutrino, or another consensus // service. BackEnd() string + + // LeaseOutput leases a wallet output for the given lock ID and + // duration, preventing it from being returned by subsequent coin + // selection calls (including ListUnspentWitnessFromDefaultAccount). + // The sweeper uses this so that wallet UTXOs it selects as fee inputs + // for one InputSet are not also selected for a sibling InputSet + // processed under the same coin-select lock cycle. The lease's expiry + // reclaims the UTXO if a sweep is abandoned, so no companion release + // API is needed here -- add one if and when an explicit-release + // caller emerges. + LeaseOutput(id wtxmgr.LockID, op wire.OutPoint, + duration time.Duration) (time.Time, error) } // SweepOutput is an output used to sweep funds from a channel output. diff --git a/sweep/mock_test.go b/sweep/mock_test.go index e6e254e8e11..6f6e165d1e1 100644 --- a/sweep/mock_test.go +++ b/sweep/mock_test.go @@ -1,9 +1,12 @@ package sweep import ( + "time" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" @@ -204,6 +207,15 @@ func (m *MockWallet) GetTransactionDetails(txHash *chainhash.Hash) ( return args.Get(0).(*lnwallet.TransactionDetail), args.Error(1) } +// LeaseOutput leases a wallet output for the given lock ID and duration. +func (m *MockWallet) LeaseOutput(id wtxmgr.LockID, op wire.OutPoint, + duration time.Duration) (time.Time, error) { + + args := m.Called(id, op, duration) + + return args.Get(0).(time.Time), args.Error(1) +} + // MockInputSet is a mock implementation of the InputSet interface. type MockInputSet struct { mock.Mock diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 7b533c232f2..aa3364e650e 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -1,6 +1,7 @@ package sweep import ( + "errors" "fmt" "math" "sort" @@ -8,10 +9,12 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" ) var ( @@ -368,12 +371,41 @@ func (b *BudgetInputSet) AddWalletInputs(wallet Wallet) error { }) // Add wallet inputs to the set until the specified budget is covered. + // Each successfully added wallet UTXO is also leased on the wallet so + // that a sibling InputSet processed under the same coin-select lock + // cycle (or before this sweep's tx confirms) does not pick the same + // UTXO and collide. The lease's duration is bounded; if the sweep is + // abandoned, the UTXO returns to the available pool on expiry. for _, utxo := range utxos { err := b.addWalletInput(utxo) if err != nil { return err } + _, err = wallet.LeaseOutput( + LndSweeperLockID, utxo.OutPoint, + chanfunding.DefaultLockDuration, + ) + switch { + // Another lnd subsystem already holds this UTXO. Treat the + // in-memory add as a no-op and try the next UTXO. We don't + // fail the sweep because the next UTXO is just as good. + case errors.Is(err, wtxmgr.ErrOutputAlreadyLocked): + log.Debugf("UTXO %v already leased, skipping", + utxo.OutPoint) + + // Roll back the addWalletInput so the in-memory set + // stays consistent with what the wallet considers + // claimable for this sweep. + b.inputs = b.inputs[:len(b.inputs)-1] + + continue + + case err != nil: + return fmt.Errorf("lease wallet output %v: %w", + utxo.OutPoint, err) + } + // Return if we've reached the minimum output amount. if !b.NeedWalletInput() { return nil diff --git a/sweep/tx_input_set_test.go b/sweep/tx_input_set_test.go index 15982487853..e3fbbc2f123 100644 --- a/sweep/tx_input_set_test.go +++ b/sweep/tx_input_set_test.go @@ -4,13 +4,16 @@ import ( "errors" "math" "testing" + "time" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" "github.com/stretchr/testify/require" ) @@ -479,6 +482,11 @@ func TestAddWalletInputsNotEnoughInputs(t *testing.T) { wallet.On("ListUnspentWitnessFromDefaultAccount", min, max).Return([]*lnwallet.Utxo{utxo}, nil).Once() + // The utxo will be leased on the wallet to prevent collisions + // with sibling InputSets. + wallet.On("LeaseOutput", LndSweeperLockID, utxo.OutPoint, + chanfunding.DefaultLockDuration).Return(time.Time{}, nil).Once() + // Initialize an input set with the pending input. set := BudgetInputSet{inputs: []*SweeperInput{pi}} @@ -603,6 +611,12 @@ func TestAddWalletInputsSuccess(t *testing.T) { wallet.On("ListUnspentWitnessFromDefaultAccount", min, max).Return([]*lnwallet.Utxo{utxo, utxo}, nil).Once() + // Both utxos will be leased on the wallet to prevent collisions + // with sibling InputSets. + wallet.On("LeaseOutput", LndSweeperLockID, utxo.OutPoint, + chanfunding.DefaultLockDuration).Return( + time.Time{}, nil).Twice() + // Initialize an input set with the pending input. set, err := NewBudgetInputSet( []SweeperInput{*pi}, deadline, fn.None[AuxSweeper](), @@ -633,3 +647,119 @@ func TestAddWalletInputsSuccess(t *testing.T) { // Weak check, a strong check is to open the slice and check each item. require.Len(t, set.inputs, 3) } + +// TestAddWalletInputsLeasesOutput asserts that AddWalletInputs leases each +// selected wallet UTXO under the sweeper's lock ID. This is what prevents a +// sibling InputSet processed in the same coin-select-lock cycle from picking +// the same fee UTXO and producing a "tx has no output" collision when the +// winning sweep spends the shared input. +func TestAddWalletInputsLeasesOutput(t *testing.T) { + t.Parallel() + + wallet := &MockWallet{} + defer wallet.AssertExpectations(t) + + minConf, maxConf := int32(1), int32(math.MaxInt32) + + const budget = 10_000 + + mockInput := &input.MockInput{} + mockInput.On("RequiredTxOut").Return(&wire.TxOut{}) + defer mockInput.AssertExpectations(t) + + sd := &input.SignDescriptor{ + Output: &wire.TxOut{Value: budget}, + } + mockInput.On("SignDesc").Return(sd).Once() + + pi := &SweeperInput{ + Input: mockInput, + params: Params{Budget: budget}, + } + + utxo := &lnwallet.Utxo{ + AddressType: lnwallet.WitnessPubKey, + Value: budget - 1, + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{0xab}, + Index: 7, + }, + } + + wallet.On("ListUnspentWitnessFromDefaultAccount", + minConf, maxConf).Return([]*lnwallet.Utxo{utxo}, nil).Once() + + // The wallet must observe a LeaseOutput call for the selected utxo + // under the sweeper's lock ID. + wallet.On("LeaseOutput", LndSweeperLockID, utxo.OutPoint, + chanfunding.DefaultLockDuration).Return(time.Time{}, nil).Once() + + set := BudgetInputSet{inputs: []*SweeperInput{pi}} + + require.NoError(t, set.AddWalletInputs(wallet)) + require.Len(t, set.inputs, 2) +} + +// TestAddWalletInputsSkipsAlreadyLeased asserts that when a candidate wallet +// UTXO is already leased by some other subsystem, AddWalletInputs rolls back +// the in-memory add for that UTXO and continues with the next candidate. +func TestAddWalletInputsSkipsAlreadyLeased(t *testing.T) { + t.Parallel() + + wallet := &MockWallet{} + defer wallet.AssertExpectations(t) + + minConf, maxConf := int32(1), int32(math.MaxInt32) + + const budget = 10_000 + + mockInput := &input.MockInput{} + mockInput.On("RequiredTxOut").Return(&wire.TxOut{}) + defer mockInput.AssertExpectations(t) + + sd := &input.SignDescriptor{ + Output: &wire.TxOut{Value: budget}, + } + mockInput.On("SignDesc").Return(sd).Once() + + pi := &SweeperInput{ + Input: mockInput, + params: Params{Budget: budget}, + } + + lockedUTXO := &lnwallet.Utxo{ + AddressType: lnwallet.WitnessPubKey, + Value: budget - 1, + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{0xaa}, + Index: 1, + }, + } + freeUTXO := &lnwallet.Utxo{ + AddressType: lnwallet.WitnessPubKey, + Value: budget - 1, + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{0xbb}, + Index: 2, + }, + } + + // AddWalletInputs sorts by value ascending; both utxos have the same + // value so the order matches the input slice order. + wallet.On("ListUnspentWitnessFromDefaultAccount", minConf, maxConf). + Return([]*lnwallet.Utxo{lockedUTXO, freeUTXO}, nil).Once() + + wallet.On("LeaseOutput", LndSweeperLockID, lockedUTXO.OutPoint, + chanfunding.DefaultLockDuration).Return( + time.Time{}, wtxmgr.ErrOutputAlreadyLocked).Once() + wallet.On("LeaseOutput", LndSweeperLockID, freeUTXO.OutPoint, + chanfunding.DefaultLockDuration).Return(time.Time{}, nil).Once() + + set := BudgetInputSet{inputs: []*SweeperInput{pi}} + + require.NoError(t, set.AddWalletInputs(wallet)) + + // Only the free UTXO should have been added; the locked one was + // rolled back. + require.Len(t, set.inputs, 2) +} diff --git a/sweep/walletsweep.go b/sweep/walletsweep.go index d814d3b3ba3..63aa43f11f7 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -31,6 +31,19 @@ var ( // ErrUnknownUTXO is returned when creating a sweeping tx using an UTXO // that's unknown to the wallet. ErrUnknownUTXO = errors.New("unknown utxo") + + // LndSweeperLockID is the binary representation of the SHA256 hash of + // the string "lnd-sweeper-lock-id" and is used by the utxo sweeper to + // lease wallet UTXOs it has selected as fee inputs for a pending + // sweep. Distinct from chanfunding.LndInternalLockID so that sweeper + // leases do not interfere with channel-funding leases. The hex value + // is 3884133f5717d2edd2a4be4e142306698297ab317b60be037cd496ecad6442e8. + LndSweeperLockID = wtxmgr.LockID{ + 0x38, 0x84, 0x13, 0x3f, 0x57, 0x17, 0xd2, 0xed, + 0xd2, 0xa4, 0xbe, 0x4e, 0x14, 0x23, 0x06, 0x69, + 0x82, 0x97, 0xab, 0x31, 0x7b, 0x60, 0xbe, 0x03, + 0x7c, 0xd4, 0x96, 0xec, 0xad, 0x64, 0x42, 0xe8, + } ) // FeePreference defines an interface that allows the caller to specify how the From 36a7dab27671a61835a10d82bc547d29a7246c5a Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Mon, 18 May 2026 20:22:38 -0230 Subject: [PATCH 2/2] sweep: don't ratchet starting fee rate on non-fee failures When a sweep fails to publish, the sweeper computes a higher retry fee rate and stores it as the input's new starting rate. That's the right move for fee-related failures, where the broadcast was rejected for not paying enough. But the same machinery also fires on resource failures: no wallet UTXO available, the input itself can't cover the desired fee, and so on. Those failures carry no fee-rate signal, and repeatedly ratcheting the rate on them can push it past the input's intrinsic budget, after which the aggregator silently skips the input and it's stranded. The bug was previously masked by a separate UTXO-collision quirk that produced UnknownSpend rather than a resource error on the loser of a collision; with that fixed (in the preceding commit), this ratchet bug surfaces. Fix: distinguish fee-rate-bearing failures from resource ones at the point where the bump result is produced, and skip the rate ratchet for the latter. The input's existing starting fee rate is preserved across non-fee failures, so the next retry attempts at the same rate the original sweep request specified. --- sweep/fee_bumper.go | 72 ++++++++++++++++++++++--------------- sweep/fee_bumper_test.go | 76 +++++++++++++++++++++++++++++++++++++++- sweep/sweeper.go | 14 ++++++++ sweep/sweeper_test.go | 40 +++++++++++++++++++++ 4 files changed, 173 insertions(+), 29 deletions(-) diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index e0d5d751616..d3ee87c7a09 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -1114,20 +1114,13 @@ func (t *TxPublisher) handleInitialTxError(r *monitorRecord, err error) { // When the error is due to budget being used up, we'll send a TxFailed // so these inputs can be retried with a different group in the next - // block. + // block. The error is fee-related (the linear fee function has run + // past its budget), so we still calculate a retry fee rate. case errors.Is(err, ErrMaxPosition): - fallthrough - - // If the tx doesn't not have enough budget, or if the inputs amounts - // are not sufficient to cover the budget, we will return a TxFailed - // event so the sweeper can handle it by re-clustering the utxos. - case errors.Is(err, ErrNotEnoughInputs), - errors.Is(err, ErrNotEnoughBudget): - result.Event = TxFailed - // Calculate the starting fee rate to be used when retry - // sweeping these inputs. + // Calculate the starting fee rate to be used when retrying + // to sweep these inputs. feeRate, err := t.calculateRetryFeeRate(r) if err != nil { result.Event = TxFatal @@ -1137,6 +1130,21 @@ func (t *TxPublisher) handleInitialTxError(r *monitorRecord, err error) { // Attach the new fee rate. result.FeeRate = feeRate + // If the tx doesn't have enough budget, or if there aren't enough + // wallet inputs available to cover fees, we return a TxFailed event + // so the sweeper can re-cluster and retry on the next block. We + // intentionally do NOT call calculateRetryFeeRate here: these + // failures have no fee-rate dimension to them (an absent wallet + // UTXO is a resource problem, not a fee problem), and ratcheting + // the starting fee rate upward on each retry can permanently + // strand an input whose intrinsic budget cannot cover the higher + // starting rate. Leaving FeeRate at zero signals the sweeper to + // preserve the input's existing starting fee rate. + case errors.Is(err, ErrNotEnoughInputs), + errors.Is(err, ErrNotEnoughBudget): + + result.Event = TxFailed + // When there are missing inputs, we'll create a TxUnknownSpend bump // result here so the rest of the inputs can be retried. case errors.Is(err, ErrInputMissing): @@ -1857,10 +1865,31 @@ func (t *TxPublisher) handleReplacementTxError(r *monitorRecord, return fn.Some(*bumpResult) } - // Return a failed event to retry the sweep. - event := TxFailed + // If the tx doesn't have enough budget, or if there aren't enough + // wallet inputs available to cover fees, we return a TxFailed event + // so the sweeper can re-cluster and retry on the next block. We + // intentionally do NOT call calculateRetryFeeRate for these failure + // modes: they have no fee-rate dimension to them (an absent wallet + // UTXO is a resource problem, not a fee problem). Leaving FeeRate + // at zero signals the sweeper to preserve the input's existing + // starting fee rate; otherwise repeated resource failures would + // ratchet the rate past the input's intrinsic budget and strand it. + if errors.Is(err, ErrNotEnoughBudget) || + errors.Is(err, ErrNotEnoughInputs) { - // Calculate the next fee rate for the retry. + log.Warnf("Fail to fee bump tx %v: %v", oldTx.TxHash(), err) + return fn.Some(BumpResult{ + Event: TxFailed, + Tx: oldTx, + Err: err, + requestID: r.requestID, + }) + } + + // For all other fee-bump failures, treat them as fee-related: ask + // the fee function for the next rate and carry it on the result so + // the sweeper can use it as the starting rate on retry. + event := TxFailed feeRate, ferr := t.calculateRetryFeeRate(r) if ferr != nil { // If there's an error with the fee calculation, we need to @@ -1868,8 +1897,6 @@ func (t *TxPublisher) handleReplacementTxError(r *monitorRecord, event = TxFatal } - // If the error is not fee related, we will return a `TxFailed` event so - // this input can be retried. result := fn.Some(BumpResult{ Event: event, Tx: oldTx, @@ -1878,18 +1905,7 @@ func (t *TxPublisher) handleReplacementTxError(r *monitorRecord, FeeRate: feeRate, }) - // If the tx doesn't not have enough budget, or if the inputs amounts - // are not sufficient to cover the budget, we will return a result so - // the sweeper can handle it by re-clustering the utxos. - if errors.Is(err, ErrNotEnoughBudget) || - errors.Is(err, ErrNotEnoughInputs) { - - log.Warnf("Fail to fee bump tx %v: %v", oldTx.TxHash(), err) - return result - } - - // Otherwise, an unexpected error occurred, we will log an error and let - // the sweeper retry the whole process. + // Log an error and let the sweeper retry the whole process. log.Errorf("Failed to bump tx %v: %v", oldTx.TxHash(), err) return result diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index d697f906b2a..e52428cb16f 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -1135,7 +1135,14 @@ func TestCreateAndPublishFail(t *testing.T) { // Create a test feerate and return it from the mock fee function. feerate := chainfee.SatPerKWeight(1000) m.feeFunc.On("FeeRate").Return(feerate) - m.feeFunc.On("Increment").Return(true, nil).Once() + + // Note: we deliberately do not expect Increment() to be called for + // ErrNotEnoughBudget. Ratcheting the starting fee rate on a non-fee + // failure mode would only strand inputs whose intrinsic budget can't + // accommodate a higher rate; the patch in createAndPublishTx (and + // the matching path in handleInitialTxError) skips the fee-function + // increment for ErrNotEnoughBudget and ErrNotEnoughInputs precisely + // for that reason. // Create a testing monitor record. req := createTestBumpRequest() @@ -1168,6 +1175,10 @@ func TestCreateAndPublishFail(t *testing.T) { require.ErrorIs(t, result.Err, ErrNotEnoughBudget) require.Equal(t, requestID, result.requestID) + // The result must NOT carry a fee rate; ErrNotEnoughBudget is not a + // fee-related failure and ratcheting would be wrong. + require.Zero(t, result.FeeRate) + // Increase the budget and call it again. This time we will mock an // error to be returned from CheckMempoolAcceptance. req.Budget = 1000 @@ -1988,6 +1999,69 @@ func TestHandleInitialBroadcastFail(t *testing.T) { require.Equal(t, 0, tp.subscriberChans.Len()) } +// TestHandleInitialTxErrorNoRatchetOnResourceFailure asserts that +// handleInitialTxError does not call the fee function's Increment method +// when the failure is ErrNotEnoughInputs or ErrNotEnoughBudget. Those +// failure modes carry no fee-rate signal; ratcheting the rate on each +// retry would only strand inputs whose intrinsic budget cannot accommodate +// the increased starting rate. +func TestHandleInitialTxErrorNoRatchetOnResourceFailure(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + err error + }{ + {"ErrNotEnoughInputs", ErrNotEnoughInputs}, + {"ErrNotEnoughBudget", ErrNotEnoughBudget}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + tp, _ := createTestPublisher(t) + + // We intentionally do not stub Increment() on the + // mock fee function: if the code under test calls + // it on a resource failure, testify will panic and + // the test will fail loudly. + + inp := createTestInput(1000, input.WitnessKeyHash) + req := &BumpRequest{ + DeliveryAddress: changePkScript, + Inputs: []input.Input{&inp}, + Budget: btcutil.Amount(1000), + MaxFeeRate: chainfee.SatPerKWeight(10000), + DeadlineHeight: 10, + } + + rec := &monitorRecord{ + requestID: 1, + req: req, + } + + // Subscribe so we can observe the bump result. + subChan := make(chan *BumpResult, 1) + tp.subscriberChans.Store(uint64(1), subChan) + + tp.handleInitialTxError(rec, tc.err) + + select { + case result := <-subChan: + require.Equal(t, TxFailed, result.Event) + require.ErrorIs(t, result.Err, tc.err) + require.Zero(t, result.FeeRate, + "FeeRate must be zero for "+ + "resource failures") + case <-time.After(time.Second): + t.Fatal("timeout waiting for result") + } + }) + } +} + // TestHasInputsSpent checks the expected outpoint:tx map is returned. func TestHasInputsSpent(t *testing.T) { t.Parallel() diff --git a/sweep/sweeper.go b/sweep/sweeper.go index e2163f6373c..9ae727a9563 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -995,6 +995,20 @@ func (s *UtxoSweeper) markInputsPublishFailed(set InputSet, // Update the input's state. pi.state = PublishFailed + // Only ratchet the starting fee rate when the BumpResult + // carries an actual rate. Failures with no fee dimension to + // them (e.g. ErrNotEnoughInputs from the wallet) leave + // FeeRate at zero; preserving the input's existing starting + // rate in that case avoids stranding inputs whose intrinsic + // budget can't accommodate a higher rate. + if feeRate == 0 { + log.Debugf("Input(%v): preserving starting fee rate "+ + "%v across non-fee failure", op, + pi.params.StartingFeeRate) + + continue + } + log.Debugf("Input(%v): updating params: starting fee rate "+ "[%v -> %v]", op, pi.params.StartingFeeRate, feeRate) diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index d97fd992504..4e1ff38a253 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -282,6 +282,46 @@ func TestMarkInputsPublishFailed(t *testing.T) { mockStore.AssertExpectations(t) } +// TestMarkInputsPublishFailedPreservesRateOnZero asserts that when +// markInputsPublishFailed is invoked with feeRate=0, the input's existing +// StartingFeeRate is preserved instead of being overwritten. This is the +// sweeper-side contract that resource-failure callers (handleInitialTxError +// and handleReplacementTxError for ErrNotEnoughInputs / ErrNotEnoughBudget) +// rely on: those failures carry no fee-rate signal, so ratcheting the +// starting rate upward on each retry would be incorrect. +func TestMarkInputsPublishFailedPreservesRateOnZero(t *testing.T) { + t.Parallel() + + mockStore := NewMockSweeperStore() + s := New(&UtxoSweeperConfig{Store: mockStore}) + + // Stage an input in PendingPublish with an existing starting fee + // rate. The rate we set here must not be touched by the call below. + existingRate := chainfee.SatPerKWeight(2500) + inp := createMockInput(t, s, PendingPublish) + s.inputs[inp.OutPoint()].params.StartingFeeRate = + fn.Some(existingRate) + + set := &MockInputSet{} + defer set.AssertExpectations(t) + set.On("Inputs").Return([]input.Input{inp}) + + // Mark the input as publish-failed with feeRate=0, signalling a + // non-fee failure (e.g. ErrNotEnoughInputs). + s.markInputsPublishFailed(set, 0) + + // The input must still be in PublishFailed (state transition is + // independent of the rate signal) but its StartingFeeRate must + // remain the value we set above, not Some(0). + pi := s.inputs[inp.OutPoint()] + require.Equal(t, PublishFailed, pi.state) + require.True(t, pi.params.StartingFeeRate.IsSome()) + require.Equal(t, existingRate, + pi.params.StartingFeeRate.UnsafeFromSome()) + + mockStore.AssertExpectations(t) +} + // TestMarkInputsSwept checks that given a list of inputs with different // states, only the non-terminal state will be marked as `Swept`. func TestMarkInputsSwept(t *testing.T) {