refactor(tx_builder): make foreign locality check not breaking

Author: nymius

crates/wallet/src/wallet/tx_builder.rs

@@ -375,28 +375,34 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
         satisfaction_weight: Weight,
         sequence: Sequence,
     ) -> Result<&mut Self, AddForeignUtxoError> {
+        let script_pubkey = if let Some(ref txout) = psbt_input.witness_utxo {
+            txout.script_pubkey.clone()
+        } else if let Some(tx) = psbt_input.non_witness_utxo.as_ref() {
+            if tx.compute_txid() != outpoint.txid {
+                return Err(AddForeignUtxoError::InvalidTxid {
+                    input_txid: tx.compute_txid(),
+                    foreign_utxo: outpoint,
+                });
+            }
+            if tx.output.len() <= outpoint.vout as usize {
+                return Err(AddForeignUtxoError::InvalidOutpoint(outpoint));
+            }
+
+            if let Ok(txout) = tx.tx_out(outpoint.vout as usize) {
+                txout.script_pubkey.clone()
+            } else {
+                return Err(AddForeignUtxoError::InvalidOutpoint(outpoint));
+            }
+        } else {
+            return Err(AddForeignUtxoError::MissingUtxo);
+        };
+
         // Avoid the inclusion of local utxos as foreign utxos
-        if self.wallet.tx_graph().get_txout(outpoint).is_some() {
-            return Err(AddForeignUtxoError::NotForeignUtxo);
+        if self.wallet.is_mine(script_pubkey) {
+            // TODO(2.0.0): return Err(AddForeignUtxoError::NotForeignUtxo);
+            // No-op to avoid breaking changes until next major release
+            return Ok(self);
         };
-        if psbt_input.witness_utxo.is_none() {
-            match psbt_input.non_witness_utxo.as_ref() {
-                Some(tx) => {
-                    if tx.compute_txid() != outpoint.txid {
-                        return Err(AddForeignUtxoError::InvalidTxid {
-                            input_txid: tx.compute_txid(),
-                            foreign_utxo: outpoint,
-                        });
-                    }
-                    if tx.output.len() <= outpoint.vout as usize {
-                        return Err(AddForeignUtxoError::InvalidOutpoint(outpoint));
-                    }
-                }
-                None => {
-                    return Err(AddForeignUtxoError::MissingUtxo);
-                }
-            }
-        }
 
         self.params.utxos.push(WeightedUtxo {
             satisfaction_weight,
@@ -716,8 +722,8 @@ pub enum AddForeignUtxoError {
     InvalidOutpoint(OutPoint),
     /// Foreign utxo missing witness_utxo or non_witness_utxo
     MissingUtxo,
-    /// UTxO is owned by wallet
-    NotForeignUtxo,
+    // Avoid breaking changes until next major release
+    // TODO(2.0.0): NotForeignUtxo,
 }
 
 impl fmt::Display for AddForeignUtxoError {
@@ -737,7 +743,8 @@ impl fmt::Display for AddForeignUtxoError {
                 outpoint.txid, outpoint.vout,
             ),
             Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"),
-            Self::NotForeignUtxo => write!(f, "UTxO is owned by wallet"),
+            // Avoid breaking changes until next major release
+            // TODO(2.0.0): Self::NotForeignUtxo => write!(f, "UTxO is owned by wallet"),
         }
     }
 }
@@ -1106,4 +1113,21 @@ mod test {
         builder.fee_rate(FeeRate::from_sat_per_kwu(feerate + 250));
         let _ = builder.finish().unwrap();
     }
+
+    #[test]
+    fn test_add_foreign_utxo_fails_when_utxo_is_owned_by_wallet() {
+        use crate::test_utils::get_funded_wallet_wpkh;
+        let (mut wallet, _) = get_funded_wallet_wpkh();
+        let outpoint = wallet.list_unspent().next().expect("must exist").outpoint;
+        let foreign_utxo_satisfaction = wallet
+            .public_descriptor(KeychainKind::External)
+            .max_weight_to_satisfy()
+            .unwrap();
+
+        let mut builder = wallet.build_tx();
+        let _ =
+            builder.add_foreign_utxo(outpoint, psbt::Input::default(), foreign_utxo_satisfaction);
+        // TODO(2.0.0): assert!(matches!(result, Err(AddForeignUtxoError::NotForeignUtxo)));
+        assert!(builder.params.utxos.is_empty());
+    }
 }

crates/wallet/tests/wallet.rs

@@ -1675,21 +1675,6 @@ fn test_add_foreign_utxo_invalid_psbt_input() {
     assert!(matches!(result, Err(AddForeignUtxoError::MissingUtxo)));
 }
 
-#[test]
-fn test_add_foreign_utxo_fails_when_utxo_is_owned_by_wallet() {
-    let (mut wallet, _) = get_funded_wallet_wpkh();
-    let outpoint = wallet.list_unspent().next().expect("must exist").outpoint;
-    let foreign_utxo_satisfaction = wallet
-        .public_descriptor(KeychainKind::External)
-        .max_weight_to_satisfy()
-        .unwrap();
-
-    let mut builder = wallet.build_tx();
-    let result =
-        builder.add_foreign_utxo(outpoint, psbt::Input::default(), foreign_utxo_satisfaction);
-    assert!(matches!(result, Err(AddForeignUtxoError::NotForeignUtxo)));
-}
-
 #[test]
 fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
     let (mut wallet1, txid1) = get_funded_wallet_wpkh();