[local_chain] API improvements and simplifications

Thank you to @LLFourn for the suggestions.

* `LocalChain::update` is now renamed back to `LocalChain::apply_update`
  and takes in a new struct `local_chain::Update` that combines a
`CheckPoint` tip and a `introduce_older_blocks: bool`. This is because
the boolean argument is determined by the chain source and not by the
caller/user.
* We've removed the premature optimization in
  `local_chain::merge_chains`.
* Deriving `PartialEq`, `Eq`, `PartialOrd`, `Ord` for `CheckPoint` does
  not make much sense. They are removed for now.
* Fix: bitcoind_rpc Cargo.toml: wrong bdk_chain version

crates/bdk/src/wallet/mod.rs

@@ -1707,10 +1707,7 @@ impl<D> Wallet<D> {
     where
         D: PersistBackend<ChangeSet>,
     {
-        let mut changeset = ChangeSet::from(
-            self.chain
-                .update(update.tip, update.introduce_older_blocks)?,
-        );
+        let mut changeset = ChangeSet::from(self.chain.apply_update(update.chain)?);
         let (_, index_additions) = self
             .indexed_graph
             .index
@@ -1739,10 +1736,7 @@ impl<D> Wallet<D> {
     where
         D: PersistBackend<ChangeSet>,
     {
-        let mut changeset = ChangeSet::from(
-            self.chain
-                .update(update.tip, update.introduce_older_blocks)?,
-        );
+        let mut changeset = ChangeSet::from(self.chain.apply_update(update.chain)?);
         let (_, index_additions) = self
             .indexed_graph
             .index

crates/bitcoind_rpc/Cargo.toml

@@ -6,6 +6,6 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-bdk_chain = { path = "../chain", version = "0.4.0", features = ["serde", "miniscript"] }
+bdk_chain = { path = "../chain", version = "0.5.0", features = ["serde", "miniscript"] }
 bitcoincore-rpc = { version = "0.16" }
 anyhow = { version = "1" }

crates/chain/src/keychain.rs

@@ -11,10 +11,7 @@
 //! [`SpkTxOutIndex`]: crate::SpkTxOutIndex
 
 use crate::{
-    collections::BTreeMap,
-    indexed_tx_graph::IndexedAdditions,
-    local_chain::{self, CheckPoint},
-    tx_graph::TxGraph,
+    collections::BTreeMap, indexed_tx_graph::IndexedAdditions, local_chain, tx_graph::TxGraph,
     Anchor, Append,
 };
 
@@ -92,7 +89,7 @@ impl<K> AsRef<BTreeMap<K, u32>> for DerivationAdditions<K> {
 /// A structure to update [`KeychainTxOutIndex`], [`TxGraph`] and [`LocalChain`] atomically.
 ///
 /// [`LocalChain`]: local_chain::LocalChain
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone)]
 pub struct LocalUpdate<K, A> {
     /// Last active derivation index per keychain (`K`).
     pub keychain: BTreeMap<K, u32>,
@@ -103,26 +100,18 @@ pub struct LocalUpdate<K, A> {
     /// Update for the [`LocalChain`].
     ///
     /// [`LocalChain`]: local_chain::LocalChain
-    pub tip: CheckPoint,
-
-    /// Whether the [`LocalChain`] update (`tip`) can introduce blocks below the original chain's
-    /// tip without invalidating blocks.
-    ///
-    /// Refer to [`LocalChain::update`] for more.
-    ///
-    /// [`LocalChain`]: local_chain::LocalChain
-    /// [`LocalChain::update`]: local_chain::LocalChain::update
-    pub introduce_older_blocks: bool,
+    pub chain: local_chain::Update,
 }
 
 impl<K, A> LocalUpdate<K, A> {
     /// Construct a [`LocalUpdate`] with a given [`CheckPoint`] tip.
-    pub fn new(tip: CheckPoint) -> Self {
+    ///
+    /// [`CheckPoint`]: local_chain::CheckPoint
+    pub fn new(chain_update: local_chain::Update) -> Self {
         Self {
             keychain: BTreeMap::new(),
             graph: TxGraph::default(),
-            tip,
-            introduce_older_blocks: false,
+            chain: chain_update,
         }
     }
 }

crates/chain/src/local_chain.rs

@@ -13,11 +13,11 @@ pub type ChangeSet = BTreeMap<u32, Option<BlockHash>>;
 /// A blockchain of [`LocalChain`].
 ///
 /// The in a linked-list with newer blocks pointing to older ones.
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone)]
 pub struct CheckPoint(Arc<CPInner>);
 
 /// The internal contents of [`CheckPoint`].
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone)]
 struct CPInner {
     /// Block id (hash and height).
     block: BlockId,
@@ -113,13 +113,33 @@ impl IntoIterator for CheckPoint {
     }
 }
 
+/// Represents an update to [`LocalChain`].
+#[derive(Debug, Clone)]
+pub struct Update {
+    /// The update's new [`CheckPoint`] tip.
+    pub tip: CheckPoint,
+
+    /// Whether the update allows for introducing older blocks.
+    ///
+    /// Refer to [`LocalChain::apply_update`] for more.
+    ///
+    /// [`LocalChain::apply_update`]: crate::local_chain::LocalChain::apply_update
+    pub introduce_older_blocks: bool,
+}
+
 /// This is a local implementation of [`ChainOracle`].
-#[derive(Debug, Default, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Default, Clone)]
 pub struct LocalChain {
     tip: Option<CheckPoint>,
     index: BTreeMap<u32, BlockHash>,
 }
 
+impl PartialEq for LocalChain {
+    fn eq(&self, other: &Self) -> bool {
+        self.index == other.index
+    }
+}
+
 impl From<LocalChain> for BTreeMap<u32, BlockHash> {
     fn from(value: LocalChain) -> Self {
         value.index
@@ -262,41 +282,28 @@ impl LocalChain {
     /// Refer to [module-level documentation] for more.
     ///
     /// [module-level documentation]: crate::local_chain
-    pub fn update(
-        &mut self,
-        update_tip: CheckPoint,
-        introduce_older_blocks: bool,
-    ) -> Result<ChangeSet, CannotConnectError> {
-        let changeset = match self.tip() {
+    pub fn apply_update(&mut self, update: Update) -> Result<ChangeSet, CannotConnectError> {
+        match self.tip() {
             Some(original_tip) => {
-                let (changeset, perfect_connection) =
-                    merge_chains(original_tip, update_tip.clone(), introduce_older_blocks)?;
-
-                if !perfect_connection {
-                    self.apply_changeset(&changeset);
-                    // return early as `apply_changeset` already calls `check_consistency`
-                    return Ok(changeset);
-                }
-
-                self.tip = Some(update_tip);
-                for (height, hash) in &changeset {
-                    match hash {
-                        Some(hash) => self.index.insert(*height, *hash),
-                        None => self.index.remove(height),
-                    };
-                }
-                changeset
+                let changeset = merge_chains(
+                    original_tip,
+                    update.tip.clone(),
+                    update.introduce_older_blocks,
+                )?;
+                self.apply_changeset(&changeset);
+
+                // return early as `apply_changeset` already calls `check_consistency`
+                Ok(changeset)
             }
             None => {
-                *self = Self::from_tip(update_tip);
-                self.initial_changeset()
-            }
-        };
+                *self = Self::from_tip(update.tip);
+                let changeset = self.initial_changeset();
 
-        #[cfg(debug_assertions)]
-        self._check_consistency(Some(&changeset));
-
-        Ok(changeset)
+                #[cfg(debug_assertions)]
+                self._check_consistency(Some(&changeset));
+                Ok(changeset)
+            }
+        }
     }
 
     /// Apply the given `changeset`.
@@ -467,7 +474,7 @@ fn merge_chains(
     original_tip: CheckPoint,
     update_tip: CheckPoint,
     introduce_older_blocks: bool,
-) -> Result<(ChangeSet, bool), CannotConnectError> {
+) -> Result<ChangeSet, CannotConnectError> {
     let mut changeset = ChangeSet::default();
     let mut orig = original_tip.into_iter();
     let mut update = update_tip.into_iter();
@@ -526,13 +533,12 @@ fn merge_chains(
                     }
                     point_of_agreement_found = true;
                     prev_orig_was_invalidated = false;
-                    // OPTIMIZATION 1 -- if we know that older blocks cannot be introduced without
-                    // invalidation, we can break after finding the point of agreement
-                    // OPTIMIZATION 2 -- if we have the same underlying references at this
-                    // point then we know everything else in the two chains will match so the
-                    // changeset is fine.
+                    // OPTIMIZATION 1 -- If we know that older blocks cannot be introduced without
+                    // invalidation, we can break after finding the point of agreement.
+                    // OPTIMIZATION 2 -- if we have the same underlying pointer at this point, we
+                    // can guarantee that no older blocks are introduced.
                     if !introduce_older_blocks || Arc::as_ptr(&o.0) == Arc::as_ptr(&u.0) {
-                        return Ok((changeset, true));
+                        return Ok(changeset);
                     }
                 } else {
                     // We have an invalidation height so we set the height to the updated hash and
@@ -566,5 +572,5 @@ fn merge_chains(
         }
     }
 
-    Ok((changeset, false))
+    Ok(changeset)
 }

crates/chain/tests/common/mod.rs

@@ -17,9 +17,12 @@ macro_rules! local_chain {
 macro_rules! chain_update {
     [ $(($height:expr, $hash:expr)), * ] => {{
         #[allow(unused_mut)]
-        bdk_chain::local_chain::LocalChain::from_blocks([$(($height, $hash).into()),*].into_iter().collect())
+        bdk_chain::local_chain::Update {
+            tip: bdk_chain::local_chain::LocalChain::from_blocks([$(($height, $hash).into()),*].into_iter().collect())
             .tip()
-            .expect("must have tip")
+            .expect("must have tip"),
+            introduce_older_blocks: true,
+        }
     }};
 }