refactor: Remove ForEachTxout trait

Author: vladimirfomene

crates/chain/src/keychain/txout_index.rs

@@ -3,7 +3,7 @@ use crate::{
     indexed_tx_graph::Indexer,
     miniscript::{Descriptor, DescriptorPublicKey},
     spk_iter::BIP32_MAX_INDEX,
-    ForEachTxOut, SpkIterator, SpkTxOutIndex,
+    SpkIterator, SpkTxOutIndex,
 };
 use alloc::vec::Vec;
 use bitcoin::{OutPoint, Script, TxOut};
@@ -112,7 +112,7 @@ impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
 }
 
 impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
-    /// Scans an object for relevant outpoints, which are stored and indexed internally.
+    /// Scans a transaction for relevant outpoints, which are stored and indexed internally.
     ///
     /// If the matched script pubkey is part of the lookahead, the last stored index is updated for
     /// the script pubkey's keychain and the [`super::ChangeSet`] returned will reflect the
@@ -124,13 +124,11 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     /// your txouts.
     /// 2. When getting new data from the chain, you usually scan it before incorporating it into
     /// your chain state (i.e., `SparseChain`, `ChainGraph`).
-    ///
-    /// See [`ForEachTxout`] for the types that support this.
-    ///
-    /// [`ForEachTxout`]: crate::ForEachTxOut
-    pub fn scan(&mut self, txouts: &impl ForEachTxOut) -> super::ChangeSet<K> {
+    pub fn scan(&mut self, tx: &bitcoin::Transaction) -> super::ChangeSet<K> {
         let mut changeset = super::ChangeSet::<K>::default();
-        txouts.for_each_txout(|(op, txout)| changeset.append(self.scan_txout(op, txout)));
+        for (op, txout) in tx.output.iter().enumerate() {
+            changeset.append(self.scan_txout(OutPoint::new(tx.txid(), op as u32), txout));
+        }
         changeset
     }
 

crates/chain/src/spk_txout_index.rs

@@ -3,7 +3,6 @@ use core::ops::RangeBounds;
 use crate::{
     collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap},
     indexed_tx_graph::Indexer,
-    ForEachTxOut,
 };
 use bitcoin::{self, OutPoint, Script, ScriptBuf, Transaction, TxOut, Txid};
 
@@ -77,49 +76,37 @@ impl<I: Clone + Ord> Indexer for SpkTxOutIndex<I> {
     }
 }
 
-/// This macro is used instead of a member function of `SpkTxOutIndex`, which would result in a
-/// compiler error[E0521]: "borrowed data escapes out of closure" when we attempt to take a
-/// reference out of the `ForEachTxOut` closure during scanning.
-macro_rules! scan_txout {
-    ($self:ident, $op:expr, $txout:expr) => {{
-        let spk_i = $self.spk_indices.get(&$txout.script_pubkey);
-        if let Some(spk_i) = spk_i {
-            $self.txouts.insert($op, (spk_i.clone(), $txout.clone()));
-            $self.spk_txouts.insert((spk_i.clone(), $op));
-            $self.unused.remove(&spk_i);
-        }
-        spk_i
-    }};
-}
-
 impl<I: Clone + Ord> SpkTxOutIndex<I> {
-    /// Scans an object containing many txouts.
+    /// Scans a transaction containing many txouts.
     ///
     /// Typically, this is used in two situations:
     ///
     /// 1. After loading transaction data from the disk, you may scan over all the txouts to restore all
     /// your txouts.
     /// 2. When getting new data from the chain, you usually scan it before incorporating it into your chain state.
-    ///
-    /// See [`ForEachTxout`] for the types that support this.
-    ///
-    /// [`ForEachTxout`]: crate::ForEachTxOut
-    pub fn scan(&mut self, txouts: &impl ForEachTxOut) -> BTreeSet<I> {
+    pub fn scan(&mut self, tx: &bitcoin::Transaction) -> BTreeSet<I> {
         let mut scanned_indices = BTreeSet::new();
 
-        txouts.for_each_txout(|(op, txout)| {
-            if let Some(spk_i) = scan_txout!(self, op, txout) {
+        for (i, txout) in tx.output.iter().enumerate() {
+            let op = OutPoint::new(tx.txid(), i as u32);
+            if let Some(spk_i) = self.scan_txout(op, txout) {
                 scanned_indices.insert(spk_i.clone());
             }
-        });
+        }
 
         scanned_indices
     }
 
     /// Scan a single `TxOut` for a matching script pubkey and returns the index that matches the
     /// script pubkey (if any).
     pub fn scan_txout(&mut self, op: OutPoint, txout: &TxOut) -> Option<&I> {
-        scan_txout!(self, op, txout)
+        let spk_i = self.spk_indices.get(&txout.script_pubkey);
+        if let Some(spk_i) = spk_i {
+            self.txouts.insert(op, (spk_i.clone(), txout.clone()));
+            self.spk_txouts.insert((spk_i.clone(), op));
+            self.unused.remove(spk_i);
+        }
+        spk_i
     }
 
     /// Get a reference to the set of indexed outpoints.

crates/chain/src/tx_data_traits.rs

@@ -2,39 +2,6 @@ use crate::collections::BTreeMap;
 use crate::collections::BTreeSet;
 use crate::BlockId;
 use alloc::vec::Vec;
-use bitcoin::{Block, OutPoint, Transaction, TxOut};
-
-/// Trait to do something with every txout contained in a structure.
-///
-/// We would prefer to just work with things that can give us an `Iterator<Item=(OutPoint, &TxOut)>`
-/// here, but rust's type system makes it extremely hard to do this (without trait objects).
-pub trait ForEachTxOut {
-    /// The provided closure `f` will be called with each `outpoint/txout` pair.
-    fn for_each_txout(&self, f: impl FnMut((OutPoint, &TxOut)));
-}
-
-impl ForEachTxOut for Block {
-    fn for_each_txout(&self, mut f: impl FnMut((OutPoint, &TxOut))) {
-        for tx in self.txdata.iter() {
-            tx.for_each_txout(&mut f)
-        }
-    }
-}
-
-impl ForEachTxOut for Transaction {
-    fn for_each_txout(&self, mut f: impl FnMut((OutPoint, &TxOut))) {
-        let txid = self.txid();
-        for (i, txout) in self.output.iter().enumerate() {
-            f((
-                OutPoint {
-                    txid,
-                    vout: i as u32,
-                },
-                txout,
-            ))
-        }
-    }
-}
 
 /// Trait that "anchors" blockchain data to a specific block of height and hash.
 ///

crates/chain/src/tx_graph.rs

@@ -52,7 +52,7 @@
 
 use crate::{
     collections::*, keychain::Balance, local_chain::LocalChain, Anchor, Append, BlockId,
-    ChainOracle, ChainPosition, ForEachTxOut, FullTxOut,
+    ChainOracle, ChainPosition, FullTxOut,
 };
 use alloc::vec::Vec;
 use bitcoin::{OutPoint, Script, Transaction, TxOut, Txid};
@@ -1072,18 +1072,6 @@ impl<A> AsRef<TxGraph<A>> for TxGraph<A> {
     }
 }
 
-impl<A> ForEachTxOut for ChangeSet<A> {
-    fn for_each_txout(&self, f: impl FnMut((OutPoint, &TxOut))) {
-        self.txouts().for_each(f)
-    }
-}
-
-impl<A> ForEachTxOut for TxGraph<A> {
-    fn for_each_txout(&self, f: impl FnMut((OutPoint, &TxOut))) {
-        self.all_txouts().for_each(f)
-    }
-}
-
 /// An iterator that traverses transaction descendants.
 ///
 /// This `struct` is created by the [`walk_descendants`] method of [`TxGraph`].