diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b4eb7709..af00201b 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -886,6 +886,31 @@ impl Wallet { .next() } + /// Inserts a full [`Transaction`] into the wallet's transaction graph. + /// + /// This is used for providing a previous transaction when wallet operations require the full + /// transaction data rather than only a [`TxOut`], such as reconstructing a `non_witness_utxo` + /// for foreign inputs. + /// + /// Transactions inserted with this method may affect wallet queries such as [`get_tx`], + /// [`list_unspent`], or [`list_output`] if they are relevant to this wallet. + /// + /// **WARNINGS:** This should only be used to add transactions that you trust. If the + /// transaction contains outputs owned by this wallet, those outputs will be indexed as wallet + /// outputs. + /// + /// You must persist the changes resulting from one or more calls to this method if you need + /// the inserted transaction data to be reloaded after closing the wallet. + /// See [`Wallet::reveal_next_address`]. + /// + /// [`get_tx`]: Self::get_tx + /// [`list_unspent`]: Self::list_unspent + /// [`list_output`]: Self::list_output + pub fn insert_tx>>(&mut self, tx: T) { + let additions = self.tx_graph.insert_tx(tx); + self.stage.merge(additions.into()); + } + /// Inserts a [`TxOut`] at [`OutPoint`] into the wallet's transaction graph. /// /// This is used for providing a previous output's value so that we can use [`calculate_fee`] diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index 767cd5a4..785fd2df 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -366,9 +366,12 @@ impl<'a, Cs> TxBuilder<'a, Cs> { /// /// This is an **EXPERIMENTAL** feature, API and other major changes are expected. /// - /// In order to use [`Wallet::calculate_fee`] or [`Wallet::calculate_fee_rate`] for a - /// transaction created with foreign UTXO(s) you must manually insert the corresponding - /// TxOut(s) into the tx graph using the [`Wallet::insert_txout`] function. + /// Before calling this method, you must preregister the foreign prev data in the wallet tx + /// graph. Use [`Wallet::insert_txout`] if `psbt_input` only provides a `witness_utxo`, or + /// [`Wallet::insert_tx`] if it provides a `non_witness_utxo`. + /// + /// This preregistration is also what allows [`Wallet::calculate_fee`] or + /// [`Wallet::calculate_fee_rate`] to work for transactions created with foreign UTXO(s). /// /// # Errors /// @@ -376,6 +379,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> { /// /// 1. The `psbt_input` does not contain a `witness_utxo` or `non_witness_utxo`. /// 2. The data in `non_witness_utxo` does not match what is in `outpoint`. + /// 3. The wallet tx graph does not already contain the prev data required by `psbt_input`. /// /// Note unless you set [`only_witness_utxo`] any non-taproot `psbt_input` you pass to this /// method must have `non_witness_utxo` set otherwise you will get an error when [`finish`] @@ -407,25 +411,32 @@ impl<'a, Cs> TxBuilder<'a, Cs> { satisfaction_weight: Weight, sequence: Sequence, ) -> Result<&mut Self, AddForeignUtxoError> { - 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); - } + if psbt_input.witness_utxo.is_none() && psbt_input.non_witness_utxo.is_none() { + return Err(AddForeignUtxoError::MissingUtxo); + } + + 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 psbt_input.non_witness_utxo.is_some() { + if self.wallet.tx_graph().get_tx(outpoint.txid).is_none() { + return Err(AddForeignUtxoError::MissingRegisteredTx(outpoint)); + } + } else if psbt_input.witness_utxo.is_some() + && self.wallet.tx_graph().get_txout(outpoint).is_none() + { + return Err(AddForeignUtxoError::MissingRegisteredTxOut(outpoint)); + } + let mut existing_index: Option = None; for (idx, wutxo) in self.params.utxos.iter().enumerate() { @@ -803,6 +814,10 @@ pub enum AddForeignUtxoError { InvalidOutpoint(OutPoint), /// Foreign utxo missing witness_utxo or non_witness_utxo MissingUtxo, + /// Foreign utxo outpoint has not been preregistered in the wallet tx graph + MissingRegisteredTxOut(OutPoint), + /// Foreign utxo parent transaction has not been preregistered in the wallet tx graph + MissingRegisteredTx(OutPoint), } impl fmt::Display for AddForeignUtxoError { @@ -822,6 +837,16 @@ impl fmt::Display for AddForeignUtxoError { outpoint.txid, outpoint.vout, ), Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"), + Self::MissingRegisteredTxOut(outpoint) => write!( + f, + "Foreign UTXO must be inserted with Wallet::insert_txout or Wallet::insert_tx before calling add_foreign_utxo for txid: {} with vout: {}", + outpoint.txid, outpoint.vout, + ), + Self::MissingRegisteredTx(outpoint) => write!( + f, + "Foreign UTXO parent transaction must be inserted with Wallet::insert_tx before calling add_foreign_utxo for txid: {} with vout: {}", + outpoint.txid, outpoint.vout, + ), } } } @@ -1340,6 +1365,9 @@ mod test { .max_weight_to_satisfy() .unwrap(); + // Preregister the full foreign parent tx for `non_witness_utxo`. + wallet2.insert_tx(tx1.as_ref().clone()); + let mut builder = wallet2.build_tx(); // add foreign UTXO with satisfaction weight x diff --git a/tests/add_foreign_utxo.rs b/tests/add_foreign_utxo.rs index 1dd0a8c9..647e136c 100644 --- a/tests/add_foreign_utxo.rs +++ b/tests/add_foreign_utxo.rs @@ -5,7 +5,7 @@ use bdk_wallet::signer::SignOptions; use bdk_wallet::test_utils::*; use bdk_wallet::tx_builder::AddForeignUtxoError; use bdk_wallet::KeychainKind; -use bitcoin::{psbt, Address, Amount}; +use bitcoin::{psbt, Address, Amount, OutPoint}; mod common; @@ -29,6 +29,8 @@ fn test_add_foreign_utxo() { ..Default::default() }; + wallet1.insert_txout(utxo.outpoint, utxo.txout); + let mut builder = wallet1.build_tx(); builder .add_recipient(addr.script_pubkey(), Amount::from_sat(60_000)) @@ -36,7 +38,6 @@ fn test_add_foreign_utxo() { .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction) .unwrap(); let mut psbt = builder.finish().unwrap(); - wallet1.insert_txout(utxo.outpoint, utxo.txout); let fee = check_fee!(wallet1, psbt); let (sent, received) = wallet1.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); @@ -83,57 +84,175 @@ fn test_add_foreign_utxo() { } #[test] -fn test_calculate_fee_with_missing_foreign_utxo() { - use bdk_chain::tx_graph::CalculateFeeError; +fn test_add_foreign_utxo_invalid_psbt_input() { + 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 err = builder + .add_foreign_utxo(outpoint, psbt::Input::default(), foreign_utxo_satisfaction) + .unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!(matches!(err, AddForeignUtxoError::MissingUtxo)); +} + +#[test] +fn test_add_foreign_utxo_requires_inserted_txout() { let (mut wallet1, _) = get_funded_wallet_wpkh(); let (wallet2, _) = get_funded_wallet_single("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); - let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX") - .unwrap() - .assume_checked(); let utxo = wallet2.list_unspent().next().expect("must take!"); let foreign_utxo_satisfaction = wallet2 .public_descriptor(KeychainKind::External) .max_weight_to_satisfy() .unwrap(); - let psbt_input = psbt::Input { - witness_utxo: Some(utxo.txout.clone()), - ..Default::default() - }; + { + let mut builder = wallet1.build_tx(); + let err = builder + .add_foreign_utxo( + utxo.outpoint, + psbt::Input { + witness_utxo: Some(utxo.txout.clone()), + ..Default::default() + }, + foreign_utxo_satisfaction, + ) + .unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!( + matches!(err, AddForeignUtxoError::MissingRegisteredTxOut(outpoint) if outpoint == utxo.outpoint) + ); + } - let mut builder = wallet1.build_tx(); - builder - .add_recipient(addr.script_pubkey(), Amount::from_sat(60_000)) - .only_witness_utxo() - .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction) + wallet1.insert_txout(utxo.outpoint, utxo.txout.clone()); + + { + let mut builder = wallet1.build_tx(); + let result = builder.add_foreign_utxo( + utxo.outpoint, + psbt::Input { + witness_utxo: Some(utxo.txout), + ..Default::default() + }, + foreign_utxo_satisfaction, + ); + assert!( + result.is_ok(), + "should succeed once the txout is inserted into wallet" + ); + } +} + +#[test] +fn test_add_foreign_utxo_requires_inserted_tx() { + let (mut wallet1, _) = get_funded_wallet_wpkh(); + let (wallet2, txid2) = + get_funded_wallet_single("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); + + let utxo = wallet2.list_unspent().next().expect("must take!"); + let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx; + let foreign_utxo_satisfaction = wallet2 + .public_descriptor(KeychainKind::External) + .max_weight_to_satisfy() .unwrap(); - let psbt = builder.finish().unwrap(); - let tx = psbt.extract_tx().expect("failed to extract tx"); - let res = wallet1.calculate_fee(&tx); - assert!( - matches!(res, Err(CalculateFeeError::MissingTxOut(outpoints)) if outpoints[0] == utxo.outpoint) - ); + + { + let mut builder = wallet1.build_tx(); + let err = builder + .add_foreign_utxo( + utxo.outpoint, + psbt::Input { + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + foreign_utxo_satisfaction, + ) + .unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!( + matches!(err, AddForeignUtxoError::MissingRegisteredTx(outpoint) if outpoint == utxo.outpoint) + ); + } + + wallet1.insert_tx(tx2.as_ref().clone()); + + { + let mut builder = wallet1.build_tx(); + let result = builder.add_foreign_utxo( + utxo.outpoint, + psbt::Input { + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + foreign_utxo_satisfaction, + ); + assert!( + result.is_ok(), + "should succeed once the parent tx is inserted into wallet" + ); + } } #[test] -fn test_add_foreign_utxo_invalid_psbt_input() { - let (mut wallet, _) = get_funded_wallet_wpkh(); - let outpoint = wallet.list_unspent().next().expect("must exist").outpoint; - let foreign_utxo_satisfaction = wallet +/// When both `non_witness_utxo` and `witness_utxo` are present, the parent tx must be inserted. +fn test_add_foreign_utxo_requires_inserted_tx_when_both_prevout_forms_are_present() { + let (mut wallet1, _) = get_funded_wallet_wpkh(); + let (wallet2, txid2) = + get_funded_wallet_single("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); + + let utxo = wallet2.list_unspent().next().expect("must take!"); + let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx; + let foreign_utxo_satisfaction = wallet2 .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::MissingUtxo))); + wallet1.insert_txout(utxo.outpoint, utxo.txout.clone()); + + { + let mut builder = wallet1.build_tx(); + let result = builder.add_foreign_utxo( + utxo.outpoint, + psbt::Input { + witness_utxo: Some(utxo.txout.clone()), + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + foreign_utxo_satisfaction, + ); + assert!( + matches!(result, Err(AddForeignUtxoError::MissingRegisteredTx(outpoint)) if outpoint == utxo.outpoint) + ); + } + + wallet1.insert_tx(tx2.as_ref().clone()); + + { + let mut builder = wallet1.build_tx(); + let result = builder.add_foreign_utxo( + utxo.outpoint, + psbt::Input { + witness_utxo: Some(utxo.txout), + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + foreign_utxo_satisfaction, + ); + assert!( + result.is_ok(), + "should require the parent tx when non_witness_utxo is present" + ); + } } #[test] -fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() { +fn test_add_foreign_utxo_rejects_non_witness_utxo_with_wrong_txid() { let (mut wallet1, txid1) = get_funded_wallet_wpkh(); let (wallet2, txid2) = get_funded_wallet_single("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); @@ -147,32 +266,78 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() { .max_weight_to_satisfy() .unwrap(); - let mut builder = wallet1.build_tx(); - assert!( - builder + { + let mut builder = wallet1.build_tx(); + let err = builder .add_foreign_utxo( utxo2.outpoint, psbt::Input { non_witness_utxo: Some(tx1.as_ref().clone()), ..Default::default() }, - satisfaction_weight + satisfaction_weight, ) - .is_err(), - "should fail when outpoint doesn't match psbt_input" - ); + .unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!( + matches!( + err, + AddForeignUtxoError::InvalidTxid { + input_txid, + foreign_utxo, + } if input_txid == tx1.compute_txid() && foreign_utxo == utxo2.outpoint + ), + "should fail when outpoint doesn't match psbt_input" + ); + } + wallet1.insert_tx(tx2.as_ref().clone()); + { + let mut builder = wallet1.build_tx(); + assert!( + builder + .add_foreign_utxo( + utxo2.outpoint, + psbt::Input { + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + satisfaction_weight + ) + .is_ok(), + "should be ok when outpoint does match psbt_input" + ); + } +} + +#[test] +fn test_add_foreign_utxo_rejects_non_witness_utxo_with_out_of_bounds_vout() { + let (mut wallet1, _) = get_funded_wallet_wpkh(); + let (wallet2, txid2) = + get_funded_wallet_single("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); + + let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx; + let invalid_outpoint = OutPoint::new(txid2, tx2.output.len() as u32); + let satisfaction_weight = wallet2 + .public_descriptor(KeychainKind::External) + .max_weight_to_satisfy() + .unwrap(); + + let mut builder = wallet1.build_tx(); + let err = builder + .add_foreign_utxo( + invalid_outpoint, + psbt::Input { + non_witness_utxo: Some(tx2.as_ref().clone()), + ..Default::default() + }, + satisfaction_weight, + ) + .unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!( - builder - .add_foreign_utxo( - utxo2.outpoint, - psbt::Input { - non_witness_utxo: Some(tx2.as_ref().clone()), - ..Default::default() - }, - satisfaction_weight - ) - .is_ok(), - "should be ok when outpoint does match psbt_input" + matches!(err, AddForeignUtxoError::InvalidOutpoint(outpoint) if outpoint == invalid_outpoint), + "should fail when vout is outside the parent tx outputs" ); } @@ -191,6 +356,8 @@ fn test_add_foreign_utxo_only_witness_utxo() { .max_weight_to_satisfy() .unwrap(); + wallet1.insert_txout(utxo2.outpoint, utxo2.txout.clone()); + { let mut builder = wallet1.build_tx(); builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000)); @@ -227,10 +394,12 @@ fn test_add_foreign_utxo_only_witness_utxo() { } { + let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx; + wallet1.insert_tx(tx2.as_ref().clone()); + let mut builder = wallet1.build_tx(); builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000)); - let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx; let psbt_input = psbt::Input { non_witness_utxo: Some(tx2.as_ref().clone()), ..Default::default() @@ -265,6 +434,8 @@ fn test_taproot_foreign_utxo() { "`non_witness_utxo` should never be populated for taproot" ); + wallet1.insert_txout(utxo.outpoint, utxo.txout); + let mut builder = wallet1.build_tx(); builder .add_recipient(addr.script_pubkey(), Amount::from_sat(60_000)) @@ -273,7 +444,6 @@ fn test_taproot_foreign_utxo() { let psbt = builder.finish().unwrap(); let (sent, received) = wallet1.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); - wallet1.insert_txout(utxo.outpoint, utxo.txout); let fee = check_fee!(wallet1, psbt); assert_eq!( diff --git a/tests/wallet.rs b/tests/wallet.rs index c6048593..00c4aa94 100644 --- a/tests/wallet.rs +++ b/tests/wallet.rs @@ -78,6 +78,34 @@ fn test_get_funded_wallet_balance() { assert_eq!(wallet.balance().confirmed, Amount::from_sat(50_000)); } +#[test] +fn test_insert_tx() { + let (desc, change_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + + let mut wallet = Wallet::create(desc, change_desc) + .network(Network::Testnet) + .create_wallet_no_persist() + .unwrap(); + + let tx = Transaction { + input: vec![], + output: vec![TxOut { + script_pubkey: wallet + .next_unused_address(KeychainKind::External) + .script_pubkey(), + value: Amount::from_sat(25_000), + }], + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + }; + let txid = tx.compute_txid(); + + wallet.insert_tx(tx); + + assert!(wallet.tx_graph().get_tx(txid).is_some()); + assert!(wallet.take_staged().is_some()); +} + #[test] fn test_get_funded_wallet_sent_and_received() { let (wallet, txid) = get_funded_wallet_wpkh();