diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index be6953fa5..2f0f994aa 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -162,6 +162,12 @@ close transaction. tests to set a lower value for faster funding confirmation timeout while keeping the default of 2016 blocks for production stability. +* Previously when sweeping inputs, the sweeper will wait until the specified + budget can be covered by the inputs, which could cause the sweep to be + delayed, and the sweeping tx ends up using large fees. This is now + [changed](https://github.com/lightningnetwork/lnd/pull/9627) so the sweeper + will always attempt the sweep as long as the budget can be partially covered. + ## RPC Additions * [Add a new rpc endpoint](https://github.com/lightningnetwork/lnd/pull/8843) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 139a1917f..168465fb8 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -690,6 +690,10 @@ var allTestCases = []*lntest.TestCase{ Name: "rbf coop close", TestFunc: testCoopCloseRbf, }, + { + Name: "bump fee low budget", + TestFunc: testBumpFeeLowBudget, + }, } // appendPrefixed is used to add a prefix to each test name in the subtests diff --git a/itest/lnd_funding_test.go b/itest/lnd_funding_test.go index 4a984a06e..52f0f7805 100644 --- a/itest/lnd_funding_test.go +++ b/itest/lnd_funding_test.go @@ -8,6 +8,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/labels" @@ -466,20 +467,29 @@ func runChannelFundingInputTypes(ht *lntest.HarnessTest, alice, burnAddr = "bcrt1qxsnqpdc842lu8c0xlllgvejt6rhy49u6fmpgyz" ) - fundMixed := func(amt btcutil.Amount, target *node.HarnessNode) { + fundMixed := func(amt btcutil.Amount, + target *node.HarnessNode) *wire.MsgTx { + ht.FundCoins(amt/5, target) ht.FundCoins(amt/5, target) ht.FundCoinsP2TR(amt/5, target) ht.FundCoinsP2TR(amt/5, target) ht.FundCoinsP2TR(amt/5, target) + + return nil } - fundMultipleP2TR := func(amt btcutil.Amount, target *node.HarnessNode) { + fundMultipleP2TR := func(amt btcutil.Amount, + target *node.HarnessNode) *wire.MsgTx { + ht.FundCoinsP2TR(amt/4, target) ht.FundCoinsP2TR(amt/4, target) ht.FundCoinsP2TR(amt/4, target) ht.FundCoinsP2TR(amt/4, target) + + return nil } - fundWithTypes := []func(amt btcutil.Amount, target *node.HarnessNode){ + fundWithTypes := []func(amt btcutil.Amount, + target *node.HarnessNode) *wire.MsgTx{ ht.FundCoins, ht.FundCoinsNP2WKH, ht.FundCoinsP2TR, fundMixed, fundMultipleP2TR, } diff --git a/itest/lnd_multi-hop_force_close_test.go b/itest/lnd_multi-hop_force_close_test.go index 84f8c8d44..c542c81ac 100644 --- a/itest/lnd_multi-hop_force_close_test.go +++ b/itest/lnd_multi-hop_force_close_test.go @@ -2570,7 +2570,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest, // We'll now mine enough blocks so Carol decides that she needs to go // on-chain to claim the HTLC as Bob has been inactive. numBlocks := padCLTV( - invoiceExpiry - ht.CurrentHeight() - incomingBroadcastDelta - 1, + invoiceExpiry - ht.CurrentHeight() - incomingBroadcastDelta, ) ht.MineBlocks(int(numBlocks)) diff --git a/itest/lnd_sweep_test.go b/itest/lnd_sweep_test.go index 63c489853..d5c16b0bd 100644 --- a/itest/lnd_sweep_test.go +++ b/itest/lnd_sweep_test.go @@ -1606,40 +1606,62 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) { assertPendingSweepResp := func(broadcastAttempts uint32, budget uint64, deadline uint32, startingFeeRate uint64) *wire.MsgTx { - // Alice should still have one pending sweep. - pendingSweep := ht.AssertNumPendingSweeps(alice, 1)[0] + err := wait.NoError(func() error { + // Alice should still have one pending sweep. + ps := ht.AssertNumPendingSweeps(alice, 1)[0] - // Validate all fields returned from `PendingSweeps` are as - // expected. - require.Equal(ht, op.TxidBytes, pendingSweep.Outpoint.TxidBytes) - require.Equal(ht, op.OutputIndex, - pendingSweep.Outpoint.OutputIndex) - require.Equal(ht, walletrpc.WitnessType_TAPROOT_PUB_KEY_SPEND, - pendingSweep.WitnessType) - require.EqualValuesf(ht, value, pendingSweep.AmountSat, - "amount not matched: want=%d, got=%d", value, - pendingSweep.AmountSat) - require.True(ht, pendingSweep.Immediate) + // Validate all fields returned from `PendingSweeps` are + // as expected. + // + // These fields should stay the same during the test so + // we assert the values without wait. + require.Equal(ht, op.TxidBytes, ps.Outpoint.TxidBytes) + require.Equal(ht, op.OutputIndex, + ps.Outpoint.OutputIndex) + require.Equal(ht, + walletrpc.WitnessType_TAPROOT_PUB_KEY_SPEND, + ps.WitnessType) + require.EqualValuesf(ht, value, ps.AmountSat, + "amount not matched: want=%d, got=%d", value, + ps.AmountSat) - require.Equal(ht, broadcastAttempts, - pendingSweep.BroadcastAttempts) - require.EqualValuesf(ht, budget, pendingSweep.Budget, - "budget not matched: want=%d, got=%d", budget, - pendingSweep.Budget) + // The following fields can change during the test so we + // return an error if they don't match, which will be + // checked again in this wait call. + if ps.Immediate != true { + return fmt.Errorf("Immediate should be true") + } - // Since the request doesn't specify a deadline, we expect the - // existing deadline to be used. - require.Equalf(ht, deadline, pendingSweep.DeadlineHeight, - "deadline height not matched: want=%d, got=%d", - deadline, pendingSweep.DeadlineHeight) + if broadcastAttempts != ps.BroadcastAttempts { + return fmt.Errorf("broadcastAttempts not "+ + "matched: want=%d, got=%d", + broadcastAttempts, ps.BroadcastAttempts) + } + if budget != ps.Budget { + return fmt.Errorf("budget not matched: "+ + "want=%d, got=%d", budget, ps.Budget) + } - // Since the request specifies a starting fee rate, we expect - // that to be used as the starting fee rate. - require.Equalf(ht, startingFeeRate, - pendingSweep.RequestedSatPerVbyte, "requested "+ - "starting fee rate not matched: want=%d, "+ - "got=%d", startingFeeRate, - pendingSweep.RequestedSatPerVbyte) + // Since the request doesn't specify a deadline, we + // expect the existing deadline to be used. + if deadline != ps.DeadlineHeight { + return fmt.Errorf("deadline height not "+ + "matched: want=%d, got=%d", deadline, + ps.DeadlineHeight) + } + + // Since the request specifies a starting fee rate, we + // expect that to be used as the starting fee rate. + if startingFeeRate != ps.RequestedSatPerVbyte { + return fmt.Errorf("requested starting fee "+ + "rate not matched: want=%d, got=%d", + startingFeeRate, + ps.RequestedSatPerVbyte) + } + + return nil + }, wait.DefaultTimeout) + require.NoError(ht, err, "timeout checking pending sweep") // We expect to see Alice's original tx and her CPFP tx in the // mempool. @@ -1879,7 +1901,7 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) { // Finally, we test the behavior of lowering the fee rate. The fee func // that has, // - starting fee rate: 1 sat/vbyte. - // - deadline: 1008. + // - deadline: 1. // - budget: 1000 sats. bumpFeeReq = &walletrpc.BumpFeeRequest{ Outpoint: op, @@ -1888,22 +1910,45 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) { SatPerVbyte: startFeeRate, // The budget and the deadline delta must be set together. Budget: smallBudget, - DeadlineDelta: uint32(sweep.DefaultDeadlineDelta), + DeadlineDelta: 1, } alice.RPC.BumpFee(bumpFeeReq) + // Calculate the ending fee rate, which is used in the above fee bump + // when fee function's max posistion is reached. + txWeight := ht.CalculateTxWeight(sweepTx6) + endingFeeRate := chainfee.NewSatPerKWeight( + btcutil.Amount(smallBudget), txWeight, + ) + + // Since the fee function has been maxed out, the starting fee rate for + // the next sweep attempt should be the ending fee rate. + // + // TODO(yy): The weight estimator used in the sweeper gives a different + // result than the weight calculated here, which is the result from + // `blockchain.GetTransactionWeight`. For this particular tx: + // - result from the `weightEstimator`: 445 wu + // - result from `GetTransactionWeight`: 444 wu + // + // This means the fee rates are different, + // - `weightEstimator`: 2247 sat/kw, or 8 sat/vb (8.988 round down) + // - here we have 2252 sat/kw, or 9 sat/vb (9.008 round down) + // + // We should investigate and check whether if it's possible to make the + // `weightEstimator` more accurate. + expectedStartFeeRate := uint64(endingFeeRate.FeePerVByte()) - 1 + // Assert the pending sweep is created with the expected values: // - broadcast attempts: 7. - // - starting fee rate: 1 sat/vbyte. - // - deadline: 1008. + // - starting fee rate: 8 sat/vbyte. + // - deadline: 1. // - budget: 1000 sats. sweepTx7 := assertPendingSweepResp( - 7, smallBudget, deadline, startFeeRate, + 7, smallBudget, uint32(currentHeight+1), expectedStartFeeRate, ) // Since this budget is too small to cover the RBF, we expect the // sweeping attempt to fail. - // require.Equal(ht, sweepTx6.TxHash(), sweepTx7.TxHash(), "tx6 should "+ "not be replaced: tx6=%v, tx7=%v", sweepTx6.TxHash(), sweepTx7.TxHash()) @@ -1912,7 +1957,7 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) { // initial broadcast always succeeds. assertFeeRateGreater(testFeeRate) - // Clean up the mempol. + // Clean up the mempool. ht.MineBlocksAndAssertNumTxes(1, 2) } @@ -2328,6 +2373,181 @@ func testFeeReplacement(ht *lntest.HarnessTest) { uint64(feeRate), 0.02, "want %d, got %d in tx=%v", currentFeeRate, feeRate, outgoingSweep2.TxHash()) - // Finally, clean the mempol. + // Finally, clean the mempool. ht.MineBlocksAndAssertNumTxes(1, 1) } + +// testBumpFeeLowBudget checks that when the requested ideal budget cannot be +// met, the sweeper still sweeps the input with the actual budget. +func testBumpFeeLowBudget(ht *lntest.HarnessTest) { + // Create a new node with a large `maxfeerate` so it's easier to run the + // test. + alice := ht.NewNode("Alice", []string{ + "--sweeper.maxfeerate=10000", + }) + + // Fund Alice 2 UTXOs, each has 100k sats. One of the UTXOs will be used + // to create a tx which she sends some coins to herself. The other will + // be used as the budget when CPFPing the above tx. + coin := btcutil.Amount(100_000) + ht.FundCoins(coin, alice) + ht.FundCoins(coin, alice) + + // Alice sends 50k sats to herself. + tx := ht.SendCoins(alice, alice, coin/2) + txid := tx.TxHash() + + // Get Alice's wallet balance to calculate the fees used in the above + // tx. + resp := alice.RPC.WalletBalance() + + // balance is the expected final balance. Alice's initial balance is + // 200k sats, with 100k sats as the budget for the sweeping tx, which + // means her final balance should be 100k sats minus the mining fees + // used in the above `SendCoins`. + balance := btcutil.Amount( + resp.UnconfirmedBalance + resp.ConfirmedBalance, + ) + fee := coin*2 - balance + ht.Logf("Alice's expected final balance=%v, fee=%v", balance, fee) + + // Alice now tries to bump the first output on this tx. + op := &lnrpc.OutPoint{ + TxidBytes: txid[:], + OutputIndex: uint32(0), + } + value := btcutil.Amount(tx.TxOut[0].Value) + + // assertPendingSweepResp is a helper closure that asserts the response + // from `PendingSweep` RPC is returned with expected values. It also + // returns the sweeping tx for further checks. + assertPendingSweepResp := func(budget uint64, + deadline uint32) *wire.MsgTx { + + // Alice should still have one pending sweep. + pendingSweep := ht.AssertNumPendingSweeps(alice, 1)[0] + + // Validate all fields returned from `PendingSweeps` are as + // expected. + require.Equal(ht, op.TxidBytes, pendingSweep.Outpoint.TxidBytes) + require.Equal(ht, op.OutputIndex, + pendingSweep.Outpoint.OutputIndex) + require.Equal(ht, walletrpc.WitnessType_TAPROOT_PUB_KEY_SPEND, + pendingSweep.WitnessType) + require.EqualValuesf(ht, value, pendingSweep.AmountSat, + "amount not matched: want=%d, got=%d", value, + pendingSweep.AmountSat) + require.True(ht, pendingSweep.Immediate) + + require.EqualValuesf(ht, budget, pendingSweep.Budget, + "budget not matched: want=%d, got=%d", budget, + pendingSweep.Budget) + + // Since the request doesn't specify a deadline, we expect the + // existing deadline to be used. + require.Equalf(ht, deadline, pendingSweep.DeadlineHeight, + "deadline height not matched: want=%d, got=%d", + deadline, pendingSweep.DeadlineHeight) + + // We expect to see Alice's original tx and her CPFP tx in the + // mempool. + txns := ht.GetNumTxsFromMempool(2) + + // Find the sweeping tx - assume it's the first item, if it has + // the same txid as the parent tx, use the second item. + sweepTx := txns[0] + if sweepTx.TxHash() == tx.TxHash() { + sweepTx = txns[1] + } + + return sweepTx + } + + // Use a budget that Alice cannot cover using her wallet UTXOs. + budget := coin * 2 + + // Use a deadlineDelta of 3 such that the fee func is initialized as, + // - starting fee rate: 1 sat/vbyte + // - deadline: 3 + // - budget: 200% of Alice's available funds. + deadlineDelta := 3 + + // First bump request - we expect it to succeed as Alice's current funds + // can cover the fees used here given the position of the fee func is at + // 0. + bumpFeeReq := &walletrpc.BumpFeeRequest{ + Outpoint: op, + Budget: uint64(budget), + Immediate: true, + DeadlineDelta: uint32(deadlineDelta), + } + alice.RPC.BumpFee(bumpFeeReq) + + // Calculate the deadline height. + deadline := ht.CurrentHeight() + uint32(deadlineDelta) + + // Assert the pending sweep is created with the expected values: + // - deadline: 3+current height. + // - budget: 2x the wallet balance. + sweepTx1 := assertPendingSweepResp(uint64(budget), deadline) + + // Mine a block to trigger Alice's sweeper to fee bump the tx. + // + // Second bump request - we expect it to succeed as Alice's current + // funds can cover the fees used here, which is 66.7% of her available + // funds given the position of the fee func is at 1. + ht.MineEmptyBlocks(1) + + // Assert the old sweeping tx has been replaced. + ht.AssertTxNotInMempool(sweepTx1.TxHash()) + + // Assert a new sweeping tx is made. + sweepTx2 := assertPendingSweepResp(uint64(budget), deadline) + + // Mine a block to trigger Alice's sweeper to fee bump the tx. + // + // Third bump request - we expect it to fail as Alice's current funds + // cannot cover the fees now, which is 133.3% of her available funds + // given the position of the fee func is at 2. + ht.MineEmptyBlocks(1) + + // Assert the above sweeping tx is still in the mempool. + ht.AssertTxInMempool(sweepTx2.TxHash()) + + // Fund Alice 200k sats, which will be used to cover the budget. + // + // TODO(yy): We are funding Alice more than enough - at this stage Alice + // has a confirmed UTXO of `coin` amount in her wallet, so ideally we + // should only fund another UTXO of `coin` amount. However, since the + // confirmed wallet UTXO has already been used in sweepTx2, there's no + // easy way to tell her wallet to reuse that UTXO in the upcoming + // sweeping tx. + // To properly fix it, we should provide more granular UTXO management + // here by leveraing `LeaseOutput` - whenever we use a wallet UTXO, we + // should lock it first. And when the sweeping attempt fails, we should + // release it so the UTXO can be used again in another batch. + walletTx := ht.FundCoinsUnconfirmed(coin*2, alice) + + // Mine a block to confirm the above funding coin. + // + // Fourth bump request - we expect it to succeed as Alice's current + // funds can cover the full budget. + ht.MineBlockWithTx(walletTx) + + flakeRaceInBitcoinClientNotifications(ht) + + // Assert Alice's previous sweeping tx has been replaced. + ht.AssertTxNotInMempool(sweepTx2.TxHash()) + + // Assert the pending sweep is created with the expected values: + // - deadline: 3+current height. + // - budget: 2x the wallet balance. + sweepTx3 := assertPendingSweepResp(uint64(budget), deadline) + require.NotEqual(ht, sweepTx2.TxHash(), sweepTx3.TxHash()) + + // Mine the sweeping tx. + ht.MineBlocksAndAssertNumTxes(1, 2) + + // Assert Alice's wallet balance. a + ht.WaitForBalanceConfirmed(alice, balance) +} diff --git a/lntest/harness.go b/lntest/harness.go index c43cdfd45..d0a36f6dd 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -2,7 +2,6 @@ package lntest import ( "context" - "encoding/hex" "fmt" "strings" "testing" @@ -1425,7 +1424,7 @@ func (h *HarnessTest) IsNeutrinoBackend() bool { // transaction that pays to the target should confirm. For neutrino backend, // the `confirmed` param is ignored. func (h *HarnessTest) fundCoins(amt btcutil.Amount, target *node.HarnessNode, - addrType lnrpc.AddressType, confirmed bool) { + addrType lnrpc.AddressType, confirmed bool) *wire.MsgTx { initialBalance := target.RPC.WalletBalance() @@ -1443,25 +1442,14 @@ func (h *HarnessTest) fundCoins(amt btcutil.Amount, target *node.HarnessNode, PkScript: addrScript, Value: int64(amt), } - h.miner.SendOutput(output, defaultMinerFeeRate) + txid := h.miner.SendOutput(output, defaultMinerFeeRate) - // Encode the pkScript in hex as this the format that it will be - // returned via rpc. - expPkScriptStr := hex.EncodeToString(addrScript) + // Get the funding tx. + tx := h.GetRawTransaction(*txid) + msgTx := tx.MsgTx() - // Now, wait for ListUnspent to show the unconfirmed transaction - // containing the correct pkscript. - // // Since neutrino doesn't support unconfirmed outputs, skip this check. if !h.IsNeutrinoBackend() { - utxos := h.AssertNumUTXOsUnconfirmed(target, 1) - - // Assert that the lone unconfirmed utxo contains the same - // pkscript as the output generated above. - pkScriptStr := utxos[0].PkScript - require.Equal(h, pkScriptStr, expPkScriptStr, - "pkscript mismatch") - expectedBalance := btcutil.Amount( initialBalance.UnconfirmedBalance, ) + amt @@ -1472,48 +1460,56 @@ func (h *HarnessTest) fundCoins(amt btcutil.Amount, target *node.HarnessNode, // the target node's unconfirmed balance reflects the expected balance // and exit. if !confirmed { - return + return msgTx } // Otherwise, we'll generate 1 new blocks to ensure the output gains a // sufficient number of confirmations and wait for the balance to // reflect what's expected. - h.MineBlocksAndAssertNumTxes(1, 1) + h.MineBlockWithTx(msgTx) expectedBalance := btcutil.Amount(initialBalance.ConfirmedBalance) + amt h.WaitForBalanceConfirmed(target, expectedBalance) + + return msgTx } // FundCoins attempts to send amt satoshis from the internal mining node to the // targeted lightning node using a P2WKH address. 1 blocks are mined after in // order to confirm the transaction. -func (h *HarnessTest) FundCoins(amt btcutil.Amount, hn *node.HarnessNode) { - h.fundCoins(amt, hn, lnrpc.AddressType_WITNESS_PUBKEY_HASH, true) +func (h *HarnessTest) FundCoins(amt btcutil.Amount, + hn *node.HarnessNode) *wire.MsgTx { + + return h.fundCoins(amt, hn, lnrpc.AddressType_WITNESS_PUBKEY_HASH, true) } // FundCoinsUnconfirmed attempts to send amt satoshis from the internal mining // node to the targeted lightning node using a P2WKH address. No blocks are // mined after and the UTXOs are unconfirmed. func (h *HarnessTest) FundCoinsUnconfirmed(amt btcutil.Amount, - hn *node.HarnessNode) { + hn *node.HarnessNode) *wire.MsgTx { - h.fundCoins(amt, hn, lnrpc.AddressType_WITNESS_PUBKEY_HASH, false) + return h.fundCoins( + amt, hn, lnrpc.AddressType_WITNESS_PUBKEY_HASH, false, + ) } // FundCoinsNP2WKH attempts to send amt satoshis from the internal mining node // to the targeted lightning node using a NP2WKH address. func (h *HarnessTest) FundCoinsNP2WKH(amt btcutil.Amount, - target *node.HarnessNode) { + target *node.HarnessNode) *wire.MsgTx { - h.fundCoins(amt, target, lnrpc.AddressType_NESTED_PUBKEY_HASH, true) + return h.fundCoins( + amt, target, lnrpc.AddressType_NESTED_PUBKEY_HASH, true, + ) } // FundCoinsP2TR attempts to send amt satoshis from the internal mining node to // the targeted lightning node using a P2TR address. func (h *HarnessTest) FundCoinsP2TR(amt btcutil.Amount, - target *node.HarnessNode) { + target *node.HarnessNode) *wire.MsgTx { - h.fundCoins(amt, target, lnrpc.AddressType_TAPROOT_PUBKEY, true) + return h.fundCoins(amt, target, lnrpc.AddressType_TAPROOT_PUBKEY, true) } // FundNumCoins attempts to send the given number of UTXOs from the internal diff --git a/lntest/harness_miner.go b/lntest/harness_miner.go index cde9663c4..010c3f8ca 100644 --- a/lntest/harness_miner.go +++ b/lntest/harness_miner.go @@ -247,7 +247,15 @@ func (h *HarnessTest) GetBestBlock() (*chainhash.Hash, int32) { // MineBlockWithTx mines a single block to include the specifies tx only. func (h *HarnessTest) MineBlockWithTx(tx *wire.MsgTx) *wire.MsgBlock { - return h.miner.MineBlockWithTx(tx) + // Update the harness's current height. + defer h.updateCurrentHeight() + + block := h.miner.MineBlockWithTx(tx) + + // Finally, make sure all the active nodes are synced. + h.AssertActiveNodesSyncedTo(block.BlockHash()) + + return block } // ConnectToMiner connects the miner to a temp miner. diff --git a/rpcserver.go b/rpcserver.go index 6a3cb9707..ad0c3392a 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -4841,12 +4841,17 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, ) } - // Create a set of the HTLCs found in the remote commitment, which is + // Create two sets of the HTLCs found in the remote commitment, which is // used to decide whether the HTLCs from the local commitment has been // locked in or not. - remoteHTLCs := fn.NewSet[[32]byte]() + remoteIncomingHTLCs := fn.NewSet[uint64]() + remoteOutgoingHTLCs := fn.NewSet[uint64]() for _, htlc := range dbChannel.RemoteCommitment.Htlcs { - remoteHTLCs.Add(htlc.RHash) + if htlc.Incoming { + remoteIncomingHTLCs.Add(htlc.HtlcIndex) + } else { + remoteOutgoingHTLCs.Add(htlc.HtlcIndex) + } } for i, htlc := range localCommit.Htlcs { @@ -4855,7 +4860,10 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, circuitMap := r.server.htlcSwitch.CircuitLookup() - var forwardingChannel, forwardingHtlcIndex uint64 + var ( + forwardingChannel, forwardingHtlcIndex uint64 + lockedIn bool + ) switch { case htlc.Incoming: circuit := circuitMap.LookupCircuit( @@ -4871,6 +4879,8 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, forwardingHtlcIndex = circuit.Outgoing.HtlcID } + lockedIn = remoteIncomingHTLCs.Contains(htlc.HtlcIndex) + case !htlc.Incoming: circuit := circuitMap.LookupOpenCircuit( htlcswitch.CircuitKey{ @@ -4890,6 +4900,8 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, forwardingHtlcIndex = circuit.Incoming.HtlcID } + + lockedIn = remoteOutgoingHTLCs.Contains(htlc.HtlcIndex) } channel.PendingHtlcs[i] = &lnrpc.HTLC{ @@ -4900,7 +4912,7 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, HtlcIndex: htlc.HtlcIndex, ForwardingChannel: forwardingChannel, ForwardingHtlcIndex: forwardingHtlcIndex, - LockedIn: remoteHTLCs.Contains(rHash), + LockedIn: lockedIn, } // Add the Pending Htlc Amount to UnsettledBalance field. @@ -9056,6 +9068,9 @@ func (r *rpcServer) getChainSyncInfo() (*chainSyncInfo, error) { // Exit early if the wallet is not synced. if !isSynced { + rpcsLog.Debugf("Wallet is not synced to height %v yet", + bestHeight) + return info, nil } @@ -9074,6 +9089,9 @@ func (r *rpcServer) getChainSyncInfo() (*chainSyncInfo, error) { // Exit early if the channel graph is not synced. if !isSynced { + rpcsLog.Debugf("Graph is not synced to height %v yet", + bestHeight) + return info, nil } @@ -9084,5 +9102,10 @@ func (r *rpcServer) getChainSyncInfo() (*chainSyncInfo, error) { // Overwrite isSynced and return. info.isSynced = height == bestHeight + if !info.isSynced { + rpcsLog.Debugf("Blockbeat is not synced to height %v yet", + bestHeight) + } + return info, nil } diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index b081fe479..1602165ea 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -1103,17 +1103,36 @@ func (t *TxPublisher) handleInitialTxError(r *monitorRecord, err error) { case errors.Is(err, ErrTxNoOutput): result.Event = TxFailed - // 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. - case errors.Is(err, ErrMaxPosition): - result.Event = TxFailed - // When the error is due to zero fee rate delta, we'll send a TxFailed // so these inputs can be retried in the next block. case errors.Is(err, ErrZeroFeeRateDelta): result.Event = TxFailed + // 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. + 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. + feeRate, err := t.calculateRetryFeeRate(r) + if err != nil { + result.Event = TxFatal + result.Err = err + } + + // Attach the new fee rate. + result.FeeRate = feeRate + // 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): @@ -1259,56 +1278,17 @@ func (t *TxPublisher) createUnknownSpentBumpResult( SpentInputs: r.spentInputs, } - // Get the fee function, which will be used to decided the next fee rate - // to use if the sweeper decides to retry sweeping this input. - feeFunc := r.feeFunction - - // When the record is failed before the initial broadcast is attempted, - // it will have a nil fee func. In this case, we'll create the fee func - // here. - // - // NOTE: Since the current record is failed and will be deleted, we - // don't need to update the record on this fee function. We only need - // the fee rate data so the sweeper can pick up where we left off. - if feeFunc == nil { - f, err := t.initializeFeeFunction(r.req) - // TODO(yy): The only error we would receive here is when the - // pkScript is not recognized by the weightEstimator. What we - // should do instead is to check the pkScript immediately after - // receiving a sweep request so we don't need to check it again, - // which will also save us from error checking from several - // callsites. - if err != nil { - log.Errorf("Failed to create fee func for record %v: "+ - "%v", r.requestID, err) - - // Overwrite the event and error so the sweeper will - // remove this input. - result.Event = TxFatal - result.Err = err - - return result - } - - feeFunc = f - } - - // Since the sweeping tx has been replaced by another party's tx, we - // missed this block window to increase its fee rate. To make sure the - // fee rate stays in the initial line, we now ask the fee function to - // give us the next fee rate as if the sweeping tx were RBFed. This new - // fee rate will be used as the starting fee rate if the upper system - // decides to continue sweeping the rest of the inputs. - _, err := feeFunc.Increment() + // Calculate the next fee rate for the retry. + feeRate, err := t.calculateRetryFeeRate(r) if err != nil { - // The fee function has reached its max position - nothing we - // can do here other than letting the user increase the budget. - log.Errorf("Failed to calculate the next fee rate for "+ - "Record(%v): %v", r.requestID, err) + // Overwrite the event and error so the sweeper will + // remove this input. + result.Event = TxFatal + result.Err = err } // Attach the new fee rate to be used for the next sweeping attempt. - result.FeeRate = feeFunc.FeeRate() + result.FeeRate = feeRate return result } @@ -1757,9 +1737,11 @@ func prepareSweepTx(inputs []input.Input, changePkScript lnwallet.AddrWithKey, // Make sure total output amount is less than total input amount. if requiredOutput+txFee > totalInput { - return 0, noChange, noLocktime, fmt.Errorf("insufficient "+ - "input to create sweep tx: input_sum=%v, "+ - "output_sum=%v", totalInput, requiredOutput+txFee) + log.Errorf("Insufficient input to create sweep tx: "+ + "input_sum=%v, output_sum=%v", totalInput, + requiredOutput+txFee) + + return 0, noChange, noLocktime, ErrNotEnoughInputs } // The value remaining after the required output and fees is the @@ -1861,18 +1843,33 @@ func (t *TxPublisher) handleReplacementTxError(r *monitorRecord, return fn.Some(*bumpResult) } - // If the error is not fee related, we will return a `TxFailed` event - // so this input can be retried. + // Return a failed event to retry the sweep. + event := TxFailed + + // Calculate the next fee rate for the retry. + feeRate, ferr := t.calculateRetryFeeRate(r) + if ferr != nil { + // If there's an error with the fee calculation, we need to + // abort the sweep. + 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: TxFailed, + Event: event, Tx: oldTx, Err: err, requestID: r.requestID, + FeeRate: feeRate, }) - // If the tx doesn't not have enought budget, we will return a result so + // 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) { + if errors.Is(err, ErrNotEnoughBudget) || + errors.Is(err, ErrNotEnoughInputs) { + log.Warnf("Fail to fee bump tx %v: %v", oldTx.TxHash(), err) return result } @@ -1883,3 +1880,59 @@ func (t *TxPublisher) handleReplacementTxError(r *monitorRecord, return result } + +// calculateRetryFeeRate calculates a new fee rate to be used as the starting +// fee rate for the next sweep attempt if the inputs are to be retried. When the +// fee function is nil it will be created here, and an error is returned if the +// fee func cannot be initialized. +func (t *TxPublisher) calculateRetryFeeRate( + r *monitorRecord) (chainfee.SatPerKWeight, error) { + + // Get the fee function, which will be used to decided the next fee rate + // to use if the sweeper decides to retry sweeping this input. + feeFunc := r.feeFunction + + // When the record is failed before the initial broadcast is attempted, + // it will have a nil fee func. In this case, we'll create the fee func + // here. + // + // NOTE: Since the current record is failed and will be deleted, we + // don't need to update the record on this fee function. We only need + // the fee rate data so the sweeper can pick up where we left off. + if feeFunc == nil { + f, err := t.initializeFeeFunction(r.req) + + // TODO(yy): The only error we would receive here is when the + // pkScript is not recognized by the weightEstimator. What we + // should do instead is to check the pkScript immediately after + // receiving a sweep request so we don't need to check it again, + // which will also save us from error checking from several + // callsites. + if err != nil { + log.Errorf("Failed to create fee func for record %v: "+ + "%v", r.requestID, err) + + return 0, err + } + + feeFunc = f + } + + // Since we failed to sweep the inputs, either the sweeping tx has been + // replaced by another party's tx, or the current output values cannot + // cover the budget, we missed this block window to increase its fee + // rate. To make sure the fee rate stays in the initial line, we now ask + // the fee function to give us the next fee rate as if the sweeping tx + // were RBFed. This new fee rate will be used as the starting fee rate + // if the upper system decides to continue sweeping the rest of the + // inputs. + _, err := feeFunc.Increment() + if err != nil { + // The fee function has reached its max position - nothing we + // can do here other than letting the user increase the budget. + log.Errorf("Failed to calculate the next fee rate for "+ + "Record(%v): %v", r.requestID, err) + } + + return feeFunc.FeeRate(), nil +} diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index 64695dbfe..aa5561b5a 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -1134,6 +1134,7 @@ func TestCreateAnPublishFail(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() // Create a testing monitor record. req := createTestBumpRequest() diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 69cf9c5d8..d240f5b34 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -951,7 +951,9 @@ func (s *UtxoSweeper) markInputsPublished(tr *TxRecord, set InputSet) error { } // markInputsPublishFailed marks the list of inputs as failed to be published. -func (s *UtxoSweeper) markInputsPublishFailed(set InputSet) { +func (s *UtxoSweeper) markInputsPublishFailed(set InputSet, + feeRate chainfee.SatPerKWeight) { + // Reschedule sweep. for _, inp := range set.Inputs() { op := inp.OutPoint() @@ -978,6 +980,15 @@ func (s *UtxoSweeper) markInputsPublishFailed(set InputSet) { // Update the input's state. pi.state = PublishFailed + + log.Debugf("Input(%v): updating params: starting fee rate "+ + "[%v -> %v]", op, pi.params.StartingFeeRate, + feeRate) + + // Update the input using the fee rate specified from the + // BumpResult, which should be the starting fee rate to use for + // the next sweeping attempt. + pi.params.StartingFeeRate = fn.Some(feeRate) } } @@ -1699,7 +1710,7 @@ func (s *UtxoSweeper) handleBumpEventTxFailed(resp *bumpResp) { // the inputs specified by the set. // // TODO(yy): should we also remove the failed tx from db? - s.markInputsPublishFailed(resp.set) + s.markInputsPublishFailed(resp.set, resp.result.FeeRate) } // handleBumpEventTxReplaced handles the case where the sweeping tx has been @@ -1948,7 +1959,7 @@ func (s *UtxoSweeper) handleUnknownSpendTx(inp *SweeperInput, tx *wire.MsgTx) { func (s *UtxoSweeper) handleBumpEventTxUnknownSpend(r *bumpResp) { // Mark the inputs as publish failed, which means they will be retried // later. - s.markInputsPublishFailed(r.set) + s.markInputsPublishFailed(r.set, r.result.FeeRate) // Get all the inputs that are not spent in the current sweeping tx. spentInputs := r.result.SpentInputs @@ -1982,15 +1993,9 @@ func (s *UtxoSweeper) handleBumpEventTxUnknownSpend(r *bumpResp) { continue } - log.Debugf("Input(%v): updating params: starting fee rate "+ - "[%v -> %v], immediate [%v -> true]", op, - input.params.StartingFeeRate, r.result.FeeRate, - input.params.Immediate) + log.Debugf("Input(%v): updating params: immediate [%v -> true]", + op, r.result.FeeRate, input.params.Immediate) - // Update the input using the fee rate specified from the - // BumpResult, which should be the starting fee rate to use for - // the next sweeping attempt. - input.params.StartingFeeRate = fn.Some(r.result.FeeRate) input.params.Immediate = true inputsToRetry = append(inputsToRetry, input) } diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index d1fbee1f6..d97fd9925 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -232,39 +232,51 @@ func TestMarkInputsPublishFailed(t *testing.T) { inputPublishFailed, inputSwept, inputExcluded, inputFatal, }) + feeRate := chainfee.SatPerKWeight(1000) + // Mark the test inputs. We expect the non-exist input and the // inputInit to be skipped, and the final input to be marked as // published. - s.markInputsPublishFailed(set) + s.markInputsPublishFailed(set, feeRate) // We expect unchanged number of pending inputs. require.Len(s.inputs, 7) // We expect the init input's state to stay unchanged. - require.Equal(Init, - s.inputs[inputInit.OutPoint()].state) + pi := s.inputs[inputInit.OutPoint()] + require.Equal(Init, pi.state) + require.True(pi.params.StartingFeeRate.IsNone()) // We expect the pending-publish input's is now marked as publish // failed. - require.Equal(PublishFailed, - s.inputs[inputPendingPublish.OutPoint()].state) + pi = s.inputs[inputPendingPublish.OutPoint()] + require.Equal(PublishFailed, pi.state) + require.Equal(feeRate, pi.params.StartingFeeRate.UnsafeFromSome()) // We expect the published input's is now marked as publish failed. - require.Equal(PublishFailed, - s.inputs[inputPublished.OutPoint()].state) + pi = s.inputs[inputPublished.OutPoint()] + require.Equal(PublishFailed, pi.state) + require.Equal(feeRate, pi.params.StartingFeeRate.UnsafeFromSome()) // We expect the publish failed input to stay unchanged. - require.Equal(PublishFailed, - s.inputs[inputPublishFailed.OutPoint()].state) + pi = s.inputs[inputPublishFailed.OutPoint()] + require.Equal(PublishFailed, pi.state) + require.True(pi.params.StartingFeeRate.IsNone()) // We expect the swept input to stay unchanged. - require.Equal(Swept, s.inputs[inputSwept.OutPoint()].state) + pi = s.inputs[inputSwept.OutPoint()] + require.Equal(Swept, pi.state) + require.True(pi.params.StartingFeeRate.IsNone()) // We expect the excluded input to stay unchanged. - require.Equal(Excluded, s.inputs[inputExcluded.OutPoint()].state) + pi = s.inputs[inputExcluded.OutPoint()] + require.Equal(Excluded, pi.state) + require.True(pi.params.StartingFeeRate.IsNone()) - // We expect the failed input to stay unchanged. - require.Equal(Fatal, s.inputs[inputFatal.OutPoint()].state) + // We expect the fatal input to stay unchanged. + pi = s.inputs[inputFatal.OutPoint()] + require.Equal(Fatal, pi.state) + require.True(pi.params.StartingFeeRate.IsNone()) // Assert mocked statements are executed as expected. mockStore.AssertExpectations(t) diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index b80d52b0e..7b533c232 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -244,6 +244,28 @@ func (b *BudgetInputSet) addInput(input SweeperInput) { b.inputs = append(b.inputs, &input) } +// addWalletInput takes a wallet UTXO and adds it as an input to be used as +// budget for the input set. +func (b *BudgetInputSet) addWalletInput(utxo *lnwallet.Utxo) error { + input, err := createWalletTxInput(utxo) + if err != nil { + return err + } + + pi := SweeperInput{ + Input: input, + params: Params{ + DeadlineHeight: fn.Some(b.deadlineHeight), + }, + } + b.addInput(pi) + + log.Debugf("Added wallet input to input set: op=%v, amt=%v", + pi.OutPoint(), utxo.Value) + + return nil +} + // NeedWalletInput returns true if the input set needs more wallet inputs. // // A set may need wallet inputs when it has a required output or its total @@ -274,7 +296,9 @@ func (b *BudgetInputSet) NeedWalletInput() bool { } // Get the amount left after covering the input's own budget. - // This amount can then be lent to the above input. + // This amount can then be lent to the above input. For a wallet + // input, its `Budget` is set to zero, which means the whole + // input can be borrowed to cover the budget. budget := inp.params.Budget output := btcutil.Amount(inp.SignDesc().Output.Value) budgetBorrowable += output - budget @@ -296,11 +320,19 @@ func (b *BudgetInputSet) NeedWalletInput() bool { return budgetBorrowable < budgetNeeded } -// copyInputs returns a copy of the slice of the inputs in the set. -func (b *BudgetInputSet) copyInputs() []*SweeperInput { - inputs := make([]*SweeperInput, len(b.inputs)) - copy(inputs, b.inputs) - return inputs +// hasNormalInput return a bool to indicate whether there exists an input that +// doesn't require a TxOut. When an input has no required outputs, it's either a +// wallet input, or an input we want to sweep. +func (b *BudgetInputSet) hasNormalInput() bool { + for _, inp := range b.inputs { + if inp.RequiredTxOut() != nil { + continue + } + + return true + } + + return false } // AddWalletInputs adds wallet inputs to the set until the specified budget is @@ -335,40 +367,35 @@ func (b *BudgetInputSet) AddWalletInputs(wallet Wallet) error { return utxos[i].Value < utxos[j].Value }) - // Make a copy of the current inputs. If the wallet doesn't have enough - // utxos to cover the budget, we will revert the current set to its - // original state by removing the added wallet inputs. - originalInputs := b.copyInputs() - // Add wallet inputs to the set until the specified budget is covered. for _, utxo := range utxos { - input, err := createWalletTxInput(utxo) + err := b.addWalletInput(utxo) if err != nil { return err } - pi := SweeperInput{ - Input: input, - params: Params{ - DeadlineHeight: fn.Some(b.deadlineHeight), - }, - } - b.addInput(pi) - - log.Debugf("Added wallet input to input set: op=%v, amt=%v", - pi.OutPoint(), utxo.Value) - // Return if we've reached the minimum output amount. if !b.NeedWalletInput() { return nil } } - // The wallet doesn't have enough utxos to cover the budget. Revert the - // input set to its original state. - b.inputs = originalInputs + // Exit if there are no inputs can contribute to the fees. + if !b.hasNormalInput() { + return ErrNotEnoughInputs + } - return ErrNotEnoughInputs + // If there's at least one input that can contribute to fees, we allow + // the sweep to continue, even though the full budget can't be met. + // Maybe later more wallet inputs will become available and we can add + // them if needed. + budget := b.Budget() + total, spendable := b.inputAmts() + log.Warnf("Not enough wallet UTXOs: need budget=%v, has spendable=%v, "+ + "total=%v, missing at least %v, sweeping anyway...", budget, + spendable, total, budget-spendable) + + return nil } // Budget returns the total budget of the set. @@ -404,6 +431,28 @@ func (b *BudgetInputSet) Inputs() []input.Input { return inputs } +// inputAmts returns two values for the set - the total input amount, and the +// spendable amount. Only the spendable amount can be used to pay the fees. +func (b *BudgetInputSet) inputAmts() (btcutil.Amount, btcutil.Amount) { + var ( + totalAmt btcutil.Amount + spendableAmt btcutil.Amount + ) + + for _, inp := range b.inputs { + output := btcutil.Amount(inp.SignDesc().Output.Value) + totalAmt += output + + if inp.RequiredTxOut() != nil { + continue + } + + spendableAmt += output + } + + return totalAmt, spendableAmt +} + // StartingFeeRate returns the max starting fee rate found in the inputs. // // NOTE: part of the InputSet interface. diff --git a/sweep/tx_input_set_test.go b/sweep/tx_input_set_test.go index 73f056a96..159824878 100644 --- a/sweep/tx_input_set_test.go +++ b/sweep/tx_input_set_test.go @@ -133,6 +133,47 @@ func TestBudgetInputSetAddInput(t *testing.T) { require.Equal(t, btcutil.Amount(200), set.Budget()) } +// TestAddWalletInput asserts `addWalletInput` successfully converts a wallet +// UTXO into a `SweeperInput` with the correct deadline. +func TestAddWalletInput(t *testing.T) { + t.Parallel() + + // Create a testing deadline. + deadline := int32(1000) + + // Initialize an empty input set. + set := &BudgetInputSet{ + deadlineHeight: deadline, + } + + // Create an utxo with unknown address type to trigger an error. + utxo := &lnwallet.Utxo{ + AddressType: lnwallet.UnknownAddressType, + } + + // Check that the error is returned from addWalletInput. + err := set.addWalletInput(utxo) + require.Error(t, err) + + // Create a wallet utxo. + utxo = &lnwallet.Utxo{ + AddressType: lnwallet.WitnessPubKey, + Value: 1000, + } + + // Check that no error is returned from addWalletInput. + err = set.addWalletInput(utxo) + require.NoError(t, err) + + // Check the input has been added to the set. + require.Len(t, set.inputs, 1) + + // Assert the wallet input is added using the set's deadline. + inp := set.inputs[0] + require.True(t, inp.params.DeadlineHeight.IsSome()) + require.Equal(t, deadline, inp.params.DeadlineHeight.UnsafeFromSome()) +} + // TestNeedWalletInput checks that NeedWalletInput correctly determines if a // wallet input is needed. func TestNeedWalletInput(t *testing.T) { @@ -345,12 +386,12 @@ func TestNeedWalletInput(t *testing.T) { } } -// TestAddWalletInputReturnErr tests the three possible errors returned from +// TestAddWalletInputsReturnErr tests the three possible errors returned from // AddWalletInputs: // - error from ListUnspentWitnessFromDefaultAccount. // - error from createWalletTxInput. // - error when wallet doesn't have utxos. -func TestAddWalletInputReturnErr(t *testing.T) { +func TestAddWalletInputsReturnErr(t *testing.T) { t.Parallel() wallet := &MockWallet{} @@ -395,10 +436,9 @@ func TestAddWalletInputReturnErr(t *testing.T) { require.ErrorIs(t, err, ErrNotEnoughInputs) } -// TestAddWalletInputNotEnoughInputs checks that when there are not enough -// wallet utxos, an error is returned and the budget set is reset to its -// initial state. -func TestAddWalletInputNotEnoughInputs(t *testing.T) { +// TestAddWalletInputsNotEnoughInputs checks that when there are not enough +// wallet utxos, no error is returned as long as the wallet is not empty. +func TestAddWalletInputsNotEnoughInputs(t *testing.T) { t.Parallel() wallet := &MockWallet{} @@ -416,6 +456,13 @@ func TestAddWalletInputNotEnoughInputs(t *testing.T) { mockInput.On("RequiredTxOut").Return(&wire.TxOut{}) defer mockInput.AssertExpectations(t) + sd := &input.SignDescriptor{ + Output: &wire.TxOut{ + Value: budget, + }, + } + mockInput.On("SignDesc").Return(sd).Once() + // Create a pending input that requires 10k satoshis. pi := &SweeperInput{ Input: mockInput, @@ -435,19 +482,83 @@ func TestAddWalletInputNotEnoughInputs(t *testing.T) { // Initialize an input set with the pending input. set := BudgetInputSet{inputs: []*SweeperInput{pi}} - // Add wallet inputs to the input set, which should give us an error as - // the wallet cannot cover the budget. + // Add wallet inputs to the input set, which should return no error + // although the wallet cannot cover the budget. err := set.AddWalletInputs(wallet) - require.ErrorIs(t, err, ErrNotEnoughInputs) + require.NoError(t, err) - // Check that the budget set is reverted to its initial state. - require.Len(t, set.inputs, 1) - require.Equal(t, pi, set.inputs[0]) + // Check that the budget set is updated. + require.Len(t, set.inputs, 2) } -// TestAddWalletInputSuccess checks that when there are enough wallet utxos, +// TestAddWalletInputsEmptyWalletSuccess checks that when the wallet is empty, +// if there is a normal input, no error is returned. +func TestAddWalletInputsEmptyWalletSuccess(t *testing.T) { + t.Parallel() + + wallet := &MockWallet{} + defer wallet.AssertExpectations(t) + + // Specify the min and max confs used in + // ListUnspentWitnessFromDefaultAccount. + minConf, maxConf := int32(1), int32(math.MaxInt32) + + // Assume the desired budget is 10k satoshis. + const budget = 10_000 + + // Create a mock input that has required outputs. + mockInput1 := &input.MockInput{} + defer mockInput1.AssertExpectations(t) + + mockInput1.On("RequiredTxOut").Return(&wire.TxOut{}) + + sd := &input.SignDescriptor{ + Output: &wire.TxOut{ + Value: budget, + }, + } + mockInput1.On("SignDesc").Return(sd).Once() + + // Create a pending input that requires 10k satoshis. + pi1 := &SweeperInput{ + Input: mockInput1, + params: Params{Budget: budget}, + } + + // Create a mock input that doesn't require outputs. + mockInput2 := &input.MockInput{} + defer mockInput2.AssertExpectations(t) + + mockInput2.On("RequiredTxOut").Return(nil) + sd2 := &input.SignDescriptor{ + Output: &wire.TxOut{ + Value: budget, + }, + } + mockInput2.On("SignDesc").Return(sd2).Once() + + // Create a pending input that requires 10k satoshis. + pi2 := &SweeperInput{ + Input: mockInput2, + params: Params{Budget: budget}, + } + + // Mock the wallet to return empty utxos. + wallet.On("ListUnspentWitnessFromDefaultAccount", + minConf, maxConf).Return([]*lnwallet.Utxo{}, nil).Once() + + // Initialize an input set with the pending inputs. + set := BudgetInputSet{inputs: []*SweeperInput{pi1, pi2}} + + // Add wallet inputs to the input set, which should return no error + // although the wallet is empty. + err := set.AddWalletInputs(wallet) + require.NoError(t, err) +} + +// TestAddWalletInputsSuccess checks that when there are enough wallet utxos, // they are added to the input set. -func TestAddWalletInputSuccess(t *testing.T) { +func TestAddWalletInputsSuccess(t *testing.T) { t.Parallel() wallet := &MockWallet{}