Skip to content

chore(l1): upgrade mainnet ef-test fixtures to execution-specs tests@v20.0.0#6969

Open
edg-l wants to merge 5 commits into
mainfrom
chore/upgrade-mainnet-fixtures-v20
Open

chore(l1): upgrade mainnet ef-test fixtures to execution-specs tests@v20.0.0#6969
edg-l wants to merge 5 commits into
mainfrom
chore/upgrade-mainnet-fixtures-v20

Conversation

@edg-l

@edg-l edg-l commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Migrates the mainnet ef-tests off the now-archived ethereum/execution-spec-tests onto ethereum/execution-specs tests@v20.0.0 (first mainnet fixture release: Osaka + BPO1 + BPO2).

Changes

  • .fixtures_url for blockchain/state/engine → tests@v20.0.0/fixtures.tar.gz (same archive layout, extraction targets unchanged)
  • Fork parsing: new fixtures rename EIP150TangerineWhistle, EIP158SpuriousDragon. Added aliases in blockchain/fork.rs, state/state_v2 deserializers, and engine/fixture.rs
  • CI hive: new .github/config/hive/mainnet.yaml (pins fixtures + eels commit), replacing the hardcoded archived fixtures_develop URL in daily_hive_report.yaml
  • Docs updated to reference execution-specs

The for_<fork>/ dir rename in the new bundle is a non-issue — runners read the fork from each fixture's network/post field. Amsterdam/glamsterdam and zkevm overlays untouched (already on execution-specs).

Verification

  • cargo check clean on all four crates
  • Engine suite vs new fixtures: 10,863 tests / 74,331 fixtures, 0 failed, 0 skipped (incl. Amsterdam overlay: 3,021 / 23,767, 0 failed)
  • Remaining suites covered by CI

@github-actions github-actions Bot added the L1 Ethereum client label Jul 7, 2026
@edg-l edg-l marked this pull request as ready for review July 7, 2026 11:02
@edg-l edg-l requested a review from a team as a code owner July 7, 2026 11:02
@ethrex-project-sync ethrex-project-sync Bot moved this to In Review in ethrex_l1 Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

These are correct — bare string matches with no risk of duplicate arms (compiler would catch that anyway). Everything checks out consistently across the four parsing sites. This is a clean, mechanical, low-risk PR.

Review: chore(l1): upgrade mainnet ef-test fixtures to execution-specs tests@v20.0.0

This is a well-scoped, mechanical migration PR. The changes are consistent across all four fork-name parsing sites (blockchain/fork.rs, state/deserialize.rs, state_v2/deserialize.rs, engine/fixture.rs), and the new mainnet.yaml config file follows the same pattern as the existing amsterdam.yaml. No correctness, security, gas-accounting, or consensus-rule concerns — this only touches test tooling and CI configuration, not client logic.

Findings:

  1. .github/config/hive/mainnet.yaml (new file) — minor observation, not a defect. The eels_commit (b6956dbe42a64d143569d5e9b20a0e7f7b9d0ac9) is an opaque hash with no inline comment explaining what branch/tag it corresponds to (unlike amsterdam.yaml, which at least names the devnet in its header comment). Not required, but a one-line comment noting which execution-specs branch/PR this commit comes from would help future maintainers doing the "Updating Repository Versions" doc procedure (docs/developers/l1/testing/hive.md) without needing to dig through git blame.

  2. Alias coverage looks complete and symmetric. EIP150/TangerineWhistle and EIP158/SpuriousDragon aliases are added consistently in all three deserializers plus the engine's manual match, so fixtures using either the old or new naming convention parse correctly. Confirmed no duplicate match arms (would be a compile error) and no missing sites.

  3. CI script uses env-var indirection for interpolated fixture URLs/commits (daily_hive_report.yaml), avoiding direct string interpolation into the shell script body — this is a good pattern since it sidesteps shell-injection concerns even though the values come from a repo-controlled YAML file rather than untrusted input.

  4. Pre-existing stale references outside this PR's scope (not a blocker): tooling/ef_tests/state/README.md, tooling/ef_tests/blockchain/README.md, and docs/eip-8025.md still reference ethereum/execution-spec-tests docs/zkevm fixtures. The PR description explicitly notes zkevm/Amsterdam overlays are untouched, so this is fine to leave for a follow-up.

Overall: sound, low-risk change with good verification numbers cited in the PR description (10,863 tests / 74,331 fixtures, 0 failures). No blocking issues found.


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

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

This PR correctly migrates Hive test fixtures from the archived ethereum/execution-spec-tests to the active ethereum/execution-specs repository. The changes are consistent and properly handle the new fixture naming conventions.

Key observations:

  1. Configuration structure (.github/config/hive/mainnet.yaml):
    Properly pins tests@v20.0.0 (Osaka + BPO1 + BPO2) with a specific EELS commit. This is a significant improvement over the previous floating forks/amsterdam branch reference.

  2. Fork name aliases (tooling/ef_tests/blockchain/fork.rs lines 117-120, engine/src/fixture.rs line 247, etc.):
    Correctly adds TangerineWhistle and SpuriousDragon as aliases for EIP150 and EIP158 respectively. This matches the canonical naming used in the new execution-specs fixtures while maintaining backward compatibility with existing internal variant names.

  3. Workflow logic (.github/workflows/daily_hive_report.yaml lines 120-149):
    The separation of Amsterdam (glamsterdam-devnet) and mainnet fixture configurations is clean. Using pinned commits ($MAINNET_EELS_COMMIT) instead of branch names for non-Amsterdam forks eliminates nondeterminism in CI.

  4. Documentation (docs/developers/l1/testing/hive.md):
    Accurately reflects the repository migration and provides clear instructions for future fixture updates.

Minor note:
Ensure that the EELS commit b6956dbe42a64d143569d5e9b20a0e7f7b9d0ac9 specified in mainnet.yaml is compatible with the Osaka/BPO1/BPO2 fixtures at tests@v20.0.0. The commit appears to reference the correct execution-specs state for consuming these fixtures.

Verdict: LGTM. The migration is complete and consistent across CI configuration, local test runners, and documentation.


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

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

  1. Local state-test fork filtering is still out of sync with the new alias support. tooling/ef_tests/state/runner/mod.rs:92 only accepts "Tangerine" and "SpuriousDragon" for --fork, while this PR teaches the fixture deserializers to accept "TangerineWhistle" / "EIP150" and "EIP158" / "SpuriousDragon" in other paths. That means a local filtered run can still fail with Unknown fork if someone uses the new execution-specs naming. I’d mirror the alias table here or centralize fork-name parsing.

  2. The new alias handling is duplicated across four parsers with no regression coverage: tooling/ef_tests/blockchain/fork.rs:118, tooling/ef_tests/engine/src/fixture.rs:247, tooling/ef_tests/state/deserialize.rs:318, and tooling/ef_tests/state_v2/src/modules/deserialize.rs:148. This kind of string mapping tends to drift again; a small table-driven test for "TangerineWhistle" and "SpuriousDragon" would make the migration safer.

I did not find a security, consensus, gas-accounting, or EVM-correctness issue in the fixture URL / Hive workflow migration itself.

I couldn’t run the Rust test targets in this environment because cargo/rustup attempted to write under /home/runner/.rustup/tmp, which is read-only here.


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

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR migrates the mainnet EF-test fixture source from the now-archived ethereum/execution-spec-tests repo to ethereum/execution-specs at tests@v20.0.0, the first mainnet release covering Osaka, BPO1, and BPO2. Fork name aliases (TangerineWhistle/SpuriousDragon) are added across all four test suite deserializers to handle the renamed fork identifiers in the new bundle.

  • Fixture URLs updated in blockchain, state, and engine .fixtures_url files to point at the new execution-specs release tarball.
  • Fork name aliases added consistently in blockchain/fork.rs (serde aliases), engine/src/fixture.rs (match arms), state/deserialize.rs, and state_v2/src/modules/deserialize.rs so both old (EIP150/EIP158) and new (TangerineWhistle/SpuriousDragon) fixture naming resolves correctly.
  • CI hive config extracted into a new .github/config/hive/mainnet.yaml, replacing the previously hardcoded archived URL in daily_hive_report.yaml, mirroring the existing Amsterdam config pattern.

Confidence Score: 4/5

Safe to merge — the code-level changes are mechanical alias additions and URL swaps, all applied consistently across the four test suites.

All fork-name aliases are added symmetrically across blockchain, state, state_v2, and engine deserializers. The CI workflow change follows the established amsterdam.yaml pattern exactly. The only blemish is a minor docs duplicate where items 2 and 3 in hive.md both link to the same ethereum/execution-specs URL with different descriptions.

docs/developers/l1/testing/hive.md has a duplicate repository entry that should be cleaned up.

Important Files Changed

Filename Overview
.github/config/hive/mainnet.yaml New config file pinning mainnet hive fixtures to execution-specs tests@v20.0.0 and an eels_commit hash; mirrors the existing amsterdam.yaml pattern cleanly.
.github/workflows/daily_hive_report.yaml Adds a Load mainnet config step that reads fixtures/eels_commit from mainnet.yaml and replaces the hardcoded archived URL in the non-Amsterdam hive flags branch; logic mirrors the existing Amsterdam config pattern.
tooling/ef_tests/blockchain/fork.rs Adds serde aliases TangerineWhistle→EIP150 and SpuriousDragon→EIP158 so both old and new fixture naming conventions deserialize correctly.
tooling/ef_tests/engine/src/fixture.rs Adds TangerineWhistle to the single_fork match arm for the Tangerine fork; SpuriousDragon was already handled, so coverage is now symmetric.
tooling/ef_tests/state/deserialize.rs Adds SpuriousDragon and TangerineWhistle aliases to the fork-name match arms, consistent with the state_v2 and engine changes.
tooling/ef_tests/state_v2/src/modules/deserialize.rs Same TangerineWhistle/SpuriousDragon aliases added to state_v2 fork deserializer, mirroring state and engine changes.
docs/developers/l1/testing/hive.md Docs updated to reference execution-specs; items 2 and 3 in the overview list now both link to the same URL (ethereum/execution-specs), creating a confusing duplicate entry.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI: daily_hive_report.yaml] --> B{Fork type?}
    B -->|Amsterdam| C[Load amsterdam.yaml]
    B -->|Mainnet Prague/Osaka/Cancun| D[Load mainnet.yaml\ntests@v20.0.0]
    C --> G[hive buildarg fixtures + branch]
    D --> G

    H[Local ef_tests runners] --> I{Suite}
    I -->|blockchain| J[blockchain/.fixtures_url]
    I -->|state and state_v2| K[state/.fixtures_url]
    I -->|engine| L[engine/.fixtures_url]

    J --> M[Fork name deserialization]
    K --> M
    L --> M
    M --> N{Fork string in fixture}
    N -->|EIP150 or TangerineWhistle| O[Fork::Tangerine]
    N -->|EIP158 or SpuriousDragon| P[Fork::SpuriousDragon]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[CI: daily_hive_report.yaml] --> B{Fork type?}
    B -->|Amsterdam| C[Load amsterdam.yaml]
    B -->|Mainnet Prague/Osaka/Cancun| D[Load mainnet.yaml\ntests@v20.0.0]
    C --> G[hive buildarg fixtures + branch]
    D --> G

    H[Local ef_tests runners] --> I{Suite}
    I -->|blockchain| J[blockchain/.fixtures_url]
    I -->|state and state_v2| K[state/.fixtures_url]
    I -->|engine| L[engine/.fixtures_url]

    J --> M[Fork name deserialization]
    K --> M
    L --> M
    M --> N{Fork string in fixture}
    N -->|EIP150 or TangerineWhistle| O[Fork::Tangerine]
    N -->|EIP158 or SpuriousDragon| P[Fork::SpuriousDragon]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
docs/developers/l1/testing/hive.md:11-14
**Duplicate repo entry in overview list**

Items 2 and 3 now both link to `https://github.com/ethereum/execution-specs` with different descriptions ("Test fixtures and vectors" vs "Fork specifications"). A reader following item 3's link lands on the same repo they just visited for item 2, which makes the list confusing. Consider merging them into a single entry that notes both roles, or clarifying that item 3 specifically refers to the `forks/amsterdam` branch for the EELS consumer.

Reviews (1): Last reviewed commit: "chore(l1): drop non-future-proof amsterd..." | Re-trigger Greptile

Comment on lines +11 to 14
2. **[ethereum/execution-specs](https://github.com/ethereum/execution-specs)** - Test fixtures and vectors (the former `ethereum/execution-spec-tests` repo is archived)
- Mainnet: `tests@v20.0.0` (Osaka + BPO1 + BPO2); Amsterdam: `tests-glamsterdam-devnet@v6.1.1`
3. **[ethereum/execution-specs](https://github.com/ethereum/execution-specs)** - Fork specifications
- Current branch: `forks/amsterdam`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicate repo entry in overview list

Items 2 and 3 now both link to https://github.com/ethereum/execution-specs with different descriptions ("Test fixtures and vectors" vs "Fork specifications"). A reader following item 3's link lands on the same repo they just visited for item 2, which makes the list confusing. Consider merging them into a single entry that notes both roles, or clarifying that item 3 specifically refers to the forks/amsterdam branch for the EELS consumer.

Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/l1/testing/hive.md
Line: 11-14

Comment:
**Duplicate repo entry in overview list**

Items 2 and 3 now both link to `https://github.com/ethereum/execution-specs` with different descriptions ("Test fixtures and vectors" vs "Fork specifications"). A reader following item 3's link lands on the same repo they just visited for item 2, which makes the list confusing. Consider merging them into a single entry that notes both roles, or clarifying that item 3 specifically refers to the `forks/amsterdam` branch for the EELS consumer.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

edg-l added 3 commits July 7, 2026 13:08
…ture recovery

Instead of skipping bad_v_r_s fixtures, decode the signed tx from post.txbytes and
recover the sender (enforcing EIP-2 low-s and r/s range checks). A rejected recovery
is the expected invalid-signature failure. Applied to both state and state_v2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

1 participant