From 07ba9d6015ea268a9a4742a4a108d939e28fef04 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 09:46:33 -0800 Subject: [PATCH 1/8] itest: add open channel locked balance test --- itest/list_on_test.go | 4 +++ itest/lnd_open_channel_test.go | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 13d7814a3..47cffcf6d 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -570,4 +570,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) +} From 4d2ab7423fa022fec619bce123344845065d448a Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 13:20:19 -0800 Subject: [PATCH 2/8] multi: move 3 vars from walletrpc+lncfg to chanfunding This commit moves the constants LndInternalLockID and DefaultLockDuration from the walletrpc package to the chanfunding package, moves DefaultReservationTimeout from lncfg to chanfunding, and also updates the lncli package with the new location. --- cmd/lncli/walletrpc_active.go | 3 ++- lncfg/config.go | 4 ---- lncfg/dev.go | 4 +++- lncfg/dev_integration.go | 4 +++- lnrpc/walletrpc/psbt.go | 14 +++++-------- lnrpc/walletrpc/walletkit_server.go | 16 ++------------- lnwallet/chanfunding/wallet_assembler.go | 25 ++++++++++++++++++++++++ server.go | 3 ++- 8 files changed, 42 insertions(+), 31 deletions(-) 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/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/lnwallet/chanfunding/wallet_assembler.go b/lnwallet/chanfunding/wallet_assembler.go index 651bc5558..062fd2328 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. diff --git a/server.go b/server.go index cb1540a2c..938221887 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" @@ -1276,7 +1277,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 From 6ad86a800c1805f80f627d7c06dc2a28f6721c0f Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 14:15:45 -0800 Subject: [PATCH 3/8] chanfunding: use `{Lease|Release}Output` not `{Lock|Unlock}Outpoint` --- lnwallet/chanfunding/assembler.go | 21 +++++++++++-------- lnwallet/chanfunding/wallet_assembler.go | 26 +++++++++++++++++------- lnwallet/test/test_interface.go | 2 +- lnwallet/wallet.go | 2 +- 4 files changed, 33 insertions(+), 18 deletions(-) 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 062fd2328..d4ee080bc 100644 --- a/lnwallet/chanfunding/wallet_assembler.go +++ b/lnwallet/chanfunding/wallet_assembler.go @@ -62,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 @@ -219,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() @@ -241,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. @@ -518,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{ @@ -528,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/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..99a2f380b 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -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, From a1d446394735464ea5ef31815b3b9bc44c6ae274 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 13:23:34 -0800 Subject: [PATCH 4/8] sweep: use `{Lease|Release}Output` instead of `{Lock|Unlock}Outpoint` --- sweep/walletsweep.go | 42 ++++++++++++++------ sweep/walletsweep_test.go | 83 ++++++++++++++++++++++----------------- 2 files changed, 77 insertions(+), 48 deletions(-) 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]) } From cefbb77b1e87fd5e50db943a45aaa6a680138c70 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 15:04:56 -0800 Subject: [PATCH 5/8] lnwallet: use `ReleaseOutput` instead of `UnlockOutpoint` --- lnwallet/wallet.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 99a2f380b..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{}) } @@ -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? From b9357fe8303de2ba3f9aef6a92ececbc6ac7acc3 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Wed, 21 Feb 2024 15:31:54 -0800 Subject: [PATCH 6/8] multi: remove unused `LockOutpoint` and `UnlockOutpoint` --- lntest/mock/walletcontroller.go | 6 ------ lnwallet/btcwallet/btcwallet.go | 22 ---------------------- lnwallet/interface.go | 14 -------------- lnwallet/mock.go | 6 ------ 4 files changed, 48 deletions(-) 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/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index 364953be4..61d53787a 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/lnwallet/btcwallet/btcwallet.go @@ -1052,28 +1052,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/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) { From 7af3d4022ffffa097e9ae9b5e037fe73123ed595 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Thu, 29 Feb 2024 15:24:21 -0800 Subject: [PATCH 7/8] docs: update release notes --- docs/release-notes/release-notes-0.18.0.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 4ef9d7dd9..80e1d2858 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 @@ -336,6 +341,7 @@ # Contributors (Alphabetical Order) +* Alex Akselrod * Amin Bashiri * Andras Banki-Horvath * BitcoinerCoderBob From 419350534180f857f6123cad5fcb1d5e561dbae0 Mon Sep 17 00:00:00 2001 From: Alex Akselrod Date: Fri, 15 Mar 2024 11:09:52 -0700 Subject: [PATCH 8/8] lntest/wait: increase DefaultTimeout for db access This helps take into account the new limits on GHA runners. --- lntest/wait/timeouts_remote_db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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.