From 59b60ae606c35851a9465cc29d2d18aa525d8029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:19:50 -0300 Subject: [PATCH 1/7] fix: tighten attestation future-slot bound (leanSpec #682) The previous time check accepted votes up to a full slot ahead of the local clock. With 4s slots that is ~3.2s of free pre-positioning room, enough for an adversary to publish next-slot aggregates before any honest validator could produce them, and the next proposer would happily include them. The bound is now expressed in interval units against the store's local time and gated by a new GOSSIP_DISPARITY_INTERVALS = 1 constant, the lean analogue of mainnet's MAXIMUM_GOSSIP_CLOCK_DISPARITY (~800 ms). --- crates/blockchain/src/lib.rs | 8 ++++++++ crates/blockchain/src/store.rs | 23 +++++++++++++---------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index 77c9882..5e47f2e 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -46,6 +46,14 @@ pub const MILLISECONDS_PER_SLOT: u64 = MILLISECONDS_PER_INTERVAL * INTERVALS_PER /// /// See: leanSpec commit 0c9528a (PR #536). pub const MAX_ATTESTATIONS_DATA: usize = 16; +/// Future-slot tolerance for gossip attestations, expressed in intervals. +/// +/// Bounds the clock skew the time check is willing to absorb when admitting a +/// vote whose slot has not yet started locally. One interval is roughly 800 ms, +/// the lean analogue of mainnet's `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. +/// +/// See: leanSpec PR #682. +pub const GOSSIP_DISPARITY_INTERVALS: u64 = 1; impl BlockChain { pub fn spawn( diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index ac37527..e7d9122 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -20,8 +20,8 @@ use ethlambda_types::{ use tracing::{info, trace, warn}; use crate::{ - INTERVALS_PER_SLOT, MAX_ATTESTATIONS_DATA, MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, - metrics, + GOSSIP_DISPARITY_INTERVALS, INTERVALS_PER_SLOT, MAX_ATTESTATIONS_DATA, + MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, metrics, }; const JUSTIFICATION_LOOKBACK_SLOTS: u64 = 3; @@ -128,7 +128,7 @@ fn update_safe_target(store: &mut Store) { /// 2. A vote cannot span backwards in time (source > target). /// 3. The head must be at least as recent as source and target. /// 4. Checkpoint slots must match the actual block slots. -/// 5. A vote cannot be for a future slot. +/// 5. The vote's slot must have started locally (a small disparity margin is allowed). fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<(), StoreError> { let _timing = metrics::time_attestation_validation(); @@ -175,13 +175,16 @@ fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<() }); } - // Time Check - Validate attestation is not too far in the future. - // We allow a small margin for clock disparity (1 slot), but no further. - let current_slot = store.time() / INTERVALS_PER_SLOT; - if data.slot > current_slot + 1 { + // Time Check - Honest validators emit votes only after their slot has begun. + // Allow a small disparity margin for clock skew between peers. + // + // The bound is in intervals, not slots: a whole-slot margin would let an + // adversary pre-publish next-slot aggregates ahead of any honest validator. + let attestation_start_interval = data.slot * INTERVALS_PER_SLOT; + if attestation_start_interval > store.time() + GOSSIP_DISPARITY_INTERVALS { return Err(StoreError::AttestationTooFarInFuture { attestation_slot: data.slot, - current_slot, + store_time: store.time(), }); } @@ -795,11 +798,11 @@ pub enum StoreError { }, #[error( - "Attestation slot {attestation_slot} is too far in future (current slot: {current_slot})" + "Attestation slot {attestation_slot} is too far in future (store time: {store_time} intervals)" )] AttestationTooFarInFuture { attestation_slot: u64, - current_slot: u64, + store_time: u64, }, #[error( From da0c3934205e780598394625ce1f0c8df4c44409 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Tue, 28 Apr 2026 14:22:12 +0200 Subject: [PATCH 2/7] Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- crates/blockchain/src/store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index e7d9122..80747ee 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -180,7 +180,7 @@ fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<() // // The bound is in intervals, not slots: a whole-slot margin would let an // adversary pre-publish next-slot aggregates ahead of any honest validator. - let attestation_start_interval = data.slot * INTERVALS_PER_SLOT; + let attestation_start_interval = data.slot.saturating_mul(INTERVALS_PER_SLOT); if attestation_start_interval > store.time() + GOSSIP_DISPARITY_INTERVALS { return Err(StoreError::AttestationTooFarInFuture { attestation_slot: data.slot, From a34f8575d1f04c5540e39d0f7e44d17f6102cf73 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, 28 Apr 2026 11:57:50 -0300 Subject: [PATCH 3/7] build: bump LEAN_SPEC_COMMIT_HASH to leanSpec main (#682 merged) --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c9c1c17..013b157 100644 --- a/Makefile +++ b/Makefile @@ -24,8 +24,8 @@ docker-build: ## 🐳 Build the Docker image -t ghcr.io/lambdaclass/ethlambda:$(DOCKER_TAG) . @echo -# 2026-04-20 -LEAN_SPEC_COMMIT_HASH:=bc17f7ae8d16caec276f4d304e04fd3c65e6de3c +# 2026-04-28: bump for leanSpec PR #682 (validate_attestation future-slot bound). +LEAN_SPEC_COMMIT_HASH:=62eff6e7e6041a283877a546a07cb3b83f4f7d5b leanSpec: git clone https://github.com/leanEthereum/leanSpec.git --single-branch From c20aebf76d3fe2d5c4529635efb4ef0d78bdd3bf 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, 28 Apr 2026 13:21:44 -0300 Subject: [PATCH 4/7] test(stf): honor expectException for fixtures with empty blocks The leanSpec test framework can finish a negative state-transition test without producing any block, when the spec filler raises during pre-block construction (e.g. `state.process_slots(spec.slot)` aborting before a block is ever appended to `_filled_blocks`). The resulting fixture carries `blocks: []` and only `expectException` records the intended outcome. Until now ethlambda's runner read failure intent solely from `post == None`, so an empty `blocks` array combined with `post == None` was reported as "expected failure but got success". Parse the new `expectException` field and treat that case as a successful negative test; the spec framework already validated the assertion at fixture-generation time, and ethlambda has no block to replay. Surfaces with the bumped pin (leanSpec PR #682 merge) by test_process_slots_target_equal_to_state_slot_rejected. --- crates/blockchain/state_transition/tests/stf_spectests.rs | 7 +++++++ crates/blockchain/state_transition/tests/types.rs | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/crates/blockchain/state_transition/tests/stf_spectests.rs b/crates/blockchain/state_transition/tests/stf_spectests.rs index 5c25c8a..b66c301 100644 --- a/crates/blockchain/state_transition/tests/stf_spectests.rs +++ b/crates/blockchain/state_transition/tests/stf_spectests.rs @@ -32,6 +32,7 @@ fn run(path: &Path) -> datatest_stable::Result<()> { // Build a block registry mapping "block_N" labels to hash tree roots. // Labels are 1-indexed: "block_1" is the first block in the array. let mut block_registry: HashMap = HashMap::new(); + let blocks_empty = test.blocks.is_empty(); for (i, block) in test.blocks.into_iter().enumerate() { let block: Block = block.into(); let label = format!("block_{}", i + 1); @@ -46,6 +47,12 @@ fn run(path: &Path) -> datatest_stable::Result<()> { (Ok(_), Some(expected_post)) => { compare_post_states(&post_state, &expected_post, &block_registry)?; } + (Ok(_), None) if blocks_empty && test.expect_exception.is_some() => { + // Negative test where the spec filler raised during pre-block + // construction (so `blocks: []` in the fixture). The intended + // failure is recorded in `expectException`; ethlambda has no + // block to replay, so the spec framework's verdict stands. + } (Ok(_), None) => { return Err( format!("Test '{name}' failed: expected failure but got success").into(), diff --git a/crates/blockchain/state_transition/tests/types.rs b/crates/blockchain/state_transition/tests/types.rs index dab07f6..515e6f9 100644 --- a/crates/blockchain/state_transition/tests/types.rs +++ b/crates/blockchain/state_transition/tests/types.rs @@ -32,6 +32,14 @@ pub struct StateTransitionTest { pub pre: TestState, pub blocks: Vec, pub post: Option, + /// Exception class the spec test framework expects (e.g. "AssertionError"). + /// + /// Some negative tests fail during pre-block construction in the spec + /// filler (e.g. `state.process_slots(spec.slot)` raising before a block + /// is built). The fixture then carries `blocks: []` and only this field + /// records the intended outcome. + #[serde(rename = "expectException")] + pub expect_exception: Option, #[serde(rename = "_info")] #[allow(dead_code)] pub info: TestInfo, From d6b9aeaa7f3a37464f5b9bd925a5ffb556cebefa 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, 28 Apr 2026 13:22:48 -0300 Subject: [PATCH 5/7] ci: always cache fixtures and production keys --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bede38..41e9235 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,10 +54,11 @@ jobs: - name: Cache test fixtures id: cache-fixtures - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: leanSpec/fixtures key: leanspec-fixtures-${{ steps.lean-spec.outputs.commit }} + save-always: true # All fixture generation steps are skipped when the cache hits - name: Checkout leanSpec at pinned commit @@ -93,10 +94,11 @@ jobs: - name: Cache production keys if: steps.cache-fixtures.outputs.cache-hit != 'true' id: cache-prod-keys - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: leanSpec/packages/testing/src/consensus_testing/test_keys/prod_scheme key: prod-keys-${{ steps.prod-keys-url.outputs.hash }} + save-always: true - name: Download production keys if: steps.cache-fixtures.outputs.cache-hit != 'true' && steps.cache-prod-keys.outputs.cache-hit != 'true' From df2703e7b86ab163b8494ec699b5f3229cc85c8a 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, 28 Apr 2026 13:28:16 -0300 Subject: [PATCH 6/7] test(stf): simplify empty-blocks fixture handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the `expectException` field plumbing and just skip any fixture whose `blocks` array is empty. With no block for ethlambda to replay, there is nothing the runner can verify regardless of how the fixture records its expected outcome — the spec filler already validated the assertion at fixture-generation time. Replaces the c20aebf approach. --- .../state_transition/tests/stf_spectests.rs | 15 ++++++++------- crates/blockchain/state_transition/tests/types.rs | 8 -------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/crates/blockchain/state_transition/tests/stf_spectests.rs b/crates/blockchain/state_transition/tests/stf_spectests.rs index b66c301..669ea83 100644 --- a/crates/blockchain/state_transition/tests/stf_spectests.rs +++ b/crates/blockchain/state_transition/tests/stf_spectests.rs @@ -26,13 +26,20 @@ fn run(path: &Path) -> datatest_stable::Result<()> { } println!("Running test: {}", name); + // Fixtures with no blocks come from spec filler runs that raised + // before any block was constructed (e.g. negative tests where + // `state.process_slots(spec.slot)` aborts pre-build). With nothing + // for ethlambda to replay, the spec framework's verdict stands. + if test.blocks.is_empty() { + continue; + } + let mut pre_state: State = test.pre.into(); let mut result = Ok(()); // Build a block registry mapping "block_N" labels to hash tree roots. // Labels are 1-indexed: "block_1" is the first block in the array. let mut block_registry: HashMap = HashMap::new(); - let blocks_empty = test.blocks.is_empty(); for (i, block) in test.blocks.into_iter().enumerate() { let block: Block = block.into(); let label = format!("block_{}", i + 1); @@ -47,12 +54,6 @@ fn run(path: &Path) -> datatest_stable::Result<()> { (Ok(_), Some(expected_post)) => { compare_post_states(&post_state, &expected_post, &block_registry)?; } - (Ok(_), None) if blocks_empty && test.expect_exception.is_some() => { - // Negative test where the spec filler raised during pre-block - // construction (so `blocks: []` in the fixture). The intended - // failure is recorded in `expectException`; ethlambda has no - // block to replay, so the spec framework's verdict stands. - } (Ok(_), None) => { return Err( format!("Test '{name}' failed: expected failure but got success").into(), diff --git a/crates/blockchain/state_transition/tests/types.rs b/crates/blockchain/state_transition/tests/types.rs index 515e6f9..dab07f6 100644 --- a/crates/blockchain/state_transition/tests/types.rs +++ b/crates/blockchain/state_transition/tests/types.rs @@ -32,14 +32,6 @@ pub struct StateTransitionTest { pub pre: TestState, pub blocks: Vec, pub post: Option, - /// Exception class the spec test framework expects (e.g. "AssertionError"). - /// - /// Some negative tests fail during pre-block construction in the spec - /// filler (e.g. `state.process_slots(spec.slot)` raising before a block - /// is built). The fixture then carries `blocks: []` and only this field - /// records the intended outcome. - #[serde(rename = "expectException")] - pub expect_exception: Option, #[serde(rename = "_info")] #[allow(dead_code)] pub info: TestInfo, From 1de54785eae32a5b2510b275e1f73e393d9071c8 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, 28 Apr 2026 17:44:00 -0300 Subject: [PATCH 7/7] ci: replace deprecated save-always with split restore/save steps actions/cache@v5 deprecated the save-always input. Follow the pattern documented at https://github.com/actions/cache/tree/main/save#always-save-cache: - Use actions/cache/restore@v5 to fetch the cache early. - Use actions/cache/save@v5 with `if: always() && .cache-hit ...` after the data-producing step, so the cache is uploaded even when a later step (e.g. test run) fails. Production-keys restore is itself gated on the fixtures cache miss, so its save uses `cache-hit == 'false'` to match exactly the case where the restore ran and missed (avoids a second cross-step gate clause). --- .github/workflows/ci.yml | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41e9235..2d23a38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,13 +52,12 @@ jobs: id: lean-spec run: echo "commit=$(sed -n 's/^LEAN_SPEC_COMMIT_HASH:= *//p' Makefile)" >> $GITHUB_OUTPUT - - name: Cache test fixtures + - name: Restore test fixtures cache id: cache-fixtures - uses: actions/cache@v5 + uses: actions/cache/restore@v5 with: path: leanSpec/fixtures key: leanspec-fixtures-${{ steps.lean-spec.outputs.commit }} - save-always: true # All fixture generation steps are skipped when the cache hits - name: Checkout leanSpec at pinned commit @@ -91,25 +90,47 @@ jobs: HASH=$(echo -n "$URL" | sha256sum | awk '{print $1}') echo "hash=$HASH" >> $GITHUB_OUTPUT - - name: Cache production keys + - name: Restore production keys cache if: steps.cache-fixtures.outputs.cache-hit != 'true' id: cache-prod-keys - uses: actions/cache@v5 + uses: actions/cache/restore@v5 with: path: leanSpec/packages/testing/src/consensus_testing/test_keys/prod_scheme key: prod-keys-${{ steps.prod-keys-url.outputs.hash }} - save-always: true - name: Download production keys if: steps.cache-fixtures.outputs.cache-hit != 'true' && steps.cache-prod-keys.outputs.cache-hit != 'true' working-directory: leanSpec run: uv run python -m consensus_testing.keys --download --scheme prod + # Save production keys even if a later step fails, so a re-run does + # not have to re-download. See: https://github.com/actions/cache/tree/main/save#always-save-cache + # + # `cache-hit == 'false'` (rather than `!= 'true'`) only matches when + # the restore step actually ran and missed: when fixtures were already + # cached, the restore was skipped and `cache-hit` is empty, so save + # is skipped too. + - name: Save production keys cache + if: always() && steps.cache-prod-keys.outputs.cache-hit == 'false' + uses: actions/cache/save@v5 + with: + path: leanSpec/packages/testing/src/consensus_testing/test_keys/prod_scheme + key: ${{ steps.cache-prod-keys.outputs.cache-primary-key }} + - name: Generate test fixtures if: steps.cache-fixtures.outputs.cache-hit != 'true' working-directory: leanSpec run: uv run fill --fork=Devnet --scheme prod -o fixtures -n 2 + # Save fixtures even if a later step fails, so a re-run does not + # have to regenerate them. See: https://github.com/actions/cache/tree/main/save#always-save-cache + - name: Save test fixtures cache + if: always() && steps.cache-fixtures.outputs.cache-hit != 'true' + uses: actions/cache/save@v5 + with: + path: leanSpec/fixtures + key: ${{ steps.cache-fixtures.outputs.cache-primary-key }} + # Ensure make sees fixtures as up-to-date (its timestamp must be # newer than leanSpec/, which intermediate steps may have modified). - name: Mark fixtures as up-to-date