Skip to content

Commit 7403aac

Browse files
Aliemekapablodeymo
andauthored
chore(storage): Add error handling on Store::get_forkchoice_store (lambdaclass#362)
## 🗒️ Description / Motivation Replace the assert_eq! in Store::get_forkchoice_store with a typed Result so callers see a descriptive error instead of a panic. The previous message ("block header doesn't match state's latest_block_header") gave no insight into which field actually differed, which made misconfigured anchor state/block pairs hard to diagnose. PR closes lambdaclass#360. ## What Changed - `crates/storage/Cargo.toml` — add `thiserror` dependency. - `crates/storage/src/store.rs` — introduce `pub enum GetForkchoiceStoreError` with a `HeaderMismatch { state_header, block_header }` variant; change `Store::get_forkchoice_store` to return `Result<Self, GetForkchoiceStoreError>`; replace the assertion with an explicit comparison that returns `Err`; update the doc comment (`# Panics` → `# Errors`). - `bin/ethlambda/src/main.rs` — added error handling to `Store::forkchoice_store` to return `CheckpointSyncError::AnchorPairingMismatch` if there is a mismatch. ## Correctness / Behavior Guarantees - The validation rule is unchanged: anchor block header must equal the state's `latest_block_header` after zeroing `state_root`. Mismatches that previously aborted via assert_eq! now return `Err(HeaderMismatch { … })`, surfacing both full headers (boxed to keep the variant small) for diff-friendly debugging. - `Store::from_anchor_state` is untouched. Internal panics in `init_store` (`state_root` mismatch and storage backend operations) are intentionally left as-is — they represent infrastructure invariants shared between both constructors and are out of scope for this change. ## Tests Added / Run - No new tests; the type change is enforced by the compiler and existing fork-choice / signature spec suites continue to exercise the success path. - Ran `make fmt`, `make lint`, `cargo test --workspace --release` (unit tests pass; spec tests require `make leanSpec/fixtures` for fixture generation) - Also ran `make test` ## Related Issues / PRs - Closes lambdaclass#360 ## ✅ Verification Checklist - [x] Ran `make fmt` — clean - [x] Ran `make lint` (clippy with `-D warnings`) — clean - [x] Ran `cargo test --workspace --release` — all passing --------- Co-authored-by: Pablo Deymonnaz <pdeymon@fi.uba.ar>
1 parent a2f6d13 commit 7403aac

7 files changed

Lines changed: 40 additions & 12 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/ethlambda/src/main.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,9 @@ async fn fetch_initial_state(
640640
// overlaps with what `get_forkchoice_store` already wrote, but it's
641641
// idempotent and the only path that also stores `BlockSignatures`.
642642
let anchor_root = signed_block.message.header().hash_tree_root();
643-
let mut store = Store::get_forkchoice_store(backend, state, signed_block.message.clone());
643+
let mut store = Store::get_forkchoice_store(backend, state, signed_block.message.clone())
644+
.inspect_err(|err| error!(%err, "Failed to initialize store from anchor state and block"))
645+
.map_err(|_| checkpoint_sync::CheckpointSyncError::AnchorPairingMismatch)?;
644646
store.insert_signed_block(anchor_root, signed_block);
645647
Ok(store)
646648
}

crates/blockchain/tests/forkchoice_spectests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
7070
}
7171

7272
let backend = Arc::new(InMemoryBackend::new());
73-
let mut store = Store::get_forkchoice_store(backend, anchor_state, anchor_block);
73+
let mut store = Store::get_forkchoice_store(backend, anchor_state, anchor_block)
74+
.expect("anchor state and block must match");
7475

7576
// Block registry: maps block labels to their roots
7677
let mut block_registry: HashMap<String, H256> = HashMap::new();

crates/blockchain/tests/signature_spectests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
4242
// Initialize the store with the anchor state and block
4343
let genesis_time = anchor_state.config.genesis_time;
4444
let backend = Arc::new(InMemoryBackend::new());
45-
let mut st = Store::get_forkchoice_store(backend, anchor_state, anchor_block);
45+
let mut st = Store::get_forkchoice_store(backend, anchor_state, anchor_block)
46+
.expect("anchor state and block must match");
4647

4748
// Step 2: Run the state transition function with the block fixture
4849
let signed_block: SignedBlock = test.signed_block.into();

crates/storage/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ ethlambda-types.workspace = true
1414

1515
tracing.workspace = true
1616
rocksdb.workspace = true
17+
thiserror.workspace = true
1718

1819
libssz.workspace = true
1920
libssz-derive.workspace = true

crates/storage/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ pub mod backend;
33
mod store;
44

55
pub use api::{ALL_TABLES, StorageBackend, StorageReadView, StorageWriteBatch, Table};
6-
pub use store::{ForkCheckpoints, Store};
6+
pub use store::{ForkCheckpoints, GetForkchoiceStoreError, Store};

crates/storage/src/store.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,22 @@ use ethlambda_types::{
2020
state::{ChainConfig, State, anchor_pair_is_consistent},
2121
};
2222
use libssz::{SszDecode, SszEncode};
23+
use thiserror::Error;
2324
use tracing::info;
2425

26+
/// Errors returned by [`Store::get_forkchoice_store`].
27+
#[derive(Debug, Error)]
28+
pub enum GetForkchoiceStoreError {
29+
#[error(
30+
"anchor block doesn't match anchor state: \
31+
state header = {anchor_state:?}, block = {anchor_block:?}"
32+
)]
33+
AnchorPairInconsistent {
34+
anchor_state: Box<State>,
35+
anchor_block: Box<Block>,
36+
},
37+
}
38+
2539
/// Checkpoints to update in the forkchoice store.
2640
///
2741
/// Used with `Store::update_checkpoints` to update head and optionally
@@ -470,20 +484,28 @@ impl Store {
470484
/// The block must match the state's `latest_block_header`.
471485
/// Named to mirror the spec's `get_forkchoice_store` function.
472486
///
473-
/// # Panics
487+
/// # Errors
474488
///
475-
/// Panics if [`anchor_pair_is_consistent`] would reject the pair.
489+
/// Returns [`GetForkchoiceStoreError::AnchorPairInconsistent`] if the block's header
490+
/// doesn't match the state's `latest_block_header` (comparing all fields
491+
/// except `state_root`, which is computed internally).
476492
pub fn get_forkchoice_store(
477493
backend: Arc<dyn StorageBackend>,
478494
mut anchor_state: State,
479495
anchor_block: Block,
480-
) -> Self {
481-
assert!(
482-
anchor_pair_is_consistent(&mut anchor_state, &anchor_block),
483-
"anchor block does not match anchor state"
484-
);
496+
) -> Result<Self, GetForkchoiceStoreError> {
497+
if !anchor_pair_is_consistent(&mut anchor_state, &anchor_block) {
498+
return Err(GetForkchoiceStoreError::AnchorPairInconsistent {
499+
anchor_state: Box::new(anchor_state),
500+
anchor_block: Box::new(anchor_block),
501+
});
502+
}
485503

486-
Self::init_store(backend, anchor_state, Some(anchor_block.body))
504+
Ok(Self::init_store(
505+
backend,
506+
anchor_state,
507+
Some(anchor_block.body),
508+
))
487509
}
488510

489511
/// Internal helper to initialize the store with anchor data.

0 commit comments

Comments
 (0)