From 2970b83f30ca7071f0502de395327eb3671a512b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= <hello@evanlinjin.me>
Date: Thu, 12 Sep 2024 16:44:21 +0800
Subject: [PATCH 1/3] fix(core): calling `CheckPoint::insert` with existing
 block must succeed

Previously, we were panicking when the caller tried to insert a block at
height 0. However, this should succeed if the block hash does not
change.
---
 crates/core/src/checkpoint.rs        |  3 +--
 crates/core/tests/common.rs          |  9 +++++++
 crates/core/tests/test_checkpoint.rs | 36 ++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 crates/core/tests/common.rs
 create mode 100644 crates/core/tests/test_checkpoint.rs

diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs
index 0abadda1d..089db230d 100644
--- a/crates/core/src/checkpoint.rs
+++ b/crates/core/src/checkpoint.rs
@@ -173,8 +173,6 @@ impl CheckPoint {
     /// passed in. Of course, if the `block_id` was already present then this just returns `self`.
     #[must_use]
     pub fn insert(self, block_id: BlockId) -> Self {
-        assert_ne!(block_id.height, 0, "cannot insert the genesis block");
-
         let mut cp = self.clone();
         let mut tail = vec![];
         let base = loop {
@@ -182,6 +180,7 @@ impl CheckPoint {
                 if cp.hash() == block_id.hash {
                     return self;
                 }
+                assert_ne!(cp.height(), 0, "cannot replace genesis block");
                 // if we have a conflict we just return the inserted block because the tail is by
                 // implication invalid.
                 tail = vec![];
diff --git a/crates/core/tests/common.rs b/crates/core/tests/common.rs
new file mode 100644
index 000000000..347a90bf6
--- /dev/null
+++ b/crates/core/tests/common.rs
@@ -0,0 +1,9 @@
+#[allow(unused_macros)]
+macro_rules! block_id {
+    ($height:expr, $hash:literal) => {{
+        bdk_chain::BlockId {
+            height: $height,
+            hash: bitcoin::hashes::Hash::hash($hash.as_bytes()),
+        }
+    }};
+}
diff --git a/crates/core/tests/test_checkpoint.rs b/crates/core/tests/test_checkpoint.rs
new file mode 100644
index 000000000..66f945fe2
--- /dev/null
+++ b/crates/core/tests/test_checkpoint.rs
@@ -0,0 +1,36 @@
+#[macro_use]
+mod common;
+
+use bdk_core::CheckPoint;
+
+/// Inserting a block that already exists in the checkpoint chain must always succeed.
+#[test]
+fn checkpoint_insert_existing() {
+    let blocks = &[
+        block_id!(0, "genesis"),
+        block_id!(1, "A"),
+        block_id!(2, "B"),
+        block_id!(3, "C"),
+    ];
+
+    // Index `i` allows us to test with chains of different length.
+    // Index `j` allows us to test inserting different block heights.
+    for i in 0..blocks.len() {
+        let cp_chain = CheckPoint::from_block_ids(blocks[..=i].iter().copied())
+            .expect("must construct valid chain");
+
+        for j in 0..i {
+            let block_to_insert = cp_chain
+                .get(j as u32)
+                .expect("cp of height must exist")
+                .block_id();
+            let new_cp_chain = cp_chain.clone().insert(block_to_insert);
+
+            assert_eq!(
+                new_cp_chain, cp_chain,
+                "must not divert from original chain"
+            );
+            assert!(new_cp_chain.eq_ptr(&cp_chain), "pointers must still match");
+        }
+    }
+}

From e6aeaea0c69472e4c2dcf0f5e2f632f43733529d Mon Sep 17 00:00:00 2001
From: valued mammal <valuedmammal@protonmail.com>
Date: Thu, 12 Sep 2024 15:18:01 -0400
Subject: [PATCH 2/3] doc(core): document panic for `CheckPoint::insert`

---
 crates/core/src/checkpoint.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs
index 089db230d..10af13277 100644
--- a/crates/core/src/checkpoint.rs
+++ b/crates/core/src/checkpoint.rs
@@ -171,6 +171,10 @@ impl CheckPoint {
     /// it. If the height already existed and has a conflicting block hash then it will be purged
     /// along with all block followin it. The returned chain will have a tip of the `block_id`
     /// passed in. Of course, if the `block_id` was already present then this just returns `self`.
+    ///
+    /// # Panics
+    ///
+    /// This panics if called with a genesis block that differs from that of `self`.
     #[must_use]
     pub fn insert(self, block_id: BlockId) -> Self {
         let mut cp = self.clone();

From 3ae9ecba8c893750fa2ed5dfdbb1f4ee84a0b228 Mon Sep 17 00:00:00 2001
From: valued mammal <valuedmammal@protonmail.com>
Date: Thu, 12 Sep 2024 15:20:16 -0400
Subject: [PATCH 3/3] test: fix off-by-one in `checkpoint_insert_existing`

---
 crates/core/tests/test_checkpoint.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/core/tests/test_checkpoint.rs b/crates/core/tests/test_checkpoint.rs
index 66f945fe2..705f1c474 100644
--- a/crates/core/tests/test_checkpoint.rs
+++ b/crates/core/tests/test_checkpoint.rs
@@ -19,7 +19,7 @@ fn checkpoint_insert_existing() {
         let cp_chain = CheckPoint::from_block_ids(blocks[..=i].iter().copied())
             .expect("must construct valid chain");
 
-        for j in 0..i {
+        for j in 0..=i {
             let block_to_insert = cp_chain
                 .get(j as u32)
                 .expect("cp of height must exist")