feat(wallet): add Wallet::insert_txout function and updated docs for fee functions

notmandatory · notmandatory · commit d37ace56286b · 2023-08-24T09:54:15.000-05:00
added
    - Wallet::insert_txout function to allow inserting foreign TxOuts
    - test to verify error when trying to calculate fee with missing foreign utxo
    - test to calculate fee with inserted foreign utxo

updated
    - docs for Wallet::calculate_fee, Wallet::calculate_fee_rate, and TxGraph::calculate_fee
      with note about missing foreign utxos
diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs
@@ -431,16 +431,40 @@ impl<D> Wallet<D> {
             .next()
     }
 
+    /// Inserts the given foreign `TxOut` at `OutPoint` into the wallet's transaction graph. Any
+    /// inserted foreign TxOuts are not persisted until [`Self::commit`] is called.
+    ///
+    /// Only insert TxOuts you trust the values for!
+    pub fn insert_txout(&mut self, outpoint: OutPoint, txout: TxOut)
+    where
+        D: PersistBackend<ChangeSet>,
+    {
+        let additions = self.indexed_graph.insert_txout(outpoint, &txout);
+        self.persist.stage(ChangeSet::from(additions));
+    }
+
     /// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction.
     ///
+    /// To calculate the fee for a [`Transaction`] that depends on foreign [`TxOut`] values you must
+    /// first manually insert the foreign TxOuts into the tx graph using the [`insert_txout`] function.
+    /// Only insert TxOuts you trust the values for!
+    ///
     /// Note `tx` does not have to be in the graph for this to work.
+    ///
+    /// [`insert_txout`]: Self::insert_txout
     pub fn calculate_fee(&self, tx: &Transaction) -> Result<u64, CalculateFeeError> {
         self.indexed_graph.graph().calculate_fee(tx)
     }
 
-    /// Calculate the `FeeRate` for a given transaction.
+    /// Calculate the [`FeeRate`] for a given transaction.
+    ///
+    /// To calculate the fee rate for a [`Transaction`] that depends on foreign [`TxOut`] values you
+    /// must first manually insert the foreign TxOuts into the tx graph using the [`insert_txout`] function.
+    /// Only insert TxOuts you trust the values for!
     ///
     /// Note `tx` does not have to be in the graph for this to work.
+    ///
+    /// [`insert_txout`]: Self::insert_txout
     pub fn calculate_fee_rate(&self, tx: &Transaction) -> Result<FeeRate, CalculateFeeError> {
         self.calculate_fee(tx).map(|fee| {
             let weight = tx.weight();
@@ -451,14 +475,55 @@ impl<D> Wallet<D> {
     /// Computes total input value going from script pubkeys in the index (sent) and the total output
     /// value going to script pubkeys in the index (received) in `tx`. For the `sent` to be computed
     /// correctly, the output being spent must have already been scanned by the index. Calculating
-    /// received just uses the transaction outputs directly, so it will be correct even if it has not
-    /// been scanned.
+    /// received just uses the [`Transaction`] outputs directly, so it will be correct even if it has
+    /// not been scanned.
     pub fn sent_and_received(&self, tx: &Transaction) -> (u64, u64) {
         self.indexed_graph.index.sent_and_received(tx)
     }
 
-    /// Return a single `CanonicalTx` made and received by the wallet or `None` if it doesn't
-    /// exist in the wallet
+    /// Get a single transaction from the wallet as a [`CanonicalTx`] (if the transaction exists).
+    ///
+    /// `CanonicalTx` contains the full transaction alongside meta-data such as:
+    /// * Blocks that the transaction is [`Anchor`]ed in. These may or may not be blocks that exist
+    ///   in the best chain.
+    /// * The [`ChainPosition`] of the transaction in the best chain - whether the transaction is
+    ///   confirmed or unconfirmed. If the transaction is confirmed, the anchor which proves the
+    ///   confirmation is provided. If the transaction is unconfirmed, the unix timestamp of when
+    ///   the transaction was last seen in the mempool is provided.
+    ///
+    /// ```rust, no_run
+    /// use bdk::{chain::ChainPosition, Wallet};
+    /// use bdk_chain::Anchor;
+    /// # let wallet: Wallet<()> = todo!();
+    /// # let my_txid: bitcoin::Txid = todo!();
+    ///
+    /// let canonical_tx = wallet.get_tx(my_txid).expect("panic if tx does not exist");
+    ///
+    /// // get reference to full transaction
+    /// println!("my tx: {:#?}", canonical_tx.tx_node.tx);
+    ///
+    /// // list all transaction anchors
+    /// for anchor in canonical_tx.tx_node.anchors {
+    ///     println!(
+    ///         "tx is anchored by block of hash {}",
+    ///         anchor.anchor_block().hash
+    ///     );
+    /// }
+    ///
+    /// // get confirmation status of transaction
+    /// match canonical_tx.chain_position {
+    ///     ChainPosition::Confirmed(anchor) => println!(
+    ///         "tx is confirmed at height {}, we know this since {}:{} is in the best chain",
+    ///         anchor.confirmation_height, anchor.anchor_block.height, anchor.anchor_block.hash,
+    ///     ),
+    ///     ChainPosition::Unconfirmed(last_seen) => println!(
+    ///         "tx is last seen at {}, it is unconfirmed as it is not anchored in the best chain",
+    ///         last_seen,
+    ///     ),
+    /// }
+    /// ```
+    ///
+    /// [`Anchor`]: bdk_chain::Anchor
     pub fn get_tx(
         &self,
         txid: Txid,
diff --git a/crates/bdk/tests/common.rs b/crates/bdk/tests/common.rs
@@ -7,7 +7,11 @@ use bitcoin::hashes::Hash;
 use bitcoin::{Address, BlockHash, Network, OutPoint, Transaction, TxIn, TxOut, Txid};
 use std::str::FromStr;
 
-/// Return a fake wallet that appears to be funded for testing.
+// Return a fake wallet that appears to be funded for testing.
+//
+// The funded wallet containing a tx with a 76_000 sats input and two outputs, one spending 25_000
+// to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+// sats are the transaction fee.
 pub fn get_funded_wallet_with_change(
     descriptor: &str,
     change: Option<&str>,
@@ -95,6 +99,11 @@ pub fn get_funded_wallet_with_change(
     (wallet, tx1.txid())
 }
 
+// Return a fake wallet that appears to be funded for testing.
+//
+// The funded wallet containing a tx with a 76_000 sats input and two outputs, one spending 25_000
+// to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+// sats are the transaction fee.
 pub fn get_funded_wallet(descriptor: &str) -> (Wallet, bitcoin::Txid) {
     get_funded_wallet_with_change(descriptor, None)
 }
diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs
@@ -6,7 +6,6 @@ use bdk::wallet::coin_selection::LargestFirstCoinSelection;
 use bdk::wallet::AddressIndex::*;
 use bdk::wallet::{AddressIndex, AddressInfo, Balance, Wallet};
 use bdk::{Error, FeeRate, KeychainKind};
-use bdk_chain::tx_graph::CalculateFeeError;
 use bdk_chain::COINBASE_MATURITY;
 use bdk_chain::{BlockId, ConfirmationTime};
 use bitcoin::hashes::Hash;
@@ -84,63 +83,60 @@ fn test_descriptor_checksum() {
 #[test]
 fn test_get_funded_wallet_balance() {
     let (wallet, _) = get_funded_wallet(get_test_wpkh());
-    assert_eq!(wallet.get_balance().confirmed, 50000);
+
+    // The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
+    // to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+    // sats are the transaction fee.
+    assert_eq!(wallet.get_balance().confirmed, 50_000);
 }
 
 #[test]
 fn test_get_funded_wallet_sent_and_received() {
-    let (wallet, _) = get_funded_wallet(get_test_wpkh());
-    assert_eq!(wallet.get_balance().confirmed, 50000);
+    let (wallet, txid) = get_funded_wallet(get_test_wpkh());
+
     let mut tx_amounts: Vec<(Txid, (u64, u64))> = wallet
         .transactions()
-        .map(|ct| (ct.node.txid, wallet.sent_and_received(ct.node.tx)))
+        .map(|ct| (ct.tx_node.txid, wallet.sent_and_received(ct.tx_node.tx)))
         .collect();
     tx_amounts.sort_by(|a1, a2| a1.0.cmp(&a2.0));
 
-    assert_eq!(tx_amounts.len(), 2);
-    assert_matches!(tx_amounts.get(0), Some((_, (76_000, 50_000))))
+    let tx = wallet.get_tx(txid).expect("transaction").tx_node.tx;
+    let (sent, received) = wallet.sent_and_received(tx);
+
+    // The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
+    // to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+    // sats are the transaction fee.
+    assert_eq!(sent, 76_000);
+    assert_eq!(received, 50_000);
 }
 
 #[test]
 fn test_get_funded_wallet_tx_fees() {
-    let (wallet, _) = get_funded_wallet(get_test_wpkh());
-    assert_eq!(wallet.get_balance().confirmed, 50000);
-    let mut tx_fee_amounts: Vec<(Txid, Result<u64, CalculateFeeError>)> = wallet
-        .transactions()
-        .map(|ct| {
-            let fee = wallet.calculate_fee(ct.node.tx);
-            (ct.node.txid, fee)
-        })
-        .collect();
-    tx_fee_amounts.sort_by(|a1, a2| a1.0.cmp(&a2.0));
+    let (wallet, txid) = get_funded_wallet(get_test_wpkh());
 
-    assert_eq!(tx_fee_amounts.len(), 2);
-    assert_matches!(
-        tx_fee_amounts.get(1),
-        Some((_, Err(CalculateFeeError::MissingTxOut(_))))
-    );
-    assert_matches!(tx_fee_amounts.get(0), Some((_, Ok(1000))))
+    let tx = wallet.get_tx(txid).expect("transaction").tx_node.tx;
+    let tx_fee = wallet.calculate_fee(tx).expect("transaction fee");
+
+    // The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
+    // to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+    // sats are the transaction fee.
+    assert_eq!(tx_fee, 1000)
 }
 
 #[test]
 fn test_get_funded_wallet_tx_fee_rate() {
-    let (wallet, _) = get_funded_wallet(get_test_wpkh());
-    assert_eq!(wallet.get_balance().confirmed, 50000);
-    let mut tx_fee_rates: Vec<(Txid, Result<FeeRate, CalculateFeeError>)> = wallet
-        .transactions()
-        .map(|ct| {
-            let fee_rate = wallet.calculate_fee_rate(ct.node.tx);
-            (ct.node.txid, fee_rate)
-        })
-        .collect();
-    tx_fee_rates.sort_by(|a1, a2| a1.0.cmp(&a2.0));
+    let (wallet, txid) = get_funded_wallet(get_test_wpkh());
 
-    assert_eq!(tx_fee_rates.len(), 2);
-    assert_matches!(
-        tx_fee_rates.get(1),
-        Some((_, Err(CalculateFeeError::MissingTxOut(_))))
-    );
-    assert_matches!(tx_fee_rates.get(0), Some((_, Ok(_))))
+    let tx = wallet.get_tx(txid).expect("transaction").tx_node.tx;
+    let tx_fee_rate = wallet.calculate_fee_rate(tx).expect("transaction fee rate");
+
+    // The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000
+    // to a foreign address and one returning 50_000 back to the wallet as change. The remaining 1000
+    // sats are the transaction fee.
+
+    // tx weight = 452 bytes, as vbytes = (452+3)/4 = 113
+    // fee rate (sats per vbyte) = fee / vbytes = 1000 / 113 = 8.8495575221 rounded to 8.849558
+    assert_eq!(tx_fee_rate.as_sat_per_vb(), 8.849558);
 }
 
 macro_rules! assert_fee_rate {
@@ -1098,6 +1094,77 @@ fn test_add_foreign_utxo() {
     assert!(finished, "all the inputs should have been signed now");
 }
 
+#[test]
+#[should_panic(
+    expected = "MissingTxOut([OutPoint { txid: 0x21d7fb1bceda00ab4069fc52d06baa13470803e9050edd16f5736e5d8c4925fd, vout: 0 }])"
+)]
+fn test_calculate_fee_with_missing_foreign_utxo() {
+    let (mut wallet1, _) = get_funded_wallet(get_test_wpkh());
+    let (wallet2, _) =
+        get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
+
+    let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX")
+        .unwrap()
+        .assume_checked();
+    let utxo = wallet2.list_unspent().next().expect("must take!");
+    #[allow(deprecated)]
+    let foreign_utxo_satisfaction = wallet2
+        .get_descriptor_for_keychain(KeychainKind::External)
+        .max_satisfaction_weight()
+        .unwrap();
+
+    let psbt_input = psbt::Input {
+        witness_utxo: Some(utxo.txout.clone()),
+        ..Default::default()
+    };
+
+    let mut builder = wallet1.build_tx();
+    builder
+        .add_recipient(addr.script_pubkey(), 60_000)
+        .only_witness_utxo()
+        .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
+        .unwrap();
+    let psbt = builder.finish().unwrap();
+    let tx = psbt.extract_tx();
+    wallet1.calculate_fee(&tx).unwrap();
+}
+
+#[test]
+fn test_calculate_fee_with_inserted_foreign_utxo() {
+    let (mut wallet1, _) = get_funded_wallet(get_test_wpkh());
+    let (wallet2, _) =
+        get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
+
+    let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX")
+        .unwrap()
+        .assume_checked();
+    let utxo = wallet2.list_unspent().next().expect("must take!");
+    #[allow(deprecated)]
+    let foreign_utxo_satisfaction = wallet2
+        .get_descriptor_for_keychain(KeychainKind::External)
+        .max_satisfaction_weight()
+        .unwrap();
+
+    let psbt_input = psbt::Input {
+        witness_utxo: Some(utxo.txout.clone()),
+        ..Default::default()
+    };
+
+    let mut builder = wallet1.build_tx();
+    builder
+        .add_recipient(addr.script_pubkey(), 60_000)
+        .only_witness_utxo()
+        .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
+        .unwrap();
+    let psbt = builder.finish().unwrap();
+    let psbt_fee = psbt.fee_amount().expect("psbt fee");
+    let tx = psbt.extract_tx();
+
+    wallet1.insert_txout(utxo.outpoint, utxo.txout);
+    let wallet1_fee = wallet1.calculate_fee(&tx).expect("wallet fee");
+    assert_eq!(psbt_fee, wallet1_fee);
+}
+
 #[test]
 #[should_panic(expected = "Generic(\"Foreign utxo missing witness_utxo or non_witness_utxo\")")]
 fn test_add_foreign_utxo_invalid_psbt_input() {
@@ -1122,8 +1189,8 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
         get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
 
     let utxo2 = wallet2.list_unspent().next().unwrap();
-    let tx1 = wallet1.get_tx(txid1).unwrap().node.tx.clone();
-    let tx2 = wallet2.get_tx(txid2).unwrap().node.tx.clone();
+    let tx1 = wallet1.get_tx(txid1).unwrap().tx_node.tx.clone();
+    let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx.clone();
 
     #[allow(deprecated)]
     let satisfaction_weight = wallet2
@@ -1212,7 +1279,7 @@ fn test_add_foreign_utxo_only_witness_utxo() {
 
     {
         let mut builder = builder.clone();
-        let tx2 = wallet2.get_tx(txid2).unwrap().node.tx;
+        let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx;
         let psbt_input = psbt::Input {
             non_witness_utxo: Some(tx2.clone()),
             ..Default::default()
@@ -2842,7 +2909,7 @@ fn test_taproot_sign_using_non_witness_utxo() {
     let mut psbt = builder.finish().unwrap();
 
     psbt.inputs[0].witness_utxo = None;
-    psbt.inputs[0].non_witness_utxo = Some(wallet.get_tx(prev_txid).unwrap().node.tx.clone());
+    psbt.inputs[0].non_witness_utxo = Some(wallet.get_tx(prev_txid).unwrap().tx_node.tx.clone());
     assert!(
         psbt.inputs[0].non_witness_utxo.is_some(),
         "Previous tx should be present in the database"
diff --git a/crates/chain/src/spk_txout_index.rs b/crates/chain/src/spk_txout_index.rs
@@ -288,8 +288,8 @@ impl<I: Clone + Ord> SpkTxOutIndex<I> {
     /// Computes total input value going from script pubkeys in the index (sent) and the total output
     /// value going to script pubkeys in the index (received) in `tx`. For the `sent` to be computed
     /// correctly, the output being spent must have already been scanned by the index. Calculating
-    /// received just uses the transaction outputs directly, so it will be correct even if it has not
-    /// been scanned.
+    /// received just uses the [`Transaction`] outputs directly, so it will be correct even if it has
+    /// not been scanned.
     pub fn sent_and_received(&self, tx: &Transaction) -> (u64, u64) {
         let mut sent = 0;
         let mut received = 0;
diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs
@@ -245,33 +245,37 @@ impl<A> TxGraph<A> {
     }
 
     /// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction.
-    /// Returns `OK(_)` if we have all the `TxOut`s being spent by `tx` in the graph (either as
+    /// Returns `OK(_)` if we have all the [`TxOut`]s being spent by `tx` in the graph (either as
     /// the full transactions or individual txouts).
     ///
+    /// To calculate the fee for a [`Transaction`] that depends on foreign [`TxOut`] values you must
+    /// first manually insert the foreign TxOuts into the tx graph using the [`insert_txout`] function.
+    /// Only insert TxOuts you trust the values for!
+    ///
     /// Note `tx` does not have to be in the graph for this to work.
+    ///
+    /// [`insert_txout`]: Self::insert_txout
     pub fn calculate_fee(&self, tx: &Transaction) -> Result<u64, CalculateFeeError> {
         if tx.is_coin_base() {
             return Ok(0);
         }
-        let inputs_sum = tx.input.iter().fold(
-            (0_u64, Vec::new()),
+
+        let (inputs_sum, missing_outputs) = tx.input.iter().fold(
+            (0_i64, Vec::new()),
             |(mut sum, mut missing_outpoints), txin| match self.get_txout(txin.previous_output) {
                 None => {
                     missing_outpoints.push(txin.previous_output);
                     (sum, missing_outpoints)
                 }
                 Some(txout) => {
-                    sum += txout.value;
+                    sum += txout.value as i64;
                     (sum, missing_outpoints)
                 }
             },
         );
-
-        let inputs_sum = if inputs_sum.1.is_empty() {
-            Ok(inputs_sum.0 as i64)
-        } else {
-            Err(CalculateFeeError::MissingTxOut(inputs_sum.1))
-        }?;
+        if !missing_outputs.is_empty() {
+            return Err(CalculateFeeError::MissingTxOut(missing_outputs));
+        }
 
         let outputs_sum = tx
             .output
