From 20f4c1f0505cf6f152a4abdfd5b08b9ffe2d846d Mon Sep 17 00:00:00 2001 From: Leonardo Lima Date: Fri, 26 Apr 2024 11:17:23 -0300 Subject: [PATCH] feat: use `Amount` on `spk_txout_index` and related - update `wallet.rs` fns: `sent_and_received` fn - update `keychain` `txout_index`fn: `sent_and_received and `net_value` --- crates/bdk/src/wallet/mod.rs | 4 +- crates/bdk/tests/wallet.rs | 118 +++++++++++++-------- crates/chain/src/keychain/txout_index.rs | 10 +- crates/chain/src/spk_txout_index.rs | 22 ++-- crates/chain/tests/test_spk_txout_index.rs | 40 +++++-- 5 files changed, 125 insertions(+), 69 deletions(-) diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index cbe1a82416..432c207878 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -953,7 +953,7 @@ impl Wallet { .map(|fee| bitcoin::Amount::from_sat(fee) / tx.weight()) } - /// Compute the `tx`'s sent and received amounts (in satoshis). + /// Compute the `tx`'s sent and received amounts (in [`bitcoin::Amount`]). /// /// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation @@ -978,7 +978,7 @@ impl Wallet { /// let tx = &psbt.clone().extract_tx().expect("tx"); /// let (sent, received) = wallet.sent_and_received(tx); /// ``` - pub fn sent_and_received(&self, tx: &Transaction) -> (u64, u64) { + pub fn sent_and_received(&self, tx: &Transaction) -> (Amount, Amount) { self.indexed_graph.index.sent_and_received(tx, ..) } diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index 1246790c9d..0211e3ada6 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -207,7 +207,7 @@ fn test_get_funded_wallet_balance() { fn test_get_funded_wallet_sent_and_received() { let (wallet, txid) = get_funded_wallet(get_test_wpkh()); - let mut tx_amounts: Vec<(Txid, (u64, u64))> = wallet + let mut tx_amounts: Vec<(Txid, (Amount, Amount))> = wallet .transactions() .map(|ct| (ct.tx_node.txid, wallet.sent_and_received(&ct.tx_node))) .collect(); @@ -219,8 +219,8 @@ fn test_get_funded_wallet_sent_and_received() { // 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); + assert_eq!(sent.to_sat(), 76_000); + assert_eq!(received.to_sat(), 50_000); } #[test] @@ -1034,7 +1034,8 @@ fn test_create_tx_add_utxo() { "should add an additional input since 25_000 < 30_000" ); assert_eq!( - sent_received.0, 75_000, + sent_received.0, + Amount::from_sat(75_000), "total should be sum of both inputs" ); } @@ -1225,7 +1226,7 @@ fn test_add_foreign_utxo() { wallet1.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); assert_eq!( - sent_received.0 - sent_received.1, + (sent_received.0 - sent_received.1).to_sat(), 10_000 + fee.unwrap_or(0), "we should have only net spent ~10_000" ); @@ -1622,8 +1623,8 @@ fn test_bump_fee_reduce_change() { assert_eq!(sent_received.0, original_sent_received.0); assert_eq!( - sent_received.1 + fee.unwrap_or(0), - original_sent_received.1 + original_fee.unwrap_or(0) + sent_received.1 + Amount::from_sat(fee.unwrap_or(0)), + original_sent_received.1 + Amount::from_sat(original_fee.unwrap_or(0)) ); assert!(fee.unwrap_or(0) > original_fee.unwrap_or(0)); @@ -1642,8 +1643,7 @@ fn test_bump_fee_reduce_change() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -1659,8 +1659,8 @@ fn test_bump_fee_reduce_change() { assert_eq!(sent_received.0, original_sent_received.0); assert_eq!( - sent_received.1 + fee.unwrap_or(0), - original_sent_received.1 + original_fee.unwrap_or(0) + sent_received.1 + Amount::from_sat(fee.unwrap_or(0)), + original_sent_received.1 + Amount::from_sat(original_fee.unwrap_or(0)) ); assert!( fee.unwrap_or(0) > original_fee.unwrap_or(0), @@ -1684,8 +1684,7 @@ fn test_bump_fee_reduce_change() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -1729,7 +1728,7 @@ fn test_bump_fee_reduce_single_recipient() { let tx = &psbt.unsigned_tx; assert_eq!(tx.output.len(), 1); assert_eq!( - tx.output[0].value.to_sat() + fee.unwrap_or(0), + tx.output[0].value + Amount::from_sat(fee.unwrap_or(0)), sent_received.0 ); @@ -1771,7 +1770,7 @@ fn test_bump_fee_absolute_reduce_single_recipient() { assert_eq!(tx.output.len(), 1); assert_eq!( - tx.output[0].value.to_sat() + fee.unwrap_or(0), + tx.output[0].value + Amount::from_sat(fee.unwrap_or(0)), sent_received.0 ); @@ -1825,7 +1824,7 @@ fn test_bump_fee_drain_wallet() { wallet .insert_tx(tx, ConfirmationTime::Unconfirmed { last_seen: 0 }) .unwrap(); - assert_eq!(original_sent_received.0, 25_000); + assert_eq!(original_sent_received.0, Amount::from_sat(25_000)); // for the new feerate, it should be enough to reduce the output, but since we specify // `drain_wallet` we expect to spend everything @@ -1838,7 +1837,7 @@ fn test_bump_fee_drain_wallet() { let psbt = builder.finish().unwrap(); let sent_received = wallet.sent_and_received(&psbt.extract_tx().expect("failed to extract tx")); - assert_eq!(sent_received.0, 75_000); + assert_eq!(sent_received.0, Amount::from_sat(75_000)); } #[test] @@ -1895,7 +1894,7 @@ fn test_bump_fee_remove_output_manually_selected_only() { wallet .insert_tx(tx, ConfirmationTime::Unconfirmed { last_seen: 0 }) .unwrap(); - assert_eq!(original_sent_received.0, 25_000); + assert_eq!(original_sent_received.0, Amount::from_sat(25_000)); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder @@ -1949,8 +1948,14 @@ fn test_bump_fee_add_input() { let sent_received = wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - assert_eq!(sent_received.0, original_details.0 + 25_000); - assert_eq!(fee.unwrap_or(0) + sent_received.1, 30_000); + assert_eq!( + sent_received.0, + original_details.0 + Amount::from_sat(25_000) + ); + assert_eq!( + Amount::from_sat(fee.unwrap_or(0)) + sent_received.1, + Amount::from_sat(30_000) + ); let tx = &psbt.unsigned_tx; assert_eq!(tx.input.len(), 2); @@ -1968,8 +1973,7 @@ fn test_bump_fee_add_input() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -2002,8 +2006,14 @@ fn test_bump_fee_absolute_add_input() { wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - assert_eq!(sent_received.0, original_sent_received.0 + 25_000); - assert_eq!(fee.unwrap_or(0) + sent_received.1, 30_000); + assert_eq!( + sent_received.0, + original_sent_received.0 + Amount::from_sat(25_000) + ); + assert_eq!( + Amount::from_sat(fee.unwrap_or(0)) + sent_received.1, + Amount::from_sat(30_000) + ); let tx = &psbt.unsigned_tx; assert_eq!(tx.input.len(), 2); @@ -2021,8 +2031,7 @@ fn test_bump_fee_absolute_add_input() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -2065,11 +2074,15 @@ fn test_bump_fee_no_change_add_input_and_change() { wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - let original_send_all_amount = original_sent_received.0 - original_fee.unwrap_or(0); - assert_eq!(sent_received.0, original_sent_received.0 + 50_000); + let original_send_all_amount = + original_sent_received.0 - Amount::from_sat(original_fee.unwrap_or(0)); + assert_eq!( + sent_received.0, + original_sent_received.0 + Amount::from_sat(50_000) + ); assert_eq!( sent_received.1, - 75_000 - original_send_all_amount - fee.unwrap_or(0) + Amount::from_sat(75_000) - original_send_all_amount - Amount::from_sat(fee.unwrap_or(0)) ); let tx = &psbt.unsigned_tx; @@ -2081,16 +2094,15 @@ fn test_bump_fee_no_change_add_input_and_change() { .find(|txout| txout.script_pubkey == addr.script_pubkey()) .unwrap() .value, - Amount::from_sat(original_send_all_amount) + original_send_all_amount ); assert_eq!( tx.output .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), - 75_000 - original_send_all_amount - fee.unwrap_or(0) + .value, + Amount::from_sat(75_000) - original_send_all_amount - Amount::from_sat(fee.unwrap_or(0)) ); assert_fee_rate!(psbt, fee.unwrap_or(0), FeeRate::from_sat_per_vb_unchecked(50), @add_signature); @@ -2145,11 +2157,17 @@ fn test_bump_fee_add_input_change_dust() { wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - assert_eq!(original_sent_received.1, 5_000 - original_fee.unwrap_or(0)); + assert_eq!( + original_sent_received.1, + Amount::from_sat(5_000 - original_fee.unwrap_or(0)) + ); - assert_eq!(sent_received.0, original_sent_received.0 + 25_000); + assert_eq!( + sent_received.0, + original_sent_received.0 + Amount::from_sat(25_000) + ); assert_eq!(fee.unwrap_or(0), 30_000); - assert_eq!(sent_received.1, 0); + assert_eq!(sent_received.1, Amount::ZERO); let tx = &psbt.unsigned_tx; assert_eq!(tx.input.len(), 2); @@ -2200,8 +2218,14 @@ fn test_bump_fee_force_add_input() { wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - assert_eq!(sent_received.0, original_sent_received.0 + 25_000); - assert_eq!(fee.unwrap_or(0) + sent_received.1, 30_000); + assert_eq!( + sent_received.0, + original_sent_received.0 + Amount::from_sat(25_000) + ); + assert_eq!( + Amount::from_sat(fee.unwrap_or(0)) + sent_received.1, + Amount::from_sat(30_000) + ); let tx = &psbt.unsigned_tx; assert_eq!(tx.input.len(), 2); @@ -2219,8 +2243,7 @@ fn test_bump_fee_force_add_input() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -2260,8 +2283,14 @@ fn test_bump_fee_absolute_force_add_input() { wallet.sent_and_received(&psbt.clone().extract_tx().expect("failed to extract tx")); let fee = check_fee!(wallet, psbt); - assert_eq!(sent_received.0, original_sent_received.0 + 25_000); - assert_eq!(fee.unwrap_or(0) + sent_received.1, 30_000); + assert_eq!( + sent_received.0, + original_sent_received.0 + Amount::from_sat(25_000) + ); + assert_eq!( + Amount::from_sat(fee.unwrap_or(0)) + sent_received.1, + Amount::from_sat(30_000) + ); let tx = &psbt.unsigned_tx; assert_eq!(tx.input.len(), 2); @@ -2279,8 +2308,7 @@ fn test_bump_fee_absolute_force_add_input() { .iter() .find(|txout| txout.script_pubkey != addr.script_pubkey()) .unwrap() - .value - .to_sat(), + .value, sent_received.1 ); @@ -3228,7 +3256,7 @@ fn test_taproot_foreign_utxo() { assert_eq!( sent_received.0 - sent_received.1, - 10_000 + fee.unwrap_or(0), + Amount::from_sat(10_000 + fee.unwrap_or(0)), "we should have only net spent ~10_000" ); diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 4b2479ec90..789db6c6f4 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -5,7 +5,7 @@ use crate::{ spk_iter::BIP32_MAX_INDEX, SpkIterator, SpkTxOutIndex, }; -use bitcoin::{OutPoint, Script, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid}; use core::{ fmt::Debug, ops::{Bound, RangeBounds}, @@ -273,7 +273,11 @@ impl KeychainTxOutIndex { /// *received* when it is on an output. For `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. - pub fn sent_and_received(&self, tx: &Transaction, range: impl RangeBounds) -> (u64, u64) { + pub fn sent_and_received( + &self, + tx: &Transaction, + range: impl RangeBounds, + ) -> (Amount, Amount) { self.inner .sent_and_received(tx, Self::map_to_inner_bounds(range)) } @@ -285,7 +289,7 @@ impl KeychainTxOutIndex { /// This calls [`SpkTxOutIndex::net_value`] internally. /// /// [`sent_and_received`]: Self::sent_and_received - pub fn net_value(&self, tx: &Transaction, range: impl RangeBounds) -> i64 { + pub fn net_value(&self, tx: &Transaction, range: impl RangeBounds) -> SignedAmount { self.inner.net_value(tx, Self::map_to_inner_bounds(range)) } } diff --git a/crates/chain/src/spk_txout_index.rs b/crates/chain/src/spk_txout_index.rs index a2e718dd47..2cad129cf9 100644 --- a/crates/chain/src/spk_txout_index.rs +++ b/crates/chain/src/spk_txout_index.rs @@ -4,7 +4,7 @@ use crate::{ collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap}, indexed_tx_graph::Indexer, }; -use bitcoin::{OutPoint, Script, ScriptBuf, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; /// An index storing [`TxOut`]s that have a script pubkey that matches those in a list. /// @@ -270,27 +270,30 @@ impl SpkTxOutIndex { self.spk_indices.get(script) } - // TODO: (@leonardo) Should this also be updated to return `(bitcoin::Amount, bitcoin::Amount)` instead of (u64, u64) /// Computes the total value transfer effect `tx` has on the script pubkeys in `range`. Value is /// *sent* when a script pubkey in the `range` is on an input and *received* when it is on an /// output. For `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. - pub fn sent_and_received(&self, tx: &Transaction, range: impl RangeBounds) -> (u64, u64) { - let mut sent = 0; - let mut received = 0; + pub fn sent_and_received( + &self, + tx: &Transaction, + range: impl RangeBounds, + ) -> (Amount, Amount) { + let mut sent = Amount::ZERO; + let mut received = Amount::ZERO; for txin in &tx.input { if let Some((index, txout)) = self.txout(txin.previous_output) { if range.contains(index) { - sent += txout.value.to_sat(); + sent += txout.value; } } } for txout in &tx.output { if let Some(index) = self.index_of_spk(&txout.script_pubkey) { if range.contains(index) { - received += txout.value.to_sat(); + received += txout.value; } } } @@ -302,9 +305,10 @@ impl SpkTxOutIndex { /// for calling [`sent_and_received`] and subtracting sent from received. /// /// [`sent_and_received`]: Self::sent_and_received - pub fn net_value(&self, tx: &Transaction, range: impl RangeBounds) -> i64 { + pub fn net_value(&self, tx: &Transaction, range: impl RangeBounds) -> SignedAmount { let (sent, received) = self.sent_and_received(tx, range); - received as i64 - sent as i64 + received.to_signed().expect("valid `SignedAmount`") + - sent.to_signed().expect("valid `SignedAmount`") } /// Whether any of the inputs of this transaction spend a txout tracked or whether any output diff --git a/crates/chain/tests/test_spk_txout_index.rs b/crates/chain/tests/test_spk_txout_index.rs index e6c3908055..2317b0a7d8 100644 --- a/crates/chain/tests/test_spk_txout_index.rs +++ b/crates/chain/tests/test_spk_txout_index.rs @@ -1,5 +1,7 @@ use bdk_chain::{indexed_tx_graph::Indexer, SpkTxOutIndex}; -use bitcoin::{absolute, transaction, Amount, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; +use bitcoin::{ + absolute, transaction, Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxIn, TxOut, +}; #[test] fn spk_txout_sent_and_received() { @@ -20,14 +22,23 @@ fn spk_txout_sent_and_received() { }], }; - assert_eq!(index.sent_and_received(&tx1, ..), (0, 42_000)); - assert_eq!(index.sent_and_received(&tx1, ..1), (0, 42_000)); - assert_eq!(index.sent_and_received(&tx1, 1..), (0, 0)); - assert_eq!(index.net_value(&tx1, ..), 42_000); + assert_eq!( + index.sent_and_received(&tx1, ..), + (Amount::from_sat(0), Amount::from_sat(42_000)) + ); + assert_eq!( + index.sent_and_received(&tx1, ..1), + (Amount::from_sat(0), Amount::from_sat(42_000)) + ); + assert_eq!( + index.sent_and_received(&tx1, 1..), + (Amount::from_sat(0), Amount::from_sat(0)) + ); + assert_eq!(index.net_value(&tx1, ..), SignedAmount::from_sat(42_000)); index.index_tx(&tx1); assert_eq!( index.sent_and_received(&tx1, ..), - (0, 42_000), + (Amount::from_sat(0), Amount::from_sat(42_000)), "shouldn't change after scanning" ); @@ -53,10 +64,19 @@ fn spk_txout_sent_and_received() { ], }; - assert_eq!(index.sent_and_received(&tx2, ..), (42_000, 50_000)); - assert_eq!(index.sent_and_received(&tx2, ..1), (42_000, 30_000)); - assert_eq!(index.sent_and_received(&tx2, 1..), (0, 20_000)); - assert_eq!(index.net_value(&tx2, ..), 8_000); + assert_eq!( + index.sent_and_received(&tx2, ..), + (Amount::from_sat(42_000), Amount::from_sat(50_000)) + ); + assert_eq!( + index.sent_and_received(&tx2, ..1), + (Amount::from_sat(42_000), Amount::from_sat(30_000)) + ); + assert_eq!( + index.sent_and_received(&tx2, 1..), + (Amount::from_sat(0), Amount::from_sat(20_000)) + ); + assert_eq!(index.net_value(&tx2, ..), SignedAmount::from_sat(8_000)); } #[test]