sweep: use {Lease|Release}Output instead of {Lock|Unlock}Outpoint

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
+// OutputLeaser allows a caller to lease/release an output. When leased, the
-// outpoints shouldn't be used for any sort of channel funding of coin
+// outputs shouldn't be used for any sort of channel funding or coin selection.
-// selection. Locked outpoints are not expected to be persisted between restarts.
+// Leased outputs are expected to be persisted between restarts.
-type OutpointLocker interface {
+type OutputLeaser interface {
-	// LockOutpoint locks a target outpoint, rendering it unusable for coin
+	// 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...)

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 {
+type mockOutputLeaser struct {
-	lockedOutpoints map[wire.OutPoint]struct{}
+	leasedOutputs map[wire.OutPoint]struct{}
 
-	unlockedOutpoints map[wire.OutPoint]struct{}
+	releasedOutputs map[wire.OutPoint]struct{}
 }
 
-func newMockOutpointLocker() *mockOutpointLocker {
+func newMockOutputLeaser() *mockOutputLeaser {
-	return &mockOutpointLocker{
+	return &mockOutputLeaser{
-		lockedOutpoints: make(map[wire.OutPoint]struct{}),
+		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) {
+func (m *mockOutputLeaser) LeaseOutput(_ wtxmgr.LockID, o wire.OutPoint,
-	m.lockedOutpoints[o] = struct{}{}
+	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 {
+		if _, ok := utxoLeaser.leasedOutputs[utxo.OutPoint]; !ok {
-			t.Fatalf("utxo %v was never locked", utxo.OutPoint)
+			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 {
+	if len(utxoLeaser.releasedOutputs) != 0 {
-		t.Fatalf("outputs have been locked, but shouldn't have been")
+		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 {
+		if _, ok := utxoLeaser.releasedOutputs[utxo.OutPoint]; !ok {
-			t.Fatalf("utxo %v was never unlocked", utxo.OutPoint)
+			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 {
+		if _, ok := utxoLeaser.leasedOutputs[utxo.OutPoint]; !ok {
-			t.Fatalf("utxo %v was never locked", utxo.OutPoint)
+			t.Fatalf("utxo %v was never leased", utxo.OutPoint)
 		}
 
-		if _, ok := utxoLocker.unlockedOutpoints[utxo.OutPoint]; !ok {
+		if _, ok := utxoLeaser.releasedOutputs[utxo.OutPoint]; !ok {
-			t.Fatalf("utxo %v was never unlocked", utxo.OutPoint)
+			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])
+	assertUtxosLeased(t, utxoLeaser, testUtxos[:2])
-	assertNoUtxosUnlocked(t, utxoLocker, 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])
 }