diff --git a/itest/lnd_onchain_test.go b/itest/lnd_onchain_test.go index a34b9a2e478..cd5d40a5a56 100644 --- a/itest/lnd_onchain_test.go +++ b/itest/lnd_onchain_test.go @@ -345,13 +345,43 @@ func testAnchorReservedValue(ht *lntest.HarnessTest) { // The reserved value is now back in Alice's wallet. aliceBalance := waitForConfirmedBalance() + // Send more coins to Alice. + ht.FundCoins(btcutil.SatoshiPerBitcoin, alice) + + // Make sure wallet balance increased. + newBalance := waitForConfirmedBalance() + require.Greater(ht, newBalance, aliceBalance, + "Alice's balance did not increase after receiving a coin") + + // Send all to external address. We already have a coin of value + // reserved to bump anchor outputs, so fee efficient SendAll should keep + // that coin untouched instead of recreating new reserved output. + sweepReq = &lnrpc.SendCoinsRequest{ + Addr: minerAddr.String(), + SendAll: true, + TargetConf: 6, + } + alice.RPC.SendCoins(sweepReq) + + // We'll mine a block which should include the sweep transaction we + // generated above. + block = ht.MineBlocksAndAssertNumTxes(1, 1)[0] + + // The sweep transaction should have 1 input and 1 output. Reserved + // coin is not touched. + sweepTx = block.Transactions[1] + assertNumTxInAndTxOut(sweepTx, 1, 1) + + // Record wallet balance before closing channels. + aliceBalance = waitForConfirmedBalance() + // Alice closes channel, should now be allowed to send everything to an // external address. for _, chanPoint := range chanPoints { ht.CloseChannel(alice, chanPoint) } - newBalance := waitForConfirmedBalance() + newBalance = waitForConfirmedBalance() require.Greater(ht, newBalance, aliceBalance, "Alice's balance did not increase after channel close") diff --git a/rpcserver.go b/rpcserver.go index 465035ea557..ac1aee3040b 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1356,12 +1356,11 @@ func (r *rpcServer) SendCoins(ctx context.Context, // transaction that will sweep ALL outputs from the wallet in a // single transaction. This will be generated in a concurrent // safe manner, so no need to worry about locking. The tx will - // pay to the change address created above if we needed to - // reserve any value, the rest will go to targetAddr. + // pay everything to targetAddr. sweepTxPkg, err := sweep.CraftSweepAllTx( feePerKw, maxFeeRate, uint32(bestHeight), nil, targetAddr, wallet, wallet, wallet.WalletController, - r.server.cc.Signer, minConfs, + r.server.cc.Signer, minConfs, 0, ) if err != nil { return nil, err @@ -1387,38 +1386,66 @@ func (r *rpcServer) SendCoins(ctx context.Context, sweepTxPkg.CancelSweepAttempt() rpcsLog.Debugf("Reserved value %v not satisfied after "+ - "send_all, trying with change output", + "send_all, trying to exclude one input", reservedVal) - // We'll request a change address from the wallet, - // where we'll send this reserved value back to. This - // ensures this is an address the wallet knows about, - // allowing us to pass the reserved value check. - changeAddr, err := r.server.cc.Wallet.NewAddress( - lnwallet.TaprootPubkey, true, - lnwallet.DefaultAccountName, + // Try to exlude an input of value exactly reservedVal. + // It is likely to exist as an output of previous + // SendAll tx. + sweepTxPkg, err = sweep.CraftSweepAllTx( + feePerKw, maxFeeRate, uint32(bestHeight), nil, + targetAddr, wallet, wallet, + wallet.WalletController, r.server.cc.Signer, + minConfs, reservedVal, ) - if err != nil { + missingInput := errors.Is( + err, sweep.ErrMissingInputToSkip, + ) + if err != nil && !missingInput { + // Unexpected error. return nil, err } - // Send the reserved value to this change address, the - // remaining funds will go to the targetAddr. - outputs := []sweep.DeliveryAddr{ - { - Addr: changeAddr, - Amt: reservedVal, - }, - } + // If there is no such input to skip of needed value to + // satisfy reservation requirements, add an output. + if missingInput { + rpcsLog.Debugf("Can not find an input of value"+ + " %v for send_all, trying with change "+ + "output", reservedVal) + + // We'll request a change address from the + // wallet, where we'll send this reserved value + // back to. This ensures this is an address the + // wallet knows about, allowing us to pass the + // reserved value check. + newAddress := r.server.cc.Wallet.NewAddress + changeAddr, err := newAddress( + lnwallet.TaprootPubkey, true, + lnwallet.DefaultAccountName, + ) + if err != nil { + return nil, err + } - sweepTxPkg, err = sweep.CraftSweepAllTx( - feePerKw, maxFeeRate, uint32(bestHeight), - outputs, targetAddr, wallet, wallet, - wallet.WalletController, - r.server.cc.Signer, minConfs, - ) - if err != nil { - return nil, err + // Send the reserved value to this change + // address, the remaining funds will go to the + // targetAddr. + outputs := []sweep.DeliveryAddr{ + { + Addr: changeAddr, + Amt: reservedVal, + }, + } + + sweepTxPkg, err = sweep.CraftSweepAllTx( + feePerKw, maxFeeRate, + uint32(bestHeight), outputs, targetAddr, + wallet, wallet, wallet.WalletController, + r.server.cc.Signer, minConfs, 0, + ) + if err != nil { + return nil, err + } } // Sanity check the new tx by re-doing the check. diff --git a/sweep/walletsweep.go b/sweep/walletsweep.go index 73385506821..fb367ae6bec 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -24,6 +24,10 @@ var ( // ErrFeePreferenceConflict is returned when both a fee rate and a conf // target is set for a fee preference. ErrFeePreferenceConflict = errors.New("fee preference conflict") + + // ErrMissingInputToSkip is returned by CraftSweepAllTx if argument + // skipInputAmount is not 0 and there is no such an input of that value. + ErrMissingInputToSkip = errors.New("no input of desired value") ) // FeePreference defines an interface that allows the caller to specify how the @@ -212,12 +216,15 @@ type DeliveryAddr struct { // leftover amount after these outputs and transaction fee, is sent to a single // output, as specified by the change address. The sweep transaction will be // crafted with the target fee rate, and will use the utxoSource and -// outputLeaser as sources for wallet funds. +// outputLeaser as sources for wallet funds. If skipInputAmount is not 0, one +// input of value exactly skipInputAmount is skipped, if it is present, or an +// error wrapping ErrMissingInputToSkip is returned and coins are not locked. func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, blockHeight uint32, deliveryAddrs []DeliveryAddr, changeAddr btcutil.Address, coinSelectLocker CoinSelectionLocker, utxoSource UtxoSource, outputLeaser OutputLeaser, - signer input.Signer, minConfs int32) (*WalletSweepPackage, error) { + signer input.Signer, minConfs int32, skipInputAmount btcutil.Amount) ( + *WalletSweepPackage, error) { // TODO(roasbeef): turn off ATPL as well when available? @@ -260,6 +267,27 @@ func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, log.Trace("[WithCoinSelectLock] finished fetching UTXOs") + if skipInputAmount != 0 { + // Filter out one input of value skipInputAmount. + utxos2 := make([]*lnwallet.Utxo, 0, len(utxos)) + skipped := false + for _, utxo := range utxos { + if !skipped && utxo.Value == skipInputAmount { + log.Trace("skipped UTXO %v of value %v", + utxo.OutPoint, utxo.Value) + skipped = true + continue + } + utxos2 = append(utxos2, utxo) + } + utxos = utxos2 + + // Make sure we have found and excluded a coin. + if !skipped { + return ErrMissingInputToSkip + } + } + // We'll now lock each UTXO to ensure that other callers don't // attempt to use these UTXOs in transactions while we're // crafting out sweep all transaction. @@ -288,7 +316,7 @@ func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, unlockOutputs() return nil, fmt.Errorf("unable to fetch+lock wallet "+ - "utxos: %v", err) + "utxos: %w", err) } // Now that we've locked all the potential outputs to sweep, we'll diff --git a/sweep/walletsweep_test.go b/sweep/walletsweep_test.go index c4d1681a0fc..9810f45f45f 100644 --- a/sweep/walletsweep_test.go +++ b/sweep/walletsweep_test.go @@ -339,7 +339,7 @@ func TestCraftSweepAllTxCoinSelectFail(t *testing.T) { _, err := CraftSweepAllTx( 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLeaser, - nil, 0, + nil, 0, 0, ) // Since we instructed the coin select locker to fail above, we should @@ -365,7 +365,7 @@ func TestCraftSweepAllTxUnknownWitnessType(t *testing.T) { _, err := CraftSweepAllTx( 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLeaser, - nil, 0, + nil, 0, 0, ) // Since passed in a p2wsh output, which is unknown, we should fail to @@ -399,7 +399,7 @@ func TestCraftSweepAllTx(t *testing.T) { sweepPkg, err := CraftSweepAllTx( 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, - utxoLeaser, signer, 0, + utxoLeaser, signer, 0, 0, ) require.NoError(t, err, "unable to make sweep tx") @@ -440,3 +440,62 @@ func TestCraftSweepAllTx(t *testing.T) { sweepPkg.CancelSweepAttempt() assertUtxosReleased(t, utxoLeaser, testUtxos[:2]) } + +// TestCraftSweepAllTxSkipInput tests that we'll skip the input of requested +// value if it exists and return wrapped ErrMissingInputToSkip, if it doesn't +// exist. In the later case coins are not locked. +func TestCraftSweepAllTxSkipInput(t *testing.T) { + t.Parallel() + + // First, we'll make a mock signer along with a fee estimator, We'll + // use zero fees to we can assert a precise output value. + signer := &mock.DummySigner{} + + // For our UTXO source, we'll pass in all the UTXOs that we know of, + // other than the final one which is of an unknown witness type. + targetUTXOs := testUtxos[:2] + utxoSource := newMockUtxoSource(targetUTXOs) + coinSelectLocker := &mockCoinSelectionLocker{} + utxoLeaser := newMockOutputLeaser() + + // Try to skip a coin of value 1500. There is no such coin. + const skipInputAmountMissing = 1500 + _, err := CraftSweepAllTx( + 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, + utxoLeaser, signer, 0, skipInputAmountMissing, + ) + require.ErrorIs(t, err, ErrMissingInputToSkip) + + // No UTXOs should be leased. + require.Empty(t, utxoLeaser.leasedOutputs) + + // Now skip a coin of value 1000. There is a coin of that value. + const skipInputAmount = 1000 + sweepPkg, err := CraftSweepAllTx( + 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, + utxoLeaser, signer, 0, skipInputAmount, + ) + require.NoError(t, err, "unable to make sweep tx") + + // At this point, the second UTXO that we made above should be locked + // and none of them unlocked. + assertUtxosLeased(t, utxoLeaser, testUtxos[1:2]) + assertNoUtxosReleased(t, utxoLeaser, testUtxos[1:2]) + + // The transaction is expected to have the second UTXO as its input + // and to have a single output. + sweepTx := sweepPkg.SweepTx + require.Equal(t, 1, len(sweepTx.TxIn)) + + // We should have a single output that pays to our sweep script + // generated above. + require.Equal(t, 1, len(sweepTx.TxOut)) + output := sweepTx.TxOut[0] + require.Equal(t, int64(2000), output.Value) + require.Equal(t, sweepScript, output.PkScript) + + // If we cancel the sweep attempt, then we should find that the second + // UTXO is now unlocked. + sweepPkg.CancelSweepAttempt() + assertUtxosReleased(t, utxoLeaser, testUtxos[1:2]) +}