diff --git a/cmd/lncli/walletrpc_active.go b/cmd/lncli/walletrpc_active.go index f09560169..3e01ecf14 100644 --- a/cmd/lncli/walletrpc_active.go +++ b/cmd/lncli/walletrpc_active.go @@ -22,6 +22,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/walletrpc" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" "github.com/urfave/cli" ) @@ -1537,7 +1538,7 @@ func releaseOutput(ctx *cli.Context) error { return fmt.Errorf("error parsing outpoint: %w", err) } - lockID := walletrpc.LndInternalLockID[:] + lockID := chanfunding.LndInternalLockID[:] lockIDStr := ctx.String("lockid") if lockIDStr != "" { var err error diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 8ec828cc8..d7a472813 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/docs/release-notes/release-notes-0.18.0.md @@ -90,6 +90,11 @@ precision issue when querying payments and invoices using the start and end date filters. +* [Fixed](https://github.com/lightningnetwork/lnd/pull/8496) an issue where + `locked_balance` is not updated in `WalletBalanceResponse` when outputs are + reserved for `OpenChannel` by using non-volatile leases instead of volatile + locks. + # New Features ## Functional Enhancements @@ -364,6 +369,7 @@ # Contributors (Alphabetical Order) +* Alex Akselrod * Amin Bashiri * Andras Banki-Horvath * BitcoinerCoderBob diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 69fdf6787..1de36b962 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -574,4 +574,8 @@ var allTestCases = []*lntest.TestCase{ Name: "coop close with htlcs", TestFunc: testCoopCloseWithHtlcs, }, + { + Name: "open channel locked balance", + TestFunc: testOpenChannelLockedBalance, + }, } diff --git a/itest/lnd_open_channel_test.go b/itest/lnd_open_channel_test.go index d8cf66485..919e6ae97 100644 --- a/itest/lnd_open_channel_test.go +++ b/itest/lnd_open_channel_test.go @@ -14,6 +14,7 @@ import ( "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/node" "github.com/lightningnetwork/lnd/lntest/rpc" + "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -822,3 +823,59 @@ func testSimpleTaprootChannelActivation(ht *lntest.HarnessTest) { // Our test is done and Alice closes her channel to Bob. ht.CloseChannel(alice, chanPoint) } + +// testOpenChannelLockedBalance tests that when a funding reservation is +// made for opening a channel, the balance of the required outputs shows +// up as locked balance in the WalletBalance response. +func testOpenChannelLockedBalance(ht *lntest.HarnessTest) { + var ( + alice = ht.Alice + bob = ht.Bob + req *lnrpc.ChannelAcceptRequest + err error + ) + + // We first make sure Alice has no locked wallet balance. + balance := alice.RPC.WalletBalance() + require.EqualValues(ht, 0, balance.LockedBalance) + + // Next, we register a ChannelAcceptor on Bob. This way, we can get + // Alice's wallet balance after coin selection is done and outpoints + // are locked. + stream, cancel := bob.RPC.ChannelAcceptor() + defer cancel() + + // Then, we request creation of a channel from Alice to Bob. We don't + // use OpenChannelSync since we want to receive Bob's message in the + // same goroutine. + openChannelReq := &lnrpc.OpenChannelRequest{ + NodePubkey: bob.PubKey[:], + LocalFundingAmount: int64(funding.MaxBtcFundingAmount), + } + _ = alice.RPC.OpenChannel(openChannelReq) + + // After that, we receive the request on Bob's side, get the wallet + // balance from Alice, and ensure the locked balance is non-zero. + err = wait.NoError(func() error { + req, err = stream.Recv() + return err + }, defaultTimeout) + require.NoError(ht, err) + + balance = alice.RPC.WalletBalance() + require.NotEqualValues(ht, 0, balance.LockedBalance) + + // Next, we let Bob deny the request. + resp := &lnrpc.ChannelAcceptResponse{ + Accept: false, + PendingChanId: req.PendingChanId, + } + err = wait.NoError(func() error { + return stream.Send(resp) + }, defaultTimeout) + require.NoError(ht, err) + + // Finally, we check to make sure the balance is unlocked again. + balance = alice.RPC.WalletBalance() + require.EqualValues(ht, 0, balance.LockedBalance) +} diff --git a/lncfg/config.go b/lncfg/config.go index 07c37875d..889b327f6 100644 --- a/lncfg/config.go +++ b/lncfg/config.go @@ -69,10 +69,6 @@ const ( // closure. DefaultOutgoingCltvRejectDelta = DefaultOutgoingBroadcastDelta + 3 - // DefaultReservationTimeout is the default time we wait until we remove - // an unfinished (zombiestate) open channel flow from memory. - DefaultReservationTimeout = 10 * time.Minute - // DefaultZombieSweeperInterval is the default time interval at which // unfinished (zombiestate) open channel flows are purged from memory. DefaultZombieSweeperInterval = 1 * time.Minute diff --git a/lncfg/dev.go b/lncfg/dev.go index 7eb295824..0c887457f 100644 --- a/lncfg/dev.go +++ b/lncfg/dev.go @@ -4,6 +4,8 @@ package lncfg import ( "time" + + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" ) // IsDevBuild returns a bool to indicate whether we are in a development @@ -37,7 +39,7 @@ func (d *DevConfig) GetUnsafeDisconnect() bool { // GetReservationTimeout returns the config value for `ReservationTimeout`. func (d *DevConfig) GetReservationTimeout() time.Duration { - return DefaultReservationTimeout + return chanfunding.DefaultReservationTimeout } // GetZombieSweeperInterval returns the config value for`ZombieSweeperInterval`. diff --git a/lncfg/dev_integration.go b/lncfg/dev_integration.go index 86a4af275..0f0227e01 100644 --- a/lncfg/dev_integration.go +++ b/lncfg/dev_integration.go @@ -4,6 +4,8 @@ package lncfg import ( "time" + + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" ) // IsDevBuild returns a bool to indicate whether we are in a development @@ -33,7 +35,7 @@ func (d *DevConfig) ChannelReadyWait() time.Duration { // GetReservationTimeout returns the config value for `ReservationTimeout`. func (d *DevConfig) GetReservationTimeout() time.Duration { if d.ReservationTimeout == 0 { - return DefaultReservationTimeout + return chanfunding.DefaultReservationTimeout } return d.ReservationTimeout diff --git a/lnrpc/walletrpc/psbt.go b/lnrpc/walletrpc/psbt.go index b05d75054..19b102473 100644 --- a/lnrpc/walletrpc/psbt.go +++ b/lnrpc/walletrpc/psbt.go @@ -6,23 +6,18 @@ package walletrpc import ( "fmt" "math" - "time" "github.com/btcsuite/btcd/wire" base "github.com/btcsuite/btcwallet/wallet" "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" ) const ( defaultMaxConf = math.MaxInt32 ) -var ( - // DefaultLockDuration is the default duration used to lock outputs. - DefaultLockDuration = 10 * time.Minute -) - // verifyInputsUnspent checks that all inputs are contained in the list of // known, non-locked UTXOs given. func verifyInputsUnspent(inputs []*wire.TxIn, utxos []*lnwallet.Utxo) error { @@ -56,13 +51,14 @@ func lockInputs(w lnwallet.WalletController, for idx := range outpoints { lock := &base.ListLeasedOutputResult{ LockedOutput: &wtxmgr.LockedOutput{ - LockID: LndInternalLockID, + LockID: chanfunding.LndInternalLockID, Outpoint: outpoints[idx], }, } expiration, pkScript, value, err := w.LeaseOutput( - lock.LockID, lock.Outpoint, DefaultLockDuration, + lock.LockID, lock.Outpoint, + chanfunding.DefaultLockDuration, ) if err != nil { // If we run into a problem with locking one output, we @@ -72,7 +68,7 @@ func lockInputs(w lnwallet.WalletController, for i := 0; i < idx; i++ { op := locks[i].Outpoint if err := w.ReleaseOutput( - LndInternalLockID, op, + chanfunding.LndInternalLockID, op, ); err != nil { log.Errorf("could not release the "+ "lock on %v: %v", op, err) diff --git a/lnrpc/walletrpc/walletkit_server.go b/lnrpc/walletrpc/walletkit_server.go index 3e71070aa..79cc3e78e 100644 --- a/lnrpc/walletrpc/walletkit_server.go +++ b/lnrpc/walletrpc/walletkit_server.go @@ -184,18 +184,6 @@ var ( // configuration file in this package. DefaultWalletKitMacFilename = "walletkit.macaroon" - // LndInternalLockID is the binary representation of the SHA256 hash of - // the string "lnd-internal-lock-id" and is used for UTXO lock leases to - // identify that we ourselves are locking an UTXO, for example when - // giving out a funded PSBT. The ID corresponds to the hex value of - // ede19a92ed321a4705f8a1cccc1d4f6182545d4bb4fae08bd5937831b7e38f98. - LndInternalLockID = wtxmgr.LockID{ - 0xed, 0xe1, 0x9a, 0x92, 0xed, 0x32, 0x1a, 0x47, - 0x05, 0xf8, 0xa1, 0xcc, 0xcc, 0x1d, 0x4f, 0x61, - 0x82, 0x54, 0x5d, 0x4b, 0xb4, 0xfa, 0xe0, 0x8b, - 0xd5, 0x93, 0x78, 0x31, 0xb7, 0xe3, 0x8f, 0x98, - } - // allWitnessTypes is a mapping between the witness types defined in the // `input` package, and the witness types in the protobuf definition. // This map is necessary because the native enum and the protobuf enum @@ -482,7 +470,7 @@ func (w *WalletKit) LeaseOutput(ctx context.Context, // Don't allow our internal ID to be used externally for locking. Only // unlocking is allowed. - if lockID == LndInternalLockID { + if lockID == chanfunding.LndInternalLockID { return nil, errors.New("reserved id cannot be used") } @@ -492,7 +480,7 @@ func (w *WalletKit) LeaseOutput(ctx context.Context, } // Use the specified lock duration or fall back to the default. - duration := DefaultLockDuration + duration := chanfunding.DefaultLockDuration if req.ExpirationSeconds != 0 { duration = time.Duration(req.ExpirationSeconds) * time.Second } diff --git a/lntest/mock/walletcontroller.go b/lntest/mock/walletcontroller.go index 6c0f3e6a9..6381b4bb3 100644 --- a/lntest/mock/walletcontroller.go +++ b/lntest/mock/walletcontroller.go @@ -186,12 +186,6 @@ func (w *WalletController) ListTransactionDetails(int32, int32, return nil, nil } -// LockOutpoint currently does nothing. -func (w *WalletController) LockOutpoint(o wire.OutPoint) {} - -// UnlockOutpoint currently does nothing. -func (w *WalletController) UnlockOutpoint(o wire.OutPoint) {} - // LeaseOutput returns the current time and a nil error. func (w *WalletController) LeaseOutput(wtxmgr.LockID, wire.OutPoint, time.Duration) (time.Time, []byte, btcutil.Amount, error) { diff --git a/lntest/wait/timeouts_remote_db.go b/lntest/wait/timeouts_remote_db.go index 902508136..bc6784ff0 100644 --- a/lntest/wait/timeouts_remote_db.go +++ b/lntest/wait/timeouts_remote_db.go @@ -20,7 +20,7 @@ const ( // DefaultTimeout is a timeout that will be used for various wait // scenarios where no custom timeout value is defined. - DefaultTimeout = time.Second * 30 + DefaultTimeout = time.Second * 45 // AsyncBenchmarkTimeout is the timeout used when running the async // payments benchmark. diff --git a/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index fd4683509..3790af507 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/lnwallet/btcwallet/btcwallet.go @@ -1053,28 +1053,6 @@ func (b *BtcWallet) CreateSimpleTx(outputs []*wire.TxOut, ) } -// LockOutpoint marks an outpoint as locked meaning it will no longer be deemed -// as eligible for coin selection. Locking outputs are utilized in order to -// avoid race conditions when selecting inputs for usage when funding a -// channel. -// -// NOTE: This method requires the global coin selection lock to be held. -// -// This is a part of the WalletController interface. -func (b *BtcWallet) LockOutpoint(o wire.OutPoint) { - b.wallet.LockOutpoint(o) -} - -// UnlockOutpoint unlocks a previously locked output, marking it eligible for -// coin selection. -// -// NOTE: This method requires the global coin selection lock to be held. -// -// This is a part of the WalletController interface. -func (b *BtcWallet) UnlockOutpoint(o wire.OutPoint) { - b.wallet.UnlockOutpoint(o) -} - // LeaseOutput locks an output to the given ID, preventing it from being // available for any future coin selection attempts. The absolute time of the // lock's expiration is returned. The expiration of the lock can be extended by diff --git a/lnwallet/chanfunding/assembler.go b/lnwallet/chanfunding/assembler.go index 08fe31e43..a694d2a2c 100644 --- a/lnwallet/chanfunding/assembler.go +++ b/lnwallet/chanfunding/assembler.go @@ -1,9 +1,12 @@ package chanfunding import ( + "time" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/wallet" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/lnwallet/chainfee" ) @@ -34,18 +37,18 @@ type CoinSelectionLocker interface { WithCoinSelectLock(func() error) error } -// OutpointLocker allows a caller to lock/unlock an outpoint. When locked, the -// outpoints shouldn't be used for any sort of channel funding of coin -// selection. Locked outpoints are not expected to be persisted between -// restarts. -type OutpointLocker interface { - // LockOutpoint locks a target outpoint, rendering it unusable for coin +// OutputLeaser allows a caller to lease/release an output. When leased, the +// outputs shouldn't be used for any sort of channel funding or coin selection. +// Leased outputs are expected to be persisted between restarts. +type OutputLeaser interface { + // LeaseOutput leases a target output, rendering it unusable for coin // selection. - LockOutpoint(o wire.OutPoint) + LeaseOutput(i wtxmgr.LockID, o wire.OutPoint, d time.Duration) ( + time.Time, []byte, btcutil.Amount, error) - // UnlockOutpoint unlocks a target outpoint, allowing it to be used for + // ReleaseOutput releases a target output, allowing it to be used for // coin selection once again. - UnlockOutpoint(o wire.OutPoint) + ReleaseOutput(i wtxmgr.LockID, o wire.OutPoint) error } // Request is a new request for funding a channel. The items in the struct diff --git a/lnwallet/chanfunding/wallet_assembler.go b/lnwallet/chanfunding/wallet_assembler.go index 651bc5558..d4ee080bc 100644 --- a/lnwallet/chanfunding/wallet_assembler.go +++ b/lnwallet/chanfunding/wallet_assembler.go @@ -3,6 +3,7 @@ package chanfunding import ( "fmt" "math" + "time" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil" @@ -10,10 +11,34 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/wallet" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" ) +const ( + // DefaultReservationTimeout is the default time we wait until we remove + // an unfinished (zombiestate) open channel flow from memory. + DefaultReservationTimeout = 10 * time.Minute + + // DefaultLockDuration is the default duration used to lock outputs. + DefaultLockDuration = 10 * time.Minute +) + +var ( + // LndInternalLockID is the binary representation of the SHA256 hash of + // the string "lnd-internal-lock-id" and is used for UTXO lock leases to + // identify that we ourselves are locking an UTXO, for example when + // giving out a funded PSBT. The ID corresponds to the hex value of + // ede19a92ed321a4705f8a1cccc1d4f6182545d4bb4fae08bd5937831b7e38f98. + LndInternalLockID = wtxmgr.LockID{ + 0xed, 0xe1, 0x9a, 0x92, 0xed, 0x32, 0x1a, 0x47, + 0x05, 0xf8, 0xa1, 0xcc, 0xcc, 0x1d, 0x4f, 0x61, + 0x82, 0x54, 0x5d, 0x4b, 0xb4, 0xfa, 0xe0, 0x8b, + 0xd5, 0x93, 0x78, 0x31, 0xb7, 0xe3, 0x8f, 0x98, + } +) + // FullIntent is an intent that is fully backed by the internal wallet. This // intent differs from the ShimIntent, in that the funding transaction will be // constructed internally, and will consist of only inputs we wholly control. @@ -37,9 +62,9 @@ type FullIntent struct { // change from the main funding transaction. ChangeOutputs []*wire.TxOut - // coinLocker is the Assembler's instance of the OutpointLocker + // coinLeaser is the Assembler's instance of the OutputLeaser // interface. - coinLocker OutpointLocker + coinLeaser OutputLeaser // coinSource is the Assembler's instance of the CoinSource interface. coinSource CoinSource @@ -194,7 +219,13 @@ func (f *FullIntent) Outputs() []*wire.TxOut { // NOTE: Part of the chanfunding.Intent interface. func (f *FullIntent) Cancel() { for _, coin := range f.InputCoins { - f.coinLocker.UnlockOutpoint(coin.OutPoint) + err := f.coinLeaser.ReleaseOutput( + LndInternalLockID, coin.OutPoint, + ) + if err != nil { + log.Warnf("Failed to release UTXO %s (%v))", + coin.OutPoint, err) + } } f.ShimIntent.Cancel() @@ -216,9 +247,9 @@ type WalletConfig struct { // access to the current set of coins returned by the CoinSource. CoinSelectLocker CoinSelectionLocker - // CoinLocker is what the WalletAssembler uses to lock coins that may + // CoinLeaser is what the WalletAssembler uses to lease coins that may // be used as inputs for a new funding transaction. - CoinLocker OutpointLocker + CoinLeaser OutputLeaser // Signer allows the WalletAssembler to sign inputs on any potential // funding transactions. @@ -493,7 +524,13 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { for _, coin := range selectedCoins { outpoint := coin.OutPoint - w.cfg.CoinLocker.LockOutpoint(outpoint) + _, _, _, err = w.cfg.CoinLeaser.LeaseOutput( + LndInternalLockID, outpoint, + DefaultReservationTimeout, + ) + if err != nil { + return err + } } newIntent := &FullIntent{ @@ -503,7 +540,7 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) { musig2: r.Musig2, }, InputCoins: selectedCoins, - coinLocker: w.cfg.CoinLocker, + coinLeaser: w.cfg.CoinLeaser, coinSource: w.cfg.CoinSource, signer: w.cfg.Signer, } diff --git a/lnwallet/interface.go b/lnwallet/interface.go index 3df0a7328..c3944d812 100644 --- a/lnwallet/interface.go +++ b/lnwallet/interface.go @@ -393,20 +393,6 @@ type WalletController interface { ListTransactionDetails(startHeight, endHeight int32, accountFilter string) ([]*TransactionDetail, error) - // LockOutpoint marks an outpoint as locked meaning it will no longer - // be deemed as eligible for coin selection. Locking outputs are - // utilized in order to avoid race conditions when selecting inputs for - // usage when funding a channel. - // - // NOTE: This method requires the global coin selection lock to be held. - LockOutpoint(o wire.OutPoint) - - // UnlockOutpoint unlocks a previously locked output, marking it - // eligible for coin selection. - // - // NOTE: This method requires the global coin selection lock to be held. - UnlockOutpoint(o wire.OutPoint) - // LeaseOutput locks an output to the given ID, preventing it from being // available for any future coin selection attempts. The absolute time // of the lock's expiration is returned. The expiration of the lock can diff --git a/lnwallet/mock.go b/lnwallet/mock.go index dc59c63ac..1c1fb61a5 100644 --- a/lnwallet/mock.go +++ b/lnwallet/mock.go @@ -192,12 +192,6 @@ func (w *mockWalletController) ListTransactionDetails(int32, int32, return nil, nil } -// LockOutpoint currently does nothing. -func (w *mockWalletController) LockOutpoint(o wire.OutPoint) {} - -// UnlockOutpoint currently does nothing. -func (w *mockWalletController) UnlockOutpoint(o wire.OutPoint) {} - // LeaseOutput returns the current time and a nil error. func (w *mockWalletController) LeaseOutput(wtxmgr.LockID, wire.OutPoint, time.Duration) (time.Time, []byte, btcutil.Amount, error) { diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index f97b6500c..caa222cf4 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -2990,7 +2990,7 @@ func testSingleFunderExternalFundingTx(miner *rpctest.Harness, chanfunding.WalletConfig{ CoinSource: lnwallet.NewCoinSource(alice), CoinSelectLocker: alice, - CoinLocker: alice, + CoinLeaser: alice, Signer: alice.Cfg.Signer, DustLimit: 600, CoinSelectionStrategy: wallet.CoinSelectionLargest, diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 6a8ced33b..7786791f9 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -588,7 +588,7 @@ func (l *LightningWallet) ResetReservations() { l.reservationIDs = make(map[[32]byte]uint64) for outpoint := range l.lockedOutPoints { - l.UnlockOutpoint(outpoint) + _ = l.ReleaseOutput(chanfunding.LndInternalLockID, outpoint) } l.lockedOutPoints = make(map[wire.OutPoint]struct{}) } @@ -851,7 +851,7 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg cfg := chanfunding.WalletConfig{ CoinSource: &CoinSource{l}, CoinSelectLocker: l, - CoinLocker: l, + CoinLeaser: l, Signer: l.Cfg.Signer, DustLimit: DustLimitForSize( input.P2WSHSize, @@ -1425,7 +1425,10 @@ func (l *LightningWallet) handleFundingCancelRequest(req *fundingReserveCancelMs // requests. for _, unusedInput := range pendingReservation.ourContribution.Inputs { delete(l.lockedOutPoints, unusedInput.PreviousOutPoint) - l.UnlockOutpoint(unusedInput.PreviousOutPoint) + _ = l.ReleaseOutput( + chanfunding.LndInternalLockID, + unusedInput.PreviousOutPoint, + ) } // TODO(roasbeef): is it even worth it to keep track of unused keys? diff --git a/server.go b/server.go index a089a2238..ea51242e4 100644 --- a/server.go +++ b/server.go @@ -53,6 +53,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" "github.com/lightningnetwork/lnd/lnwallet/rpcwallet" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/nat" @@ -1279,7 +1280,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, // For the reservationTimeout and the zombieSweeperInterval different // values are set in case we are in a dev environment so enhance test // capacilities. - reservationTimeout := lncfg.DefaultReservationTimeout + reservationTimeout := chanfunding.DefaultReservationTimeout zombieSweeperInterval := lncfg.DefaultZombieSweeperInterval // Get the development config for funding manager. If we are not in diff --git a/sweep/walletsweep.go b/sweep/walletsweep.go index 831b0151e..bcfd90315 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -3,13 +3,16 @@ package sweep import ( "fmt" "math" + "time" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/lightningnetwork/lnd/lnwallet/chanfunding" ) const ( @@ -134,17 +137,18 @@ type CoinSelectionLocker interface { WithCoinSelectLock(func() error) error } -// OutpointLocker allows a caller to lock/unlock an outpoint. When locked, the -// outpoints shouldn't be used for any sort of channel funding of coin -// selection. Locked outpoints are not expected to be persisted between restarts. -type OutpointLocker interface { - // LockOutpoint locks a target outpoint, rendering it unusable for coin +// OutputLeaser allows a caller to lease/release an output. When leased, the +// outputs shouldn't be used for any sort of channel funding or coin selection. +// Leased outputs are expected to be persisted between restarts. +type OutputLeaser interface { + // LeaseOutput leases a target output, rendering it unusable for coin // selection. - LockOutpoint(o wire.OutPoint) + LeaseOutput(i wtxmgr.LockID, o wire.OutPoint, d time.Duration) ( + time.Time, []byte, btcutil.Amount, error) - // UnlockOutpoint unlocks a target outpoint, allowing it to be used for + // ReleaseOutput releases a target output, allowing it to be used for // coin selection once again. - UnlockOutpoint(o wire.OutPoint) + ReleaseOutput(i wtxmgr.LockID, o wire.OutPoint) error } // WalletSweepPackage is a package that gives the caller the ability to sweep @@ -179,11 +183,11 @@ 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 -// outpointLocker as sources for wallet funds. +// outputLeaser as sources for wallet funds. func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, blockHeight uint32, deliveryAddrs []DeliveryAddr, changeAddr btcutil.Address, coinSelectLocker CoinSelectionLocker, - utxoSource UtxoSource, outpointLocker OutpointLocker, + utxoSource UtxoSource, outputLeaser OutputLeaser, signer input.Signer, minConfs int32) (*WalletSweepPackage, error) { // TODO(roasbeef): turn off ATPL as well when available? @@ -196,7 +200,15 @@ func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, // can actually craft a sweeping transaction. unlockOutputs := func() { for _, utxo := range allOutputs { - outpointLocker.UnlockOutpoint(utxo.OutPoint) + // Log the error but continue since we're already + // handling an error. + err := outputLeaser.ReleaseOutput( + chanfunding.LndInternalLockID, utxo.OutPoint, + ) + if err != nil { + log.Warnf("Failed to release UTXO %s (%v))", + utxo.OutPoint, err) + } } } @@ -219,7 +231,13 @@ func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, // attempt to use these UTXOs in transactions while we're // crafting out sweep all transaction. for _, utxo := range utxos { - outpointLocker.LockOutpoint(utxo.OutPoint) + _, _, _, err = outputLeaser.LeaseOutput( + chanfunding.LndInternalLockID, utxo.OutPoint, + chanfunding.DefaultLockDuration, + ) + if err != nil { + return err + } } allOutputs = append(allOutputs, utxos...) diff --git a/sweep/walletsweep_test.go b/sweep/walletsweep_test.go index 4a7ceec7a..f36e472fa 100644 --- a/sweep/walletsweep_test.go +++ b/sweep/walletsweep_test.go @@ -4,11 +4,13 @@ import ( "bytes" "fmt" "testing" + "time" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcwallet/wtxmgr" "github.com/lightningnetwork/lnd/lntest/mock" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -151,25 +153,34 @@ func (m *mockCoinSelectionLocker) WithCoinSelectLock(f func() error) error { } -type mockOutpointLocker struct { - lockedOutpoints map[wire.OutPoint]struct{} +type mockOutputLeaser struct { + leasedOutputs map[wire.OutPoint]struct{} - unlockedOutpoints map[wire.OutPoint]struct{} + releasedOutputs map[wire.OutPoint]struct{} } -func newMockOutpointLocker() *mockOutpointLocker { - return &mockOutpointLocker{ - lockedOutpoints: make(map[wire.OutPoint]struct{}), +func newMockOutputLeaser() *mockOutputLeaser { + return &mockOutputLeaser{ + leasedOutputs: make(map[wire.OutPoint]struct{}), - unlockedOutpoints: make(map[wire.OutPoint]struct{}), + releasedOutputs: make(map[wire.OutPoint]struct{}), } } -func (m *mockOutpointLocker) LockOutpoint(o wire.OutPoint) { - m.lockedOutpoints[o] = struct{}{} +func (m *mockOutputLeaser) LeaseOutput(_ wtxmgr.LockID, o wire.OutPoint, + t time.Duration) (time.Time, []byte, btcutil.Amount, error) { + + m.leasedOutputs[o] = struct{}{} + + return time.Now().Add(t), nil, 0, nil } -func (m *mockOutpointLocker) UnlockOutpoint(o wire.OutPoint) { - m.unlockedOutpoints[o] = struct{}{} + +func (m *mockOutputLeaser) ReleaseOutput(_ wtxmgr.LockID, + o wire.OutPoint) error { + + m.releasedOutputs[o] = struct{}{} + + return nil } var sweepScript = []byte{ @@ -234,53 +245,53 @@ var testUtxos = []*lnwallet.Utxo{ }, } -func assertUtxosLocked(t *testing.T, utxoLocker *mockOutpointLocker, +func assertUtxosLeased(t *testing.T, utxoLeaser *mockOutputLeaser, utxos []*lnwallet.Utxo) { t.Helper() for _, utxo := range utxos { - if _, ok := utxoLocker.lockedOutpoints[utxo.OutPoint]; !ok { - t.Fatalf("utxo %v was never locked", utxo.OutPoint) + if _, ok := utxoLeaser.leasedOutputs[utxo.OutPoint]; !ok { + t.Fatalf("utxo %v was never leased", utxo.OutPoint) } } } -func assertNoUtxosUnlocked(t *testing.T, utxoLocker *mockOutpointLocker, +func assertNoUtxosReleased(t *testing.T, utxoLeaser *mockOutputLeaser, utxos []*lnwallet.Utxo) { t.Helper() - if len(utxoLocker.unlockedOutpoints) != 0 { - t.Fatalf("outputs have been locked, but shouldn't have been") + if len(utxoLeaser.releasedOutputs) != 0 { + t.Fatalf("outputs have been released, but shouldn't have been") } } -func assertUtxosUnlocked(t *testing.T, utxoLocker *mockOutpointLocker, +func assertUtxosReleased(t *testing.T, utxoLeaser *mockOutputLeaser, utxos []*lnwallet.Utxo) { t.Helper() for _, utxo := range utxos { - if _, ok := utxoLocker.unlockedOutpoints[utxo.OutPoint]; !ok { - t.Fatalf("utxo %v was never unlocked", utxo.OutPoint) + if _, ok := utxoLeaser.releasedOutputs[utxo.OutPoint]; !ok { + t.Fatalf("utxo %v was never released", utxo.OutPoint) } } } -func assertUtxosLockedAndUnlocked(t *testing.T, utxoLocker *mockOutpointLocker, +func assertUtxosLeasedAndReleased(t *testing.T, utxoLeaser *mockOutputLeaser, utxos []*lnwallet.Utxo) { t.Helper() for _, utxo := range utxos { - if _, ok := utxoLocker.lockedOutpoints[utxo.OutPoint]; !ok { - t.Fatalf("utxo %v was never locked", utxo.OutPoint) + if _, ok := utxoLeaser.leasedOutputs[utxo.OutPoint]; !ok { + t.Fatalf("utxo %v was never leased", utxo.OutPoint) } - if _, ok := utxoLocker.unlockedOutpoints[utxo.OutPoint]; !ok { - t.Fatalf("utxo %v was never unlocked", utxo.OutPoint) + if _, ok := utxoLeaser.releasedOutputs[utxo.OutPoint]; !ok { + t.Fatalf("utxo %v was never released", utxo.OutPoint) } } } @@ -294,10 +305,10 @@ func TestCraftSweepAllTxCoinSelectFail(t *testing.T) { coinSelectLocker := &mockCoinSelectionLocker{ fail: true, } - utxoLocker := newMockOutpointLocker() + utxoLeaser := newMockOutputLeaser() _, err := CraftSweepAllTx( - 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, + 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLeaser, nil, 0, ) @@ -309,7 +320,7 @@ func TestCraftSweepAllTxCoinSelectFail(t *testing.T) { // At this point, we'll now verify that all outputs were initially // locked, and then also unlocked due to the failure. - assertUtxosLockedAndUnlocked(t, utxoLocker, testUtxos) + assertUtxosLeasedAndReleased(t, utxoLeaser, testUtxos) } // TestCraftSweepAllTxUnknownWitnessType tests that if one of the inputs we @@ -320,10 +331,10 @@ func TestCraftSweepAllTxUnknownWitnessType(t *testing.T) { utxoSource := newMockUtxoSource(testUtxos) coinSelectLocker := &mockCoinSelectionLocker{} - utxoLocker := newMockOutpointLocker() + utxoLeaser := newMockOutputLeaser() _, err := CraftSweepAllTx( - 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, + 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLeaser, nil, 0, ) @@ -336,7 +347,7 @@ func TestCraftSweepAllTxUnknownWitnessType(t *testing.T) { // At this point, we'll now verify that all outputs were initially // locked, and then also unlocked since we weren't able to find a // witness type for the last output. - assertUtxosLockedAndUnlocked(t, utxoLocker, testUtxos) + assertUtxosLeasedAndReleased(t, utxoLeaser, testUtxos) } // TestCraftSweepAllTx tests that we'll properly lock all available outputs @@ -354,18 +365,18 @@ func TestCraftSweepAllTx(t *testing.T) { targetUTXOs := testUtxos[:2] utxoSource := newMockUtxoSource(targetUTXOs) coinSelectLocker := &mockCoinSelectionLocker{} - utxoLocker := newMockOutpointLocker() + utxoLeaser := newMockOutputLeaser() sweepPkg, err := CraftSweepAllTx( 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, - utxoLocker, signer, 0, + utxoLeaser, signer, 0, ) require.NoError(t, err, "unable to make sweep tx") // At this point, all of the UTXOs that we made above should be locked // and none of them unlocked. - assertUtxosLocked(t, utxoLocker, testUtxos[:2]) - assertNoUtxosUnlocked(t, utxoLocker, testUtxos[:2]) + assertUtxosLeased(t, utxoLeaser, testUtxos[:2]) + assertNoUtxosReleased(t, utxoLeaser, testUtxos[:2]) // Now that we have our sweep transaction, we should find that we have // a UTXO for each input, and also that our final output value is the @@ -397,5 +408,5 @@ func TestCraftSweepAllTx(t *testing.T) { // If we cancel the sweep attempt, then we should find that all the // UTXOs within the sweep transaction are now unlocked. sweepPkg.CancelSweepAttempt() - assertUtxosUnlocked(t, utxoLocker, testUtxos[:2]) + assertUtxosReleased(t, utxoLeaser, testUtxos[:2]) }