fix(wallet): ensure there are not duplicated foreign utxos

Author: nymius

crates/wallet/src/types.rs

@@ -66,7 +66,7 @@ pub struct LocalOutput {
 }
 
 /// A [`Utxo`] with its `satisfaction_weight`.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub struct WeightedUtxo {
     /// The weight of the witness data and `scriptSig` expressed in [weight units]. This is used to
     /// properly maintain the feerate when adding this input to a transaction during coin selection.
@@ -77,7 +77,7 @@ pub struct WeightedUtxo {
     pub utxo: Utxo,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
 /// An unspent transaction output (UTXO).
 pub enum Utxo {
     /// A UTXO owned by the local wallet.

crates/wallet/src/wallet/mod.rs

@@ -1640,9 +1640,12 @@ impl Wallet {
             .map_err(|_| BuildFeeBumpError::FeeRateUnavailable)?;
 
         // remove the inputs from the tx and process them
-        let (local, foreign): (HashSet<LocalOutput>, Vec<WeightedUtxo>) =
+        let (local, foreign): (HashSet<LocalOutput>, HashSet<WeightedUtxo>) =
             tx.input.drain(..).try_fold(
-                (HashSet::<LocalOutput>::new(), Vec::<WeightedUtxo>::new()),
+                (
+                    HashSet::<LocalOutput>::new(),
+                    HashSet::<WeightedUtxo>::new(),
+                ),
                 |(mut local, mut foreign), txin| -> Result<_, BuildFeeBumpError> {
                     let prev_tx = graph
                         .get_tx(txin.previous_output.txid)
@@ -1683,7 +1686,7 @@ impl Wallet {
                                     ..Default::default()
                                 })
                             };
-                            foreign.push(WeightedUtxo {
+                            foreign.insert(WeightedUtxo {
                                 utxo: Utxo::Foreign {
                                     outpoint: txin.previous_output,
                                     sequence: txin.sequence,

crates/wallet/src/wallet/tx_builder.rs

@@ -126,7 +126,7 @@ pub(crate) struct TxParams {
     pub(crate) internal_policy_path: Option<BTreeMap<String, Vec<usize>>>,
     pub(crate) external_policy_path: Option<BTreeMap<String, Vec<usize>>>,
     pub(crate) utxos: HashSet<LocalOutput>,
-    pub(crate) foreign_utxos: Vec<WeightedUtxo>,
+    pub(crate) foreign_utxos: HashSet<WeightedUtxo>,
     pub(crate) unspendable: HashSet<OutPoint>,
     pub(crate) manually_selected_only: bool,
     pub(crate) sighash: Option<psbt::PsbtSighashType>,
@@ -382,7 +382,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
             }
         }
 
-        self.params.foreign_utxos.push(WeightedUtxo {
+        self.params.foreign_utxos.insert(WeightedUtxo {
             satisfaction_weight,
             utxo: Utxo::Foreign {
                 outpoint,
@@ -1100,4 +1100,34 @@ mod test {
             params.utxos.into_iter().collect::<Vec<_>>()
         );
     }
+
+    #[test]
+    fn not_duplicated_foreign_utxos() {
+        use crate::test_utils::get_funded_wallet_wpkh;
+        // Use two different wallets to avoid adding local utxos
+        let (wallet1, txid1) = get_funded_wallet_wpkh();
+        let (mut wallet2, _) = get_funded_wallet_wpkh();
+
+        let utxo1 = wallet1.list_unspent().next().unwrap();
+        let tx1 = wallet1.get_tx(txid1).unwrap().tx_node.tx.clone();
+
+        let satisfaction_weight = wallet1
+            .public_descriptor(KeychainKind::External)
+            .max_weight_to_satisfy()
+            .unwrap();
+
+        let mut builder = wallet2.build_tx();
+        for _ in 0..2 {
+            let _ = builder.add_foreign_utxo(
+                utxo1.outpoint,
+                psbt::Input {
+                    non_witness_utxo: Some(tx1.as_ref().clone()),
+                    ..Default::default()
+                },
+                satisfaction_weight,
+            );
+        }
+
+        assert_eq!(builder.params.foreign_utxos.len(), 1);
+    }
 }