fix(l1): statetest CLI follow-ups (optional _info, Osaka, BLOCKHASH convention)

[ElFantasma]

Motivation

Follow-ups to #6663 (statetest CLI for goevmlab fuzzing), addressing three issues raised in Slack feedback after the PR landed:

1. _info is required in the statetest fixture deserializer, but goevmlab-generated fixtures don't necessarily include it.
2. default_forks (the allow-list of forks the runner will execute) is missing Osaka, so any fixture with an Osaka post-state was silently skipped or surfaced as an error.
3. BLOCKHASH(0) returns ethrex's synthesized genesis hash rather than the EF state-test convention keccak256(decimal_string(n)) that geth uses. This trips the goevmlab fuzzer on the very first block-hash lookup.

Description

Two commits, reviewable independently:

Commit 1 — fix(l1): make statetest _info optional and add Osaka to runnable forks

• Test._info becomes Option<Info>; build_test no longer errors when _info is absent.
• Both readers (runner::run_tests for EIP filtering and report::generate_report for the description/reference lines) are updated to handle None with sensible defaults — empty reference_spec, "No description or comment" for the report.
• DEFAULT_FORKS gets "Osaka" inserted between Prague and Amsterdam, matching the fork ordering used elsewhere.
• Two unit tests pin the new contract: fixture-without-_info parses, fixture-with-_info still parses.

Commit 2 — fix(l1): apply EF state-test BLOCKHASH convention in statetest harness

The bug: at block N=1, the test contract 0x60004060005500 (PUSH1 0 / BLOCKHASH / PUSH1 0 / SSTORE / STOP) calls BLOCKHASH(0). ethrex returned the genesis hash it derived from the test's pre-state (0x86a0…); geth (and the EF spec) returns keccak256("0") = 0x0448…116d. The state root differs from the very first SSTORE, which kills any differential fuzzing.

Fix: in load_initial_state (shared by the statetest CLI and runner.rs's EF state-test runner), wrap the levm Database in a thin StatetestDatabase shim that delegates everything except get_block_hash, which returns keccak256(decimal_string(n)). Matches geth's vmTestBlockHash in tests/state_test_util.go. Three unit tests pin the contract (n=0, n=1, n=10 to distinguish decimal vs hex encoding).

block_runner.rs has a separate pre-existing problem: its phase-3 real-block re-execution path uses the actual genesis hash because it goes through add_block_pipeline, not load_initial_state. Out of scope here — orthogonal code path, separate fix.

Checklist

• Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

Why and resolution path

PR #6527 broadened
test-stateless to extract the entire for_amsterdam/ tree from the
zkevm bundle and run all of it under --features stateless; combined with
this branch's bal-devnet-7 semantics that scope produces ~549
GasUsedMismatch / ReceiptsRootMismatch /
BlockAccessListHashMismatch failures.

test-stateless-zkevm filters cargo to the eip8025_optional_proofs
suite, which still validates the stateless harness without the bal-version
mismatch.

Re-broaden by switching test: back to test-stateless in
tooling/ef_tests/blockchain/Makefile once the zkevm bundle is regenerated
against the current bal spec.

[github-actions]

🤖 Kimi Code Review

The PR correctly makes the _info field optional to support goevmlab-generated fixtures and implements the EF test convention for BLOCKHASH operations. No security vulnerabilities or consensus-critical bugs found.

Files reviewed: report.rs, runner.rs, types.rs, utils.rs

Observations

1. types.rs: Missing fork comment update

• Line 30: The DEFAULT_FORKS constant was updated to 6 elements but the comment/reference docs (if any exist outside this diff) should reflect the addition of "Osaka".

2. utils.rs: StatetestDatabase implementation

• Lines 28–56: The wrapper correctly delegates all database operations while overriding get_block_hash to return keccak256(decimal_string(block_number)). This matches the EF test specification and geth's vmTestBlockHash behavior.
• Lines 112–113: The double-Arc wrapping (Arc::new(store) then Arc::new(StatetestDatabase::new(...))) is necessary because StatetestDatabase stores Arc<dyn LevmDatabase> internally, not Arc<StoreVmDatabase>. This is correct.

3. types.rs: Deserialization logic

• Lines 120–130: The match on test_data.get("_info") properly distinguishes between missing fields (returns None) and malformed fields (returns deserialization error). This preserves error reporting for corrupted EF fixtures while allowing goevmlab fixtures to omit the field.

4. Test coverage

• Lines 563–631 (types.rs): Good test coverage for both _info present and absent cases.
• Lines 103–176 (utils.rs): Excellent verification that BLOCKHASH uses decimal encoding ("10" not "a") and specific hash values for blocks 0 and 1.

Minor suggestions

report.rs lines 78–88: The description extraction could avoid cloning both fields by restructuring slightly, though the current implementation is clear and correct:

// Current (acceptable):
info.description.clone().or_else(|| info.comment.clone())

// Alternative avoiding double clone when description exists:
info.description.as_ref().or(info.comment.as_ref()).cloned()

runner.rs lines 40–44: The filter logic for Osaka/Fusaka EIPs remains correct with the optional _info field.

Verdict: LGTM. The changes properly handle the EF test BLOCKHASH convention and gracefully degrade when metadata is absent.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Claude Code Review

Here is my code review for PR #6721:

---

PR #6721 — statetest CLI follow-ups (optional _info, Osaka, BLOCKHASH convention)

Overview

Three targeted bug fixes for the statetest CLI / goevmlab fuzzing harness:

• _info field made optional to support goevmlab-generated fixtures
• Osaka added to DEFAULT_FORKS
• BLOCKHASH(n) overridden to return keccak256(decimal_string(n)), matching the EF convention

The changes are well-scoped, the test coverage is solid, and the PR description is excellent. A few things worth flagging:

---

Correctness

runner.rs — silent skip of goevmlab Osaka fixtures

runner.rs:40-44

let test_eip = test
    ._info
    .as_ref()
    .and_then(|info| info.reference_spec.clone())
    .unwrap_or_default();

if test.path.to_str().unwrap().contains("osaka")
    && !fusaka_eips_to_test.iter().any(|eip| test_eip.contains(eip))

When _info is None (the goevmlab case), test_eip is "". The condition then evaluates to "osaka" in path && no EIP matches "" — which is always true, so the test is silently skipped. This directly contradicts one of the PR's goals: running goevmlab-generated Osaka fixtures that lack _info.

If the intent is to skip only EF Osaka tests that are not in the fusaka allow-list (and to always run goevmlab ones), the guard should be:

if test.path.to_str().unwrap().contains("osaka")
    && test._info.is_some()   // only apply EIP filter when we have metadata
    && !fusaka_eips_to_test.iter().any(|eip| test_eip.contains(eip))

Alternatively, if the caller guarantees goevmlab fixtures never have "osaka" in their path, this is a non-issue — but that assumption should be documented.

---

DEFAULT_FORKS ordering — Amsterdam after Osaka

types.rs:30

const DEFAULT_FORKS: [&str; 6] = ["Merge", "Shanghai", "Cancun", "Prague", "Osaka", "Amsterdam"];

Amsterdam (a pre-Merge Ethereum "fork") appearing after Osaka is chronologically reversed. If DEFAULT_FORKS is used as an ordered allow-list where position matters (e.g., for range comparisons), this ordering is a latent bug. If it's treated purely as a set for membership checks, it's harmless but confusing. A comment clarifying the ordering semantics would help.

---

Design

StatetestDatabase — double-Arc wrapping

utils.rs:89-92

let store: DynVmDatabase = Box::new(StoreVmDatabase::new(...).unwrap());
let inner: Arc<dyn LevmDatabase> = Arc::new(store);
let wrapped: Arc<dyn LevmDatabase> = Arc::new(StatetestDatabase::new(inner));

StatetestDatabase stores its inner as Arc<dyn LevmDatabase>, which then wraps another Arc<dyn LevmDatabase> (the store). Every delegation call goes through two fat pointers. For a test harness this is acceptable, but storing inner as Box<dyn LevmDatabase> would eliminate one indirection and one reference-count pair with no API change (since StatetestDatabase itself is the thing being wrapped in an Arc by callers).

pub struct StatetestDatabase

The struct is pub but only used within this module. Unless it needs to be accessed from other test tooling, pub(crate) would be more appropriate.

---

Minor

• _info field inline comment (types.rs:229) is long enough that a /// doc comment above the field would be more idiomatic.
• SentinelInner's unreachable! bodies in the test are a good pattern for documenting which methods a test exercises — no change needed, just worth noting as a positive pattern.
• report.rs refactor from nested clone().unwrap_or(...) to as_ref().and_then(...) chains is a clear improvement in both correctness and readability.

---

Summary

The BLOCKHASH convention fix (StatetestDatabase shim) and the _info optionality change are correct and well-tested. The main actionable item is the silent-skip behavior for goevmlab Osaka fixtures in runner.rs — whether that's a bug depends on how goevmlab names its fixture paths. The DEFAULT_FORKS ordering warrants a comment or verification that position is irrelevant. Everything else is style-level.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

Findings:

1. tooling/ef_tests/state_v2/src/modules/utils.rs:92 injects StatetestDatabase into every load_initial_state() caller, but only the statetest flow appears to want synthetic BLOCKHASH values. In block mode, the warm-up LEVM execution now sees keccak(decimal(n)) from utils.rs:46, while the actual Blockchain::add_block_pipeline() run still executes against the raw Store in tooling/ef_tests/state_v2/src/modules/block_runner.rs:151. That means any tx touching BLOCKHASH can produce different gas/receipts/state between pre-exec (block_runner.rs:45-53) and the real import, which is a correctness bug. This wrapper should be scoped to statetest, or block mode needs to use identical DB semantics end-to-end.

2. tooling/ef_tests/state_v2/src/modules/types.rs:30 now parses Osaka, but block mode is not updated for it. tooling/ef_tests/state_v2/src/modules/block_runner.rs:83-113 only treats Fork::Prague | Fork::Cancun as blob-era forks, so Osaka blocks will be built without inherited header fields like excess_blob_gas, blob_gas_used, parent_beacon_block_root, and requests_hash. Separately, block_runner.rs:25-34 has no Osaka EIP allowlist, unlike runner.rs:34-50, so block-mode runs will start executing all parsed Osaka fixtures unconditionally. That is likely to create false failures as soon as Osaka fixtures are present.

3. The new workaround still ignores the fixture’s explicit previousHash. Env does not deserialize it in tooling/ef_tests/state_v2/src/modules/types.rs:370-387, and StatetestDatabase::get_block_hash() hardcodes keccak(decimal_string(block_number)) for every lookup in utils.rs:46-47. If a fixture ever provides a non-default previousHash, or only expects the immediate parent hash to be synthetic, this runner will silently execute the wrong semantics. Parsing previousHash and threading it into the state-test DB would be safer than a global formula.

The optional _info handling itself looks fine and the added regression tests are useful.

I couldn’t run cargo test here because the environment blocked Rust toolchain temp-file creation under /home/runner/.rustup/tmp.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

Three focused follow-ups to the statetest CLI landing: _info is made optional to support goevmlab fixtures, Osaka is added to the runnable forks allow-list, and a StatetestDatabase shim is introduced to override BLOCKHASH with the EF convention (keccak256(decimal_string(n))) instead of ethrex's genesis-derived hash.

• Test._info is widened to Option<Info>; all read sites in runner.rs and report.rs are updated with as_ref().and_then(...) fallbacks; two unit tests pin both the with- and without-_info parse paths.
• StatetestDatabase wraps the levm Database and unconditionally returns keccak256(n.to_string()) from get_block_hash, matching geth's vmTestBlockHash; three unit tests including a decimal-vs-hex pin at n=10 verify correctness.
• DEFAULT_FORKS gains \"Osaka\", but the genesis_from_test_and_fork blob-gas target calculation still has an else { 0 } branch that will apply to Osaka fixtures containing currentExcessBlobGas.

Confidence Score: 4/5

Safe to merge for the statetest CLI use-case; the two observations are about edge cases that only activate when Osaka fixtures include blob gas fields or when goevmlab Osaka fixtures flow through the EF runner path.

The StatetestDatabase and optional-_info changes are well-tested and correct. Adding Osaka to DEFAULT_FORKS without updating the blob-gas target formula in genesis_from_test_and_fork leaves an incorrect else-0 path for Osaka fixtures that carry currentExcessBlobGas. Similarly, the Osaka skip logic in run_tests will silently drop any fixture with _info=None and osaka in its path.

types.rs — the genesis excess-blob-gas target branch does not cover Osaka; runner.rs — the Osaka filter behaves incorrectly when _info is absent.

Important Files Changed

Filename	Overview
tooling/ef_tests/state_v2/src/modules/utils.rs	Adds StatetestDatabase shim that overrides get_block_hash to return keccak256(decimal_string(n)), matching geth's vmTestBlockHash convention; well-tested with 3 unit tests including the n=10 decimal-vs-hex pin.
tooling/ef_tests/state_v2/src/modules/types.rs	Makes Test._info optional, adds Osaka to DEFAULT_FORKS, and adds two unit tests; genesis_from_test_and_fork blob-gas target calculation does not handle Osaka (falls to the else-0 branch), which may be incorrect if Osaka uses a non-zero blob target.
tooling/ef_tests/state_v2/src/modules/runner.rs	Correctly updates reference_spec extraction to handle Option; Osaka skip logic still works for EF fixtures (which always have _info).
tooling/ef_tests/state_v2/src/modules/report.rs	Clean adaptation of description/reference_spec extraction to Option with sensible fallback strings.

Comments Outside Diff (2)

1. tooling/ef_tests/state_v2/src/modules/types.rs, line 294-300 (link)

   Osaka blob target missing from genesis excess-blob-gas calculation. Now that Osaka is in DEFAULT_FORKS, any Osaka fixture that sets currentExcessBlobGas will land in the else { 0 } branch, making genesis_excess_blob_gas = current_excess_blob_gas + 0. If Osaka uses a non-zero blob target (like Prague does), the genesis block will carry the wrong excess blob gas, causing the base fee calculation on block 1 to diverge from the expected value and silently misfiring Osaka blob-gas tests.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tooling/ef_tests/state_v2/src/modules/types.rs
   Line: 294-300
   
   Comment:
   Osaka blob target missing from genesis excess-blob-gas calculation. Now that Osaka is in `DEFAULT_FORKS`, any Osaka fixture that sets `currentExcessBlobGas` will land in the `else { 0 }` branch, making `genesis_excess_blob_gas = current_excess_blob_gas + 0`. If Osaka uses a non-zero blob target (like Prague does), the genesis block will carry the wrong excess blob gas, causing the base fee calculation on block 1 to diverge from the expected value and silently misfiring Osaka blob-gas tests.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

2. tooling/ef_tests/state_v2/src/modules/runner.rs, line 40-50 (link)

   Osaka filter silently skips goevmlab fixtures

   When _info is None, test_eip becomes "". Any fixture whose file path contains "osaka" will satisfy !fusaka_eips_to_test.iter().any(|eip| test_eip.contains(eip)) (no EIP can be found in an empty string) and be silently dropped via continue. If run_tests is ever invoked for goevmlab-generated Osaka fixtures, they will be skipped without notice. The workaround is to treat None as "no EIP filter applies" rather than as "no matching EIP found".

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tooling/ef_tests/state_v2/src/modules/runner.rs
   Line: 40-50
   
   Comment:
   **Osaka filter silently skips goevmlab fixtures**
   
   When `_info` is `None`, `test_eip` becomes `""`. Any fixture whose file path contains `"osaka"` will satisfy `!fusaka_eips_to_test.iter().any(|eip| test_eip.contains(eip))` (no EIP can be found in an empty string) and be silently dropped via `continue`. If `run_tests` is ever invoked for goevmlab-generated Osaka fixtures, they will be skipped without notice. The workaround is to treat `None` as "no EIP filter applies" rather than as "no matching EIP found".
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
tooling/ef_tests/state_v2/src/modules/types.rs:294-300
Osaka blob target missing from genesis excess-blob-gas calculation. Now that Osaka is in `DEFAULT_FORKS`, any Osaka fixture that sets `currentExcessBlobGas` will land in the `else { 0 }` branch, making `genesis_excess_blob_gas = current_excess_blob_gas + 0`. If Osaka uses a non-zero blob target (like Prague does), the genesis block will carry the wrong excess blob gas, causing the base fee calculation on block 1 to diverge from the expected value and silently misfiring Osaka blob-gas tests.

```suggestion
        let target = if *fork == Fork::Cancun {
            schedule.cancun.target
        } else if *fork >= Fork::Prague {
            // Osaka (and future forks) inherit the Prague blob schedule until
            // a dedicated schedule entry is added.
            schedule.prague.target
        } else {
            0
        };
```

### Issue 2 of 2
tooling/ef_tests/state_v2/src/modules/runner.rs:40-50
**Osaka filter silently skips goevmlab fixtures**

When `_info` is `None`, `test_eip` becomes `""`. Any fixture whose file path contains `"osaka"` will satisfy `!fusaka_eips_to_test.iter().any(|eip| test_eip.contains(eip))` (no EIP can be found in an empty string) and be silently dropped via `continue`. If `run_tests` is ever invoked for goevmlab-generated Osaka fixtures, they will be skipped without notice. The workaround is to treat `None` as "no EIP filter applies" rather than as "no matching EIP found".

_{Reviews (1): Last reviewed commit: "fix(l1): apply EF state-test BLOCKHASH c..." | Re-trigger Greptile}

[ElFantasma]

Re Codex finding 3 (parse previousHash from the fixture):

The hardcoded keccak256(decimal_string(n)) formula is intentional. It matches geth's vmTestBlockHash in tests/state_test_util.go exactly, and the EF state-test spec defines the formula — not the fixture field — as the source of truth.

env.previousHash is informational: it's always equal to keccak256(decimal_string(currentNumber - 1)) by construction, so reading it would be a no-op for any fixture that follows the spec. If a fixture ever provided a previousHash that didn't match the formula, geth would still return the formula's value, so honoring previousHash instead would diverge from geth — the opposite of what this PR is trying to fix.

So: not parsing previousHash is the deliberate behavior here.