fix(tooling): review fixups for the real ethrex block fixture - #884
Open
MauroToscano wants to merge 10 commits into
Open
fix(tooling): review fixups for the real ethrex block fixture#884MauroToscano wants to merge 10 commits into
MauroToscano wants to merge 10 commits into
Conversation
… changes The cache filename is keyed on the block number alone and its download rule had no prerequisites, so make treated an already-present `caches/cache_hoodi_<n>.json` as up to date across an `ETHREX_REPLAY_REV` bump and kept converting the old input. Depending on a rev-stamped marker makes a re-pin discard the stale cache and refetch. Without this the mismatch only surfaced downstream as a `conversion_is_reproducible` digest failure whose message reads "regenerate the fixture", pointing at the wrong cause. CI never hit it (fresh checkout, `caches/` is gitignored); local re-pins did.
…uild
Two independent costs in the `test-executor` job, both paid on every PR run:
- `Swatinem/rust-cache` defaulted to `. -> target`, so neither detached
ethrex workspace was cached. `tooling/ethrex-real-block` alone locks ~335
packages, including the blst, c-kzg and secp256k1-sys C builds, malachite
and ark-ff/asm. `cache-all-crates` covers the registry, not artifacts.
Adds both tool workspaces to `workspaces:`.
- k256's `expose-field` was requested only by `lambda-vm-ethrex-crypto`, a
dev-dependency. Under resolver 3 dev-dependency features are not unified
into builds that exclude them, so `cargo run` (the fixture step) and the
`cargo test` right after it resolved different k256 feature sets and
rebuilt k256 plus every ethrex crate above it. Moving that one dependency
to `[dependencies]` makes both resolutions identical:
cargo tree -e features,no-dev -i k256 # lists expose-field
cargo tree -e features -i k256 # matches
sha2 stays a dev-dependency; it has no feature delta either way.
Also corrects the rkyv sync note: it named `executor/Cargo.toml`, which
declares no rkyv. The readers are the guest and the two other detached
ethrex tool workspaces.
… digest `conversion_is_reproducible`'s sha256 is the assertion an ethrex rev bump forces someone to rewrite: the layout moves, the test goes red, a fresh digest gets pasted in. At that moment it stops covering the substituted- chain-config case the README picked it for. Assert the Hoodi chain id directly so that check survives the churn. Docs, all measured rather than estimated: - `ethrex_bench_20.bin` is 32,766 B, not 17 KB. The benchmark scripts generate it in `distinct` mode (20 senders to 20 recipients); 17 KB is the committed `ethrex_bench_4.bin`. `same` mode would be 16,811 B. Names the mode so the row is reproducible, and replaces "a handful" of trie nodes with the 40 accounts that mode actually touches. - "How to test" and the adopt-a-new-block steps omitted `make test-ethrex`, i.e. the only check that screens precompile 0x0a. This crate links a working c-kzg, so a block needing point evaluation passes here and fails in the guest — following the README alone would have missed it. - The validation list was labelled "cheapest first" but is ordered by the argument it builds, and the ~60s cold-build figure understated a ~335 package release tree. The per-test "milliseconds" claim is accurate (0.01-0.03s each) and is kept. Cache-absent failures now say to run `make ethrex-real-block-fixture` instead of surfacing a bare ENOENT from `unwrap()`.
Oppen
reviewed
Jul 31, 2026
Comment on lines
+35
to
+47
| # The `Crypto` impl the guest injects. Off riscv64 its keccak and ECSM paths both | ||
| # fall back to software, so this exercises the guest's *fallback* crypto semantics | ||
| # — not the accelerators themselves. | ||
| # | ||
| # Used only by the tests, but declared here rather than in `[dev-dependencies]` on | ||
| # purpose: it is the only thing that asks for k256's `expose-field`, and under | ||
| # resolver 3 a dev-dependency's features are not unified into builds that exclude | ||
| # dev-dependencies. As a dev-dep, `cargo run` (the `make ethrex-real-block-fixture` | ||
| # step) and the `cargo test` that follows it in CI resolve two different k256 | ||
| # feature sets, so k256 and every ethrex crate above it compile a second time in | ||
| # the same job. Confirm with: | ||
| # cargo tree -e features,no-dev -i k256 # must list expose-field | ||
| # cargo tree -e features -i k256 # must match the above |
Collaborator
There was a problem hiding this comment.
Wouldn't it make more sense to just set the k256 expose-field feature here?
Oppen
approved these changes
Jul 31, 2026
The real-block fixture from tooling/ethrex-real-block existed but nothing benchmarked with it: every perf measurement still ran on ethrex_bench_20, a synthetic block of 20 plain transfers. Measured on the same guest ELF, the two workloads are not the same benchmark: ethrex_bench_20 14.2M cycles 411 keccak 80 ecsm real block 168.3M cycles 9,046 keccak 44 ecsm so the synthetic block is ecrecover-bound where a real one is keccak- and trie-bound, and a prover change can move the two in opposite directions. Wire the real block in as the headline, gated exactly like the growth sweep — `/bench-real`, push to main, workflow_dispatch — and NOT on plain `/bench`. One real-block prove is ~15 min on the CPU bench runner against ~25s for the synthetic block, and that runner is single and serialized across all PRs. Because push-to-main always publishes a real-block number into the baseline artifact, `/bench-real` on a PR pays for its own side only. The synthetic block stays as the fast per-comment screen, relabelled as such; GROWTH_PROGRAMS stays synthetic because it plots heap against block size, which needs a family of blocks and cannot come from one real one. Continuations are forced for the real block rather than chosen: monolithic peak heap grows ~3.1 GB per million cycles on this workload family (2007 MB/transfer at R^2 = 0.998, from the current main baseline artifact), so 168M cycles would need ~500 GB. At epoch 2^21 it fits in ~16 GB. Supporting changes: - cli: report `Peak heap` from `prove --continuations`. The tracker only existed on the monolithic path, so the continuation path had no heap metric at all and the benchmark's parser would have failed on it. - Makefile: derive every real-block path from ETHREX_REAL_BLOCK_NETWORK + ETHREX_REAL_BLOCK, and expose `print-real-block-fixture`. Repointing the block is now those two lines; scripts and CI resolve the path from make instead of naming the fixture themselves. - benchmark-pr.yml: anchor the baseline-artifact greps. `real_time_s` contains `time_s`, so the existing unanchored greps would have matched two lines and written a multi-line step output. - benchmark-pr.yml: record which block was measured and refuse to diff against a baseline that measured a different one, so a fixture swap cannot masquerade as a regression. - bench_verify.sh / perf_diff.sh: WORKLOAD=synthetic|real, defaulting to synthetic. Both auto-generate the gitignored fixture when absent, and bench_verify.sh carries the prove flags in its proof-cache marker so a WORKLOAD switch cannot reuse the other workload's proof. - executor/.gitignore: ignore every accepted network's fixture name, not just hoodi's, so repointing cannot make a ~1 MB fixture committable.
…R gate Two changes with one root: the real-block fixture is a benchmark INPUT, and it was being treated as a build artifact and a correctness gate. Fetch it instead of building it. `make ethrex-real-block-fixture` now downloads the finished .bin from a pinned URL and verifies its sha256 before moving it into place, mirroring prepare-sysroot (same dual sha256sum/shasum handling, same hard error when neither exists). Off the fixture's path, and therefore deleted from it: the 3.8 MB ethrex-replay cache download, the converter build, the ~335-package ethrex host dependency tree, and the ETHREX_REPLAY_REV pin. That also unblocks the block we actually want. Upstream hosts an ethrex-replay cache for Hoodi and nothing else, so a mainnet block cannot be produced by the convert-locally route at all — generating its cache takes ~4 min and ~700 archive RPC calls. Fetching a verified binary makes the benchmark block independent of what lambdaclass happens to host, and independent of this crate's own pins, which stay on Hoodi because that is what conversion_is_reproducible reads. ETHREX_REAL_BLOCK_FIXTURE_URL is deliberately left EMPTY — the artifact is not hosted yet, and a guessed URL fails with a 404 nobody can act on. Unset, it fails with an actionable message, and every consumer degrades instead of breaking: benchmark-pr.yml warns and skips its real-block section rather than reddening every push to main, and the usability screen skips too. Adopting mainnet 25453112 once hosted is four Makefile lines plus REAL_BLOCK_FIXTURE in ethrex-tests; its digest and measured cost are recorded in the crate README. Take it out of the required gate. pr_main.yaml's test-executor job no longer generates the fixture, no longer builds tooling/ethrex-real-block, and skips both real-block tests. No product code reads this fixture: it has to be right when it changes, not on every PR, and it was costing every PR a network download plus a cold ~335-package build (blst, c-kzg, secp256k1-sys, malachite, ark-ff/asm) inside the branch-protection gate. Those checks are not dropped, they move to a new path-filtered workflow (.github/workflows/ethrex-real-block.yml) triggered by tooling/ethrex-real-block, tooling/ethrex-tests and the Makefile — i.e. by the things that can actually invalidate them. It runs the converter's tests (host-side parity, network rejection, reproducibility digest) and the block-usability screen (test_ethrex_real_block_native, which is what catches a block needing precompile 0x0a). no_kzg_backend_linked stays in the PR gate: it is a pure unit test and it is the property the usability screen depends on, so it should fail on the PR that breaks it. This supersedes part of 50dfca4: `tooling/ethrex-real-block -> target` is gone from test-executor's rust-cache workspaces, since that job no longer builds the workspace; the new job carries its own cache entry under its own key. `tooling/ethrex-tests -> target` stays and is still doing its job. Corrections while in here, all against a CURRENT guest ELF (counts move ~14% with ELF vintage, so every quoted figure now names its ELF): - synthetic 9.06M cycles, real 147.5M — a 16x ratio, not the 11.9x that came from comparing measurements taken on two different ELF vintages - monolithic peak heap ~4.9 GB/Mcycle from the measured growth fit, so ~700 GB for the real block rather than ~500 GB - continuation peak ~21 GB at epoch 2^21 (15.8 GB working set plus ~69 MB/epoch of accumulated proofs), not ~16 GB; the bundle is ~5 GB on disk and needs rkyv pointer_width_64 to serialize past 2 GiB - ~13 min per prove, not ~15 - bench_vs/run_ethrex.sh claimed ethrex_10_transfers is ~42M cycles and OOMs at 36 GB. It is 6.8M; the figure predates ecrecover becoming an ECSM accelerator.
…sured costs Follow-up to the fetch change. Two things. No block number appears outside the Makefile's four variables. Comments in benchmark-pr.yml and this README previously named a specific candidate block as the destination of a repoint, which would have gone stale the moment a different candidate won and read as if the wiring depended on it. Every figure that is a property of the block is now labelled "current default", and the repoint procedure shows placeholders rather than one candidate's values. Verified: no workflow, script or env var names a block. Recorded the measured cost of each candidate, since prove time is a property of the block and moves with the repoint. mainnet 25453112 is 125,932,956 cycles on the mid-July ELF against hoodi 1265656's 168,319,360 on the same ELF — ~110M vs 147.5M on a current one, so ~10 min and ~19 GB rather than ~13 min and ~21 GB. Cheaper despite the larger fixture: cycles per gas is not constant across blocks, so the earlier gas-ratio estimate for it was wrong. Costs are stated as ranges where they span the candidates.
Executes the documented repoint now that block selection is final. The chosen block was the only one in a 90-day Dune sweep matching the shape constraints (exactly 2 heavy txs, no whale tx, sane plain-transfer share) in the 1.6-2.6M gas band, so it is structurally typical rather than merely large. It is also the cheapest candidate measured, which matters because this workload sits on a single shared bench runner: ~65.6M cycles on a current guest ELF (74,819,518 on the mid-July one) against hoodi 1265656's 147.5M, i.e. ~6 min per prove rather than ~13, ~18 GB peak and a ~1.9 GB bundle. Note cycles/gas is not constant across blocks (~26-38 here), so ranking candidates by gas would have picked wrong: 25453112 carries MORE gas than the hoodi block yet costs ~25% fewer cycles. Wiring: unchanged. Nothing outside the Makefile's variables names a block, so the repoint is those variables plus the test constants that prove it self-consistent. Verified by grep that no workflow, script or env var mentions a block number. The cache is now fetched and verified like the fixture rather than pulled from raw.githubusercontent.com at a pinned ethrex-replay rev. That route only ever worked for the one block upstream publishes, which is what previously forced the converter's tests onto a different block from the benchmarks. Hosting both artifacts collapses that split: conversion_is_reproducible now pins the SAME block the benchmarks prove, so its digest assert covers the artifact in use. ETHREX_REPLAY_REV and its stamp are gone with it — this supersedes 05e0285, whose staleness guard is carried forward in a stronger form below. Fetch targets are now phony with an internal guard (the prepare-sysroot shape) instead of file rules. A file rule does not run when its target exists, so it could never catch the case that matters: an artifact already on disk that is stale after a re-upload under the same block number. The digest is now re-checked on every invocation, which also catches corruption and hand-placed copies. Validation, all run: - make test-ethrex-real-block-converter: 3/3 pass against the new constants. conversion_is_reproducible passing is the self-consistency proof — the cache, the block stats, MAINNET_CHAIN_ID and the digest all describe one block. - regenerating from the cache reproduces the fixture byte for byte (61eba49b...), so the handed-over .bin and the hosted cache agree. - test_ethrex_real_block_native and no_kzg_backend_linked pass: the block clears the guest's precompile surface with the KZG screen live. - fetch guard exercised: matching digest is a silent no-op, a tampered file is detected and reported, and an unset URL fails with an actionable message. Incidental: tooling/ethrex-tests/Cargo.lock gains ecsm's num-integer entry. That manifest has declared it all along and the detached lockfile was stale, so any build regenerates this and a --locked build in that workspace would have failed.
Repoints the benchmark fixture only. The converter's test constants stay on Hoodi 1265656 and do NOT move with it: what they exercise is the CONVERSION, which any real block demonstrates equally well, and Hoodi's is the one cache ethrex-replay publishes upstream — so pinning there costs us no hosting and cannot drift. The fetch is what makes that decoupling possible, and it means a future repoint touches the Makefile plus one constant in tooling/ethrex-tests, never this crate. That restores ETHREX_REPLAY_REV and its rev stamp for the converter's cache, so 05e0285 is no longer superseded. The benchmark block's own cache is fetched from our release alongside the fixture and read only by regen-real-block-fixture; the two caches are now separate variables (ETHREX_CONVERTER_CACHE vs ETHREX_REAL_BLOCK_CACHE) because they serve different purposes and move independently. The fixture URLs are live (bench-fixtures-v1 release), so the TBDs are filled in. The unset-URL guards stay: they now cover the window after a repoint but before the new artifact is uploaded, which is exactly when a push to main would otherwise go red over an upload nobody in CI can perform. Block: 2,428,684 gas, 29 txs, ~65.6M cycles on a current guest ELF (74,819,518 on the mid-July one), 10,478 keccak, 116 ECSM, ~6 min per prove, ~18 GB peak, ~1.9 GB bundle. Only block in a 90-day Dune sweep matching the shape constraints in the 1.6-2.6M gas band (2 heavy txs, no whale, sane transfer mix; heavy-gas 44.3%, top-tx 22.5%, p50 tx gas 41,297), and pre-FWA so transient-clean. Also the cheapest of the three measured candidates, which matters on a shared runner. Validation, all run end to end against the live release: - make ethrex-real-block-fixture from a clean slate: fetch -> checksum -> place, digest 61eba49b... matches. Same for both caches (release + upstream). - unset-URL path still reads correctly for the default block. - converter tests 3/3 against the Hoodi pins. - test_ethrex_real_block_native and no_kzg_backend_linked pass on the mainnet block.
The GPU benchmark ran the synthetic 100-transfer fixture; it now runs the real block, and unlike the CPU path the real block is the DEFAULT here rather than opt-in. The two paths differ because their scarce resource differs: /bench-gpu rents its own Vast box per run, so the cost is money and nothing queues behind it, while the CPU bench shares one runner with every /bench, /bench-abba and /bench-verify in the repo. Same reasoning, opposite conclusion. Synthetic workloads stay reachable through the existing `cont[TX]` / `mono[TX]` tokens with unchanged semantics, so every number recorded before this reproduces with the exact syntax that produced it. A new `real` token names the default explicitly. Implementation follows the pattern already used by bench_verify.sh and perf_diff.sh: scripts/bench_abba.sh gains WORKLOAD=synthetic|real (defaulting to synthetic, so CPU /bench-abba is untouched), resolves the fixture through `make -s print-real-block-fixture`, and fetches it when absent — which a rented box always needs, since it is a fresh checkout. That fetch is a ~1 MB URL + sha256 download, not a build, so it costs seconds of rental. WORKLOAD=real forces --continuations rather than offering it: monolithic would only OOM at this trace length. Deliberately NOT touched, per the GPU sweep's tuning: LAMBDA_VM_VRAM_BUDGET_MB, mempool settings, EPOCH_SIZE_LOG2, K, and the offer query's values. Changing any of them silently would break comparability with the sweep that chose them. No GPU duration is asserted anywhere, because none has been measured — the CPU rate does not transfer, and the sweep's finding that the prover is CPU-bound at the serial producer above epoch 2^21 cuts against assuming a large speedup. The "running…" comment now says the real-block runtime is unmeasured and that the run establishes the baseline, instead of quoting the synthetic workload's ~2.5 hr. Two things checked rather than assumed: - The box RAM floor stays 48 GB and needs no change. Continuation peak heap is set by the epoch size, not the block, so the real block costs the same ~10 GB working set at the default epoch 2^20 plus ~4.6 GB of accumulated epoch proofs — ~14 GB, or ~18 GB at 2^21. The comment now records that instead of leaving the next reader to re-derive it. - The rkyv pointer_width_64 fail-fast is now scoped to synthetic >=40tx. It exists because a large synthetic continuation bundle exceeds the old 2 GiB cap; the real block's bundle is ~1.9 GB, so applying the guard to it would reject PR branches over a limit they do not reach. This workflow never checks the repo out (it orchestrates over the Vast API and SSH), so the workload label is a fixed string rather than a make query; bench_abba.sh prints the resolved fixture path on the box and that lands in the run log.
…anual The knob exists (WORKLOAD=real, which benchmark-gpu.yml drives) but the CPU ABBA workflow keeps its synthetic default: 20 pairs x 2 proves x ~6 min is ~4 hours on the one shared bench runner that every other bench queues behind. Documented as an option to reach for deliberately rather than left as an undocumented capability.
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.
What this became
Started as review fixups for #880; grew into the wiring that PR was missing. #880 built a validated real-block fixture but nothing benchmarked with it — every perf number still came from the synthetic 20-transfer block its own motivation section calls unrepresentative. This branch closes that gap and swaps the block for a measured, screened mainnet one.
The block: mainnet 25368371
2,428,684 gas, 29 txs — selected via Dune sweeps against measured mainnet composition (baseline: 3.5M txs + a 6-month weekly profile). It is the only block in 90 days in the 1.6–2.6M gas band with 2 heavy txs, no whale tx (top tx 22.5% of gas), and a mainnet-like transfer mix (heavy-gas 44.3% vs mainnet ~39%, p50 tx gas 41,297 vs ~38k, tx/Mgas 11.9). Verified: executes statelessly to its post-state root on the guest's precompile surface (no KZG 0x0a — the no-KZG screen was live during the check), free of transient-contract dependence (predates the FWA lottery deployment), 1 EIP-7702 authorization.
ethrex_bench_20(synthetic screen)* Jul-15 guest ELF; counts move ~14% with ELF vintage. Cycles/gas is not constant (26–38 across measured blocks) — candidates must be measured, not extrapolated from gas. ECSM = exactly 4 × signature recoveries (116 = 4×29), confirming the per-ecrecover cost model.
Benchmark tiers
/bench— unchanged: synthetic screen, 3+5 sampled runs, ~25 s/prove. Sensitive to ~1.5% regressions./bench-real— new: the real block, 1 run, ~6 min. Push-to-main publishes the baseline, so a PR pays for its side only.benchmark-pr.ymlrecords which block it measured and refuses to diff across a repoint./bench-gpu— the real block is now the default workload (golden target). Synthetic reachable via the existingcont[TX]/mono[TX]tokens; GPU env/tuning untouched. Real-block GPU runtime is deliberately undocumented until the first run establishes it (prior: the GPU prover was CPU-bound at the serial producer above epoch 2^21, so no speedup is assumed).bench_verify.sh/perf_diff.sh/bench_abba.sh:WORKLOAD=synthetic|real, defaulting to synthetic.Continuations are mandatory for real blocks, not tuning: monolithic peak heap grows ~3–5 GB/Mcycle (measured, R²≈0.998) — ~330 GB for this block.
prove --continuationsalso now printsPeak heap(it printed nothing; the benchmark parser would have hard-failed).Fixture mechanism
Fetched by URL + sha256 (the
prepare-sysrootpattern:.tmp+ verify + move, digest re-checked every invocation), from thebench-fixtures-v1pre-release — both fixtures plus their ethrex-replay provenance caches, all digest-pinned. No binaries in git. The converter (tooling/ethrex-real-block) moves off the per-PR path into a path-filtered workflow (ethrex-real-block.yml); its ~335-package build, cache download and rev pin leave the required gate.no_kzg_backend_linkedstays in the gate (microseconds, and it's the property the usability screen rests on). Converter pins stay on Hoodi — the one upstream-hosted cache — decoupled from the benchmarked block; repointing the benchmark is five Makefile lines + one test constant.Review fixups (original scope)
ETHREX_REPLAY_REVbump silently kept converting the old cache (now guarded).expose-fieldcame only from a dev-dependency, socargo runandcargo testresolved different feature sets and rebuilt the ethrex tree twice per job.HOODI_CHAIN_ID → MAINNET_CHAIN_IDflip during the repoint instead of letting a substituted config hide behind a regenerated hash.ethrex_bench_20is 32,766 B indistinctmode, not 17 KB; rkyv note; "cheapest first" ordering).Known items
/bench-abba's escalation text points from a 20-tx monolithic result to a 100-tx continuations tiebreaker — pre-existing mismatch, flagged not fixed.tooling/ethrex-tests/Cargo.lockgains one line (ecsm/num-integer): pre-existing staleness;cargo build --lockedin that workspace currently fails on main. Separable on request.make test-ethrex's sysroot step wasn't runnable locally (sudo); the real-block screen ran via the exact command CI uses.How to test