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]) }