From dcbfd2d614b15b6b6a7868e460c38b9376a2d068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 29 May 2026 15:06:52 -0300 Subject: [PATCH 1/2] fix(stf): reject blocks justifying a target whose source is below finalized leanSpec's `Slot.is_justifiable_after` asserts `self >= finalized_slot`. ethlambda's `slot_is_justifiable_after` instead returned `false` on the underflow, silently accepting blocks that justify a target while the finalization scan would inspect pre-finalization slots. The spec and other clients reject such blocks (zeam errors with InvalidJustifiableSlot), so accepting them risks a consensus split: ethlambda could follow a chain its peers reject. Surface the missing assert as `Error::SlotBeforeFinalized`, propagated through `is_valid_vote`/`try_finalize` so the block is rejected. Also guard the proposer (`entry_passes_filters`) to drop candidate attestations whose source is below the finalized slot, so we never build a block our own state transition would reject. Observed on a mixed ethlambda/zeam devnet: block slot 21 packed an attestation {source=2, target=9} that reached supermajority while finalized=8; justifying 9 then scanned slots 3..9 (slot 3 < finalized 8). Adds regression tests covering the rejection path and the boundary behavior of slot_is_justifiable_after. --- crates/blockchain/src/block_builder.rs | 19 ++- crates/blockchain/src/store.rs | 3 + crates/blockchain/state_transition/src/lib.rs | 153 +++++++++++++++--- 3 files changed, 150 insertions(+), 25 deletions(-) diff --git a/crates/blockchain/src/block_builder.rs b/crates/blockchain/src/block_builder.rs index c34aba45..4279fee4 100644 --- a/crates/blockchain/src/block_builder.rs +++ b/crates/blockchain/src/block_builder.rs @@ -302,6 +302,14 @@ fn entry_passes_filters( ) { return Err("source_not_justified"); } + // The source must not be below the finalized slot. Such a vote, once it + // justifies its target, drives the finalization scan over pre-finalization + // slots, which the STF now rejects (leanSpec `is_justifiable_after` + // assert). Exclude these candidates so we never build a block our own + // state transition would reject. + if att_data.source.slot < projected_finalized_slot { + return Err("source_before_finalized"); + } if !attestation_data_matches_chain(extended_historical_block_hashes, att_data) { return Err("chain_mismatch"); } @@ -320,6 +328,9 @@ fn entry_passes_filters( } if !is_genesis_self_vote && !slot_is_justifiable_after(att_data.target.slot, projected_finalized_slot) + // `target_already_justified` above rejects target.slot <= finalized, + // so target.slot > finalized here and this cannot error. + .expect("target slot > finalized: target <= finalized rejected above") { return Err("target_not_justifiable"); } @@ -368,8 +379,12 @@ fn score_entry( // and target.slot to still be justifiable (so source and target are // consecutive justified checkpoints in the projected post-state). let finalizes = crosses_2_3 - && (att_data.source.slot + 1..att_data.target.slot) - .all(|s| !slot_is_justifiable_after(s, projected_finalized_slot)); + && (att_data.source.slot + 1..att_data.target.slot).all(|s| { + // `entry_passes_filters` rejects source.slot < finalized, so every + // scanned slot is >= finalized and this cannot error. + !slot_is_justifiable_after(s, projected_finalized_slot) + .expect("source slot >= finalized: enforced by entry_passes_filters") + }); let tier = if is_genesis_self_vote(att_data) || !crosses_2_3 { Tier::Build diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 97c06615..4703d9a6 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -589,6 +589,9 @@ pub fn get_attestation_target_with_checkpoints( // relative to the latest finalized checkpoint. while target_header.slot > finalized_slot && !slot_is_justifiable_after(target_header.slot, finalized_slot) + // The `target_header.slot > finalized_slot` guard short-circuits + // before this call, so the slot is always after finalized. + .expect("target_header.slot > finalized_slot guaranteed by the loop guard") { target_block_root = target_header.parent_root; target_header = store diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index ae0a766e..f9eb7e7f 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -37,6 +37,8 @@ pub enum Error { }, #[error("zero hash found in justifications_roots")] ZeroHashInJustificationRoots, + #[error("slot {slot} is before the finalized slot {finalized_slot}")] + SlotBeforeFinalized { slot: u64, finalized_slot: u64 }, } /// Transition the given pre-state to the block's post-state. @@ -272,7 +274,7 @@ fn process_attestations( let source = attestation_data.source; let target = attestation_data.target; - if !is_valid_vote(state, attestation_data) { + if !is_valid_vote(state, attestation_data)? { continue; } @@ -325,7 +327,7 @@ fn process_attestations( justifications.remove(&target.root); - try_finalize(state, source, target, &mut justifications, &root_to_slot); + try_finalize(state, source, target, &mut justifications, &root_to_slot)?; } } @@ -343,7 +345,7 @@ fn process_attestations( /// rejects zero-hash source or target roots) /// 4. Target slot > source slot /// 5. Target slot is justifiable after the finalized slot -fn is_valid_vote(state: &State, data: &AttestationData) -> bool { +fn is_valid_vote(state: &State, data: &AttestationData) -> Result { let source = data.source; let target = data.target; @@ -354,7 +356,7 @@ fn is_valid_vote(state: &State, data: &AttestationData) -> bool { source.slot, ) { // TODO: why doesn't this make the block invalid? - return false; + return Ok(false); } // Ignore votes for targets that have already reached consensus @@ -363,26 +365,28 @@ fn is_valid_vote(state: &State, data: &AttestationData) -> bool { state.latest_finalized.slot, target.slot, ) { - return false; + return Ok(false); } // Ensure the vote refers to blocks that actually exist on our chain; // also rejects zero-hash source or target inline. if !attestation_data_matches_chain(&state.historical_block_hashes, data) { - return false; + return Ok(false); } // Ensure time flows forward if target.slot <= source.slot { - return false; + return Ok(false); } - // Ensure the target falls on a slot that can be justified after the finalized one. - if !slot_is_justifiable_after(target.slot, state.latest_finalized.slot) { - return false; + // Ensure the target falls on a slot that can be justified after the + // finalized one. The prior `target_already_justified` check rejects + // `target.slot <= finalized_slot`, so this call cannot error here. + if !slot_is_justifiable_after(target.slot, state.latest_finalized.slot)? { + return Ok(false); } - true + Ok(true) } /// Attempt to advance finalization from source to target. @@ -396,13 +400,20 @@ fn try_finalize( target: Checkpoint, justifications: &mut HashMap>, root_to_slot: &HashMap, -) { - // Consider whether finalization can advance. - if ((source.slot + 1)..target.slot) - .any(|slot| slot_is_justifiable_after(slot, state.latest_finalized.slot)) - { - metrics::inc_finalizations("error"); - return; +) -> Result<(), Error> { + // Consider whether finalization can advance: source finalizes only when no + // slot strictly between source and target is itself justifiable. + // + // `slot_is_justifiable_after` errors when a scanned slot is before the + // finalized slot (the leanSpec assert). That happens precisely when + // `source.slot < latest_finalized.slot`, which is an invalid state: a block + // justified a target while pointing its vote at a source below finalization. + // Propagating the error rejects the block, matching the spec. + for slot in (source.slot + 1)..target.slot { + if slot_is_justifiable_after(slot, state.latest_finalized.slot)? { + metrics::inc_finalizations("error"); + return Ok(()); + } } let old_finalized_slot = state.latest_finalized.slot; @@ -438,6 +449,8 @@ fn try_finalize( false } }); + + Ok(()) } /// Convert the in-memory vote HashMap back into SSZ-compatible state fields. @@ -513,15 +526,19 @@ pub fn attestation_data_matches_chain( /// scenarios, validators may vote for many different slots, making none of them /// reach the supermajority threshold. By having unjustifiable slots, we can /// funnel votes towards only some slots, increasing finalization chances. -pub fn slot_is_justifiable_after(slot: u64, finalized_slot: u64) -> bool { +pub fn slot_is_justifiable_after(slot: u64, finalized_slot: u64) -> Result { + // Justifiable slot checks before the finalized slot result in an assertion error + // according to the spec. let Some(delta) = slot.checked_sub(finalized_slot) else { - // Candidate slot must not be before finalized slot - return false; + return Err(Error::SlotBeforeFinalized { + slot, + finalized_slot, + }); }; // Rule 1: The first 5 slots after finalization are always justifiable. // // Examples: delta = 0, 1, 2, 3, 4, 5 - delta <= 5 + Ok(delta <= 5 // Rule 2: Slots at perfect square distances are justifiable. // // Examples: delta = 1, 4, 9, 16, 25, 36, 49, 64, ... @@ -536,7 +553,7 @@ pub fn slot_is_justifiable_after(slot: u64, finalized_slot: u64) -> bool { || delta .checked_mul(4) .and_then(|v| v.checked_add(1)) - .is_some_and(|val| val.isqrt().pow(2) == val && val % 2 == 1) + .is_some_and(|val| val.isqrt().pow(2) == val && val % 2 == 1)) } #[cfg(test)] @@ -683,4 +700,94 @@ mod tests { ); assert_eq!(state.latest_justified.root, r9); } + + /// A block must be rejected when an attestation justifies a target whose + /// source lies below the finalized slot, which would drive the + /// finalization scan over pre-finalization slots. + /// + /// This is the leanSpec `Slot.is_justifiable_after` invariant + /// (`assert self >= finalized_slot`). ethlambda previously swallowed the + /// underflow (returning `false`) and silently accepted such blocks, while + /// the spec and other clients (zeam) reject them — a consensus-split risk. + /// + /// Observed on a mixed ethlambda/zeam devnet at block slot 21: a packed + /// attestation `{source.slot=2, target.slot=9}` reached supermajority while + /// `finalized=8`; justifying target 9 then scanned slots 3..9, hitting + /// slot 3 < finalized 8. + #[test] + fn block_rejected_when_source_below_finalized() { + const NUM_VALIDATORS: usize = 4; + let r2 = H256([2u8; 32]); + let r8 = H256([8u8; 32]); + let r9 = H256([9u8; 32]); + + let mut hashes: Vec = vec![H256::ZERO; 12]; + hashes[2] = r2; + hashes[8] = r8; + hashes[9] = r9; + + let validators = make_validators(NUM_VALIDATORS); + let mut justified_slots = JustifiedSlots::new(); + // Window is relative to finalized=8; track slots 9..=11. Slot 9 stays + // unjustified so the (2, 9) vote is not skipped as already-justified. + justified_slots_ops::extend_to_slot(&mut justified_slots, 8, 11); + + let mut state = State { + config: ChainConfig { genesis_time: 0 }, + slot: 12, + latest_block_header: BlockHeader { + slot: 11, + proposer_index: 0, + parent_root: H256::ZERO, + state_root: H256::ZERO, + body_root: BlockBody::default().hash_tree_root(), + }, + latest_justified: Checkpoint { slot: 8, root: r8 }, + latest_finalized: Checkpoint { slot: 8, root: r8 }, + historical_block_hashes: SszList::try_from(hashes).unwrap(), + justified_slots, + validators: SszList::try_from(validators).unwrap(), + justifications_roots: Default::default(), + justifications_validators: JustificationValidators::new(), + }; + + // Supermajority (3 of 4) vote with source below finalized: source=(2, r2), + // target=(9, r9). Target 9 is justifiable after finalized 8 (Δ=1) and not + // yet justified, so it crosses the threshold and justifies. + let atts: Vec = vec![make_attestation( + 10, + (2, r2), + (9, r9), + (9, r9), + &[0, 1, 2], + NUM_VALIDATORS, + )]; + let atts: AggregatedAttestations = atts.try_into().unwrap(); + + let result = process_attestations(&mut state, &atts); + assert!( + matches!(result, Err(Error::SlotBeforeFinalized { .. })), + "expected SlotBeforeFinalized, got {result:?}" + ); + } + + #[test] + fn slot_is_justifiable_after_errors_below_finalized() { + // slot < finalized: the missing assert -> error. + assert!(matches!( + slot_is_justifiable_after(3, 8), + Err(Error::SlotBeforeFinalized { + slot: 3, + finalized_slot: 8 + }) + )); + // slot == finalized: Δ=0, justifiable. + assert!(slot_is_justifiable_after(8, 8).unwrap()); + // Δ within window / square / pronic are justifiable. + assert!(slot_is_justifiable_after(13, 8).unwrap()); // Δ=5 (window) + assert!(slot_is_justifiable_after(17, 8).unwrap()); // Δ=9 = 3^2 + assert!(slot_is_justifiable_after(14, 8).unwrap()); // Δ=6 = 2*3 pronic + // Δ=7 is none of window/square/pronic. + assert!(!slot_is_justifiable_after(15, 8).unwrap()); + } } From e849aa8fd8399ee8b7830705abdd9a37263cacb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 29 May 2026 15:23:49 -0300 Subject: [PATCH 2/2] docs: improve comments --- crates/blockchain/src/block_builder.rs | 2 +- crates/blockchain/state_transition/src/lib.rs | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/blockchain/src/block_builder.rs b/crates/blockchain/src/block_builder.rs index 4279fee4..047e38cc 100644 --- a/crates/blockchain/src/block_builder.rs +++ b/crates/blockchain/src/block_builder.rs @@ -304,7 +304,7 @@ fn entry_passes_filters( } // The source must not be below the finalized slot. Such a vote, once it // justifies its target, drives the finalization scan over pre-finalization - // slots, which the STF now rejects (leanSpec `is_justifiable_after` + // slots, which the STF rejects (leanSpec `is_justifiable_after` // assert). Exclude these candidates so we never build a block our own // state transition would reject. if att_data.source.slot < projected_finalized_slot { diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index f9eb7e7f..6618dc72 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -404,11 +404,8 @@ fn try_finalize( // Consider whether finalization can advance: source finalizes only when no // slot strictly between source and target is itself justifiable. // - // `slot_is_justifiable_after` errors when a scanned slot is before the - // finalized slot (the leanSpec assert). That happens precisely when - // `source.slot < latest_finalized.slot`, which is an invalid state: a block - // justified a target while pointing its vote at a source below finalization. - // Propagating the error rejects the block, matching the spec. + // NOTE: `slot_is_justifiable_after` errors when a scanned slot is before the + // finalized slot (the leanSpec assert). for slot in (source.slot + 1)..target.slot { if slot_is_justifiable_after(slot, state.latest_finalized.slot)? { metrics::inc_finalizations("error");