eest: add EIP-8025 execution witness test suite (93 zkevm fixtures)#20938
Closed
awskii wants to merge 37 commits into
Closed
eest: add EIP-8025 execution witness test suite (93 zkevm fixtures)#20938awskii wants to merge 37 commits into
awskii wants to merge 37 commits into
Conversation
Add witness_block_test_util.go with WitnessBlockTest struct that wraps BlockTest with per-block execution witness expectations (state, codes, headers). Includes UnmarshalJSON for dual deserialization and accessors for querying expected witness data per block index.
Add public M field to BlockTest struct and assign it in both Run() and RunCLI() methods so the witness test runner can access the ExecModuleTester after block execution.
Create testmain_test.go and witness_test.go in execution/tests/eest_zkevm_witness/ to validate Erigon's execution witness generation against 93 EIP-8025 fixtures from ethereum/execution-spec-tests zkevm@v0.3.3. All 93 fixtures are marked as expected failures via bt.Fails for known State/Codes ordering mismatches (#20442, #20534).
Add execution-eest-zkevm test group to tools/test-groups and append blockchain_tests_zkevm/**/*.json to the git-lfs pull include pattern in setup-erigon action.
make lint (0 issues, run twice) and make erigon integration both pass cleanly with the new witness test files from Tasks 1-4.
- Guard NewWitness against genesis block (number 0) to prevent uint64 underflow in parent block number calculation - Simplify headers slice initialization to direct literal - Add TestNewWitnessRejectsGenesisBlock test
This reverts commit 90456fd.
This reverts commit be57953.
…ewmain # Conflicts: # execution/tests/testutil/block_test_util.go # tools/test-groups
…ss Headers format After merge with main, ExecutionWitnessResult.Headers changed from []hexutil.Bytes to []map[string]any in PR #21224 ("align response debug_executionWitness to GETH"). The EEST zkevm fixture format still encodes headers as RLP hex bytes, so compareWitness can't pass them to compareByteSlices anymore. Add a small jsonHeadersToRLP helper that mirrors marshalWitnessHeader in reverse: it renames balHash back to blockAccessListHash, drops the synthetic "hash" and "size" fields that aren't part of types.Header's JSON schema, and round-trips each map through Header.UnmarshalJSON + rlp.EncodeToBytes. The resulting []hexutil.Bytes is comparable element-by-element against the fixture data.
Main's #21092 removed both execution-eest-blockchain and execution-eest-devnet from tools/test-groups when spec tests were extracted out of make test-all. The merge resolution mistakenly kept the PR-side branch with the devnet entry plus a stale "Temporarily disable the blockchain shard" comment. Drop the devnet entry and the comment; keep only the PR's own execution-eest-zkevm contribution.
The witness test only consumes BlockTest.M from the Run(t) path, so the RunCLI() assignment is a dead store with a real cost: cmd/evm's runBlockTest holds a map[string]*BlockTest across an entire fixture file, and bt.M kept each iteration's ExecModuleTester reachable past m.Close(). Close() releases DB/snapshots/engine but doesn't nil the struct fields, so StateCache and other heavyweights stayed pinned for the rest of the file's iteration. Under evm.race with --workers 12 in the EEST blockchain-tests CI matrix that pushed the runners past the OOM threshold (jobs SIGTERM'd at 9-57 minutes), while main without the field assignment runs fine. Removing the RunCLI line restores the prior memory profile. Run(t) keeps the assignment since the witness test needs it; t.Cleanup handles the cleanup there.
awskii
requested review from
AskAlexSharov,
bloxster,
mh0lt and
yperbasis
as code owners
AskAlexSharov
approved these changes
May 22, 2026
Adopts the lazy-download manifest machinery introduced by #21002 instead of relying on the prior Git LFS path under execution/tests/execution-spec-tests/. - test-fixtures.json: add eest_zkevm entry pinning zkevm@v0.3.4 fixtures_zkevm.tar.gz (sha256 b9e3ab70..., 231977945 bytes). - Makefile: add a focused test-fixtures-zkevm target so workflows can pull just the zkevm tarball without forcing eest_stable/devnet/benchmark. - execution/tests/eest_zkevm_witness/witness_test.go: repoint the fixture walk root at test-fixtures-cache/eest_zkevm/fixtures/blockchain_tests, matching the tarball's internal layout (the script does not strip --strip-components). - .github/workflows/test-all-erigon-race.yml: when the matrix entry is execution-eest-zkevm, restore the cached tarball (keyed on hashFiles('test-fixtures.json')) and run make test-fixtures-zkevm before make test-group. Other matrix entries are unaffected.
awskii
commented
May 22, 2026
Member
Author
awskii
left a comment
awskii
left a comment
There was a problem hiding this comment.
Migrated to the #21002 manifest in ceb81f9. Summary:
test-fixtures.json— addedeest_zkevmpinningzkevm@v0.3.4(fixtures_zkevm.tar.gz, sha256b9e3ab70…, 231,977,945 bytes).Makefile— addedtest-fixtures-zkevm(focused; pulls only the zkevm tarball, doesn't piggy-back ontest-fixtures-eestsince most jobs don't need it).execution/tests/eest_zkevm_witness/witness_test.go— repointed the walk root attest-fixtures-cache/eest_zkevm/fixtures/blockchain_tests, matching the tarball's internalfixtures/blockchain_tests/...layout (the script doesn't strip)..github/workflows/test-all-erigon-race.yml— when the matrix entry isexecution-eest-zkevm, restore the cached tarball (keyed onhashFiles('test-fixtures.json')) and runmake test-fixtures-zkevmbeforemake test-group. Other matrix entries are untouched.
Verified locally: tools/test-fixtures.sh downloads, sha256-verifies, and extracts; the whitelisted fixture (for_amsterdam/.../validation_state_unsorted_but_complete.json) is reachable at the new path and the test walks the corpus correctly.
PR #21224 added a DB-level precondition to debug_executionWitness: the RPC now reads the commitment-history flag via rawdb.ReadDBCommitmentHistoryEnabled and returns an error if it's unset. statecfg.EnableHistoricalCommitment sets the schema but not the DB flag (in production the flag is written by node/eth/backend on startup). The test framework builds its DB from scratch, so write the flag explicitly right after setting up the API. Confirmed locally that the whitelisted fixture now reaches the comparator instead of failing on the precondition.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an EIP-8025 execution witness validation suite (zkevm fixtures) to Erigon’s execution tests, including fixture download wiring and small witness-generation/test harness adjustments.
Changes:
- Add
eest_zkevmfixture manifest entry plusmake test-fixtures-zkevmtarget and CI cache/download steps. - Introduce a witness-aware
BlockTestwrapper and a newexecution/tests/eest_zkevm_witnessrunner that comparesdebug_executionWitnessoutput to fixture expectations. - Harden witness creation by explicitly rejecting genesis block witnesses and add a unit test for it.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tools/test-groups |
Adds a dedicated CI test group for the zkevm witness suite. |
test-fixtures.json |
Pins the eest_zkevm fixture tarball (url/sha/size). |
Makefile |
Adds test-fixtures-zkevm target to fetch/extract the zkevm witness fixtures. |
execution/types/stateless/witness.go |
Rejects genesis witness creation; minor slice init cleanup. |
execution/types/stateless/witness_test.go |
Adds a unit test asserting genesis witness creation is rejected. |
execution/tests/testutil/witness_block_test_util.go |
Adds WitnessBlockTest to parse per-block executionWitness expectations. |
execution/tests/testutil/block_test_util.go |
Exposes the underlying ExecModuleTester from BlockTest.Run(). |
execution/tests/eest_zkevm_witness/witness_test.go |
Adds the witness fixture runner and comparison helpers. |
execution/tests/eest_zkevm_witness/testmain_test.go |
Adds TestMain hook for the suite. |
docs/plans/completed/20260429-eest-witness-test-suite-port.md |
Documents the port plan and completion notes for the suite. |
.github/workflows/test-all-erigon-race.yml |
Caches the zkevm fixture tarball and runs make test-fixtures-zkevm for the new group. |
Comments suppressed due to low confidence (2)
execution/tests/eest_zkevm_witness/witness_test.go:97
execmoduletester.New(t, ...)already registerst.Cleanup(mock.Close), so the additionaldefer test.M.Close()will close the tester twice.ExecModuleTester.Close()is not idempotent (it closes Engine/DB/snapshots), so this can cause panics or flaky failures.
if test.M == nil {
t.Fatal("ExecModuleTester not set after BlockTest.Run")
}
defer test.M.Close()
docs/plans/completed/20260429-eest-witness-test-suite-port.md:101
- The verification steps here still instruct initializing the
execution-spec-testssubmodule and mention LFS checkout, but the suite in this PR is wired tomake test-fixtures-zkevm/test-fixtures-cache/eest_zkevm. Consider updating the recorded steps so someone following the plan doesn't fetch the wrong fixture source.
### Task 6: Run witness test suite
- [x] Initialize submodule: `git submodule update --init execution/tests/execution-spec-tests`
- [x] Fetch LFS fixtures: fixtures already checked out as real JSON (git-lfs not needed)
- [x] Run test suite: `go test -count=1 -v -run TestExecutionSpecWitness ./execution/tests/eest_zkevm_witness/...`
- [x] Verify all 93 fixtures run and pass (via expected-failure annotations) — 176 subtests, 0 failures
- [x] No deviations from expected results
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The M field added by this PR was a data-race hazard (Copilot also flagged a related double-close: execmoduletester.New already registers t.Cleanup(m.Close), so the test's defer test.M.Close() ran Close twice on the same instance — Close is not idempotent and closes Engine/DB/snapshots). Replace the field with an explicit RunWithTester(t) (*ExecModuleTester, error) method that returns the tester from Run's existing setup path. Run(t) error becomes a thin wrapper. The witness test grabs the tester from the return value, drops the field access, and removes the double-close defer (auto-cleanup via t.Cleanup is sufficient). Also drop the zkevm fixture version (zkevm@v0.3.3) from the package doc comment — the pin lives in test-fixtures.json now, so the comment would just drift with every bump.
- witness_test.go: drop bt.Whitelist so the full corpus runs. All
mismatches are still absorbed by bt.Fails(".") for the documented
state/codes ordering issues (#20442, #20534), so the CI signal stays
green; CI logs now reflect the whole 93-fixture surface.
- stateless/witness.go: fix "wtness" → "witness" in a comment touched by
this PR's surrounding refactor.
- docs/plans/completed/20260429-eest-witness-test-suite-port.md: replace
the v0.3.3 + submodule/LFS references with the manifest-driven flow
(test-fixtures.json + make test-fixtures-zkevm) so the completed-plan
record matches what shipped.
…ilure After dropping the whitelist (bd01f8d) many fixtures fail at block execution rather than at witness comparison, because Erigon doesn't yet implement every EIP in zkevm v0.3.4 (EIP-7708 burn logs, EIP-7954, EIP-8037, etc.). The previous t.Fatalf bypassed the suite-wide bt.Fails(".") absorption mechanism, so each gap surfaced as a hard failure. Route the error through bt.CheckFailure (mirroring the witness-comparison branch) so it's absorbed as a documented gap. Return early after absorption since there's no tester to drive the subsequent debug RPC calls. Unexpected failures (no matching Fails pattern) still raise via t.Error.
…s too Same shape as the block-execution-failure fix (9c70e4d): with the whitelist gone, many fixtures reach the RPC path but the RPC returns "block not found: 1" for fixtures whose state shape differs from the single fixture the test was originally tuned against. The previous t.Fatalf bypassed bt.Fails absorption, leaving each gap as a hard failure. Collect RPC errors (and nil results) into witnessErrs alongside comparison mismatches, then let the bt.CheckFailure call at the end of the closure absorb them uniformly. Unexpected failures (no matching Fails pattern) still raise via t.Error.
…xtures skip message and set-diff wording
… Walk guard The os.Stat pre-check only branched on os.IsNotExist, so a dir that exists as a non-directory (or a non-IsNotExist stat error) fell through to the shared Walk, re-surfacing the misleading 'clone the tests submodule' message the pre-check was meant to replace (and hitting Walk's nil-deref on the error case). Mirror Walk's full 'missing OR not-a-directory' guard.
…gain
The widen-corpus suite contains Amsterdam EIPs (eip7708/7778/7843/7928) that
Erigon does not yet fully implement, so their blocks fail to execute (gas
mismatches). Hard-failing on block execution (prior commit) turned ~12.8k
known-failing subtests red in CI. Route block-execution errors back through
bt.CheckFailure so the suite-wide bt.Fails(".") absorbs them, matching the
witness-comparison gaps. The whole corpus is a known-failing tracker today.
yperbasis
requested changes
May 26, 2026
Member
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
Infra changes look good (RunWithTester, manifest migration, genesis guard). One blocking concern on the test itself:
bt.Fails(".", ...) + bt.CheckFailure on both error classes makes this a no-op regression detector.
. matches every fixture; routing block-execution errors and witness mismatches through bt.CheckFailure means the only signal the suite produces is "did anything pass unexpectedly." The commit message on 8c38b22ab7 confirms the two failure modes are genuinely different (Amsterdam EIPs unimplemented vs. State/Codes ordering #20442/#20534), and the prior hard-fail commit c488b1ca43 was reverted specifically because they got conflated.
Minimum fix: split bt.Fails by category (Amsterdam EIPs by name pattern → "not yet implemented"; the rest → "ordering #20442/#20534"), and let block-execution errors outside the Amsterdam set fall through to t.Errorf. That way a regression that turns one category into the other surfaces as unexpected.
Smaller items:
TestNewWitnessRejectsGenesisBlockcompareserr.Error()verbatim — use a sentinel orerrors.Is.RunWithTester's no-close contract is docstring-only; consider hidingCloseon the returned type so the double-close footgun can't recur.- Branch needs rebase against
7827ec7268(newexec_modematrix intest-all-erigon-race.yml). - Re-request review — @AskAlexSharov's approval is on
cd9d157e58, 11 substantive commits ago.
Member
Author
|
ZKevm corpus is geth compatible, we need to make it RETH compatible since Zilk is using it. We are not looking for 100% compatibility - its unbearable, and im closing this in favor of newer zkevm corpus v0.4.0 at #21487 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Status
Testing