From 7bb8bbbd7fe0bd397f60c570bf9a8ced8e3ce420 Mon Sep 17 00:00:00 2001
From: Ononiwu Maureen <59079323+Chinwendu20@users.noreply.github.com>
Date: Mon, 13 May 2024 21:11:18 +0100
Subject: [PATCH] wallet: select utxos should not contain duplicates

Signed-off-by: Ononiwu Maureen <59079323+Chinwendu20@users.noreply.github.com>
---
 wallet/createtx.go      |   7 +++
 wallet/createtx_test.go | 119 ++++++++++++++++++++++++++++++----------
 2 files changed, 98 insertions(+), 28 deletions(-)

diff --git a/wallet/createtx.go b/wallet/createtx.go
index 9c2b061984..08d6791eae 100644
--- 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,12 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut,
 
 		var inputSource txauthor.InputSource
 		if len(selectedUtxos) > 0 {
+			if len(fn.NewSet(selectedUtxos...)) !=
+				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
index c0badf35d7..b56c690b31 100644
--- a/wallet/createtx_test.go
+++ b/wallet/createtx_test.go
@@ -449,42 +449,105 @@ 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: "part select utxo 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, "unexpected outpoint in txin")
+			}
+
+			// Expect two outputs, change and the actual payment to the address.
+			require.Len(t, tx1.Tx.TxOut, 2)
+		})
+	}
 }