fix(chain)!: report `prev_blockhash` mismatches from `insert_block`

`insert_block` mapped every `apply_changeset` error to an
`AlterCheckPointError` about removing genesis. The only reachable error
there is a `prev_blockhash` mismatch, so callers inserting an unlinked
`Header` were told genesis could not be removed.

`insert_block` now returns `InsertBlockError`, which reports either the
existing conflicting block or the `prev_blockhash` mismatch. With no
users left, `AlterCheckPointError` is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs
index 8b1fef0b86370ae35cbb6a4fd68d2b17c544e763..901f279172a5fb20a33626846be24196cf2b5129 100644 (file)
--- a/crates/chain/src/local_chain.rs
+++ b/crates/chain/src/local_chain.rs
@@ -420,19 +420,16 @@ where
     ///
     /// # Errors
     ///
-    /// Replacing the block hash of an existing checkpoint will result in an error.
-    pub fn insert_block(
-        &mut self,
-        height: u32,
-        data: D,
-    ) -> Result<ChangeSet<D>, AlterCheckPointError> {
+    /// Replacing the block hash of an existing checkpoint will result in an error. So will a block
+    /// that does not link to its neighbors by `prev_blockhash`.
+    pub fn insert_block(&mut self, height: u32, data: D) -> Result<ChangeSet<D>, InsertBlockError> {
         if let Some(original_cp) = self.tip.get(height) {
             let original_hash = original_cp.hash();
             if original_hash != data.to_blockhash() {
-                return Err(AlterCheckPointError {
+                return Err(InsertBlockError::CannotReplace {
                     height,
                     original_hash,
-                    update_hash: Some(data.to_blockhash()),
+                    update_hash: data.to_blockhash(),
                 });
             }
             return Ok(ChangeSet::default());
@@ -440,17 +437,14 @@ where
 
         let mut changeset = ChangeSet::<D>::default();
         changeset.blocks.insert(height, Some(data));
-        self.apply_changeset(&changeset)
-            .map_err(|_| AlterCheckPointError {
-                height: 0,
-                original_hash: self.genesis_hash(),
-                update_hash: changeset
-                    .blocks
-                    .get(&0)
-                    .cloned()
-                    .flatten()
-                    .map(|d| d.to_blockhash()),
-            })?;
+        self.apply_changeset(&changeset).map_err(|err| match err {
+            ApplyChangeSetError::CannotAlterGenesis { .. } => {
+                unreachable!("`height` is not in the chain, so it cannot be genesis")
+            }
+            ApplyChangeSetError::PrevBlockhashMismatch { expected } => {
+                InsertBlockError::PrevBlockhashMismatch { expected }
+            }
+        })?;
         Ok(changeset)
     }
 
@@ -658,35 +652,47 @@ impl core::fmt::Display for MissingGenesisError {
 
 impl core::error::Error for MissingGenesisError {}
 
-/// Represents a failure when trying to insert/remove a checkpoint to/from [`LocalChain`].
+/// The error type for [`LocalChain::insert_block`].
 #[derive(Clone, Debug, PartialEq)]
-pub struct AlterCheckPointError {
-    /// The checkpoint's height.
-    pub height: u32,
-    /// The original checkpoint's block hash which cannot be replaced/removed.
-    pub original_hash: BlockHash,
-    /// The attempted update to the `original_block` hash.
-    pub update_hash: Option<BlockHash>,
+#[non_exhaustive]
+pub enum InsertBlockError {
+    /// The block conflicts with an existing checkpoint at the same height.
+    CannotReplace {
+        /// The checkpoint's height.
+        height: u32,
+        /// The original checkpoint's block hash which cannot be replaced.
+        original_hash: BlockHash,
+        /// The attempted replacement block hash.
+        update_hash: BlockHash,
+    },
+    /// Block's `prev_blockhash` doesn't match the expected block.
+    PrevBlockhashMismatch {
+        /// The block that `prev_blockhash` should reference.
+        expected: BlockId,
+    },
 }
 
-impl core::fmt::Display for AlterCheckPointError {
+impl core::fmt::Display for InsertBlockError {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self.update_hash {
-            Some(update_hash) => write!(
+        match self {
+            InsertBlockError::CannotReplace {
+                height,
+                original_hash,
+                update_hash,
+            } => write!(
                 f,
-                "failed to insert block at height {}: original={} update={}",
-                self.height, self.original_hash, update_hash
+                "failed to insert block at height {height}: original={original_hash} update={update_hash}"
             ),
-            None => write!(
+            InsertBlockError::PrevBlockhashMismatch { expected } => write!(
                 f,
-                "failed to remove block at height {}: original={}",
-                self.height, self.original_hash
+                "`prev_blockhash` doesn't match block at height {} ({})",
+                expected.height, expected.hash
             ),
         }
     }
 }
 
-impl core::error::Error for AlterCheckPointError {}
+impl core::error::Error for InsertBlockError {}
 
 /// Occurs when an update does not have a common checkpoint with the original chain.
 #[derive(Clone, Debug, PartialEq)]

diff --git a/crates/chain/tests/test_local_chain.rs b/crates/chain/tests/test_local_chain.rs
index 2e9757c7a63a25401d8efb268de2be4bce5bb5c3..827b9c3526b7050178ca001b089e4fc544208444 100644 (file)
--- a/crates/chain/tests/test_local_chain.rs
+++ b/crates/chain/tests/test_local_chain.rs
@@ -5,8 +5,8 @@ use std::ops::{Bound, RangeBounds};
 
 use bdk_chain::{
     local_chain::{
-        AlterCheckPointError, ApplyBlockError, ApplyChangeSetError, ApplyHeaderError,
-        CannotConnectError, ChangeSet, CheckPoint, LocalChain, MissingGenesisError,
+        ApplyBlockError, ApplyChangeSetError, ApplyHeaderError, CannotConnectError, ChangeSet,
+        CheckPoint, InsertBlockError, LocalChain, MissingGenesisError,
     },
     BlockId,
 };
@@ -342,7 +342,7 @@ fn local_chain_insert_block() {
     struct TestCase {
         original: LocalChain,
         insert: (u32, BlockHash),
-        expected_result: Result<ChangeSet, AlterCheckPointError>,
+        expected_result: Result<ChangeSet, InsertBlockError>,
         expected_final: LocalChain,
     }
 
@@ -374,10 +374,10 @@ fn local_chain_insert_block() {
         TestCase {
             original: local_chain![(0, hash!("_")), (2, hash!("K"))],
             insert: (2, hash!("J")),
-            expected_result: Err(AlterCheckPointError {
+            expected_result: Err(InsertBlockError::CannotReplace {
                 height: 2,
                 original_hash: hash!("K"),
-                update_hash: Some(hash!("J")),
+                update_hash: hash!("J"),
             }),
             expected_final: local_chain![(0, hash!("_")), (2, hash!("K"))],
         },
@@ -432,7 +432,7 @@ fn local_chain_insert_header() {
     struct TestCase {
         original: LocalChain<Header>,
         insert: (u32, Header),
-        expected_result: Result<ChangeSet<Header>, AlterCheckPointError>,
+        expected_result: Result<ChangeSet<Header>, InsertBlockError>,
         expected_final: LocalChain<Header>,
     }
 
@@ -470,13 +470,39 @@ fn local_chain_insert_header() {
         TestCase {
             original: local_chain(vec![(0, headers[0]), (2, headers[2])]),
             insert: (2, header(hash!("conflict"))),
-            expected_result: Err(AlterCheckPointError {
+            expected_result: Err(InsertBlockError::CannotReplace {
                 height: 2,
                 original_hash: headers[2].block_hash(),
-                update_hash: Some(header(hash!("conflict")).block_hash()),
+                update_hash: header(hash!("conflict")).block_hash(),
             }),
             expected_final: local_chain(vec![(0, headers[0]), (2, headers[2])]),
         },
+        // Test case 6: start with headers at heights 0 and 2. Insert header at height 3 that does
+        // not link to height 2.
+        TestCase {
+            original: local_chain(vec![(0, headers[0]), (2, headers[2])]),
+            insert: (3, header(hash!("not_2"))),
+            expected_result: Err(InsertBlockError::PrevBlockhashMismatch {
+                expected: BlockId {
+                    height: 2,
+                    hash: headers[2].block_hash(),
+                },
+            }),
+            expected_final: local_chain(vec![(0, headers[0]), (2, headers[2])]),
+        },
+        // Test case 7: start with headers at heights 0 and 3. Insert header at height 2 that
+        // header 3 does not link to.
+        TestCase {
+            original: local_chain(vec![(0, headers[0]), (3, headers[3])]),
+            insert: (2, header(hash!("not_1"))),
+            expected_result: Err(InsertBlockError::PrevBlockhashMismatch {
+                expected: BlockId {
+                    height: 2,
+                    hash: header(hash!("not_1")).block_hash(),
+                },
+            }),
+            expected_final: local_chain(vec![(0, headers[0]), (3, headers[3])]),
+        },
     ];
 
     for (i, t) in test_cases.into_iter().enumerate() {