Skip to content

Commit 4432455

Browse files
evanlinjinclaude
andcommitted
feat(chain)!: Add ApplyBlockError for prev_blockhash validation
Introduce `ApplyBlockError` enum with two variants: - `MissingGenesis`: genesis block is missing or would be altered - `PrevBlockhashMismatch`: block's `prev_blockhash` doesn't match expected This replaces `MissingGenesisError` in several `LocalChain` methods: - `from_blocks` - `from_changeset` - `apply_changeset` Also adds test cases for `merge_chains` with `prev_blockhash` scenarios: - Update displaces invalid block below point of agreement - Update fills gap with matching `prev_blockhash` - Cascading eviction through multiple blocks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 5a568dc commit 4432455

File tree

3 files changed

+185
-24
lines changed

3 files changed

+185
-24
lines changed

crates/chain/src/local_chain.rs

Lines changed: 84 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bitcoin::BlockHash;
1515
fn apply_changeset_to_checkpoint<D>(
1616
mut init_cp: CheckPoint<D>,
1717
changeset: &ChangeSet<D>,
18-
) -> Result<CheckPoint<D>, MissingGenesisError>
18+
) -> Result<CheckPoint<D>, ApplyBlockError>
1919
where
2020
D: ToBlockHash + fmt::Debug + Clone,
2121
{
@@ -48,7 +48,12 @@ where
4848
let new_tip = match base {
4949
Some(base) => base
5050
.extend(extension)
51-
.expect("extension is strictly greater than base"),
51+
// Since `extension` is in height order, the only failure case is `prev_blockhash`
52+
// mismatch.
53+
.map_err(|last_cp| ApplyBlockError::PrevBlockhashMismatch {
54+
height: last_cp.height(),
55+
expected: last_cp.hash(),
56+
})?,
5257
None => LocalChain::from_blocks(extension)?.tip(),
5358
};
5459
init_cp = new_tip;
@@ -251,22 +256,28 @@ where
251256
///
252257
/// The [`BTreeMap`] enforces the height order. However, the caller must ensure the blocks are
253258
/// all of the same chain.
254-
pub fn from_blocks(blocks: BTreeMap<u32, D>) -> Result<Self, MissingGenesisError> {
259+
pub fn from_blocks(blocks: BTreeMap<u32, D>) -> Result<Self, ApplyBlockError> {
255260
if !blocks.contains_key(&0) {
256-
return Err(MissingGenesisError);
261+
return Err(ApplyBlockError::MissingGenesis);
257262
}
258263

259-
Ok(Self {
260-
tip: CheckPoint::from_blocks(blocks).expect("blocks must be in order"),
261-
})
264+
CheckPoint::from_blocks(blocks)
265+
.map(|tip| Self { tip })
266+
.map_err(|err| {
267+
let last_cp = err.expect("must have atleast one block (genesis)");
268+
ApplyBlockError::PrevBlockhashMismatch {
269+
height: last_cp.height(),
270+
expected: last_cp.hash(),
271+
}
272+
})
262273
}
263274

264275
/// Construct a [`LocalChain`] from an initial `changeset`.
265-
pub fn from_changeset(changeset: ChangeSet<D>) -> Result<Self, MissingGenesisError> {
276+
pub fn from_changeset(changeset: ChangeSet<D>) -> Result<Self, ApplyBlockError> {
266277
let genesis_entry = changeset.blocks.get(&0).cloned().flatten();
267278
let genesis_data = match genesis_entry {
268279
Some(data) => data,
269-
None => return Err(MissingGenesisError),
280+
None => return Err(ApplyBlockError::MissingGenesis),
270281
};
271282

272283
let (mut chain, _) = Self::from_genesis(genesis_data);
@@ -310,7 +321,7 @@ where
310321
}
311322

312323
/// Apply the given `changeset`.
313-
pub fn apply_changeset(&mut self, changeset: &ChangeSet<D>) -> Result<(), MissingGenesisError> {
324+
pub fn apply_changeset(&mut self, changeset: &ChangeSet<D>) -> Result<(), ApplyBlockError> {
314325
let old_tip = self.tip.clone();
315326
let new_tip = apply_changeset_to_checkpoint(old_tip, changeset)?;
316327
self.tip = new_tip;
@@ -485,6 +496,37 @@ impl<D> FromIterator<(u32, D)> for ChangeSet<D> {
485496
}
486497
}
487498

499+
/// Error when applying blocks to a local chain.
500+
#[derive(Clone, Debug, PartialEq)]
501+
pub enum ApplyBlockError {
502+
/// Genesis block is missing or would be altered.
503+
MissingGenesis,
504+
/// Block's `prev_blockhash` doesn't match the expected block.
505+
PrevBlockhashMismatch {
506+
/// Height of the block that `prev_blockhash` should reference.
507+
height: u32,
508+
/// The hash of the block at `height`.
509+
expected: BlockHash,
510+
},
511+
}
512+
513+
impl core::fmt::Display for ApplyBlockError {
514+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
515+
match self {
516+
ApplyBlockError::MissingGenesis => {
517+
write!(f, "genesis block is missing or would be altered")
518+
}
519+
ApplyBlockError::PrevBlockhashMismatch { height, expected } => write!(
520+
f,
521+
"`prev_blockhash` doesn't match block at height {height} ({expected})"
522+
),
523+
}
524+
}
525+
}
526+
527+
#[cfg(feature = "std")]
528+
impl std::error::Error for ApplyBlockError {}
529+
488530
/// An error which occurs when a [`LocalChain`] is constructed without a genesis checkpoint.
489531
#[derive(Clone, Debug, PartialEq)]
490532
pub struct MissingGenesisError;
@@ -596,6 +638,30 @@ fn merge_chains<D>(
596638
where
597639
D: ToBlockHash + fmt::Debug + Clone,
598640
{
641+
// Apply the changeset to produce the final merged chain.
642+
//
643+
// `PrevBlockhashMismatch` should never happen because the merge iteration detects
644+
// `prev_blockhash` conflicts and resolves them by invalidating conflicting blocks (setting
645+
// them to `None` in the changeset) before we reach this point.
646+
fn finish<D>(
647+
original_tip: CheckPoint<D>,
648+
changeset: ChangeSet<D>,
649+
) -> Result<(CheckPoint<D>, ChangeSet<D>), CannotConnectError>
650+
where
651+
D: ToBlockHash + fmt::Debug + Clone,
652+
{
653+
let new_tip = apply_changeset_to_checkpoint(original_tip, &changeset).map_err(|err| {
654+
debug_assert!(
655+
matches!(err, ApplyBlockError::MissingGenesis),
656+
"PrevBlockhashMismatch should never happen"
657+
);
658+
CannotConnectError {
659+
try_include_height: 0,
660+
}
661+
})?;
662+
Ok((new_tip, changeset))
663+
}
664+
599665
let mut changeset = ChangeSet::<D>::default();
600666

601667
let mut orig = original_tip.entry_iter();
@@ -680,11 +746,13 @@ where
680746
if is_update_height_superset_of_original {
681747
return Ok((update_tip, changeset));
682748
} else {
683-
let new_tip = apply_changeset_to_checkpoint(original_tip, &changeset)
684-
.map_err(|_| CannotConnectError {
685-
try_include_height: 0,
686-
})?;
687-
return Ok((new_tip, changeset));
749+
return finish(original_tip, changeset);
750+
}
751+
}
752+
// Update placeholder with real data (if necessary).
753+
if let Some(u_data) = u.data_ref() {
754+
if o.is_placeholder() {
755+
changeset.blocks.insert(u.height(), Some(u_data.clone()));
688756
}
689757
}
690758
} else {
@@ -719,10 +787,5 @@ where
719787
}
720788
}
721789

722-
let new_tip = apply_changeset_to_checkpoint(original_tip, &changeset).map_err(|_| {
723-
CannotConnectError {
724-
try_include_height: 0,
725-
}
726-
})?;
727-
Ok((new_tip, changeset))
790+
finish(original_tip, changeset)
728791
}

crates/chain/tests/test_local_chain.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,14 @@ fn merge_chains_with_prev_blockhash() {
10621062
hash: hash!("B'"),
10631063
prev_hash: hash!("A"),
10641064
};
1065+
let block_a_alt = TestBlock {
1066+
hash: hash!("A_alt"),
1067+
prev_hash: hash!("_"),
1068+
};
1069+
let block_d = TestBlock {
1070+
hash: hash!("D"),
1071+
prev_hash: hash!("C"),
1072+
};
10651073

10661074
[
10671075
// Test: prev_blockhash can invalidate blocks in the original chain
@@ -1195,6 +1203,96 @@ fn merge_chains_with_prev_blockhash() {
11951203
try_include_height: 1,
11961204
}),
11971205
},
1206+
// Test: update displaces invalid block below point of agreement
1207+
//
1208+
// ```text
1209+
// | 0 | 1 | 2 | 4 |
1210+
// chain | _ C(prev=B) D
1211+
// update | _ A D
1212+
// result | _ A D
1213+
// ```
1214+
//
1215+
// Both chains agree on D at height 4. Chain has C at height 2 with `prev_blockhash = B`,
1216+
// but no B exists. Update introduces A at height 1, which displaces C because
1217+
// C's `prev_blockhash` ("B") doesn't match A's hash ("A").
1218+
//
1219+
// Note: This can only happen if chains are constructed incorrectly.
1220+
TestLocalChain {
1221+
name: "update displaces invalid block below point of agreement",
1222+
chain: LocalChain::from_blocks(
1223+
[(0, block_genesis), (2, block_c), (4, block_d_linked)].into(),
1224+
)
1225+
.unwrap(),
1226+
update: CheckPoint::from_blocks([
1227+
(0, block_genesis),
1228+
(1, block_a),
1229+
(4, block_d_linked),
1230+
])
1231+
.unwrap(),
1232+
exp: ExpectedResult::Ok {
1233+
changeset: &[(1, Some(block_a)), (2, None)],
1234+
init_changeset: &[
1235+
(0, Some(block_genesis)),
1236+
(1, Some(block_a)),
1237+
(4, Some(block_d_linked)),
1238+
],
1239+
},
1240+
},
1241+
// Test: update fills gap with matching prev_blockhash
1242+
//
1243+
// ```text
1244+
// | 0 | 1 | 2 |
1245+
// chain | _ B(prev=A)
1246+
// update | _ A B
1247+
// result | _ A B
1248+
// ```
1249+
//
1250+
// Chain has gap at height 1. Update provides A which matches B's `prev_blockhash`.
1251+
// The chains connect perfectly.
1252+
TestLocalChain {
1253+
name: "update fills gap with matching prev_blockhash",
1254+
chain: LocalChain::from_blocks([(0, block_genesis), (2, block_b)].into()).unwrap(),
1255+
update: CheckPoint::from_blocks([(0, block_genesis), (1, block_a), (2, block_b)])
1256+
.unwrap(),
1257+
exp: ExpectedResult::Ok {
1258+
changeset: &[(1, Some(block_a))],
1259+
init_changeset: &[
1260+
(0, Some(block_genesis)),
1261+
(1, Some(block_a)),
1262+
(2, Some(block_b)),
1263+
],
1264+
},
1265+
},
1266+
// Test: cascading eviction through multiple blocks
1267+
//
1268+
// ```text
1269+
// | 0 | 1 | 2 | 3 | 4 |
1270+
// chain | _ A B(prev=A) C(prev=B) D(prev=C)
1271+
// update | _ A_alt
1272+
// result | _ A_alt
1273+
// ```
1274+
//
1275+
// Update replaces A with A_alt. B's `prev_blockhash = A` doesn't match A_alt,
1276+
// so B is evicted. C and D depend on B via prev_blockhash, so they cascade evict.
1277+
TestLocalChain {
1278+
name: "cascading eviction through multiple blocks",
1279+
chain: LocalChain::from_blocks(
1280+
[
1281+
(0, block_genesis),
1282+
(1, block_a),
1283+
(2, block_b),
1284+
(3, block_c),
1285+
(4, block_d),
1286+
]
1287+
.into(),
1288+
)
1289+
.unwrap(),
1290+
update: CheckPoint::from_blocks([(0, block_genesis), (1, block_a_alt)]).unwrap(),
1291+
exp: ExpectedResult::Ok {
1292+
changeset: &[(1, Some(block_a_alt)), (2, None), (3, None), (4, None)],
1293+
init_changeset: &[(0, Some(block_genesis)), (1, Some(block_a_alt))],
1294+
},
1295+
},
11981296
]
11991297
.into_iter()
12001298
.for_each(TestLocalChain::run);

crates/core/src/checkpoint.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,13 @@ where
284284
/// Construct from an iterator of block data.
285285
///
286286
/// Returns `Err(None)` if `blocks` doesn't yield any data. If the blocks are not in ascending
287-
/// height order, then returns an `Err(..)` containing the last checkpoint that would have been
288-
/// extended.
287+
/// height order, or there are any `prev_blockhash` mismatches, then returns an `Err(..)`
288+
/// containing the last checkpoint that would have been extended.
289289
pub fn from_blocks(blocks: impl IntoIterator<Item = (u32, D)>) -> Result<Self, Option<Self>> {
290290
let mut blocks = blocks.into_iter();
291291
let (height, data) = blocks.next().ok_or(None)?;
292292
let mut cp = CheckPoint::new(height, data);
293-
cp = cp.extend(blocks)?;
293+
cp = cp.extend(blocks).map_err(Some)?;
294294

295295
Ok(cp)
296296
}

0 commit comments

Comments
 (0)