Merge bitcoindevkit/bdk#1380: Simplified EsploraExt API

96a9aa6e63 feat(chain): refactor `merge_chains` (志宇)
2f22987c9e chore(chain): fix comment (志宇)
daf588f016 feat(chain): optimize `merge_chains` (志宇)
77d35954c1 feat(chain)!: rm `local_chain::Update` (志宇)
1269b0610e test(chain): fix incorrect test case (志宇)
72fe65b65f feat(esplora)!: simplify chain update logic (志宇)
eded1a7ea0 feat(chain): introduce `CheckPoint::insert` (志宇)
519cd75d23 test(esplora): move esplora tests into src files (志宇)
a6e613e6b9 test(esplora): add `test_finalize_chain_update` (志宇)
494d253493 feat(testenv): add `genesis_hash` method (志宇)
886d72e3d5 chore(chain)!: rm `missing_heights` and `missing_heights_from` methods (志宇)
bd62aa0fe1 feat(esplora)!: remove `EsploraExt::update_local_chain` (志宇)
1e99793983 feat(testenv): add `make_checkpoint_tip` (志宇)

Pull request description:

  Fixes #1354

  ### Description

  Built on top of both #1369 and #1373, we simplify the `EsploraExt` API by removing the `update_local_chain` method and having `full_scan` and `sync` update the local chain in the same call. The `full_scan` and `sync` methods now takes in an additional input (`local_tip`) which provides us with the view of the `LocalChain` before the update. These methods now return structs `FullScanUpdate` and `SyncUpdate`.

  The examples are updated to use this new API. `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` are no longer needed, therefore they are removed.

  Additionally, we used this opportunity to simplify the logic which updates `LocalChain`. We got rid of the `local_chain::Update` struct (which contained the update `CheckPoint` tip and a `bool` which signaled whether we want to introduce blocks below point of agreement). It turns out we can use something like `CheckPoint::insert` so the chain source can craft an update based on the old tip. This way, we can make better use of `merge_chains`' optimization that compares the `Arc` pointers of the local and update chain (before we were crafting the update chain NOT based on top of the previous local chain). With this, we no longer need the `Update::introduce_older_block` field since the logic will naturally break when we reach a matching `Arc` pointer.

  ### Notes to the reviewers

  * Obtaining the `LocalChain`'s update now happens within `EsploraExt::full_scan` and `EsploraExt::sync`. Creating the `LocalChain` update is now split into two methods (`fetch_latest_blocks` and `chain_update`) that are called before and after fetching transactions and anchors.
  * We need to duplicate code for `bdk_esplora`. One for blocking and one for async.

  ### Changelog notice

  * Changed `EsploraExt` API so that sync only requires one round of fetching data. The `local_chain_update` method is removed and the `local_tip` parameter is added to the `full_scan` and `sync` methods.
  * Removed `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` methods.
  * Introduced `CheckPoint::insert` which allows convenient checkpoint-insertion. This is intended for use by chain-sources when crafting an update.
  * Refactored `merge_chains` to also return the resultant `CheckPoint` tip.
  * Optimized the update `LocalChain` logic - use the update `CheckPoint` as the new `CheckPoint` tip when possible.

  ### 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

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature

ACKs for top commit:
  LLFourn:
    ACK 96a9aa6e63

Tree-SHA512: 3d4f2eab08a1fe94eb578c594126e99679f72e231680b2edd4bfb018ba1d998ca123b07acb2d19c644d5887fc36b8e42badba91cd09853df421ded04de45bf69

crates/chain/tests/common/mod.rs

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

crates/chain/tests/test_local_chain.rs

@@ -3,7 +3,7 @@ use std::ops::{Bound, RangeBounds};
 use bdk_chain::{
     local_chain::{
         AlterCheckPointError, ApplyHeaderError, CannotConnectError, ChangeSet, CheckPoint,
-        LocalChain, MissingGenesisError, Update,
+        LocalChain, MissingGenesisError,
     },
     BlockId,
 };
@@ -17,7 +17,7 @@ mod common;
 struct TestLocalChain<'a> {
     name: &'static str,
     chain: LocalChain,
-    update: Update,
+    update: CheckPoint,
     exp: ExpectedResult<'a>,
 }
 
@@ -577,6 +577,77 @@ fn checkpoint_query() {
     }
 }
 
+#[test]
+fn checkpoint_insert() {
+    struct TestCase<'a> {
+        /// The name of the test.
+        name: &'a str,
+        /// The original checkpoint chain to call [`CheckPoint::insert`] on.
+        chain: &'a [(u32, BlockHash)],
+        /// The `block_id` to insert.
+        to_insert: (u32, BlockHash),
+        /// The expected final checkpoint chain after calling [`CheckPoint::insert`].
+        exp_final_chain: &'a [(u32, BlockHash)],
+    }
+
+    let test_cases = [
+        TestCase {
+            name: "insert_above_tip",
+            chain: &[(1, h!("a")), (2, h!("b"))],
+            to_insert: (4, h!("d")),
+            exp_final_chain: &[(1, h!("a")), (2, h!("b")), (4, h!("d"))],
+        },
+        TestCase {
+            name: "insert_already_exists_expect_no_change",
+            chain: &[(1, h!("a")), (2, h!("b")), (3, h!("c"))],
+            to_insert: (2, h!("b")),
+            exp_final_chain: &[(1, h!("a")), (2, h!("b")), (3, h!("c"))],
+        },
+        TestCase {
+            name: "insert_in_middle",
+            chain: &[(2, h!("b")), (4, h!("d")), (5, h!("e"))],
+            to_insert: (3, h!("c")),
+            exp_final_chain: &[(2, h!("b")), (3, h!("c")), (4, h!("d")), (5, h!("e"))],
+        },
+        TestCase {
+            name: "replace_one",
+            chain: &[(3, h!("c")), (4, h!("d")), (5, h!("e"))],
+            to_insert: (5, h!("E")),
+            exp_final_chain: &[(3, h!("c")), (4, h!("d")), (5, h!("E"))],
+        },
+        TestCase {
+            name: "insert_conflict_should_evict",
+            chain: &[(3, h!("c")), (4, h!("d")), (5, h!("e")), (6, h!("f"))],
+            to_insert: (4, h!("D")),
+            exp_final_chain: &[(3, h!("c")), (4, h!("D"))],
+        },
+    ];
+
+    fn genesis_block() -> impl Iterator<Item = BlockId> {
+        core::iter::once((0, h!("_"))).map(BlockId::from)
+    }
+
+    for (i, t) in test_cases.into_iter().enumerate() {
+        println!("Running [{}] '{}'", i, t.name);
+
+        let chain = CheckPoint::from_block_ids(
+            genesis_block().chain(t.chain.iter().copied().map(BlockId::from)),
+        )
+        .expect("test formed incorrectly, must construct checkpoint chain");
+
+        let exp_final_chain = CheckPoint::from_block_ids(
+            genesis_block().chain(t.exp_final_chain.iter().copied().map(BlockId::from)),
+        )
+        .expect("test formed incorrectly, must construct checkpoint chain");
+
+        assert_eq!(
+            chain.insert(t.to_insert.into()),
+            exp_final_chain,
+            "unexpected final chain"
+        );
+    }
+}
+
 #[test]
 fn local_chain_apply_header_connected_to() {
     fn header_from_prev_blockhash(prev_blockhash: BlockHash) -> Header {
@@ -601,9 +672,9 @@ fn local_chain_apply_header_connected_to() {
 
     let test_cases = [
         {
-            let header = header_from_prev_blockhash(h!("A"));
+            let header = header_from_prev_blockhash(h!("_"));
             let hash = header.block_hash();
-            let height = 2;
+            let height = 1;
             let connected_to = BlockId { height, hash };
             TestCase {
                 name: "connected_to_self_header_applied_to_self",

crates/chain/tests/test_tx_graph.rs

@@ -1087,139 +1087,6 @@ fn update_last_seen_unconfirmed() {
     assert_eq!(graph.full_txs().next().unwrap().last_seen_unconfirmed, 2);
 }
 
-#[test]
-fn test_missing_blocks() {
-    /// An anchor implementation for testing, made up of `(the_anchor_block, random_data)`.
-    #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord, core::hash::Hash)]
-    struct TestAnchor(BlockId);
-
-    impl Anchor for TestAnchor {
-        fn anchor_block(&self) -> BlockId {
-            self.0
-        }
-    }
-
-    struct Scenario<'a> {
-        name: &'a str,
-        graph: TxGraph<TestAnchor>,
-        chain: LocalChain,
-        exp_heights: &'a [u32],
-    }
-
-    const fn new_anchor(height: u32, hash: BlockHash) -> TestAnchor {
-        TestAnchor(BlockId { height, hash })
-    }
-
-    fn new_scenario<'a>(
-        name: &'a str,
-        graph_anchors: &'a [(Txid, TestAnchor)],
-        chain: &'a [(u32, BlockHash)],
-        exp_heights: &'a [u32],
-    ) -> Scenario<'a> {
-        Scenario {
-            name,
-            graph: {
-                let mut g = TxGraph::default();
-                for (txid, anchor) in graph_anchors {
-                    let _ = g.insert_anchor(*txid, anchor.clone());
-                }
-                g
-            },
-            chain: {
-                let (mut c, _) = LocalChain::from_genesis_hash(h!("genesis"));
-                for (height, hash) in chain {
-                    let _ = c.insert_block(BlockId {
-                        height: *height,
-                        hash: *hash,
-                    });
-                }
-                c
-            },
-            exp_heights,
-        }
-    }
-
-    fn run(scenarios: &[Scenario]) {
-        for scenario in scenarios {
-            let Scenario {
-                name,
-                graph,
-                chain,
-                exp_heights,
-            } = scenario;
-
-            let heights = graph.missing_heights(chain).collect::<Vec<_>>();
-            assert_eq!(&heights, exp_heights, "scenario: {}", name);
-        }
-    }
-
-    run(&[
-        new_scenario(
-            "2 txs with the same anchor (2:B) which is missing from chain",
-            &[
-                (h!("tx_1"), new_anchor(2, h!("B"))),
-                (h!("tx_2"), new_anchor(2, h!("B"))),
-            ],
-            &[(1, h!("A")), (3, h!("C"))],
-            &[2],
-        ),
-        new_scenario(
-            "2 txs with different anchors at the same height, one of the anchors is missing",
-            &[
-                (h!("tx_1"), new_anchor(2, h!("B1"))),
-                (h!("tx_2"), new_anchor(2, h!("B2"))),
-            ],
-            &[(1, h!("A")), (2, h!("B1"))],
-            &[],
-        ),
-        new_scenario(
-            "tx with 2 anchors of same height which are missing from the chain",
-            &[
-                (h!("tx"), new_anchor(3, h!("C1"))),
-                (h!("tx"), new_anchor(3, h!("C2"))),
-            ],
-            &[(1, h!("A")), (4, h!("D"))],
-            &[3],
-        ),
-        new_scenario(
-            "tx with 2 anchors at the same height, chain has this height but does not match either anchor",
-            &[
-                (h!("tx"), new_anchor(4, h!("D1"))),
-                (h!("tx"), new_anchor(4, h!("D2"))),
-            ],
-            &[(4, h!("D3")), (5, h!("E"))],
-            &[],
-        ),
-        new_scenario(
-            "tx with 2 anchors at different heights, one anchor exists in chain, should return nothing",
-            &[
-                (h!("tx"), new_anchor(3, h!("C"))),
-                (h!("tx"), new_anchor(4, h!("D"))),
-            ],
-            &[(4, h!("D")), (5, h!("E"))],
-            &[],
-        ),
-        new_scenario(
-            "tx with 2 anchors at different heights, first height is already in chain with different hash, iterator should only return 2nd height",
-            &[
-                (h!("tx"), new_anchor(5, h!("E1"))),
-                (h!("tx"), new_anchor(6, h!("F1"))),
-            ],
-            &[(4, h!("D")), (5, h!("E")), (7, h!("G"))],
-            &[6],
-        ),
-        new_scenario(
-            "tx with 2 anchors at different heights, neither height is in chain, both heights should be returned",
-            &[
-                (h!("tx"), new_anchor(3, h!("C"))),
-                (h!("tx"), new_anchor(4, h!("D"))),
-            ],
-            &[(1, h!("A")), (2, h!("B"))],
-            &[3, 4],
-        ),
-    ]);
-}
-
 #[test]
 /// The `map_anchors` allow a caller to pass a function to reconstruct the [`TxGraph`] with any [`Anchor`],
 /// even though the function is non-deterministic.