From 718768e3c3607ca379716ea2c83296db0b1a09d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:08:51 -0300 Subject: [PATCH 1/5] fix: align fork-choice finality and aggregate validation with leanSpec Three spec-test fixtures from the latest leanSpec release failed: - test_losing_fork_higher_finalized_does_not_latch: finalized was latched monotonically in on_block_core. The spec derives the finalized checkpoint from the chosen head's post-state in update_head and pins it to the head's own ancestry, so a fork switch can move finalization downward. Move that computation into update_head; keep justified monotonic (advance_to) as the spec does. - test_gossip_aggregated_attestation_empty_participants_rejected: an aggregate naming no participants slipped through the mocked-proof path. Reject empty participants (EMPTY_AGGREGATION_BITS) before bounds-check and verification, matching the spec's to_validator_indices(). - test_genesis_registry_holds_exact_validator_entries: the stf runner rejected the new post-state 'validators' registry check (deny_unknown_fields). Parse it and compare each validator's index and public keys. --- crates/blockchain/src/store.rs | 58 +++++++++++++++++-- .../state_transition/tests/stf_spectests.rs | 34 +++++++++++ .../state_transition/tests/types.rs | 4 ++ 3 files changed, 90 insertions(+), 6 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 8fd1b36c..8c394f4e 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -52,7 +52,17 @@ pub fn update_head(store: &mut Store, log_tree: bool) { metrics::observe_fork_choice_reorg_depth(depth); info!(%old_head, %new_head, depth, "Fork choice reorg detected"); } - store.update_checkpoints(ForkCheckpoints::head_only(new_head)); + + // Keep the finalized checkpoint on the head's chain (leanSpec `update_head`). + // + // Finalization is *not* a monotonic max over everything ever seen: it is + // derived from the chosen head's post-state and pinned to the block at that + // slot on the head's own ancestry. When fork choice switches to a fork that + // finalized less than a now-losing fork, finalized must follow the head + // downward rather than latch the higher value (which would let a losing + // fork's finalization persist on a chain that never contained it). + let finalized = recompute_finalized(store, new_head); + store.update_checkpoints(ForkCheckpoints::new(new_head, None, finalized)); if old_head != new_head { let old_slot = store @@ -88,6 +98,33 @@ pub fn update_head(store: &mut Store, log_tree: bool) { } } +/// Resolve the finalized checkpoint that sits on `head`'s chain. +/// +/// Mirrors leanSpec `update_head`: take the finalized slot recorded in the +/// head's post-state, then climb the head's parent chain to the block at that +/// slot. Returns `None` (caller keeps the existing checkpoint) when no block is +/// stored at that slot, which happens only for a checkpoint-sync anchor whose +/// pre-anchor history is absent. +fn recompute_finalized(store: &Store, head: H256) -> Option { + let finalized_slot = store.get_state(&head)?.latest_finalized.slot; + + let mut finalized_root = head; + while let Some(header) = store.get_block_header(&finalized_root) { + if header.slot <= finalized_slot { + break; + } + finalized_root = header.parent_root; + } + + store + .get_block_header(&finalized_root) + .filter(|header| header.slot == finalized_slot) + .map(|_| Checkpoint { + root: finalized_root, + slot: finalized_slot, + }) +} + /// Update the safe target for attestation. /// /// Safe target is an *availability* signal, not a durable-knowledge signal: @@ -406,6 +443,12 @@ fn on_gossip_aggregated_attestation_core( let num_validators = validators.len() as u64; let participant_indices: Vec = aggregated.proof.participant_indices().collect(); + // An aggregate must name at least one signer. The spec rejects empty + // participants (EMPTY_AGGREGATION_BITS) when resolving validator indices, + // before any bounds check or signature verification. + if participant_indices.is_empty() { + return Err(StoreError::EmptyAggregationBits); + } if participant_indices.iter().any(|&vid| vid >= num_validators) { return Err(StoreError::InvalidValidatorIndex); } @@ -547,14 +590,14 @@ fn on_block_core( let state_root = block.state_root; post_state.latest_block_header.state_root = state_root; - // Update justified/finalized checkpoints if they have higher slots + // Advance the justified checkpoint when the post-state names a higher one + // (leanSpec `advance_to`: monotonic by slot). Finalized is intentionally not + // latched here; it is recomputed from the head's chain in `update_head`. let justified = (post_state.latest_justified.slot > store.latest_justified().slot) .then_some(post_state.latest_justified); - let finalized = (post_state.latest_finalized.slot > store.latest_finalized().slot) - .then_some(post_state.latest_finalized); - if justified.is_some() || finalized.is_some() { - store.update_checkpoints(ForkCheckpoints::new(store.head(), justified, finalized)); + if let Some(justified) = justified { + store.update_checkpoints(ForkCheckpoints::new(store.head(), Some(justified), None)); } // Store signed block and state @@ -866,6 +909,9 @@ pub enum StoreError { #[error("Missing target state for block: {0}")] MissingTargetState(H256), + #[error("Aggregated attestation references no participants")] + EmptyAggregationBits, + #[error("Validator {validator_index} is not the proposer for slot {slot}")] NotProposer { validator_index: u64, slot: u64 }, diff --git a/crates/blockchain/state_transition/tests/stf_spectests.rs b/crates/blockchain/state_transition/tests/stf_spectests.rs index 669ea835..8a92fd4e 100644 --- a/crates/blockchain/state_transition/tests/stf_spectests.rs +++ b/crates/blockchain/state_transition/tests/stf_spectests.rs @@ -110,6 +110,7 @@ fn compare_post_states( justifications_roots, justifications_validators, validator_count, + validators, latest_justified_root_label, latest_finalized_root_label, justifications_roots_labels, @@ -275,6 +276,39 @@ fn compare_post_states( .into()); } } + if let Some(validators) = validators { + let expected = &validators.data; + let post_validators: Vec<_> = post_state.validators.iter().collect(); + if post_validators.len() != expected.len() { + return Err(format!( + "validators count mismatch: expected {}, got {}", + expected.len(), + post_validators.len() + ) + .into()); + } + for (i, expected_validator) in expected.iter().enumerate() { + let expected_domain: ethlambda_types::state::Validator = + expected_validator.clone().into(); + let actual = post_validators[i]; + if actual.index != expected_domain.index + || actual.attestation_pubkey != expected_domain.attestation_pubkey + || actual.proposal_pubkey != expected_domain.proposal_pubkey + { + return Err(format!( + "validator[{i}] mismatch: expected index={} att={} prop={}, \ + got index={} att={} prop={}", + expected_domain.index, + hex::encode(expected_domain.attestation_pubkey), + hex::encode(expected_domain.proposal_pubkey), + actual.index, + hex::encode(actual.attestation_pubkey), + hex::encode(actual.proposal_pubkey), + ) + .into()); + } + } + } if let Some(label) = latest_justified_root_label { let expected = resolve_label(label, block_registry)?; if post_state.latest_justified.root != expected { diff --git a/crates/blockchain/state_transition/tests/types.rs b/crates/blockchain/state_transition/tests/types.rs index dab07f68..e8e1d60a 100644 --- a/crates/blockchain/state_transition/tests/types.rs +++ b/crates/blockchain/state_transition/tests/types.rs @@ -82,6 +82,10 @@ pub struct PostState { #[serde(rename = "validatorCount")] pub validator_count: Option, + /// Full validator registry check: each entry's index and public keys must + /// match the post-state registry exactly. + pub validators: Option>, + // Label-based root checks: "block_N" labels resolved to hash_tree_root of the Nth block. #[serde(rename = "latestJustifiedRootLabel")] pub latest_justified_root_label: Option, From 6a58a3c392663430e13785b20ca6b697bb4b424b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:23:25 -0300 Subject: [PATCH 2/5] refactor: read finalized checkpoint directly instead of climbing The head's post-state already records latest_finalized with both root and slot, and that root is an ancestor of the head by construction. Read it directly and confirm the DB holds the block, rather than re-deriving the root by walking the parent chain. Returns None (keep existing checkpoint) when the finalized block is absent, e.g. a checkpoint-sync anchor's pre-anchor history. --- crates/blockchain/src/store.rs | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 8c394f4e..89f74fe9 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -100,29 +100,22 @@ pub fn update_head(store: &mut Store, log_tree: bool) { /// Resolve the finalized checkpoint that sits on `head`'s chain. /// -/// Mirrors leanSpec `update_head`: take the finalized slot recorded in the -/// head's post-state, then climb the head's parent chain to the block at that -/// slot. Returns `None` (caller keeps the existing checkpoint) when no block is -/// stored at that slot, which happens only for a checkpoint-sync anchor whose -/// pre-anchor history is absent. +/// Mirrors leanSpec `update_head`: the finalized checkpoint is the one recorded +/// in the head's post-state. That checkpoint's root is, by construction, an +/// ancestor of `head` (the state was produced by a transition over `head`'s +/// own history), so it always stays on the canonical chain. +/// +/// Returns `None` (caller keeps the existing checkpoint) when the finalized +/// block is not in the store. This happens only for a checkpoint-sync anchor +/// whose pre-anchor history we never downloaded, or before any block has +/// finalized (genesis post-state names the zero root). fn recompute_finalized(store: &Store, head: H256) -> Option { - let finalized_slot = store.get_state(&head)?.latest_finalized.slot; + let finalized = store.get_state(&head)?.latest_finalized; - let mut finalized_root = head; - while let Some(header) = store.get_block_header(&finalized_root) { - if header.slot <= finalized_slot { - break; - } - finalized_root = header.parent_root; - } + // Confirm the finalized block actually exists in the DB before adopting it. + store.get_block_header(&finalized.root)?; - store - .get_block_header(&finalized_root) - .filter(|header| header.slot == finalized_slot) - .map(|_| Checkpoint { - root: finalized_root, - slot: finalized_slot, - }) + Some(finalized) } /// Update the safe target for attestation. From c2e9b32f3d84df06b58dfb6e0d799c6ebba76900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:24:33 -0300 Subject: [PATCH 3/5] refactor: inline finalized-checkpoint resolution into update_head The single-use recompute_finalized helper added little over an inline expression; fold it into update_head. --- crates/blockchain/src/store.rs | 44 +++++++++++++--------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 89f74fe9..1cdabe75 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -55,13 +55,23 @@ pub fn update_head(store: &mut Store, log_tree: bool) { // Keep the finalized checkpoint on the head's chain (leanSpec `update_head`). // - // Finalization is *not* a monotonic max over everything ever seen: it is - // derived from the chosen head's post-state and pinned to the block at that - // slot on the head's own ancestry. When fork choice switches to a fork that - // finalized less than a now-losing fork, finalized must follow the head - // downward rather than latch the higher value (which would let a losing - // fork's finalization persist on a chain that never contained it). - let finalized = recompute_finalized(store, new_head); + // Finalization is *not* a monotonic max over everything ever seen: it is the + // checkpoint recorded in the chosen head's post-state. That root is, by + // construction, an ancestor of the head (the state was produced by a + // transition over the head's own history), so it always stays on the + // canonical chain. When fork choice switches to a fork that finalized less + // than a now-losing fork, finalized must follow the head downward rather + // than latch the higher value (which would let a losing fork's finalization + // persist on a chain that never contained it). + // + // Adopt it only when the finalized block is actually in the DB. It is absent + // for a checkpoint-sync anchor whose pre-anchor history we never downloaded, + // or before any block has finalized (genesis post-state names the zero + // root); in those cases keep the existing checkpoint. + let finalized = store + .get_state(&new_head) + .map(|state| state.latest_finalized) + .filter(|finalized| store.get_block_header(&finalized.root).is_some()); store.update_checkpoints(ForkCheckpoints::new(new_head, None, finalized)); if old_head != new_head { @@ -98,26 +108,6 @@ pub fn update_head(store: &mut Store, log_tree: bool) { } } -/// Resolve the finalized checkpoint that sits on `head`'s chain. -/// -/// Mirrors leanSpec `update_head`: the finalized checkpoint is the one recorded -/// in the head's post-state. That checkpoint's root is, by construction, an -/// ancestor of `head` (the state was produced by a transition over `head`'s -/// own history), so it always stays on the canonical chain. -/// -/// Returns `None` (caller keeps the existing checkpoint) when the finalized -/// block is not in the store. This happens only for a checkpoint-sync anchor -/// whose pre-anchor history we never downloaded, or before any block has -/// finalized (genesis post-state names the zero root). -fn recompute_finalized(store: &Store, head: H256) -> Option { - let finalized = store.get_state(&head)?.latest_finalized; - - // Confirm the finalized block actually exists in the DB before adopting it. - store.get_block_header(&finalized.root)?; - - Some(finalized) -} - /// Update the safe target for attestation. /// /// Safe target is an *availability* signal, not a durable-knowledge signal: From 39f78c7814d8ca50a708e3497f320a434553726f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:30:56 -0300 Subject: [PATCH 4/5] docs: simplify wall of text --- crates/blockchain/src/store.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 1cdabe75..c44092e4 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -53,21 +53,7 @@ pub fn update_head(store: &mut Store, log_tree: bool) { info!(%old_head, %new_head, depth, "Fork choice reorg detected"); } - // Keep the finalized checkpoint on the head's chain (leanSpec `update_head`). - // - // Finalization is *not* a monotonic max over everything ever seen: it is the - // checkpoint recorded in the chosen head's post-state. That root is, by - // construction, an ancestor of the head (the state was produced by a - // transition over the head's own history), so it always stays on the - // canonical chain. When fork choice switches to a fork that finalized less - // than a now-losing fork, finalized must follow the head downward rather - // than latch the higher value (which would let a losing fork's finalization - // persist on a chain that never contained it). - // - // Adopt it only when the finalized block is actually in the DB. It is absent - // for a checkpoint-sync anchor whose pre-anchor history we never downloaded, - // or before any block has finalized (genesis post-state names the zero - // root); in those cases keep the existing checkpoint. + // Override the store's latest finalized with the head state's let finalized = store .get_state(&new_head) .map(|state| state.latest_finalized) From 1b60b9ab6c6cc9e2a62da8da08369ddc5bbb043b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:31:12 -0300 Subject: [PATCH 5/5] docs: remove superfluous comment --- crates/blockchain/src/store.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index c44092e4..07c278d8 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -412,9 +412,6 @@ fn on_gossip_aggregated_attestation_core( let num_validators = validators.len() as u64; let participant_indices: Vec = aggregated.proof.participant_indices().collect(); - // An aggregate must name at least one signer. The spec rejects empty - // participants (EMPTY_AGGREGATION_BITS) when resolving validator indices, - // before any bounds check or signature verification. if participant_indices.is_empty() { return Err(StoreError::EmptyAggregationBits); }