refactor(chain)!: update KeychainTxOutIndex methods to use owned ScriptBuf

Author: notmandatory

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<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     /// 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<ScriptBuf> {
         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<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     pub fn revealed_spks(
         &self,
         range: impl RangeBounds<K>,
-    ) -> impl Iterator<Item = KeychainIndexed<K, &Script>> {
+    ) -> impl Iterator<Item = KeychainIndexed<K, ScriptBuf>> + '_ {
         let start = range.start_bound();
         let end = range.end_bound();
         let mut iter_last_revealed = self
@@ -522,7 +522,7 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
             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,24 +534,24 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     pub fn revealed_keychain_spks(
         &self,
         keychain: K,
-    ) -> impl DoubleEndedIterator<Item = Indexed<&Script>> {
+    ) -> impl DoubleEndedIterator<Item = Indexed<ScriptBuf>> + '_ {
         let end = self
             .last_revealed_index(keychain.clone())
             .map(|v| v + 1)
             .unwrap_or(0);
         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<Item = KeychainIndexed<K, &Script>> + Clone {
+    ) -> impl DoubleEndedIterator<Item = KeychainIndexed<K, ScriptBuf>> + 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<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     pub fn unused_keychain_spks(
         &self,
         keychain: K,
-    ) -> impl DoubleEndedIterator<Item = Indexed<&Script>> + Clone {
+    ) -> impl DoubleEndedIterator<Item = Indexed<ScriptBuf>> + 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<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
             .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

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<I: Clone + Ord + core::fmt::Debug> SpkTxOutIndex<I> {
     /// 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<ScriptBuf> {
+        self.spks.get(index).cloned()
     }
 
     /// The script pubkeys that are being tracked by the index.
@@ -217,7 +217,10 @@ impl<I: Clone + Ord + core::fmt::Debug> SpkTxOutIndex<I> {
     /// let unused_change_spks =
     ///     txout_index.unused_spks((change_index, u32::MIN)..(change_index, u32::MAX));
     /// ```
-    pub fn unused_spks<R>(&self, range: R) -> impl DoubleEndedIterator<Item = (&I, &Script)> + Clone
+    pub fn unused_spks<R>(
+        &self,
+        range: R,
+    ) -> impl DoubleEndedIterator<Item = (&I, ScriptBuf)> + Clone + '_
     where
         R: RangeBounds<I>,
     {
@@ -268,8 +271,8 @@ impl<I: Clone + Ord + core::fmt::Debug> SpkTxOutIndex<I> {
     }
 
     /// 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<I: Clone + Ord + core::fmt::Debug> SpkTxOutIndex<I> {
             }
         }
         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;
                 }

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<A: Anchor> TxGraph<A> {
         chain: &C,
         chain_tip: BlockId,
         outpoints: impl IntoIterator<Item = (OI, OutPoint)>,
-        mut trust_predicate: impl FnMut(&OI, &Script) -> bool,
+        mut trust_predicate: impl FnMut(&OI, ScriptBuf) -> bool,
     ) -> Result<Balance, C::Error> {
         let mut immature = Amount::ZERO;
         let mut trusted_pending = Amount::ZERO;
@@ -1182,7 +1182,7 @@ impl<A: Anchor> TxGraph<A> {
                     }
                 }
                 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<A: Anchor> TxGraph<A> {
         chain: &C,
         chain_tip: BlockId,
         outpoints: impl IntoIterator<Item = (OI, OutPoint)>,
-        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")

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(),

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

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,

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

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));
 }

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:{}",