Merge #1084: Enhance bdk chain structures

1ff806c fix(chain)!: rm weird `From` impl (志宇)
d43ae02 refactor: improve docs, cleanup unnecessary types and improve code (Vladimir Fomene)
4104206 feat: impl Append for lots of tuples (LLFourn)
c56728f refactor: Remove `scan` and `scan_txout` from SpkTxoutIndex and KeychainTxoutIndex (Vladimir Fomene)
32c40ac feat(electrum)!: change signature of `ElectrumExt` (志宇)
a28748c refactor: Implement Default for WalletUpdate (Vladimir Fomene)
f42f8b8 refactor: Allow for no chain update (Vladimir Fomene)
68572bf refactor: move WalletChangeset to wallet module (Vladimir Fomene)
2392e50 refactor: Move WalletUpdate to wallet module (Vladimir Fomene)
7c12dc9 refactor: Remove ForEachTxout trait (Vladimir Fomene)
6bcbb93 refactor: Edit ElectrumExt not to use WalletUpdate (Vladimir Fomene)

Pull request description:

  ### Description

  Fixes #1061

  ### Changelog notice

  - Move WalletUpdate to the wallet module
  - Remove ForEachTxout trait completely
  - Refactor ElectrumExt to not use WalletUpdate.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  evanlinjin:
    ACK 1ff806c

Tree-SHA512: 05349713af9d2efa14a522ceaabb7513bb437d786adf2f93055765589a67e4eb68bda36ff415aeba07816c4d30988d4d55bac018e7697019270a219105ed65a2

crates/bdk/src/wallet/mod.rs

@@ -22,7 +22,7 @@ use alloc::{
 pub use bdk_chain::keychain::Balance;
 use bdk_chain::{
     indexed_tx_graph,
-    keychain::{KeychainTxOutIndex, WalletChangeSet, WalletUpdate},
+    keychain::{self, KeychainTxOutIndex},
     local_chain::{self, CannotConnectError, CheckPoint, CheckPointIter, LocalChain},
     tx_graph::{CanonicalTx, TxGraph},
     Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeAnchor, FullTxOut,
@@ -95,11 +95,74 @@ pub struct Wallet<D = ()> {
     secp: SecpCtx,
 }
 
-/// The update to a [`Wallet`] used in [`Wallet::apply_update`]. This is usually returned from blockchain data sources.
-pub type Update = WalletUpdate<KeychainKind, ConfirmationTimeAnchor>;
+/// An update to [`Wallet`].
+///
+/// It updates [`bdk_chain::keychain::KeychainTxOutIndex`], [`bdk_chain::TxGraph`] and [`local_chain::LocalChain`] atomically.
+#[derive(Debug, Clone, Default)]
+pub struct Update {
+    /// Contains the last active derivation indices per keychain (`K`), which is used to update the
+    /// [`KeychainTxOutIndex`].
+    pub last_active_indices: BTreeMap<KeychainKind, u32>,
+
+    /// Update for the wallet's internal [`TxGraph`].
+    pub graph: TxGraph<ConfirmationTimeAnchor>,
+
+    /// Update for the wallet's internal [`LocalChain`].
+    ///
+    /// [`LocalChain`]: local_chain::LocalChain
+    pub chain: Option<local_chain::Update>,
+}
+
+/// The changes made to a wallet by applying an [`Update`].
+#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize, Default)]
+pub struct ChangeSet {
+    /// Changes to the [`LocalChain`].
+    ///
+    /// [`LocalChain`]: local_chain::LocalChain
+    pub chain: local_chain::ChangeSet,
+
+    /// Changes to [`IndexedTxGraph`].
+    ///
+    /// [`IndexedTxGraph`]: bdk_chain::indexed_tx_graph::IndexedTxGraph
+    pub indexed_tx_graph:
+        indexed_tx_graph::ChangeSet<ConfirmationTimeAnchor, keychain::ChangeSet<KeychainKind>>,
+}
+
+impl Append for ChangeSet {
+    fn append(&mut self, other: Self) {
+        Append::append(&mut self.chain, other.chain);
+        Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph);
+    }
+
+    fn is_empty(&self) -> bool {
+        self.chain.is_empty() && self.indexed_tx_graph.is_empty()
+    }
+}
+
+impl From<local_chain::ChangeSet> for ChangeSet {
+    fn from(chain: local_chain::ChangeSet) -> Self {
+        Self {
+            chain,
+            ..Default::default()
+        }
+    }
+}
 
-/// The changeset produced internally by [`Wallet`] when mutated.
-pub type ChangeSet = WalletChangeSet<KeychainKind, ConfirmationTimeAnchor>;
+impl From<indexed_tx_graph::ChangeSet<ConfirmationTimeAnchor, keychain::ChangeSet<KeychainKind>>>
+    for ChangeSet
+{
+    fn from(
+        indexed_tx_graph: indexed_tx_graph::ChangeSet<
+            ConfirmationTimeAnchor,
+            keychain::ChangeSet<KeychainKind>,
+        >,
+    ) -> Self {
+        Self {
+            indexed_tx_graph,
+            ..Default::default()
+        }
+    }
+}
 
 /// The address index selection strategy to use to derived an address from the wallet's external
 /// descriptor. See [`Wallet::get_address`]. If you're unsure which one to use use `WalletIndex::New`.
@@ -1857,7 +1920,11 @@ impl<D> Wallet<D> {
     where
         D: PersistBackend<ChangeSet>,
     {
-        let mut changeset = ChangeSet::from(self.chain.apply_update(update.chain)?);
+        let mut changeset = match update.chain {
+            Some(chain_update) => ChangeSet::from(self.chain.apply_update(chain_update)?),
+            None => ChangeSet::default(),
+        };
+
         let (_, index_changeset) = self
             .indexed_graph
             .index

crates/chain/src/indexed_tx_graph.rs

@@ -225,15 +225,18 @@ impl<A, K> From<keychain::ChangeSet<K>> for ChangeSet<A, keychain::ChangeSet<K>>
     }
 }
 
-/// Represents a structure that can index transaction data.
+/// Utilities for indexing transaction data.
+///
+/// Types which implement this trait can be used to construct an [`IndexedTxGraph`].
+/// This trait's methods should rarely be called directly.
 pub trait Indexer {
     /// The resultant "changeset" when new transaction data is indexed.
     type ChangeSet;
 
     /// Scan and index the given `outpoint` and `txout`.
     fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet;
 
-    /// Scan and index the given transaction.
+    /// Scans a transaction for relevant outpoints, which are stored and indexed internally.
     fn index_tx(&mut self, tx: &Transaction) -> Self::ChangeSet;
 
     /// Apply changeset to itself.

crates/chain/src/keychain.rs

@@ -10,9 +10,7 @@
 //!
 //! [`SpkTxOutIndex`]: crate::SpkTxOutIndex
 
-use crate::{
-    collections::BTreeMap, indexed_tx_graph, local_chain, tx_graph::TxGraph, Anchor, Append,
-};
+use crate::{collections::BTreeMap, Append};
 
 #[cfg(feature = "miniscript")]
 mod txout_index;
@@ -82,98 +80,6 @@ impl<K> AsRef<BTreeMap<K, u32>> for ChangeSet<K> {
     }
 }
 
-/// A structure to update [`KeychainTxOutIndex`], [`TxGraph`] and [`LocalChain`] atomically.
-///
-/// [`LocalChain`]: local_chain::LocalChain
-#[derive(Debug, Clone)]
-pub struct WalletUpdate<K, A> {
-    /// Contains the last active derivation indices per keychain (`K`), which is used to update the
-    /// [`KeychainTxOutIndex`].
-    pub last_active_indices: BTreeMap<K, u32>,
-
-    /// Update for the [`TxGraph`].
-    pub graph: TxGraph<A>,
-
-    /// Update for the [`LocalChain`].
-    ///
-    /// [`LocalChain`]: local_chain::LocalChain
-    pub chain: local_chain::Update,
-}
-
-impl<K, A> WalletUpdate<K, A> {
-    /// Construct a [`WalletUpdate`] with a given [`local_chain::Update`].
-    pub fn new(chain_update: local_chain::Update) -> Self {
-        Self {
-            last_active_indices: BTreeMap::new(),
-            graph: TxGraph::default(),
-            chain: chain_update,
-        }
-    }
-}
-
-/// A structure that records the corresponding changes as result of applying an [`WalletUpdate`].
-#[derive(Debug, Clone, PartialEq)]
-#[cfg_attr(
-    feature = "serde",
-    derive(serde::Deserialize, serde::Serialize),
-    serde(
-        crate = "serde_crate",
-        bound(
-            deserialize = "K: Ord + serde::Deserialize<'de>, A: Ord + serde::Deserialize<'de>",
-            serialize = "K: Ord + serde::Serialize, A: Ord + serde::Serialize",
-        )
-    )
-)]
-pub struct WalletChangeSet<K, A> {
-    /// Changes to the [`LocalChain`].
-    ///
-    /// [`LocalChain`]: local_chain::LocalChain
-    pub chain: local_chain::ChangeSet,
-
-    /// ChangeSet to [`IndexedTxGraph`].
-    ///
-    /// [`IndexedTxGraph`]: crate::indexed_tx_graph::IndexedTxGraph
-    pub indexed_tx_graph: indexed_tx_graph::ChangeSet<A, ChangeSet<K>>,
-}
-
-impl<K, A> Default for WalletChangeSet<K, A> {
-    fn default() -> Self {
-        Self {
-            chain: Default::default(),
-            indexed_tx_graph: Default::default(),
-        }
-    }
-}
-
-impl<K: Ord, A: Anchor> Append for WalletChangeSet<K, A> {
-    fn append(&mut self, other: Self) {
-        Append::append(&mut self.chain, other.chain);
-        Append::append(&mut self.indexed_tx_graph, other.indexed_tx_graph);
-    }
-
-    fn is_empty(&self) -> bool {
-        self.chain.is_empty() && self.indexed_tx_graph.is_empty()
-    }
-}
-
-impl<K, A> From<local_chain::ChangeSet> for WalletChangeSet<K, A> {
-    fn from(chain: local_chain::ChangeSet) -> Self {
-        Self {
-            chain,
-            ..Default::default()
-        }
-    }
-}
-
-impl<K, A> From<indexed_tx_graph::ChangeSet<A, ChangeSet<K>>> for WalletChangeSet<K, A> {
-    fn from(indexed_tx_graph: indexed_tx_graph::ChangeSet<A, ChangeSet<K>>) -> Self {
-        Self {
-            indexed_tx_graph,
-            ..Default::default()
-        }
-    }
-}
-
 /// Balance, differentiated into various categories.
 #[derive(Debug, PartialEq, Eq, Clone, Default)]
 #[cfg_attr(

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};
@@ -91,11 +91,18 @@ impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
     type ChangeSet = super::ChangeSet<K>;
 
     fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet {
-        self.scan_txout(outpoint, txout)
+        match self.inner.scan_txout(outpoint, txout).cloned() {
+            Some((keychain, index)) => self.reveal_to_target(&keychain, index).1,
+            None => super::ChangeSet::default(),
+        }
     }
 
     fn index_tx(&mut self, tx: &bitcoin::Transaction) -> Self::ChangeSet {
-        self.scan(tx)
+        let mut changeset = super::ChangeSet::<K>::default();
+        for (op, txout) in tx.output.iter().enumerate() {
+            changeset.append(self.index_txout(OutPoint::new(tx.txid(), op as u32), txout));
+        }
+        changeset
     }
 
     fn initial_changeset(&self) -> Self::ChangeSet {
@@ -112,38 +119,6 @@ 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.
-    ///
-    /// 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
-    /// change.
-    ///
-    /// Typically, this method 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 (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> {
-        let mut changeset = super::ChangeSet::<K>::default();
-        txouts.for_each_txout(|(op, txout)| changeset.append(self.scan_txout(op, txout)));
-        changeset
-    }
-
-    /// Scan a single outpoint for a matching script pubkey.
-    ///
-    /// If it matches, this will store and index it.
-    pub fn scan_txout(&mut self, op: OutPoint, txout: &TxOut) -> super::ChangeSet<K> {
-        match self.inner.scan_txout(op, txout).cloned() {
-            Some((keychain, index)) => self.reveal_to_target(&keychain, index).1,
-            None => super::ChangeSet::default(),
-        }
-    }
-
     /// Return a reference to the internal [`SpkTxOutIndex`].
     pub fn inner(&self) -> &SpkTxOutIndex<(K, u32)> {
         &self.inner
@@ -199,15 +174,14 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
 
     /// Set the lookahead count for `keychain`.
     ///
-    /// The lookahead is the number of scripts to cache ahead of the last stored script index. This
-    /// is useful during a scan via [`scan`] or [`scan_txout`].
+    /// The lookahead is the number of scripts to cache ahead of the last revealed script index. This
+    /// is useful to find outputs you own when processing block data that lie beyond the last revealed
+    /// index. In certain situations, such as when performing an initial scan of the blockchain during
+    /// wallet import, it may be uncertain or unknown what the last revealed index is.
     ///
     /// # Panics
     ///
     /// This will panic if the `keychain` does not exist.
-    ///
-    /// [`scan`]: Self::scan
-    /// [`scan_txout`]: Self::scan_txout
     pub fn set_lookahead(&mut self, keychain: &K, lookahead: u32) {
         self.lookahead.insert(keychain.clone(), lookahead);
         self.replenish_lookahead(keychain);