Repository navigation
feat(ci): validate filled blockchain fixtures with EELS - #3552
Merged
danceratopz merged 4 commits intoSep 10, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3552 +/- ##
===================================================
+ Coverage 94.01% 94.39% +0.37%
===================================================
Files 624 624
Lines 36927 36927
Branches 3326 3326
===================================================
+ Hits 34717 34857 +140
+ Misses 1533 1459 -74
+ Partials 677 611 -66
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Run an existing blockchain_tests fixture directory through the json_loader path, which calls state_transition for every block and so exercises execute_block, validate_header and apply_body. Fill's t8n never reaches these; the test framework assembles the block header itself. The json_loader conftest is loaded as a plugin with -p so any fixture directory can be passed directly, without symlinking it under tests/json_loader. `python -m pytest` puts the working directory on sys.path so the plugin module resolves. Coverage data is isolated under .just/validate-blocks so the fill data file read by `just coverage` is left intact; pass --no-cov to skip coverage. Based on the block-coverage recipe proposed by spencer-tb in ethereum#3526. Co-authored-by: spencer-tb <spencer.tb@ethereum.org>
Add a step to each fill matrix job that runs the fixtures just written to .just/fill/fixtures/blockchain_tests through validate-blocks, and upload its coverage report alongside the fill report. The step checks on every push that framework-built headers and declared block exceptions agree with the EELS block validator.
Run validate-blocks on the blockchain_tests directory of every split EELS release build before its artifact is uploaded. Unsplit builds write a tarball and non-EELS builds use another t8n, so both are skipped. Coverage is disabled; the step is a consistency gate only.
danceratopz
force-pushed
the
ci-block-validation
branch
from
September 9, 2026 08:22
9b8ead4 to
6ac3c33
Compare
2 tasks done
danceratopz
marked this pull request as ready for review
September 9, 2026 11:11
Contributor
|
I need more cats and puzzles in my life :) |
spencer-tb
approved these changes
Sep 10, 2026
spencer-tb
left a comment
spencer-tb
left a comment
Contributor
There was a problem hiding this comment.
LGTM!! Really nice with Codecov :)
One suggestion from Codex I agree with copied below!
Required to avoid a `ValueError` when the specified fixtures directory is located under `tests/json_loader`. Co-authored-by: spencer <spencer.tb@ethereum.org>
danceratopz
added
C-feat
2 tasks done
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
2 participants
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.
Description
Filling never runs the EELS block validator. The t8n path calls
process_transactionand the block-level helpers, but the test framework assembles the block header itself, sostate_transition,execute_block,validate_headerandapply_bodyare only reached when a fixture is consumed. Header-level invalid blocks (rlp_modifier, requests and BAL overrides) are also exempt from t8n exception verification during fill.This PR takes @spencer-tb's idea from #3526 to add a
just validate-blocks <blockchain_tests dir> [pytest args]recipe, which runs an existingblockchain_testsdirectory through thetests/json_loaderpath. In contrast to #3526, the conftest is loaded as a pytest plugin, so any fixture directory can be passed directly and no symlink undertests/json_loaderis needed. Coverage data is written under.just/validate-blocks/;--no-covskips it.This PR then additionally makes these CI changes:
test.yaml: each fill matrix job validates the fixtures it just filled and uploads the validation coverage next to the fill coverage, so codecov sees the union.build-fixturesaction: each split EELS release build validates itsblockchain_testsbefore uploading the artifact. Unsplit (tarball) and non-EELS builds are skipped.Cost on the CI runners
The tables compare the fill matrix jobs of the
Python Specificationworkflow without and with the new step. The two baseline runs are the two most recentforks/amsterdampush runs, which have no validation step. The two PR runs are the runs on this branch; both commits have the same tree and differ only in the first commit message. Durations are job and stepstarted_attocompleted_atfrom the Actions API, so runner queue time is excluded. Fixture counts are the pytest summary lines of the new step.Fill job duration:
The "Run fill" step itself is unchanged within noise (amsterdam 2m48s to 2m52s across the four runs), and the codecov upload with two files still takes 3 to 4 s.
The new step on its own:
All 14,682 fixtures passed EELS block validation. Coverage instrumentation is most of the step's cost: locally a 95-fixture sample took 16 s with coverage and 5 s with
--no-cov.Coverage change
Codecov merges the uploads of all fill jobs and the json-loader job under the
unittestsflag. The tables compare the codecov report of the base commit with the report of this PR's head, both read from the codecov API (/report/?sha=<commit>). No source or test file changed, so the whole difference comes from the validation upload. The PR report shows the same totals.Every file whose hit count changed:
src/ethereum/forks/amsterdam/fork.pysrc/ethereum/forks/osaka/fork.pysrc/ethereum/forks/prague/fork.pysrc/ethereum/forks/cancun/fork.pysrc/ethereum/forks/shanghai/fork.pysrc/ethereum/forks/london/fork.pysrc/ethereum/forks/paris/fork.pysrc/ethereum/forks/berlin/fork.pysrc/ethereum/forks/byzantium/fork.pysrc/ethereum/forks/constantinople/fork.pysrc/ethereum/forks/frontier/fork.pysrc/ethereum/forks/homestead/fork.pysrc/ethereum/forks/istanbul/fork.pysrc/ethereum/forks/cancun/transactions.pysrc/ethereum/forks/prague/transactions.pysrc/ethereum/forks/berlin/transactions.pysrc/ethereum/forks/london/transactions.pysrc/ethereum/forks/paris/transactions.pysrc/ethereum/forks/shanghai/transactions.pysrc/ethereum/state_mpt.pyEvery gain is in
fork.py(the header comparison branches,validate_headerandapply_body), in transaction RLP decoding, or in the MPT state root path. The gain is smaller than a fill-only comparison would suggest because the json-loader job already reports the base-coverage subset throughstate_transition. The new step adds the lines reached only by fixtures outside that subset, for every fork range.Notes:
networksuch asOsakaToAmsterdamAtTime15k) are skipped by the loader, as before.release_fixturesdispatch or the nightly schedule, so this PR's checks do not exercise it.Related Issues or PRs
Complements #3526, which uses the same json_loader path for per-EIP coverage measurement.
Checklist
just static<type>(<area>): <title>, where<type>and<area>come from an appropriateC-<type>, respectivelyA-<area>, label. The title should match the target squash commit message.Cute Animal Picture