Skip to content

Bump ethrex to 26.0.0 and rebuild the benchmark workload - #983

Open
jotabulacios wants to merge 56 commits into
mainfrom
chore/bump-ethrex-26
Open

jotabulacios wants to merge 56 commits into
mainfrom
chore/bump-ethrex-26

Conversation

@jotabulacios

@jotabulacios jotabulacios commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Bump ethrex to 26.0.0 and rebuild the benchmark workload

Motivation

The pin moves from 797df554 (23.0.0) to 8effcb06, a commit on ethrex main declaring
26.0.0. The bump is mechanical; what it breaks is the benchmark workload.

execution-specs #3278 replaced the guest's input with the spec's SSZ SszStatelessInput,
prefixed by a two-byte schema id that is the fork declaration (0x1501 = Amsterdam)
rather than a version tag on a container. A pre-Amsterdam block is not representable, so the
retired fixture cannot be re-serialized — and on the new guest it does not fail, it
benchmarks nothing: 496 cycles, exit 0, because a decode failure commits an all-zero
result. A stale fixture reports −99 % prove time as a green win. Closing that is most of
this PR.

Description

The fixture is generated, not fetched. tooling/ethrex-fixtures --bin real_block
rebuilds a real mainnet block as an Amsterdam block: it installs the pre-state by hashed
key
, so no key preimages are needed (132 accounts, 261 slots, 132 codes), registers a
parent header at the real height, replays the transactions in block order, and validates the
result through the guest before writing. Nothing is left to publish; the only fetched input
is the block's replay cache, which is fork-independent and pinned by sha256.

The block is mainnet 25453112, picked by a screen of twelve real blocks on cycle weight:
37.1M against the 30.5M the retired fixture costs on today's guest (+22 %), where the next
block up is +197 %. The generator is not universal — 25087563 fails with
StateRootMismatch — so screening is required before pinning a different block.

Four guardrails, all aimed at work that disappears quietly:

  • the fixture is pinned by sha256 and verified where it is used, not only where it is
    produced: generation only runs when the file is missing and the bench runner is persistent,
    so otherwise a fixture built before a rev bump survives it;
  • bench_abba.sh executes the workload before the batch and requires >1M cycles, which
    turns those 496 cycles into a hard error;
  • the generator refuses to write a fixture whose transactions were not all applied — a block
    with fewer transactions is still valid, so nothing else would have caught it;
  • likewise for a witness leaf that yields no 32-byte trie key, which would silently drop an
    account or slot.

Also: pins moved together by scripts/set_ethrex_rev.sh; the four synthetic fixtures
regenerated (SSZ is 26-38 % smaller than rkyv); docs and thresholds re-pinned; the GPU arm's
cpu_ram floor 48 → 64 GB, because 44.81 GiB is 98 % of a 48 GB box.

tooling/ethrex-fixtures/README.md carries the rest: what the workload costs, and why the
reverts below are the fork's doing rather than the reconstruction's — rebuilt under Osaka the
retired block reverts 2 transactions where Amsterdam reverts 12. What this fixture is, is a real-mix Amsterdam
block; what it is not, is a replay of mainnet economics.

Measured

before after
cycles 50.78M 37 137 748
keccak / ecsm 10 478 / 116 6 003 / 164
prove (continuations, 2^22) 158.8 s 125.33 s (σ 1.58, CV 1.26 %, n=14)
peak RSS ~52 GB 44.81 GiB
proof bundle 1.2 GB 790 MB
ABBA pair 5.3 min 4.2 min

Verified, not assumed:

  • the workload moves only with the pin — measured on the bench runner against one
    variable: the same tree, re-pinned and rebuilt, reports 37,137,386 cycles at the previous
    pin and 37,137,748 at this one. The +362 cycles (0.001 %) are therefore the ethrex crates'
    version metadata — no .rs file in the guest's dependency graph differs between the two
    revs and no crates.io dependency moved. Two separate builds at the previous pin, from
    trees before and after the merge with main, produce different ELF binaries (dc38c029
    and 12a692b3) and the same 37,137,386 to the cycle: the ELF bytes move with things the
    count does not, and rebuilding at this pin reproduces its binary byte-for-byte;
  • the fixture is byte-identical — 549,144 bytes, sha256 08a52e10…, regenerated from a
    clean clone on the runner at both pins with the same installed state root, which is what
    makes pinning its digest an invariant rather than a snapshot;
  • the prove spread is contention, not the prover — wall time and the CPU share a prove
    obtains correlate at −0.98, and two proves that overlapped another job on the shared runner
    took +73 % while producing byte-identical bundles. Gated to the proves that got the most
    CPU the CV is 0.34 %; ungated 1.26 %. bench_abba.sh now reports that share per prove.

Why nothing here compares to a pre-bump number

Four things moved at once, and they are not independent:

  • the input format, which puts new work inside the measurement. rkyv was zero-copy —
    cast the buffer in place, deserialization cost nothing. SSZ has to be parsed, and the spec
    requires committing to the payload's hash_tree_root: a sha256 Merkleization over the
    whole payload, on every block. We override keccak256 and secp256k1_ecrecover and
    nothing else, so that sha256 runs as a software default;
  • the fork, which changes how the same transactions execute — cold account access
    2600 → 3000 and EIP-8037 drawing state gas from the excess gas limit, so 10 of this
    block's 38 transactions run out of gas and stop early;
  • the block, changed deliberately;
  • the guest, three releases of levm, trie and the new EIP-7928 block-access-list
    validation.

They cannot be separated, because they are chained: the format change forces the fork — the
schema id is the fork declaration, so no SSZ input can say "pre-Amsterdam" — and the fork
forces the block, because a pre-Amsterdam block under Amsterdam rules is a degraded
workload: 25368371 rebuilds to 20.36M cycles against the 30.50M the retired fixture costs on
today's guest. There is no experiment that holds three fixed and moves one, so the delta has
no valid decomposition. Continuity was not available at any price.

One comparison is apples-to-apples, and it is the one the block was chosen with: hold the
guest fixed and vary only the workload. The retired fixture costs 30.50M cycles on today's
guest, and this block costs 37.1M — the +22 % quoted above. (The 50.78M in the old workflow
comments is not that reference; it was measured on an older ELF.)

Comparison within a PR is unaffected — both sides prove the same ELF and the same fixture
— and better protected now, by the digest pin, the cycle floor and the exclusivity line.

How to Test

make ethrex-real-block-fixture      # fetch cache, generate, verify digest
make regen-real-block-fixture       # forced; must come out byte-identical
make check-ethrex-fixture-checksums && node scripts/render_bench_comment.js
cd tooling/ethrex-tests && cargo test --release -- --include-ignored --skip test_ethrex_real_block_vm
make test-ethrex-real-block-converter

cli execute should report 37 137 748 cycles / 6 003 keccak / 164 ecsm, and a
prove/verify round trip at --epoch-size-log2 22 should pass. The verify output ends
| 01 | 0100000000000000 | 0115successful_validation = 1, chain_id = 1,
schema_id = 0x1501 — so the proof itself carries the evidence that an Amsterdam block was
validated, which is the direct antidote to the 496-cycle failure.

jotabulacios and others added 26 commits August 3, 2026 16:31
The rkyv layout moved with the ethrex rev, so the block's fixture did too, and
the release now carries both: the pre-bump bytes under the original name and the
post-bump ones under a name carrying the rev.

Uploading under a new name rather than replacing the asset keeps `main` working
while this branch is open — its Makefile still pins the pre-bump sha256, and a
replaced asset would fail that checksum on every push, taking the published
benchmark baseline with it.
The Makefile fetches ethrex_mainnet_25368371_4f658c2b.bin (the suffixed
upload keeps main's pre-bump checksum working while the branch is open),
but the converter README's asset table still listed the un-suffixed name
with the post-bump checksum next to a 'Re-upload pending' note that the
suffixed upload made moot, and executor/tests/README.md named the
un-suffixed asset with the post-bump byte count. Also drop the dangling
ETHREX_BUMP_CYCLE_RESULTS.md reference — the numbers are quoted inline.
Conflict in crypto/ethrex-crypto/Cargo.lock resolved by regenerating the
lockfile from the merged Cargo.toml (cargo update); the crate's 24 tests
pass and the workspace checks clean.
feat/lambdavm-prover-backend branch and onto ethrex main, and stop declaring the
lambdavm feature.

Requiring that feature is what tied the guest to the backend branch, since it
exists nowhere else — and that branch is 69 commits behind main and has not moved
since 2026-08-04, so waiting for it to merge was costing real performance. On the
real benchmark block the guest goes from 34,241,608 to 30,498,818 cycles and from
120.715 s to 110.259 s of proving time (median of 3, spread under 0.8%), dropping
from 9 epochs to 8; against main's pin the cycle count falls 22.9%.

No feature replaces it. ethrex's per-zkVM features are not backend selectors —
each is only a list of optional crypto dependencies, and none gates any method of
the Crypto trait. Measured against the sibling feature activating the widest
dependency set, ethrex's precompile stress fixtures come out identical to the
cycle (stress_modexp_150M 6,373,285,966 and stress_alt_bn128_150M 22,986,061,145
both ways) with a smaller ELF. What makes this guest LambdaVM never travelled
through a feature: lambda-vm-syscalls, lambda-vm-ethrex-crypto and the
riscv64im-lambda-vm-elf target are direct, and every run still reports its 116
ECSM and 10,659 keccak precompile calls.

797df554 is the last commit where this is only a rev change. b5271885 rewrites the
guest entry point to run_stateless_guest over SSZ input, which needs a new shim
and a new fixture format; and between 797df554 and that boundary there is no
perf(levm) work, while the two commits that matter (#7105, #7104) are already in.
Release tags do not help — v24.0.0 predates both and is diverged from main,
because ethrex cuts releases from a side branch.

The rkyv ProgramInput layout moved with the rev, so every committed fixture is
regenerated and the converter's reproducibility digest updated, as that test asks
for on a legitimate bump. scripts/set_ethrex_rev.sh moves all 11 pins together:
a guest and tooling on different revs do not fail to build, they produce a fixture
the guest silently misreads.

The real-block fixture asset for the new rev still has to be uploaded to the
bench-fixtures-v1 release before CI can fetch it.
Since execution-specs #3278 the guest takes the spec's schema-prefixed SSZ
statelessInputBytes instead of an rkyv ProgramInput, with the fork coming from
the schema id — ethrex 25 implements one, 0x1501 (Amsterdam) — so the guest, the
converter, the fixture generator and the host-reference tests move to that
format and the four synthetic fixtures are regenerated.

The benchmark fixture becomes generated rather than fetched: mainnet has no
Amsterdam fork, so no hosted artifact for that block can be valid, and the old
one now runs 496 cycles with successful_validation = 0 instead of failing.
real_block rebuilds mainnet 25368371 as an Amsterdam block from its replay cache,
and bench_abba.sh refuses to measure a workload the guest rejected.
…load

Every figure carrying seconds or bytes in the bench workflows described the
retired pre-Amsterdam block: 158.8 s per prove, 50.78M cycles, ~52 GB of peak
heap, ~5.3 min per ABBA pair. The workload is now 20,360,647 cycles and 69.10 s
at epoch 2^22 — mean of 10 proves on the bench runner, sd 0.41 s, CV 0.59%,
35.75 GiB peak, 466 MB bundle — so the durations the bot quotes, the ABBA pair
table, the epoch trade-off and the run-count dial are re-measured rather than
inherited, and the pre-bump figures are labelled as having measured a different
fixture. The three deterministic counters are stated as such, since they are
what tells a reader the workload is the one the job thinks it is.

Two defects the re-read surfaced. The fixture's file rule depended on a phony
target, so it regenerated on every invocation instead of only when missing. And
the epoch and cost tables the converter's README used to carry went away when it
was rewritten for SSZ, leaving seven references pointing at nothing; they now
live in tooling/ethrex-fixtures/README.md, next to the generator that produces
the workload. pr_main's justification for the detached ethrex-tests workspace
also still cited rkyv's `unaligned` feature, which the bump removed.
…ilt equivalent

Under Amsterdam, 25368371 loses so much of its gas to the new state-gas model
that the workload it rebuilds into is 20.36M cycles against the 30.50M the
retired fixture costs on today's guest — a third lighter than what /bench used to
prove. Screening twelve real mainnet blocks (both release caches plus ethrex's
curated zkevm_bench corpus) through the generator put 25453112 closest at 37.14M
cycles, +22% against that target, where the next candidate up lands at +197%.
Its cache is already hosted in bench-fixtures-v1 with a published digest, so the
repoint publishes nothing: block number, cache URL and sha256 in the Makefile,
and the fixture name the usability screen reads.

Reverts are not a selection criterion. Every pre-Amsterdam block loses 26-50% of
its transactions to the fork, which is a property of the fork rather than of any
block, and several of the twelve end up consuming more gas than they did on
mainnet because Amsterdam makes the surviving transactions dearer.

Measured on the bench runner at epoch 2^22, five proves, box idle: 125.17 s mean,
sd 0.71 s, CV 0.57%, 43.00 GiB peak, 790 MB bundle, 12.0 s to verify. Every
seconds-or-bytes figure in the bench workflows moves with it, as does what three
runs can resolve (~2.00%).

One behavioural change comes with it: the GPU arm's cpu_ram floor goes from 48 GB
to 64. At 43.00 GiB the peak is 96% of a 48 GB box, which is an OOM waiting for a
rental with a hungrier allocator; 64 GB restores the headroom the old floor had
and matches the CPU runner's. It costs offer availability.
…cache its

build. The usability screen now builds the fixture with tooling/ethrex-fixtures instead of
downloading it, so a change to real_block.rs or genesis.json could land without the job that
validates it ever running, and the ethrex host tree it compiles was outside the rust-cache
workspaces list.
…omment's diagnostic

at ETHREX_REAL_BLOCK_CACHE_URL, which is the variable that actually gates the real-block
step now that the fixture is generated instead of fetched: anyone following the old
message would have gone looking at a variable that is permanently empty and read by
nothing.

Limit the minimum-cycle floor in bench_abba.sh to WORKLOAD=real. The synthetic fixtures
are already gated twice, by their committed checksums and by the successful_validation
assert in the host tests, while a small TX_COUNT lands near the floor legitimately -- a
one-transfer block ran 1.80M cycles before the bump and an empty one 0.99M -- so applying
it there would reject valid runs.

Correct three comments the rebuild left stale: the EEST probe is committed rather than
untracked, the real-block fixture is produced by tooling/ethrex-fixtures rather than by
the converter, and the real block costs 37M cycles rather than 20M. Record in the Makefile
that the fixture name is pinned in the host tests as well, so repointing the block needs
that edit too; a glob there would silently pick up a stale fixture from an earlier block
and pass.
…less work than the

block it claims to reproduce. Two paths could shrink it silently, and neither was caught
by any existing guard, because both leave a block that is perfectly valid: the guest still
commits successful_validation = 1, so the native-validation check passes and the benchmark
reports the missing work as an improvement.

The first is a dropped transaction. build_payload_t8n returns the ones it could not apply
and the generator only printed them; the transaction mix is the entire reason this fixture
exists over a synthetic block, so applying fewer than all of them is now an error.
Screening candidate blocks does need the partial ones reported rather than refused -- the
twelve-block screen saw shares from 2/3 to 557/676 -- so REAL_BLOCK_ALLOW_DROPS=1 keeps
that usable.

The second is a witness leaf that does not yield a 32-byte trie key. collect_leaves
skipped those without a counter, which loses an account or a storage slot; the transaction
reading it then sees zero and reverts, and the only symptom is a revert count that is
already non-zero and expected. It now returns how many it could not use and the caller
aborts. The printed installed-versus-collected counts could never have caught this: both
are derived from collect_leaves' output, so they agree by construction.

Also record in the README that reverting and being dropped are different things, since the
revert discussion there reads as if a lost transaction were normal.
…r. They arrived with

the file and no CI job would have reported them: `make lint` only covers the root
workspace, and tooling/ethrex-fixtures is a detached one, so the ethrex host tree it pulls
stays out of the main build. Checked with cargo check --all-targets in the three detached
crates the branch touches; this was the only warning in any of them.
…e fixture is used, not

only where it is produced. The variable was already there and empty; filling it closes the
one path by which a benchmark could measure the wrong workload without anything going red.

Generating the fixture validates the block through the guest, but generation only runs when
the file is MISSING -- deliberately, so that a benchmark does not pay for a cargo build --
and the bench runner is persistent. A fixture built before a rev bump therefore survives
it, and every benchmark keeps proving the old workload. The minimum-cycle floor in
bench_abba.sh cannot see it either: a stale fixture still runs tens of millions of cycles,
and both sides of an A/B read the same file, so the delta looks healthy while the absolute
numbers belong to a different block. The Makefile comment claimed a stale fixture could not
reach a benchmark; that claim is now true rather than aspirational.

Hashing 549 KB is free and only a mismatch pays for a rebuild, so the original reason for
not re-checking on every invocation is preserved. On mismatch the fixture is regenerated
and re-checked, which self-heals the stale case; if the fresh output still differs, the
build stops and says that the inputs or the output format moved and the recorded numbers no
longer describe the workload. An empty digest disables the check and announces it, which is
the state right after repointing the block and before the new digest is known.

Safe to pin because the generator is deterministic across machines, not just across runs:
the same 549,144 bytes and the same 08a52e10 digest come out on macOS arm64 and on the
Linux x86-64 bench runner, from the same installed state root 0xce4c19cd.
…benchmark harness

publishes about its own resolving power. The five-prove figure it was pinned to, CV 0.57%,
came with a 95% interval of [0.34%, 1.64%]; the fuller sample lands at 1.26%, near the top
of that interval, so the pair table was promising roughly twice the resolution it can
deliver -- 12 pairs resolve ~1.0%, not ~0.51%.

The reason is not prover jitter. On this runner wall time and the share of CPU a prove
actually gets correlate at -0.98 across a ten-prove sweep, perfectly monotonic: 7583% of a
CPU takes 123.98 s and 7318% takes 129.61 s. Two proves that overlapped another job on the
box got 4769% and 4687% and took 215.58 s and 220.03 s, +73%, while producing
byte-identical proof bundles -- nothing but the clock moved. Gated to the proves that got
the most CPU the same sweep has a CV of 0.34%, which is better than what was published.
So the number was never wrong so much as conditional on something nobody recorded.

bench_abba.sh now records the CPU share of every prove and prints an exclusivity line that
flags any prove under 90% of the batch's best. The threshold is relative because the script
also runs on rented 16-32 core GPU hosts where an absolute percentage means nothing, and it
flags rather than discards because the ABBA pairing already absorbs most of the effect --
what a flag means is that the batch's spread must not be read as prover noise. The box is
shared and everyone logs in as the same account, so contention cannot be attributed after
the fact; it has to be recorded while measuring.

The pair table now carries both conditions instead of bracketing an unknown, and the peak
RSS figure moves from 43.00 GiB to the measured maximum of 44.81 GiB. That last one
tightens an argument rather than weakening it: 47.0 GB is 98% of a 48 GB box, so the GPU
arm's floor was already past the point of no headroom, not near it.
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Low — Conflicting fixture tests. eest_amsterdam_probe.rs:29 includes every .bin, including legacy_rkyv_real_block.bin, and asserts successful validation. The second test requires that same file to fail validation. Exclude the legacy fixture from the positive test or store it separately.

  • Low — Retired converter remains unused. ethrex-block-converter/src/main.rs:136 now rejects the supplied replay cache, while fixture generation has moved to real_block. Its remaining tests exercise only rejection, yet the PR retains the converter CLI, dependencies, and dedicated CI job. Remove this unused conversion path and its obsolete publishing instructions.

Static review only; no builds or tests run.

Comment thread tooling/ethrex-fixtures/src/bin/real_block.rs Outdated
Comment thread tooling/ethrex-block-converter/src/main.rs
@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Review

Read the whole diff (Makefile, scripts, workflows, the new real_block generator and shared SSZ encoder, the converter rewrite, the guest and test changes). The bump itself looks sound, and the four guardrails do close the 496-cycle hole. Four findings, one of which will break the GPU bench arm.

High

scripts/bench_abba.sh — the backgrounded fixture build is killed on the success path. cleanup() now also kills FIXTURE_PID, but cleanup is called explicitly right after both prover builds succeed (the unchanged cleanup / trap - EXIT pair after build_cli "$SHA_A" cli_A), not only from the EXIT trap. So whenever need_build=1, the fixture build gets SIGTERMed as soon as the two cargo builds finish; wait "$FIXTURE_PID" then returns 143 and the run aborts with ERROR: the real-block fixture could not be built. It survives only when the fixture build happens to finish first — i.e. it works on the warm bench runner and fails on a cold rented GPU box, which is precisely the case the backgrounding was added for. Inline, with a suggested split.

Medium

tooling/ethrex-block-converter is now dead weight, and is a third copy of the encoder. Nothing in the repo invokes it after this PR (regen-real-block-fixture moved to ethrex-fixtures --bin real_block), yet ETHREX_REAL_BLOCK_CACHE still points into its caches/ directory and ethrex-block-converter.yml still pays its documented ~335-package cold build — to run two rejection asserts. Its stateless_input_from_cache duplicates build_stateless_input almost line for line, including empty_execution_requests(), which is the duplication src/lib.rs own doc comment says it was extracted to end. Either delete the crate or have it call build_stateless_input by path. Inline.

Low

  • legacy_cache_is_rejected_without_amsterdam_fields passes vacuously when the gitignored cache is absent — it accepts a bare No such file as the rejection. The old .expect(CACHE_MISSING) failed loudly instead. Inline.
  • output.len() != 43 || output[32] == 0 is repeated in four files with nothing naming the 43 or the 32. src/lib.rs is the natural home. Inline.
  • tooling/ethrex-fixtures/README.md points at ETHREX_BENCH_WORKLOAD_AFTER_BUMP.md in the repository root ("has both sets measured side by side"), which exists neither in this PR nor on main.

Checked, no issue

  • cfg(not(target_arch = "riscv64")) on the keccak_hash import matches its only use site (crypto/ethrex-crypto/src/lib.rs:64).
  • collect_leaves recursion is trie-depth bounded; the unusable-leaf counter is accumulated across both the state and storage walks and checked before anything is written.
  • The generator determinism holds: codes_by_hash is a HashMap but only read by key, and the leaf order comes from the trie walk.
  • set -euo pipefail interactions in the new bench code: ${FIXTURE_PID:-} is guarded everywhere it is killed, and || workload_cycles="" correctly keeps the pipefail failure inside a compound so the diagnostic can print.
  • scripts/set_ethrex_rev.sh already covers every manifest this PR touches; executor/.gitignore /tests/ethrex_mainnet_*.bin still covers the repointed block.
  • The GPU arm cpu_ram 48 to 64 GB is well justified by the 44.81 GiB peak; the availability trade-off is called out in the comment.

I have not built or run anything — CI covers that.

`cleanup` is not only the EXIT trap: it is called explicitly once both prover binaries are
built, so folding the fixture job's `kill` into it SIGTERM'd the background build the moment
the cargo builds finished, and the `wait` below then read 143 and aborted the run with an
empty log. It survived only when the fixture happened to finish first — which on a freshly
rented box, where that build is the long one, it does not.

`cleanup` now removes the worktree and nothing else, and the kill lives in `kill_fixture`,
named once and installed as the EXIT trap in all three states: while the fixture builds,
while the provers build (`cleanup; kill_fixture`), and again after the successful build,
where `trap - EXIT` used to leave the window between there and the wait uncovered.
`caches/` is gitignored and fetched on demand, and the test accepted a plain
`No such file or directory` as if it were the Amsterdam rejection, so on a clean checkout it
went green having asserted nothing — and `contains("os error")` was broad enough to swallow
a permissions failure or a truncated download too. It asserts the cache is there first, with
the same message `unmappable_network_is_rejected` already uses, and then asserts the
rejection is the Amsterdam one.
The Makefile's header for the workload still introduced both artifacts as "FETCHED rather
than built", and justified it with a paragraph about keeping the ethrex host dependency
tree off the path of everyone who just wants to run a benchmark — which is exactly what a
checkout without the fixture now pays, and what the backgrounding in bench_abba.sh exists
to hide. It now separates the two: the cache is fetched because it is fork-independent, the
fixture is generated from it because the schema id is a fork declaration and a hosted one
would have to be rebuilt and published by hand on every rev bump. The cost of that choice
is named rather than implied.

`benchmark-pr.yml` said the same thing about the fixture while guarding on the CACHE url,
which is what it actually needs. And the line saying the converter is the regeneration tool
for rev bumps is gone: `ethrex-fixtures --bin real_block` is.
`ETHREX_BENCH_WORKLOAD_AFTER_BUMP.md` exists in neither this branch nor `main` — it is a
local report — so the sentence sent the reader to nothing. It now says what the EEST
fixtures are and where they come from, which is the part that was worth saying.
`ethrex-fixtures/src/lib.rs` was added in this branch to end the copy between the two
generators, and the converter kept a third copy of the same ~90 lines — the one whose
success path has no automated coverage, so a divergence on the next rev bump would surface
only when somebody ran it by hand. The encoder is now `tooling/ethrex-ssz-input`, a
lib-only crate both depend on by path; what stays in each caller is where the block comes
from, which is what actually differs.

The shared crate carries nothing extra: `ethrex-common`, `ethrex-guest-program`,
`ethrex-rlp` and the two libssz pins, all of which both consumers already had. Having the
converter depend on `ethrex-fixtures` directly would instead have pulled in `ethrex-l2-rpc`
— the one dependency of the generators its graph does not already carry.

It also absorbs the output layout, which was written out as `output.len() != 43 ||
output[32] == 0` in three files with nothing naming either number. `validate_natively` does
that check, `GUEST_OUTPUT_LEN` and `VALIDATION_FLAG` name the fields, and each call site
keeps its own message. `tooling/ethrex-tests` keeps its two asserts: they distinguish a
wrong length from a rejected block, which a single call would flatten.

Verified rather than assumed: the fixture regenerates byte-for-byte identical (sha256
`08a52e10…`, the digest the Makefile pins), the converter's two tests pass against the real
Hoodi cache, and both crates build under `--locked`, which is how CI builds them.
`scripts/set_ethrex_rev.sh` and the path filters of both workflows now name the new
manifest, so a rev bump re-pins it and a change to it still triggers the jobs that build it.
@jotabulacios
jotabulacios marked this pull request as ready for review September 14, 2026 19:31
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Medium — Background fixture builds survive cleanup. scripts/bench_abba.sh:150: kill_fixture signals only the background make PID. Its recursive make, Cargo, and compiler processes are not reliably terminated. If a prover build fails while fixture generation is running, those processes can keep consuming CPU and memory on the shared runner, distorting the next benchmark. Run fixture generation in a dedicated process group and terminate and reap that group during cleanup.

Static review only; no builds or tests run.

The comment claimed the cache was seeded with "the ancestor hashes the witness carries",
and the code inserted exactly one: the parent. A transaction reading BLOCKHASH at any of
the other 255 depths would have seen zero where mainnet gave it a hash, and the fixture
would have executed something other than what it claims to reproduce, with nothing to say
so. The witness already carries the headers — they are decoded a few lines above to build
the execution witness — so the cache is built from them, with the local head overwriting
the real ancestor at its own height because that is the block this one builds on.

Nothing about the current fixture moves: regenerated, it is byte-for-byte the pinned
`08a52e10…`, which says no transaction in this block reads BLOCKHASH past its parent. The
seeding is there for the next block someone pins. A disagreement with the guest could not
pass silently either — the generator validates the result through it, and a BLOCKHASH the
guest resolves differently fails the post-state-root check.
The doc on `GUEST_OUTPUT_LEN` called the first 32 bytes a state root. They are
`new_payload_request_root`, the payload request's `hash_tree_root`: ethrex's
`SszStatelessValidationResult` is `new_payload_request_root || successful_validation ||
chain_id || schema_id`, in that order. The post-state root is checked inside
`validate_stateless_execution`, whose verdict is the flag this module reads, so the flag is
the whole answer and the output carries no root to compare against the block.
`kill "$FIXTURE_PID"` signalled the `make` and nothing under it. Its cargo and rustc
children were reparented and kept compiling, which on the shared bench runner is precisely
the contention the next batch would have measured — the failure path was leaving behind the
load it existed to prevent. Reproduced on a harness that mirrors the launch: signalling the
parent alone left the child running; killing the group took both down.

The job now leads its own process group (`set -m` around the launch, off again immediately,
since job control also changes how the shell signals and reports jobs) and `kill_fixture`
signals the group, falling back to the bare PID, then reaps.
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Medium — Stale synthetic fixtures can silently benchmark rejected input. scripts/bench_abba.sh:207 excludes synthetic workloads from validation, but existing generated files such as ethrex_5_transfers.bin survive this migration and are reused whenever present. With the new guest ELF, their old rkyv input exits successfully after roughly 496 cycles, producing misleading benchmark results. The cited checksum/tests cover committed fixtures, not arbitrary TX_COUNT files. Regenerate cached synthetic inputs on format changes or check successful_validation before measuring.

Comment thread scripts/bench_verify.sh
Comment thread tooling/ethrex-block-converter/src/main.rs
Comment thread tooling/ethrex-fixtures/Cargo.toml Outdated
Comment thread tooling/ethrex-fixtures/src/bin/real_block.rs Outdated
@claude

claude Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Review: ethrex 26.0.0 bump + rebuilt benchmark workload

Read the whole diff: guest entry point, ethrex-ssz-input, real_block.rs, both fixture generators, the converter, Makefile, bench scripts and workflows. No safety or memory-safety issues — no new unsafe, no unbounded allocation, and every new error path returns Err rather than panicking (collect_leaves is bounded by trie depth, and the malformed-leaf count is refused before anything is written). The four guardrails are well chosen, and refusing a fixture whose transactions were not all applied is the right call — that failure genuinely has no other symptom.

Four findings, all posted inline:

Medium

  • The cycle floor guards one consumer out of four. bench_verify.sh, perf_diff.sh and benchmark-pr.yml prove the real block after only the digest check, which catches a stale fixture but not a guest whose rev moved without the generator's — precisely what set_ethrex_rev.sh --guest-only does by design. In that state all three happily measure a rejected input.

Low

  • ethrex-block-converter is now a dead path: no published cache can get past its Amsterdam checks, both remaining tests assert rejections, and it still costs a PR-blocking ~335-package build. Delete it, or take the workflow off pull_request.
  • Unused deps: libssz/libssz-types in ethrex-fixtures and ethrex-block-converter, libssz in ethrex-tests — only ethrex-ssz-input uses them. The ethrex-tests one does pin a version; it should say so.
  • real_block.rs keeps genesis.json's cancun+prague blob schedule over amsterdam_chain_config's, where src/main.rs deliberately fills in amsterdam. Harmless for this block, confusing for the next screen.

Doc nits, not worth inline comments:

  • executor/tests/README.md still says "Five manifests carry that pin" and "(all five manifests at once)" — the new crate makes it six. ("five locks" stays correct; it has no lockfile of its own.)
  • scripts/set_ethrex_rev.sh's header still describes "the rkyv stateless input the converter writes and the guest decodes"; the PR updated its manifest list but not the rationale above it.
  • GUEST_OUTPUT_LEN/VALIDATION_FLAG are exported but re-spelled as bare 43/32 in tooling/ethrex-tests, and empty_execution_requests() is pub with one in-crate caller.

On the guest change itself: dropping execution_program(...).unwrap() for run_stateless_guest means a malformed input is no longer a VM abort but a clean exit committing successful_validation = 0. That is upstream's design and the public flag makes it checkable, so it is the right call — but it moves the whole burden onto the harness, and native_output()'s assert_eq!(output[32], 1) plus the new bench floor are now the only things asserting it anywhere. Worth remembering when adding future prove paths.

🤖 Generated with Claude Code

A fixture the guest rejects still proves, still verifies and still
profiles: since this branch moved the guest to run_stateless_guest, an
input whose schema it does not recognise is committed as
successful_validation = 0 and exits cleanly instead of aborting.
Measured on the bench box, the same guest runs a pre-SSZ fixture in 496
cycles against 3,730,771 for its own 10-transfer fixture, so a whole
batch reads as a ~99% improvement, in green, on both sides of an A/B.

bench_abba.sh already checked this, but only for the real workload, and
it was the only entry point that did. bench_verify.sh, perf_diff.sh and
benchmark-pr.yml prove the real block after nothing but the fixture's
sha256, which pins the fixture and not the pair: moving only the guest's
rev (set_ethrex_rev.sh --guest-only, which is how a benchmark varies the
guest across revs) leaves a fixture that still matches its digest and an
ELF that may no longer accept it. The synthetic side is worse, because
it is generated only when MISSING and its names carry neither the rev
nor the schema -- ethrex_bench_20.bin is gitignored, so a pre-bump copy
survives a rev bump, a git clean -fd and every git status.

So the check moves into scripts/assert_workload_cycles.sh and all four
callers use it, for both workloads. The synthetic floor is 100k rather
than the real 1M because a small TX_COUNT is a legitimate workload: an
empty block ran 0.99M cycles and a 1-transfer block 1.80M, an order of
magnitude above a rejected run. A run that never reaches a cycle count
at all (a missing file, an execution error, a cli that does not
implement a syscall the ELF uses) gets its own message and the tail of
its output, because that is a different fault from a rejected input and
the two were easy to confuse.
Neither generator references libssz or libssz-types: both reach the
encoder through the ethrex-ssz-input path dependency, which declares the
same exact =0.3.0 pin and is resolved inside these crates' own
workspaces, so the pin they were spelling out arrives anyway.
ethrex-tests asserted on a 43-byte output and on byte 32 as literals
while ethrex-ssz-input already exported GUEST_OUTPUT_LEN and
VALIDATION_FLAG for exactly that, so the guest's output layout was
written down twice and only one copy was next to the encoder that
produces it. Depending on that crate also replaces the hand-declared
libssz pin, which this crate does not use and only carried to keep
ethrex-guest-program's ^0.3 from floating.

empty_execution_requests becomes private with it: one caller, in its own
module.
amsterdam_chain_config ends in ..Default::default(), so the config it
installs already carries BlobSchedule::default(): cancun, prague, osaka,
bpo1 and bpo2 filled, amsterdam None. BlobSchedule's serde defaults are
per field and are those same constructors, and genesis.json's two
entries are literally the default values, so preserving its schedule
across the assignment copied the default over the default.

No amsterdam entry is needed either way: amsterdam_chain_config
activates BPO1 and BPO2, and EIP-7892 has named forks inherit the
highest activated BPO entry, so get_fork_blob_schedule resolves BPO2's
14/21/11684671. The comment now says that, instead of claiming only the
chain id and the fee floor change when the whole fork schedule is
replaced. Regenerating from the cache after the deletion gives the
fixture digest already pinned in the Makefile, byte for byte.
The sixth manifest is ethrex-ssz-input, so set_ethrex_rev.sh re-pins six
and not five (the five lockfiles are unchanged -- that crate has none of
its own, being a path dependency of the two that do). Its header still
described the rkyv input the converter writes and a skew that decodes as
the all-zero default; the skew now reports itself as
successful_validation = 0 after ~500 cycles. And the converter job's
comment advertised a crypto-parity test and a reproducibility digest
that no longer exist: both remaining tests assert a rejection, which is
all a pre-Amsterdam cache can produce.
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

No concrete issues found in the PR changes. Reviewed statically; no builds or tests were run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants