Bump ethrex to 26.0.0 and rebuild the benchmark workload - #983
Bump ethrex to 26.0.0 and rebuild the benchmark workload#983jotabulacios wants to merge 36 commits into
Conversation
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.
…a_vm into chore/bump-ethrex-rev
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.
|
/ai-review |
Codex Code Review
Static review only; no builds or tests run. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| //! Usage: | ||
| //! cargo run --release --bin real_block -- <cache.json> <out.bin> | ||
| //! | ||
| //! NOTE: `build_stateless_input` and its helpers are copied from `main.rs` |
There was a problem hiding this comment.
Medium — duplication the PR flags against itself. This note says "A real PR should lift them into a module". This is the real PR — please do it here.
build_stateless_input + empty_execution_requests now exist in three near-identical copies (~130 lines each): here, tooling/ethrex-fixtures/src/main.rs, and tooling/ethrex-block-converter/src/main.rs. They all have to move together on the next ethrex rev bump, and nothing enforces that — the two in this crate can at least share a src/lib.rs module.
|
|
||
| let (program_input, _) = program_input_from_cache(CACHE).expect(CACHE_MISSING); | ||
| execution_program(program_input, Arc::new(LambdaVmEcsmCrypto)).unwrap(); | ||
| fn legacy_cache_is_rejected_without_amsterdam_fields() { |
There was a problem hiding this comment.
Medium — the converter's success path is now unreachable and untested.
After this PR:
regen-real-block-fixtureinvokesethrex-fixtures --bin real_block, not this crate, so nothing in the repo runs the binary.stateless_input_from_cacherequiresblock_access_list_hash+slot_number, i.e. an Amsterdam replay cache. None exists (mainnet has no Amsterdam fork, and the pinned upstream test cache is Hoodi 1265656).- The two tests that exercised conversion (
real_block_executes_under_guest_crypto,conversion_is_reproducible) are deleted. The two that remain only assert rejection — this one asserts the pinned cache fails.
So ~200 lines of new SSZ conversion code ship with zero success-path coverage, while ethrex-block-converter.yml still pays a cold build of ~335 packages (blst, c-kzg, secp256k1-sys, malachite) to run two negative tests. Suggest deleting the crate, or at minimum trimming its now-unused deps: lambda-vm-ethrex-crypto and sha2 in [dev-dependencies] have no remaining users, and the k256 entry in [dependencies] exists only to align features with that dev-dep.
Also note this test passes either way — if caches/ is absent it asserts on "No such file", and if the cache has ≠1 block the error won't contain "Amsterdam" and it fails for the wrong reason.
| MIN_PLAUSIBLE_CYCLES=1000000 | ||
| if [ ! -x ./target/release/cli ]; then | ||
| cargo build --release -p cli >/dev/null | ||
| fi | ||
| workload_cycles="$(./target/release/cli execute "$ELF" --private-input "$INPUT" --cycles \ | ||
| | awk '/^Cycles:/ {print $2}')" |
There was a problem hiding this comment.
Medium — this builds a third cli in a target dir nothing else shares.
cli_A/cli_B are built in the isolated worktree /tmp/abba_wt ($WT/target) with --features "$BENCH_FEATURES". This builds -p cli at the repo root with default features, so it shares neither the target dir nor the feature resolution — a full extra release build of the prover stack. On the bench runner that's amortised by a warm ./target, but benchmark-gpu.yml runs this script on a freshly rented Vast box where it is a cold build, on paid time, with disk_space>=64 already accounted for two other target dirs.
The floor check doesn't need its own binary — $WORK/cli_B is the baseline prover and is built a few lines below. Moving this block after step 2 and running "$WORK/cli_B" execute ... gets the same guardrail for free.
Two smaller things:
- the
cli executeruns forWORKLOAD=synthetictoo, but the result is only compared against the floor forreal. Gate the execute itself onWORKLOAD = real. - under
set -euo pipefaila failedcli executeaborts the pipeline before the message below can print. Fail-closed is right, but the operator never sees the "rebuild the fixture" hint.
| # of the repo — including uncommitted local edits — rather than whatever commit | ||
| # ethrex last pinned. (At the current ethrex rev the two happen to coincide; this | ||
| # override is what keeps that coincidence from being load-bearing.) | ||
| [patch."https://github.com/yetanotherco/lambda_vm.git"] |
There was a problem hiding this comment.
Low — this patch matches nothing, so cargo will warn on every guest build.
executor/programs/rust/ethrex/Cargo.lock contains no git+https://github.com/yetanotherco/lambda_vm.git source, before or after this PR — ethrex-guest-program is declared default-features = false, and dep:lambda-vm-syscalls only comes in through the lambdavm feature, which is never enabled. Cargo prints
warning: Patch `lambda-vm-syscalls v...` was not used in the crate graph.
for an unmatched [patch] entry, on every build of the guest.
If the intent is defence-in-depth against a future rev that does pull the git dep, the comment should say the patch is inert today and accept the warning; otherwise drop it. Either way "the two happen to coincide" is misleading — the git dep is not in the graph at all.
| # 128 GiB box (tooling/ethrex-fixtures/README.md, "Choosing the epoch size"). | ||
| # | ||
| # Pick the workload that matches the run you are localizing, because the symbol | ||
| # mix follows the block: the real default is 50.78M cycles, 10,478 keccak calls and |
There was a problem hiding this comment.
Low — re-pinned everywhere but here. The PR updated the README path on line 19 and left the numbers below it at the retired workload: 50.78M cycles / 10,478 keccak / 116 ecsm (line 22), ~240 GB (line 29), 158.8 s (line 30), ~1.2 GB bundle and ~52 GB (line 31). Same for the sizing table in scripts/bench_abba.sh lines 50–61, which still carries the "sd NOT yet measured, 1.0%/2.0% bracket" text that bench-abba.yml replaced in this PR — the script and the workflow that invokes it now disagree.
| let schema_id = u16::from_be_bytes([inputs[0], inputs[1]]); | ||
| let started = std::time::Instant::now(); | ||
| let output = run_stateless_guest(&inputs, Arc::new(NativeCrypto)); | ||
| let elapsed = started.elapsed(); | ||
| let name = path.file_name().unwrap().to_string_lossy(); | ||
| println!( | ||
| "{:<60} in={:>9} B schema={:#06x} out={} B ok={} chain_id={} native={:?}", | ||
| &name[..name.len().min(60)], | ||
| inputs.len(), | ||
| schema_id, | ||
| output.len(), | ||
| output.get(32).copied().unwrap_or(255), | ||
| u64::from_le_bytes(output[33..41].try_into().unwrap()), |
There was a problem hiding this comment.
Low — inert in CI, and indexes before it checks.
Both tests in this file return early on a missing ../../eest_amsterdam_bins. Nothing in the repo — no make target, no workflow, no .gitignore entry — ever produces that directory, so on every checkout these tests print "absent — skipping" and pass. That is dead test code by construction; either wire up a fetch/generate step or keep the probe out of the tree until the EEST workload lands.
Also, inputs[0]/inputs[1] (line 36) and output[33..41] (line 48) index before the output.len() == 43 assert below, so a short file or a short output panics on a slice index instead of the assert message — inconsistent with the defensive output.get(32).copied().unwrap_or(255) two lines up. &name[..name.len().min(60)] is also a byte slice into a str and will panic on a non-ASCII filename.
AI ReviewPR #983 · 35 changed files
Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-001: bench_abba.sh CPU share measurement uses GNU time-specific flags
Claim The script uses Evidence Line 185: Suggested fix Add a comment noting the GNU time dependency, or use a portable method like parsing /proc/stat or using AI-002: Converter tests no longer cover successful SSZ conversion
Claim The ethrex-block-converter test suite no longer exercises a successful cache-to-SSZ conversion: the only conversion-related test expects the checked-in Hoodi cache to be rejected for lacking Amsterdam fields. The successful path is only reached implicitly by the binary's main(), so regressions in successful conversion would not be caught by cargo test. Evidence src/main.rs tests now contain Suggested fix Once an Amsterdam-compatible Hoodi replay cache is available, restore a test that performs a successful conversion and validates the output byte length/hash or runs run_stateless_guest on it. Until then, add a code comment or TODO marking the temporary coverage gap. AI-003: GPU benchmark error message reports outdated 48 GB RAM floor
Claim The Vast offer error message in benchmark-gpu.yml says the RAM floor is >=48GB, but the QUERY string was raised to cpu_ram>=64 in the same PR. A user whose query comes back empty will read the wrong constraint in the error. Evidence Line 257 sets QUERY="... cpu_ram>=64 ..." while line 279 prints "... >=48GB RAM ...". The comment above correctly discusses the 64 GB floor, so the error message is the only stale value. Suggested fix Change ">=48GB RAM" to ">=64GB RAM" in the ::error:: message on line 279 to match the query. AI-004: Stale comment: benchmark-pr.yml still describes fixture as fetched
Claim The comment block in benchmark-pr.yml's "Determine run count" step still says the real-block fixture is fetched from a pinned URL and not built, but the PR changed the workflow to build it from the replay cache. The guard was updated to print-real-block-cache-url but the explanatory comment was not. Evidence Lines in .github/workflows/benchmark-pr.yml still read: "The fixture is FETCHED from a pinned URL + sha256, not built (see the Makefile)." while the actual step now runs Suggested fix Update the comment to state that the fixture is built from the fetched replay cache, and that the URL guard checks ETHREX_REAL_BLOCK_CACHE_URL. AI-005: bench_abba.sh uses fixed /tmp paths that could conflict with concurrent runs
Claim The script uses fixed paths Evidence Lines 45-47: Suggested fix Include PID or random suffix in temp paths (e.g., AI-008: awk mn==0 sentinel in exclusivity report breaks if any CPU share is exactly 0
Claim The awk exclusivity report uses Evidence The awk: Suggested fix Use a separate sentinel: AI-009: Stale comments in perf_diff.sh describe fixture as fetched
Claim scripts/perf_diff.sh still describes the real-block fixture step as a URL+sha256 fetch, but Evidence Lines 84-88 of scripts/perf_diff.sh read: "~1 MB, gitignored, never in a fresh checkout; fetch rather than abort. This is a URL + sha256 download, not a build... echo '==> Verifying ethrex real-block fixture (fetches on a digest miss)'". The Makefile target now runs Suggested fix Update the echo and comments to say the fixture is built from the replay cache (e.g., "Building ethrex real-block fixture from its replay cache"), matching bench_abba.yml. AI-010: Stale comments in ethrex-block-converter Cargo.toml reference removed k256 dependency and removed dev-dependency
Claim The k256 dependency was removed from [dependencies] (it was Evidence Lines 38-49 describe why k256 is declared and how it prevents feature-set thrashing with lambda-vm-ethrex-crypto, but the k256 line is gone and lambda-vm-ethrex-crypto/sha2 were removed from [dev-dependencies]. The section now contains only comments referring to removed dependencies. Suggested fix Remove the orphaned comment at lines 38-48 (it explains a removed dependency) and update or remove the [dev-dependencies] comment at lines 51-54 to reflect that there are no dev-dependencies (or remove the empty section). AI-012: Converter README still instructs publishing the built fixture
Claim tooling/ethrex-block-converter/README.md instructs users to publish the resulting SSZ fixture and update its URL in the Makefile, but the PR's Makefile design builds the fixture locally and has no ETHREX_REAL_BLOCK_FIXTURE_URL. The converter README contradicts the Makefile and tooling/ethrex-fixtures/README.md. Evidence The converter README says "Publish the resulting SSZ artifact and matching Amsterdam cache, then update their URLs and checksums in the Makefile before enabling the real-block benchmark." The Makefile now defines only ETHREX_REAL_BLOCK_CACHE_URL and ETHREX_REAL_BLOCK_FIXTURE_SHA256, and its comments state "Nothing to publish" and "The fixture is GENERATED from the cache below, not fetched." Suggested fix Remove the instruction to publish the SSZ fixture and update a fixture URL; instead document that only the replay cache is hosted and the fixture is generated locally by make regen-real-block-fixture. AI-014: Stale doc comment in real_block.rs says build_stateless_input is copied, but it's shared via the lib
Claim The doc comment at lines 21-23 says 'build_stateless_input and its helpers are copied from main.rs rather than shared. A real PR should lift them into a module; keeping this binary self-contained leaves the committed generator untouched.' But the code at line 33 ( Evidence Line 33: Suggested fix Remove or update the NOTE comment at lines 21-23 to reflect that build_stateless_input is now shared via ethrex_fixtures::build_stateless_input (src/lib.rs), not copied. Reviewer Lanes
Verification Lanes
Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report. Discarded candidates (3) — rejected by the verifier
Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
`build_stateless_input` and `empty_execution_requests` existed as two near-identical copies, one per binary in this crate, differing only in how the block type was spelled. The file that carried the second copy said a real PR should lift them into a module, and this is that PR. They now live in src/lib.rs and both binaries import them, so the next ethrex rev bump moves one encoder instead of two that nothing forced to stay in step. Behaviour-preserving, and checked that way rather than assumed: after the move all five generated fixtures still come out at the digests they already had, the real-block one byte-for-byte at 08a52e10 with the same installed state root.
Both were left describing the retired block. perf_diff.sh advertised 50.78M cycles, 10,478 keccak and 116 ecsm as the real default, plus ~240 GB monolithic, 158.8 s per recording, ~1.2 GB per bundle and ~52 GB of RAM; the real numbers are 37,137,748 / 6,003 / 164, ~182 GB, ~125 s, ~790 MB and ~45 GiB. Its synthetic figures were stale too, so they were re-measured on the current guest rather than dropped: 7,029,393 cycles, 486 keccak, 80 ecsm for the 20-transfer block, which keeps the point the comment is making — the keccak:ecsm ratio is 37:1 against 6:1, so a hot symbol in one workload need not be hot in the other. bench_abba.sh's sizing table still carried "the pair-delta sd is NOT yet measured" with a 1.0%/2.0% bracket, which the workflow that invokes it no longer says: it now quotes the two measured conditions. A script disagreeing with its own workflow is worse than either being out of date, so the table is the same one, with a note to keep them in step. Also drops perf_diff.sh's "~2% with the clang major" figure. It came from a headline whose own evidence put the effect at 0.13%, and the guest reports the same count here on two toolchains, so the file now points at the README's caveat instead of quoting a number nothing measured.
…hird one The floor check ran before the A/B binaries were built, so it built its own `cli` — at the repo root with default features, sharing neither the worktree's target directory nor its feature set. That is a full extra release build of the prover stack: free on a warm bench runner, a cold build on paid time on the rented GPU box that also runs this script. It now runs after step 2 and executes with `cli_B`, the baseline prover, which measures the workload just as well. Two smaller things in the same block. The execute is now gated on WORKLOAD=real, matching the comparison it feeds — a synthetic run was paying for a measurement nobody read. And the failure path keeps its diagnostic: a bare assignment from a failing command substitution aborts under `set -e` before the message can print, so the operator got a silent death instead of the line telling them to regenerate the fixture. Wrapping it makes the guardrail fail closed *and* say why.
The probe read `inputs[0..2]` and `output[33..41]` inside the print, above the asserts that establish those lengths, so a short file or a short output panicked on a slice index instead of failing with the message that names the fixture. The same print had a defensive `output.get(32).unwrap_or(255)` next to a raw `output[33..41]`, which is the inconsistency that hid it. Asserts first, then the print reads plainly. The filename was also byte-sliced to 60 characters, which panics on a multi-byte boundary; it takes 60 chars now.
`lambda-vm-ethrex-crypto`, `sha2` and `k256` have no remaining users in the crate: the first two were needed by tests that the SSZ rewrite removed, and the `k256` entry existed only to align features with one of them. They are not free — this crate's CI job pays a cold build of the ethrex host tree, C libraries included, to run its two tests.
The comment claimed the patched and the pinned copies "happen to coincide" at the current rev, which reads as though the git dependency were in the graph and the override were holding it in place. It is not there at all: the lockfile carries no git+https://github.com/yetanotherco/lambda_vm.git source, because the only thing that pulls those syscalls is `ethrex-guest-program`'s `lambdavm` feature and this manifest does not enable it. Cargo does not warn about the unmatched entry either, which was worth checking before writing it down. The patch stays. The hazard it covers is real and silent — a future ethrex rev that pulls the syscalls from git would have the guest link a different copy than the rest of the repo — and the guest is the one binary where that divergence would matter.
|
/ai-review |
Codex Code Review
Static review only; no builds or tests run. |
| //! Usage: | ||
| //! cargo run --release --bin real_block -- <cache.json> <out.bin> | ||
| //! | ||
| //! NOTE: `build_stateless_input` and its helpers are copied from `main.rs` |
There was a problem hiding this comment.
Low — stale doc note. This NOTE contradicts the code: line 39 imports ethrex_fixtures::build_stateless_input from the new src/lib.rs, so nothing is copied here and there is no "committed generator left untouched". The "A real PR should lift them into a module" phrasing also reads as a leftover draft note. Suggest deleting the whole NOTE block.
Nothing in the repository produces `eest_amsterdam_bins` — no make target, no workflow, no ignore entry — so both tests took their early return and passed on every checkout: dead test code by construction. The inputs are lifted by hand out of an execution-specs release tarball with a script that is not in the repository either. It comes back together with the EEST workload, and with the step that produces its inputs, so that the tests it adds actually run. Its two tests need splitting first: the positive scan took every `.bin` in the directory and required each to validate, including the legacy rkyv fixture that the second test requires to be REJECTED, so supplying that fixture would have made the first test fail.
The SSZ rewrite deleted the two tests that exercised conversion and left two that only assert a rejection, but three comments still described the old ones: the Makefile target promised "host-side parity through the guest's own Crypto impl ... and the reproducibility digest", the rev-stamp comment warned about a `conversion_is_reproducible` failure that can no longer happen, and the crate's own comment claimed the tests exercise the conversion itself. What can be reached today is the two rejection paths; the success path needs a cache from a network running Amsterdam, carrying `slot_number` and `block_access_list_hash`, and none is published. The comments say that now, including that the encoder is exercised by hand and by nothing automatic. Two leftovers in the same crate: the README still described the benchmark fixture as a fetched release artifact with an SSZ artifact to publish, which this branch replaced with a generator, and dropping the unused dependencies left their justifications behind — eleven lines explaining a `k256` entry that is gone, and an empty `[dev-dependencies]` holding only comments about two deleted ones.
The comment on `EPOCH_SIZE_LOG2` still sized the epoch against the retired workload's ~52 GB peak, while the header of the same file already carried the rebuilt one. It is 44.81 GiB at 2^22 on that runner and 2^23 crosses 50 GiB, which is what benchmark-pr.yml and bench-abba.yml already say.
`build_stateless_input` and its helpers now live in `src/lib.rs` and this binary imports them, so the note saying they are copied from `main.rs` — and that lifting them into a module was something "a real PR" should do — describes the tree before that change.
2 participants
Bump ethrex to 26.0.0 and rebuild the benchmark workload
Motivation
The pin moves from
797df554(23.0.0) to8effcb06, a commit on ethrexmaindeclaring26.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_blockrebuilds 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 —
25087563fails withStateRootMismatch— so screening is required before pinning a different block.Four guardrails, all aimed at work that disappears quietly:
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.shexecutes the workload first and requires >1M cycles, which turns those 496cycles into a hard error;
with fewer transactions is still valid, so nothing else would have caught it;
account or slot.
Also: pins moved together by
scripts/set_ethrex_rev.sh; the four synthetic fixturesregenerated (SSZ is 30-40 % smaller than rkyv); docs and thresholds re-pinned; the GPU arm's
cpu_ramfloor 48 → 64 GB, because 44.81 GiB is 98 % of a 48 GB box.tooling/ethrex-fixtures/README.mdcarries the rest: what the workload costs, and why thereverts below are the fork's doing rather than the reconstruction's — the same rebuild under
Osaka reverts 2 where Amsterdam reverts 10. What this fixture is, is a real-mix Amsterdam
block; what it is not, is a replay of mainnet economics.
Measured
Verified, not assumed:
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
.rsfile in the guest's dependency graph differs between the tworevs 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 (dc38c029and
12a692b3) and the same 37,137,386 to the cycle: the ELF bytes move with things thecount does not, and rebuilding at this pin reproduces its binary byte-for-byte;
08a52e10…, regenerated from aclean 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;
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.shnow reports that share per prove.Why nothing here compares to a pre-bump number
Four things moved at once, and they are not independent:
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 thewhole payload, on every block. We override
keccak256andsecp256k1_ecrecoverandnothing else, so that sha256 runs as a software default;
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;
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
cli executeshould report 37 137 748 cycles / 6 003 keccak / 164 ecsm, and aprove/verify round trip at
--epoch-size-log2 22should pass. The verify output ends| 01 | 0100000000000000 | 0115—successful_validation = 1,chain_id = 1,schema_id = 0x1501— so the proof itself carries the evidence that an Amsterdam block wasvalidated, which is the direct antidote to the 496-cycle failure.