Collect small fixes from stale contributor PRs

.golangci.yml

@@ -89,6 +89,17 @@ linters:
     # The linter is too aggressive and doesn't add much value since reviewers
     # will also catch magic numbers that make sense to extract.
     - gomnd
+
+    # Some of the tests cannot be parallelized. On the other hand, we don't
+    # gain much performance with this check so we disable it for now until
+    # unit tests become our CI bottleneck.
+    - paralleltest
+
+    # Allow dynamic errors.
+    - goerr113
+
+    # New linters that we haven't had time to address yet.
+    - depguard
 
 issues:
   # Only check issues in the new code.

go.mod

@@ -21,6 +21,7 @@ require (
 	github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf
 	github.com/lightninglabs/neutrino v0.16.0
 	github.com/lightninglabs/neutrino/cache v1.1.2
+	github.com/lightningnetwork/lnd/fn/v2 v2.0.2
 	github.com/lightningnetwork/lnd/ticker v1.0.0
 	github.com/lightningnetwork/lnd/tlv v1.0.2
 	github.com/stretchr/testify v1.9.0
@@ -45,6 +46,7 @@ require (
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
 	github.com/stretchr/objx v0.5.2 // indirect
 	go.etcd.io/bbolt v1.3.11 // indirect
+	golang.org/x/exp v0.0.0-20231226003508-02704c960a9b // indirect
 	golang.org/x/sys v0.19.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20231030173426-d783a09b4405 // indirect

go.sum

@@ -103,6 +103,8 @@ github.com/lightninglabs/neutrino/cache v1.1.2 h1:C9DY/DAPaPxbFC+xNNEI/z1SJY9GS3
 github.com/lightninglabs/neutrino/cache v1.1.2/go.mod h1:XJNcgdOw1LQnanGjw8Vj44CvguYA25IMKjWFZczwZuo=
 github.com/lightningnetwork/lnd/clock v1.0.1 h1:QQod8+m3KgqHdvVMV+2DRNNZS1GRFir8mHZYA+Z2hFo=
 github.com/lightningnetwork/lnd/clock v1.0.1/go.mod h1:KnQudQ6w0IAMZi1SgvecLZQZ43ra2vpDNj7H/aasemg=
+github.com/lightningnetwork/lnd/fn/v2 v2.0.2 h1:M7o2lYrh/zCp+lntPB3WP/rWTu5U+4ssyHW+kqNJ0fs=
+github.com/lightningnetwork/lnd/fn/v2 v2.0.2/go.mod h1:TOzwrhjB/Azw1V7aa8t21ufcQmdsQOQMDtxVOQWNl8s=
 github.com/lightningnetwork/lnd/queue v1.0.1 h1:jzJKcTy3Nj5lQrooJ3aaw9Lau3I0IwvQR5sqtjdv2R0=
 github.com/lightningnetwork/lnd/queue v1.0.1/go.mod h1:vaQwexir73flPW43Mrm7JOgJHmcEFBWWSl9HlyASoms=
 github.com/lightningnetwork/lnd/ticker v1.0.0 h1:S1b60TEGoTtCe2A0yeB+ecoj/kkS4qpwh6l+AkQEZwU=
@@ -138,6 +140,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
 golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
+golang.org/x/exp v0.0.0-20231226003508-02704c960a9b h1:kLiC65FbiHWFAOu+lxwNPujcsl8VYyTYYEZnsOO1WK4=
+golang.org/x/exp v0.0.0-20231226003508-02704c960a9b/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
 golang.org/x/net v0.0.0-20180719180050-a680a1efc54d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=

rpc/legacyrpc/methods.go

@@ -1713,6 +1713,9 @@ func signRawTransaction(icmd interface{}, w *wallet.Wallet, chainClient *chain.R
 		if err != nil {
 			return nil, err
 		}
+		if result == nil {
+			return nil, errors.New("the utxo has been spent")
+		}
 		script, err := hex.DecodeString(result.ScriptPubKey.Hex)
 		if err != nil {
 			return nil, err

wallet/createtx.go

@@ -20,6 +20,7 @@ import (
 	"github.com/btcsuite/btcwallet/wallet/txsizes"
 	"github.com/btcsuite/btcwallet/walletdb"
 	"github.com/btcsuite/btcwallet/wtxmgr"
+	"github.com/lightningnetwork/lnd/fn/v2"
 )
 
 func makeInputSource(eligible []Coin) txauthor.InputSource {
@@ -191,6 +192,11 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut,
 
 		var inputSource txauthor.InputSource
 		if len(selectedUtxos) > 0 {
+			dedupUtxos := fn.NewSet(selectedUtxos...)
+			if len(dedupUtxos) != len(selectedUtxos) {
+				return errors.New("selected UTXOs contain " +
+					"duplicate values")
+			}
 			eligibleByOutpoint := make(
 				map[wire.OutPoint]wtxmgr.Credit,
 			)

wallet/createtx_test.go

@@ -441,50 +441,115 @@ func TestSelectUtxosTxoToOutpoint(t *testing.T) {
 	}
 	addUtxo(t, w, incomingTx)
 
-	// We expect 4 unspent utxos.
+	// We expect 4 unspent UTXOs.
 	unspent, err := w.ListUnspent(0, 80, "")
-	require.NoError(t, err, "unexpected error while calling "+
-		"list unspent")
-
-	require.Len(t, unspent, 4, "expected 4 unspent "+
-		"utxos")
+	require.NoError(t, err)
+	require.Len(t, unspent, 4, "expected 4 unspent UTXOs")
 
-	selectUtxos := []wire.OutPoint{
+	tCases := []struct {
+		name        string
+		selectUTXOs []wire.OutPoint
+		errString   string
+	}{
+		{
+			name: "Duplicate utxo values",
+			selectUTXOs: []wire.OutPoint{
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+			},
+			errString: "selected UTXOs contain duplicate values",
+		},
+		{
+			name: "all selected UTXOs not eligible for spending",
+			selectUTXOs: []wire.OutPoint{
+				{
+					Hash:  chainhash.Hash([32]byte{1}),
+					Index: 1,
+				},
+				{
+					Hash:  chainhash.Hash([32]byte{3}),
+					Index: 1,
+				},
+			},
+			errString: "selected outpoint not eligible for " +
+				"spending",
+		},
 		{
-			Hash:  incomingTx.TxHash(),
-			Index: 1,
+			name: "some select UTXOs not eligible for spending",
+			selectUTXOs: []wire.OutPoint{
+				{
+					Hash:  chainhash.Hash([32]byte{1}),
+					Index: 1,
+				},
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+			},
+			errString: "selected outpoint not eligible for " +
+				"spending",
 		},
 		{
-			Hash:  incomingTx.TxHash(),
-			Index: 2,
+			name: "select utxo, no duplicates and all eligible " +
+				"for spending",
+			selectUTXOs: []wire.OutPoint{
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 2,
+				},
+			},
 		},
 	}
 
-	// Test by sending 200_000.
-	targetTxOut := &wire.TxOut{
-		Value:    200_000,
-		PkScript: p2trScript,
-	}
-	tx1, err := w.txToOutputs(
-		[]*wire.TxOut{targetTxOut}, nil, nil, 0, 1, 1000,
-		CoinSelectionLargest, true, selectUtxos, alwaysAllowUtxo,
-	)
-	require.NoError(t, err)
+	for _, tc := range tCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Test by sending 200_000.
+			targetTxOut := &wire.TxOut{
+				Value:    200_000,
+				PkScript: p2trScript,
+			}
+			tx1, err := w.txToOutputs(
+				[]*wire.TxOut{targetTxOut}, nil, nil, 0, 1,
+				1000, CoinSelectionLargest, true,
+				tc.selectUTXOs, alwaysAllowUtxo,
+			)
+			if tc.errString != "" {
+				require.ErrorContains(t, err, tc.errString)
+				require.Nil(t, tx1)
 
-	// We expect all and only our select utxos to be input in this
-	// transaction.
-	require.Len(t, tx1.Tx.TxIn, len(selectUtxos))
+				return
+			}
 
-	lookupSelectUtxos := make(map[wire.OutPoint]struct{})
-	for _, utxo := range selectUtxos {
-		lookupSelectUtxos[utxo] = struct{}{}
-	}
+			require.NoError(t, err)
+			require.NotNil(t, tx1)
 
-	for _, tx := range tx1.Tx.TxIn {
-		_, ok := lookupSelectUtxos[tx.PreviousOutPoint]
-		require.True(t, ok, "unexpected outpoint in txin")
-	}
+			// We expect all and only our select UTXOs to be input
+			// in this transaction.
+			require.Len(t, tx1.Tx.TxIn, len(tc.selectUTXOs))
 
-	// Expect two outputs, change and the actual payment to the address.
-	require.Len(t, tx1.Tx.TxOut, 2)
+			lookupSelectUtxos := make(map[wire.OutPoint]struct{})
+			for _, utxo := range tc.selectUTXOs {
+				lookupSelectUtxos[utxo] = struct{}{}
+			}
+
+			for _, tx := range tx1.Tx.TxIn {
+				_, ok := lookupSelectUtxos[tx.PreviousOutPoint]
+				require.True(t, ok)
+			}
+
+			// Expect two outputs, change and the actual payment to
+			// the address.
+			require.Len(t, tx1.Tx.TxOut, 2)
+		})
+	}
 }

wallet/rescan.go

@@ -251,14 +251,22 @@ out:
 			// Log the newly-started rescan.
 			numAddrs := len(batch.addrs)
 			noun := pickNoun(numAddrs, "address", "addresses")
-			log.Infof("Started rescan from block %v (height %d) for %d %s",
-				batch.bs.Hash, batch.bs.Height, numAddrs, noun)
+			log.Infof("Started rescan from block %v (height %d) "+
+				"for %d %s", batch.bs.Hash, batch.bs.Height,
+				numAddrs, noun)
 
-			err := chainClient.Rescan(&batch.bs.Hash, batch.addrs,
-				batch.outpoints)
+			if numAddrs == 0 {
+				batch.done(nil)
+
+				continue
+			}
+
+			err := chainClient.Rescan(
+				&batch.bs.Hash, batch.addrs, batch.outpoints,
+			)
 			if err != nil {
-				log.Errorf("Rescan for %d %s failed: %v", numAddrs,
-					noun, err)
+				log.Errorf("Rescan for %d %s failed: %v",
+					numAddrs, noun, err)
 			}
 			batch.done(err)
 		case <-quit: