From 37cef1da7834d6d29f91c382ae841baf66764c51 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Tue, 9 Jul 2024 16:51:12 -0500 Subject: [PATCH] refactor(chain)!: update KeychainTxOutIndex methods to use owned ScriptBuf --- crates/chain/src/indexer/keychain_txout.rs | 22 +++++++-------- crates/chain/src/indexer/spk_txout.rs | 17 +++++++----- crates/chain/src/tx_graph.rs | 8 +++--- crates/chain/tests/common/tx_template.rs | 2 +- crates/chain/tests/test_indexed_tx_graph.rs | 6 ++--- crates/chain/tests/test_tx_graph_conflicts.rs | 4 +-- crates/wallet/src/wallet/mod.rs | 27 ++++++++++--------- crates/wallet/tests/wallet.rs | 10 +++---- example-crates/example_cli/src/lib.rs | 2 +- 9 files changed, 50 insertions(+), 48 deletions(-) diff --git a/crates/chain/src/indexer/keychain_txout.rs b/crates/chain/src/indexer/keychain_txout.rs index 3ccf5a71e1..24b0323276 100644 --- a/crates/chain/src/indexer/keychain_txout.rs +++ b/crates/chain/src/indexer/keychain_txout.rs @@ -8,7 +8,7 @@ use crate::{ DescriptorExt, DescriptorId, Indexed, Indexer, KeychainIndexed, SpkIterator, SpkTxOutIndex, }; use alloc::{borrow::ToOwned, vec::Vec}; -use bitcoin::{Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; use core::{ fmt::Debug, ops::{Bound, RangeBounds}, @@ -254,14 +254,14 @@ impl KeychainTxOutIndex { /// Return the script that exists under the given `keychain`'s `index`. /// /// This calls [`SpkTxOutIndex::spk_at_index`] internally. - pub fn spk_at_index(&self, keychain: K, index: u32) -> Option<&Script> { + pub fn spk_at_index(&self, keychain: K, index: u32) -> Option { self.inner.spk_at_index(&(keychain.clone(), index)) } /// Returns the keychain and keychain index associated with the spk. /// /// This calls [`SpkTxOutIndex::index_of_spk`] internally. - pub fn index_of_spk(&self, script: &Script) -> Option<&(K, u32)> { + pub fn index_of_spk(&self, script: ScriptBuf) -> Option<&(K, u32)> { self.inner.index_of_spk(script) } @@ -495,7 +495,7 @@ impl KeychainTxOutIndex { pub fn revealed_spks( &self, range: impl RangeBounds, - ) -> impl Iterator> { + ) -> impl Iterator> + '_ { let start = range.start_bound(); let end = range.end_bound(); let mut iter_last_revealed = self @@ -522,7 +522,7 @@ impl KeychainTxOutIndex { let (current_keychain, last_revealed) = current_keychain?; if current_keychain == keychain && Some(*index) <= last_revealed { - break Some(((keychain.clone(), *index), spk.as_script())); + break Some(((keychain.clone(), *index), spk.clone())); } }) } @@ -534,7 +534,7 @@ impl KeychainTxOutIndex { pub fn revealed_keychain_spks( &self, keychain: K, - ) -> impl DoubleEndedIterator> { + ) -> impl DoubleEndedIterator> + '_ { let end = self .last_revealed_index(keychain.clone()) .map(|v| v + 1) @@ -542,16 +542,16 @@ impl KeychainTxOutIndex { self.inner .all_spks() .range((keychain.clone(), 0)..(keychain.clone(), end)) - .map(|((_, index), spk)| (*index, spk.as_script())) + .map(|((_, index), spk)| (*index, spk.clone())) } /// Iterate over revealed, but unused, spks of all keychains. pub fn unused_spks( &self, - ) -> impl DoubleEndedIterator> + Clone { + ) -> impl DoubleEndedIterator> + Clone + '_ { self.keychain_to_descriptor_id.keys().flat_map(|keychain| { self.unused_keychain_spks(keychain.clone()) - .map(|(i, spk)| ((keychain.clone(), i), spk)) + .map(|(i, spk)| ((keychain.clone(), i), spk.clone())) }) } @@ -560,7 +560,7 @@ impl KeychainTxOutIndex { pub fn unused_keychain_spks( &self, keychain: K, - ) -> impl DoubleEndedIterator> + Clone { + ) -> impl DoubleEndedIterator> + Clone + '_ { let end = match self.keychain_to_descriptor_id.get(&keychain) { Some(did) => self.last_revealed.get(did).map(|v| *v + 1).unwrap_or(0), None => 0, @@ -701,7 +701,7 @@ impl KeychainTxOutIndex { .inner .spk_at_index(&(keychain.clone(), next_index)) .expect("we just inserted it"); - Some(((next_index, script.into()), changeset)) + Some(((next_index, script), changeset)) } /// Gets the next unused script pubkey in the keychain. I.e., the script pubkey with the lowest diff --git a/crates/chain/src/indexer/spk_txout.rs b/crates/chain/src/indexer/spk_txout.rs index ead446a76e..bfb558fcaf 100644 --- a/crates/chain/src/indexer/spk_txout.rs +++ b/crates/chain/src/indexer/spk_txout.rs @@ -6,7 +6,7 @@ use crate::{ collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap}, Indexer, }; -use bitcoin::{Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; /// An index storing [`TxOut`]s that have a script pubkey that matches those in a list. /// @@ -176,8 +176,8 @@ impl SpkTxOutIndex { /// Returns the script that has been inserted at the `index`. /// /// If that index hasn't been inserted yet, it will return `None`. - pub fn spk_at_index(&self, index: &I) -> Option<&Script> { - self.spks.get(index).map(|s| s.as_script()) + pub fn spk_at_index(&self, index: &I) -> Option { + self.spks.get(index).cloned() } /// The script pubkeys that are being tracked by the index. @@ -217,7 +217,10 @@ impl SpkTxOutIndex { /// let unused_change_spks = /// txout_index.unused_spks((change_index, u32::MIN)..(change_index, u32::MAX)); /// ``` - pub fn unused_spks(&self, range: R) -> impl DoubleEndedIterator + Clone + pub fn unused_spks( + &self, + range: R, + ) -> impl DoubleEndedIterator + Clone + '_ where R: RangeBounds, { @@ -268,8 +271,8 @@ impl SpkTxOutIndex { } /// Returns the index associated with the script pubkey. - pub fn index_of_spk(&self, script: &Script) -> Option<&I> { - self.spk_indices.get(script) + pub fn index_of_spk(&self, script: ScriptBuf) -> Option<&I> { + self.spk_indices.get(script.as_script()) } /// Computes the total value transfer effect `tx` has on the script pubkeys in `range`. Value is @@ -293,7 +296,7 @@ impl SpkTxOutIndex { } } for txout in &tx.output { - if let Some(index) = self.index_of_spk(&txout.script_pubkey) { + if let Some(index) = self.index_of_spk(txout.script_pubkey.clone()) { if range.contains(index) { received += txout.value; } diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 8c11e737a4..192b122e2f 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -94,7 +94,7 @@ use crate::{ use alloc::collections::vec_deque::VecDeque; use alloc::sync::Arc; use alloc::vec::Vec; -use bitcoin::{Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; use core::fmt::{self, Formatter}; use core::{ convert::Infallible, @@ -1163,7 +1163,7 @@ impl TxGraph { chain: &C, chain_tip: BlockId, outpoints: impl IntoIterator, - mut trust_predicate: impl FnMut(&OI, &Script) -> bool, + mut trust_predicate: impl FnMut(&OI, ScriptBuf) -> bool, ) -> Result { let mut immature = Amount::ZERO; let mut trusted_pending = Amount::ZERO; @@ -1182,7 +1182,7 @@ impl TxGraph { } } ChainPosition::Unconfirmed(_) => { - if trust_predicate(&spk_i, &txout.txout.script_pubkey) { + if trust_predicate(&spk_i, txout.txout.script_pubkey) { trusted_pending += txout.txout.value; } else { untrusted_pending += txout.txout.value; @@ -1209,7 +1209,7 @@ impl TxGraph { chain: &C, chain_tip: BlockId, outpoints: impl IntoIterator, - trust_predicate: impl FnMut(&OI, &Script) -> bool, + trust_predicate: impl FnMut(&OI, ScriptBuf) -> bool, ) -> Balance { self.try_balance(chain, chain_tip, outpoints, trust_predicate) .expect("oracle is infallible") diff --git a/crates/chain/tests/common/tx_template.rs b/crates/chain/tests/common/tx_template.rs index 3337fb4368..b95c45aeef 100644 --- a/crates/chain/tests/common/tx_template.rs +++ b/crates/chain/tests/common/tx_template.rs @@ -119,7 +119,7 @@ pub fn init_graph<'a, A: Anchor + Clone + 'a>( }, Some(index) => TxOut { value: Amount::from_sat(output.value), - script_pubkey: spk_index.spk_at_index(index).unwrap().to_owned(), + script_pubkey: spk_index.spk_at_index(index).unwrap(), }, }) .collect(), diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 2f974fa284..6ad0f95380 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -12,9 +12,7 @@ use bdk_chain::{ local_chain::LocalChain, tx_graph, Balance, ChainPosition, ConfirmationHeightAnchor, DescriptorExt, Merge, }; -use bitcoin::{ - secp256k1::Secp256k1, Amount, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut, -}; +use bitcoin::{secp256k1::Secp256k1, Amount, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; use miniscript::Descriptor; /// Ensure [`IndexedTxGraph::insert_relevant_txs`] can successfully index transactions NOT presented @@ -289,7 +287,7 @@ fn test_list_owned_txouts() { &local_chain, chain_tip, graph.index.outpoints().iter().cloned(), - |_, spk: &Script| trusted_spks.contains(&spk.to_owned()), + |_, spk: ScriptBuf| trusted_spks.contains(&spk), ); let confirmed_txouts_txid = txouts diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 802ba5c7e4..45370b6763 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -6,7 +6,7 @@ mod common; use std::collections::{BTreeSet, HashSet}; use bdk_chain::{Balance, BlockId}; -use bitcoin::{Amount, OutPoint, Script}; +use bitcoin::{Amount, OutPoint, ScriptBuf}; use common::*; #[allow(dead_code)] @@ -659,7 +659,7 @@ fn test_tx_conflict_handling() { &local_chain, chain_tip, spk_index.outpoints().iter().cloned(), - |_, spk: &Script| spk_index.index_of_spk(spk).is_some(), + |_, spk: ScriptBuf| spk_index.index_of_spk(spk).is_some(), ); assert_eq!( balance, scenario.exp_balance, diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 9de544805b..4413f6d193 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -33,8 +33,8 @@ use bdk_chain::{ }; use bitcoin::sighash::{EcdsaSighashType, TapSighashType}; use bitcoin::{ - absolute, psbt, Address, Block, FeeRate, Network, OutPoint, Script, ScriptBuf, Sequence, - Transaction, TxOut, Txid, Witness, + absolute, psbt, Address, Block, FeeRate, Network, OutPoint, ScriptBuf, Sequence, Transaction, + TxOut, Txid, Witness, }; use bitcoin::{consensus::encode::serialize, transaction, BlockHash, Psbt}; use bitcoin::{constants::genesis_block, Amount}; @@ -764,20 +764,21 @@ impl Wallet { .unused_keychain_spks(keychain) .map(move |(index, spk)| AddressInfo { index, - address: Address::from_script(spk, self.network).expect("must have address form"), + address: Address::from_script(spk.as_script(), self.network) + .expect("must have address form"), keychain, }) } /// Return whether or not a `script` is part of this wallet (either internal or external) - pub fn is_mine(&self, script: &Script) -> bool { + pub fn is_mine(&self, script: ScriptBuf) -> bool { self.indexed_graph.index.index_of_spk(script).is_some() } /// Finds how the wallet derived the script pubkey `spk`. /// /// Will only return `Some(_)` if the wallet has given out the spk. - pub fn derivation_of_spk(&self, spk: &Script) -> Option<(KeychainKind, u32)> { + pub fn derivation_of_spk(&self, spk: ScriptBuf) -> Option<(KeychainKind, u32)> { self.indexed_graph.index.index_of_spk(spk).cloned() } @@ -1364,7 +1365,7 @@ impl Wallet { return Err(CreateTxError::OutputBelowDustLimit(index)); } - if self.is_mine(script_pubkey) { + if self.is_mine(script_pubkey.clone()) { received += Amount::from_sat(value); } @@ -1471,7 +1472,7 @@ impl Wallet { remaining_amount, .. } => fee_amount += remaining_amount, Change { amount, fee } => { - if self.is_mine(&drain_script) { + if self.is_mine(drain_script.clone()) { received += Amount::from_sat(*amount); } fee_amount += fee; @@ -1592,7 +1593,7 @@ impl Wallet { .cloned() .into(); - let weighted_utxo = match txout_index.index_of_spk(&txout.script_pubkey) { + let weighted_utxo = match txout_index.index_of_spk(txout.script_pubkey.clone()) { Some(&(keychain, derivation_index)) => { let satisfaction_weight = self .public_descriptor(keychain) @@ -1637,7 +1638,7 @@ impl Wallet { let mut change_index = None; for (index, txout) in tx.output.iter().enumerate() { let change_keychain = KeychainKind::Internal; - match txout_index.index_of_spk(&txout.script_pubkey) { + match txout_index.index_of_spk(txout.script_pubkey.clone()) { Some((keychain, _)) if *keychain == change_keychain => { change_index = Some(index) } @@ -1899,7 +1900,7 @@ impl Wallet { pub fn cancel_tx(&mut self, tx: &Transaction) { let txout_index = &mut self.indexed_graph.index; for txout in &tx.output { - if let Some((keychain, index)) = txout_index.index_of_spk(&txout.script_pubkey) { + if let Some((keychain, index)) = txout_index.index_of_spk(txout.script_pubkey.clone()) { // NOTE: unmark_used will **not** make something unused if it has actually been used // by a tx in the tracker. It only removes the superficial marking. txout_index.unmark_used(*keychain, *index); @@ -1911,7 +1912,7 @@ impl Wallet { let &(keychain, child) = self .indexed_graph .index - .index_of_spk(&txout.script_pubkey)?; + .index_of_spk(txout.script_pubkey.clone())?; let descriptor = self.public_descriptor(keychain); descriptor.at_derivation_index(child).ok() } @@ -2126,7 +2127,7 @@ impl Wallet { let &(keychain, child) = self .indexed_graph .index - .index_of_spk(&utxo.txout.script_pubkey) + .index_of_spk(utxo.txout.script_pubkey) .ok_or(CreateTxError::UnknownUtxo)?; let mut psbt_input = psbt::Input { @@ -2172,7 +2173,7 @@ impl Wallet { // Try to figure out the keychain and derivation for every input and output for (is_input, index, out) in utxos.into_iter() { if let Some(&(keychain, child)) = - self.indexed_graph.index.index_of_spk(&out.script_pubkey) + self.indexed_graph.index.index_of_spk(out.script_pubkey) { let desc = self.public_descriptor(keychain); let desc = desc diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 8a0c4f7f1b..a845330868 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -4070,7 +4070,7 @@ fn test_tx_cancellation() { .unsigned_tx .output .iter() - .find_map(|txout| wallet.derivation_of_spk(&txout.script_pubkey)) + .find_map(|txout| wallet.derivation_of_spk(txout.script_pubkey.clone())) .unwrap(); assert_eq!(change_derivation_1, (KeychainKind::Internal, 0)); @@ -4080,7 +4080,7 @@ fn test_tx_cancellation() { .unsigned_tx .output .iter() - .find_map(|txout| wallet.derivation_of_spk(&txout.script_pubkey)) + .find_map(|txout| wallet.derivation_of_spk(txout.script_pubkey.clone())) .unwrap(); assert_eq!(change_derivation_2, (KeychainKind::Internal, 1)); @@ -4091,7 +4091,7 @@ fn test_tx_cancellation() { .unsigned_tx .output .iter() - .find_map(|txout| wallet.derivation_of_spk(&txout.script_pubkey)) + .find_map(|txout| wallet.derivation_of_spk(txout.script_pubkey.clone())) .unwrap(); assert_eq!(change_derivation_3, (KeychainKind::Internal, 0)); @@ -4100,7 +4100,7 @@ fn test_tx_cancellation() { .unsigned_tx .output .iter() - .find_map(|txout| wallet.derivation_of_spk(&txout.script_pubkey)) + .find_map(|txout| wallet.derivation_of_spk(txout.script_pubkey.clone())) .unwrap(); assert_eq!(change_derivation_3, (KeychainKind::Internal, 2)); @@ -4111,7 +4111,7 @@ fn test_tx_cancellation() { .unsigned_tx .output .iter() - .find_map(|txout| wallet.derivation_of_spk(&txout.script_pubkey)) + .find_map(|txout| wallet.derivation_of_spk(txout.script_pubkey.clone())) .unwrap(); assert_eq!(change_derivation_4, (KeychainKind::Internal, 2)); } diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 67b87428f1..a290420c53 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -502,7 +502,7 @@ where false => Keychain::External, }; for (spk_i, spk) in index.revealed_keychain_spks(target_keychain) { - let address = Address::from_script(spk, network) + let address = Address::from_script(spk.as_script(), network) .expect("should always be able to derive address"); println!( "{:?} {} used:{}",