Skip to content

Commit

Permalink
fix(core): calling CheckPoint::insert with existing block must succeed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
evanlinjin committed Sep 12, 2024
1 parent 8760653 commit 2970b83
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
3 changes: 1 addition & 2 deletions crates/core/src/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,14 @@ 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 {
if cp.height() == block_id.height {
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![];
Expand Down
9 changes: 9 additions & 0 deletions crates/core/tests/common.rs
Original file line number Diff line number Diff line change
@@ -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()),
}
}};
}
36 changes: 36 additions & 0 deletions crates/core/tests/test_checkpoint.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
}

0 comments on commit 2970b83

Please sign in to comment.