wallet: select utxos should not contain duplicates

Signed-off-by: Ononiwu Maureen <[email protected]>
btcsuite · May 17, 2024 · c9c70a5 · c9c70a5
1 parent 852789b
commit c9c70a5
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 28 deletions.
diff --git a/wallet/createtx.go b/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"
 )
 
 func makeInputSource(eligible []Coin) txauthor.InputSource {
@@ -179,6 +180,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,
 			)

diff --git a/wallet/createtx_test.go b/wallet/createtx_test.go
@@ -449,42 +449,109 @@ func TestSelectUtxosTxoToOutpoint(t *testing.T) {
 	require.Len(t, unspent, 4, "expected 4 unspent "+
 		"utxos")
 
-	selectUtxos := []wire.OutPoint{
+	tCases := []struct {
+		name        string
+		selectUtxos []wire.OutPoint
+		errString   string
+	}{
 		{
-			Hash:  incomingTx.TxHash(),
-			Index: 1,
+			name: "Duplicate utxo values",
+			selectUtxos: []wire.OutPoint{
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+			},
+			errString: "selected UTXOs contain duplicate values",
 		},
 		{
-			Hash:  incomingTx.TxHash(),
-			Index: 2,
+			name: "all select utxo not part of unspent utxos",
+			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",
+		},
+		{
+			name: "some select utxos not part of unspent utxos",
+			selectUtxos: []wire.OutPoint{
+				{
+					Hash:  chainhash.Hash([32]byte{1}),
+					Index: 1,
+				},
+				{
+					Hash:  incomingTx.TxHash(),
+					Index: 1,
+				},
+			},
+			errString: "selected outpoint not eligible for " +
+				"spending",
+		},
+		{
+			name: "select utxo, no error",
+			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)
+		})
+	}
 }