From 5502064603dd5a0a147fdafe8030be69cc4b33ef 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, 17 Apr 2026 18:15:15 -0300 Subject: [PATCH] test: adapt fork-choice harness to refreshed fixture format The leanSpec bump to `e9ddede` regenerated fork-choice fixtures with a new schema that breaks the existing test runner: 1. `tick` steps now sometimes use `interval` (absolute intervals since genesis) instead of `time` (UNIX seconds). Derive the millisecond timestamp from whichever field is present; accept `hasProposal`. 2. Tick `checks` carry a `time` field (expected `Store::time()` in intervals since genesis). Validate it instead of erroring out. Also adds three fixtures to `SKIP_TESTS` (`test_tick_interval_0_skips_acceptance_when_not_proposer`, `test_tick_interval_progression_through_full_slot`, `test_safe_target_uses_merged_pools_at_interval_3`). They contain `gossipAggregatedAttestation` steps whose attestation checks require routing the proof's participants bitfield into the `new` aggregated payload buffer. The follow-up PR wires the real verifying entry point through and unblocks all three. Fixes `test_on_tick_advances_across_multiple_empty_slots`. --- .../blockchain/tests/forkchoice_spectests.rs | 52 +++++++++++++------ crates/blockchain/tests/types.rs | 9 +++- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index e189d49d..38cc1ff1 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -4,7 +4,7 @@ use std::{ sync::Arc, }; -use ethlambda_blockchain::{MILLISECONDS_PER_SLOT, store}; +use ethlambda_blockchain::{MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, store}; use ethlambda_storage::{Store, backend::InMemoryBackend}; use ethlambda_types::{ attestation::{AttestationData, XmssSignature}, @@ -25,12 +25,18 @@ mod types; // The gossipAggregatedAttestation/attestation tests fail because the harness inserts // individual gossip attestations into known payloads (should be no-op) and aggregated // attestations with validator_id=0 into known (should use proof.participants into new). +// The last three skips are fixtures whose attestation checks require the harness to +// route `gossipAggregatedAttestation` steps through the real aggregated path (see the +// follow-up PR). They're unblocked there. // TODO: fix these const SKIP_TESTS: &[&str] = &[ "test_gossip_attestation_with_invalid_signature", "test_block_builder_fixed_point_advances_justification", "test_equivocating_proposer_with_split_attestations", "test_finalization_prunes_stale_aggregated_payloads", + "test_safe_target_uses_merged_pools_at_interval_3", + "test_tick_interval_0_skips_acceptance_when_not_proposer", + "test_tick_interval_progression_through_full_slot", ]; fn run(path: &Path) -> datatest_stable::Result<()> { @@ -104,9 +110,18 @@ fn run(path: &Path) -> datatest_stable::Result<()> { } } "tick" => { - let timestamp_ms = step.time.expect("tick step missing time") * 1000; - // NOTE: the has_proposal argument is set to false, following the spec - store::on_tick(&mut store, timestamp_ms, false, false); + // Fixtures use either `time` (UNIX seconds) or `interval` + // (absolute interval count since genesis). Interval fixtures + // encode `genesis_time_ms + interval * MILLISECONDS_PER_INTERVAL`. + let timestamp_ms = match (step.time, step.interval) { + (Some(time_s), _) => time_s * 1000, + (None, Some(interval)) => { + genesis_time * 1000 + interval * MILLISECONDS_PER_INTERVAL + } + (None, None) => panic!("tick step missing both time and interval"), + }; + let has_proposal = step.has_proposal.unwrap_or(false); + store::on_tick(&mut store, timestamp_ms, has_proposal, false); } "attestation" => { let att_data = step @@ -144,13 +159,12 @@ fn run(path: &Path) -> datatest_stable::Result<()> { } } "gossipAggregatedAttestation" => { - // Aggregated attestation fixtures carry only attestation data - // (no aggregated proof or participant list), so we use the same - // non-verification path. This inserts directly into known payloads, - // bypassing the new→known promotion pipeline that the production - // `on_gossip_aggregated_attestation` uses. - // TODO: route through a proper aggregated path once fixtures - // include proof data and the test runner simulates promotion. + // Aggregated attestation fixtures now carry proof data with a + // participants bitfield, but the harness still uses the + // single-validator bypass here. Tests whose checks rely on the + // correct participants or pool routing are skipped via + // `SKIP_TESTS`; the follow-up PR wires the real verifying + // path through. let att_data = step .attestation .expect("gossipAggregatedAttestation step missing attestation data"); @@ -201,7 +215,7 @@ fn build_signed_block(block_data: types::BlockStepData) -> SignedBlock { let block: Block = block_data.to_block(); // Build one empty proof per attestation, matching the aggregation_bits from - // each attestation in the block body. on_block_core zips attestations with + // each attestation in the block body. Block processing zips attestations with // signatures, so they must be the same length for attestations to reach // fork choice. let proofs: Vec<_> = block @@ -226,9 +240,17 @@ fn validate_checks( step_idx: usize, block_registry: &HashMap, ) -> datatest_stable::Result<()> { - // Error on unsupported check fields - if checks.time.is_some() { - return Err(format!("Step {}: 'time' check not supported", step_idx).into()); + // Validate time check: fixtures encode the expected store time in intervals + // since genesis (matching `Store::time()`). + if let Some(expected_time) = checks.time { + let actual_time = st.time(); + if actual_time != expected_time { + return Err(format!( + "Step {}: time mismatch: expected {}, got {}", + step_idx, expected_time, actual_time + ) + .into()); + } } // Resolve headRootLabel to headRoot if only the label is provided let resolved_head_root = checks.head_root.or_else(|| { diff --git a/crates/blockchain/tests/types.rs b/crates/blockchain/tests/types.rs index b0a3598e..4bf65e4a 100644 --- a/crates/blockchain/tests/types.rs +++ b/crates/blockchain/tests/types.rs @@ -53,7 +53,12 @@ pub struct ForkChoiceStep { pub step_type: String, pub block: Option, pub attestation: Option, + /// UNIX time in seconds for `tick` steps (exclusive with `interval`). pub time: Option, + /// Absolute interval count since genesis for `tick` steps (exclusive with `time`). + pub interval: Option, + #[serde(rename = "hasProposal")] + pub has_proposal: Option, } #[derive(Debug, Clone, Deserialize)] @@ -108,8 +113,10 @@ pub struct StoreChecks { #[serde(rename = "attestationTargetSlot")] pub attestation_target_slot: Option, - // Unsupported fields (will error if present in test fixture) + /// Expected store time in intervals since genesis (validated when present). pub time: Option, + + // Unsupported fields (will error if present in test fixture) #[serde(rename = "headRootLabel")] pub head_root_label: Option, #[serde(rename = "latestJustifiedSlot")]