From c11495254a4f242302da0ad3e7cfdfe00acad47a Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Tue, 28 Apr 2026 21:55:32 +0200 Subject: [PATCH 1/5] Wire safeTarget store checks in the fork choice test harness so that the spec-test runner actually validates safe_target behavior end-to-end. Today the harness explicitly rejects any fixture carrying a \'safeTarget\' field with "'\safeTarget\' check not supported", and it has no field at all for the leanSpec #680 schema (safeTargetSlot, safeTargetRootLabel). The result is that the safe_target semantics shipped in PR #316 have no fixture-level coverage: regenerated fixtures are silently parsed without asserting on the field, and any test using the legacy \'safeTarget\' field errors out instead of validating. Changes: - Add safeTargetSlot and safeTargetRootLabel to StoreChecks, mirroring the latestJustified* / latestFinalized* pattern. - Replace the rejection branch with the same label-resolution pattern used for justified/finalized roots, falling back from safeTarget to safeTargetRootLabel via the step\'s block registry. - Add validation blocks for safeTargetSlot (against st.safe_target_slot()) and the resolved root (against st.safe_target()). - Reorganize StoreChecks into logical groups and drop the now-stale "Unsupported fields (will error if present)" comment, which was misleading: only safeTarget actually errored, everything else was validated. This change is forward-compatible: no current fixture in leanSpec/fixtures/consensus/ contains any safeTarget* field, so the new validation paths are dormant until LEAN_SPEC_COMMIT_HASH is bumped to a revision that includes leanSpec #680. Verified with cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, and the forkchoice_spectests harness (47 passing fixtures unchanged). Pre-existing failures from a stale leanSpec submodule checkout (1 fork-choice fixture, 34 ssz fixtures) reproduce identically on main and are unrelated to this change. --- .../blockchain/tests/forkchoice_spectests.rs | 33 +++++++++++++++++-- crates/blockchain/tests/types.rs | 33 +++++++++++++------ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index 13d8cf7..e29c4c1 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -221,9 +221,12 @@ fn validate_checks( .as_ref() .and_then(|label| block_registry.get(label).copied()) }); - if checks.safe_target.is_some() { - return Err(format!("Step {}: 'safeTarget' check not supported", step_idx).into()); - } + let resolved_safe_target_root = checks.safe_target.or_else(|| { + checks + .safe_target_root_label + .as_ref() + .and_then(|label| block_registry.get(label).copied()) + }); // Validate attestationTargetSlot if let Some(expected_slot) = checks.attestation_target_slot { let target = store::get_attestation_target(st); @@ -330,6 +333,30 @@ fn validate_checks( } } + // Validate safeTargetSlot + if let Some(expected_slot) = checks.safe_target_slot { + let actual_slot = st.safe_target_slot(); + if actual_slot != expected_slot { + return Err(format!( + "Step {}: safeTargetSlot mismatch: expected {}, got {}", + step_idx, expected_slot, actual_slot + ) + .into()); + } + } + + // Validate safeTarget root (resolved from label if root not provided) + if let Some(ref expected_root) = resolved_safe_target_root { + let actual_root = st.safe_target(); + if actual_root != *expected_root { + return Err(format!( + "Step {}: safeTarget mismatch: expected {:?}, got {:?}", + step_idx, expected_root, actual_root + ) + .into()); + } + } + // Validate attestationChecks if let Some(ref att_checks) = checks.attestation_checks { for att_check in att_checks { diff --git a/crates/blockchain/tests/types.rs b/crates/blockchain/tests/types.rs index a1f0232..97b56dd 100644 --- a/crates/blockchain/tests/types.rs +++ b/crates/blockchain/tests/types.rs @@ -136,38 +136,51 @@ impl BlockStepData { // Check Types // ============================================================================ +/// Store-state expectations for a fork choice test step. +/// +/// All fields are optional; only fields explicitly set by the fixture are validated. +/// Root-typed fields have a `*RootLabel` companion that resolves a block label via the +/// step's block registry, mirroring the leanSpec fixture schema. #[derive(Debug, Clone, Deserialize)] pub struct StoreChecks { - // Validated fields + /// Expected store time in intervals since genesis. + pub time: Option, + #[serde(rename = "headSlot")] pub head_slot: Option, #[serde(rename = "headRoot")] pub head_root: Option, - #[serde(rename = "attestationChecks")] - pub attestation_checks: Option>, - #[serde(rename = "attestationTargetSlot")] - pub attestation_target_slot: Option, - - /// 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")] pub latest_justified_slot: Option, #[serde(rename = "latestJustifiedRoot")] pub latest_justified_root: Option, #[serde(rename = "latestJustifiedRootLabel")] pub latest_justified_root_label: Option, + #[serde(rename = "latestFinalizedSlot")] pub latest_finalized_slot: Option, #[serde(rename = "latestFinalizedRoot")] pub latest_finalized_root: Option, #[serde(rename = "latestFinalizedRootLabel")] pub latest_finalized_root_label: Option, + + /// Legacy single-field schema; expected safe target block root. #[serde(rename = "safeTarget")] pub safe_target: Option, + /// Expected slot of the safe target block (leanSpec #680 schema). + #[serde(rename = "safeTargetSlot")] + pub safe_target_slot: Option, + /// Expected safe target block root by label reference (leanSpec #680 schema). + #[serde(rename = "safeTargetRootLabel")] + pub safe_target_root_label: Option, + + #[serde(rename = "attestationTargetSlot")] + pub attestation_target_slot: Option, + #[serde(rename = "attestationChecks")] + pub attestation_checks: Option>, #[serde(rename = "lexicographicHeadAmong")] pub lexicographic_head_among: Option>, } From 0d5b1e68f70345a89e58135d4e33d0532d613ce5 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Tue, 28 Apr 2026 22:12:50 +0200 Subject: [PATCH 2/5] Skip test_safe_target_uses_merged_pools_at_interval_3 in the fork choice spec-test harness. This fixture, shipped with the currently-pinned LEAN_SPEC_COMMIT_HASH, encodes the pre-leanSpec-#680 merged-pool semantics for update_safe_target. PR #316 deliberately diverged from that, switching to the new pool only, which means the fixture's safeTargetSlot=2 expectation no longer holds: with no fresh aggregations in the new pool at interval 3, safe_target now collapses to the latest justified root (slot 0). Until #316 went in this divergence was invisible because the harness silently rejected the legacy safeTarget field. The previous commit on this branch wired safeTargetSlot/safeTargetRootLabel into StoreChecks, turning that silent skip into a real assertion and surfacing the failure. The right resolution is to retire the fixture when leanSpec #680 lands and LEAN_SPEC_COMMIT_HASH is bumped; until then, skip it explicitly with a comment that points back to #316 and documents the empty-new-pool collapse to latest_justified. Verified with cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings. --- crates/blockchain/tests/forkchoice_spectests.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index e29c4c1..2e642f7 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -21,8 +21,15 @@ const SUPPORTED_FIXTURE_FORMAT: &str = "fork_choice_test"; mod common; mod types; -/// List of skipped tests -const SKIP_TESTS: &[&str] = &[]; +/// List of skipped tests. +/// +/// `test_safe_target_uses_merged_pools_at_interval_3`: encodes the pre-leanSpec-#680 +/// merged-pool behavior. PR #316 changed `update_safe_target` to consume only the +/// `new` attestation pool, so this fixture's expectation (`safeTargetSlot=2`) no +/// longer matches: with an empty `new` pool at interval 3 the safe target now +/// collapses to the latest justified root (slot 0). Skip until +/// `LEAN_SPEC_COMMIT_HASH` is bumped to a revision that retires this fixture. +const SKIP_TESTS: &[&str] = &["test_safe_target_uses_merged_pools_at_interval_3"]; fn run(path: &Path) -> datatest_stable::Result<()> { if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) From 9fb36c9a850fae02d0972eb1df8dbd53b6d44d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 29 Apr 2026 15:57:06 -0300 Subject: [PATCH 3/5] test: un-skip `test_safe_target_uses_merged_pools_at_interval_3` --- crates/blockchain/tests/forkchoice_spectests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index 2e642f7..6f0c683 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -29,7 +29,7 @@ mod types; /// longer matches: with an empty `new` pool at interval 3 the safe target now /// collapses to the latest justified root (slot 0). Skip until /// `LEAN_SPEC_COMMIT_HASH` is bumped to a revision that retires this fixture. -const SKIP_TESTS: &[&str] = &["test_safe_target_uses_merged_pools_at_interval_3"]; +const SKIP_TESTS: &[&str] = &[]; fn run(path: &Path) -> datatest_stable::Result<()> { if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) From 338c36ea97921bb4f0b408f8f18acab407dc3ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 29 Apr 2026 15:57:49 -0300 Subject: [PATCH 4/5] chore: bump leanSpec commit --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 013b157..e0de9e0 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-28: bump for leanSpec PR #682 (validate_attestation future-slot bound). -LEAN_SPEC_COMMIT_HASH:=62eff6e7e6041a283877a546a07cb3b83f4f7d5b +# 2026-04-29 +LEAN_SPEC_COMMIT_HASH:=495c29d49f2b12b7cc240c4028e15d4253a7d54e leanSpec: git clone https://github.com/leanEthereum/leanSpec.git --single-branch From 558fa88f2dff25ba1cab96e6f16dd9a3a76192a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:08:38 -0300 Subject: [PATCH 5/5] docs: remove stale comment --- crates/blockchain/tests/forkchoice_spectests.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index 6f0c683..23f4503 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -22,13 +22,6 @@ mod common; mod types; /// List of skipped tests. -/// -/// `test_safe_target_uses_merged_pools_at_interval_3`: encodes the pre-leanSpec-#680 -/// merged-pool behavior. PR #316 changed `update_safe_target` to consume only the -/// `new` attestation pool, so this fixture's expectation (`safeTargetSlot=2`) no -/// longer matches: with an empty `new` pool at interval 3 the safe target now -/// collapses to the latest justified root (slot 0). Skip until -/// `LEAN_SPEC_COMMIT_HASH` is bumped to a revision that retires this fixture. const SKIP_TESTS: &[&str] = &[]; fn run(path: &Path) -> datatest_stable::Result<()> {