Merge pull request #8496 from aakselrod/locks-to-leases

multi: replace `LockOutpoint` with `LeaseOutput`

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

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

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,
 		}

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

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) {

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,

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?