Skip to content

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

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

Bump ethrex to 26.0.0 and rebuild the benchmark workload#983
jotabulacios wants to merge 32 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 first 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 30-40 % 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 — 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

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.

//! Usage:
//! cargo run --release --bin real_block -- <cache.json> <out.bin>
//!
//! NOTE: `build_stateless_input` and its helpers are copied from `main.rs`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium — the converter's success path is now unreachable and untested.

After this PR:

  • regen-real-block-fixture invokes ethrex-fixtures --bin real_block, not this crate, so nothing in the repo runs the binary.
  • stateless_input_from_cache requires block_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.

Comment thread scripts/bench_abba.sh Outdated
Comment on lines +179 to +184
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}')"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 execute runs for WORKLOAD=synthetic too, but the result is only compared against the floor for real. Gate the execute itself on WORKLOAD = real.
  • under set -euo pipefail a failed cli execute aborts 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"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread scripts/perf_diff.sh Outdated
# 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +36 to +48
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()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

Copy link
Copy Markdown

AI Review

PR #983 · 34 changed files

Warning: the diff was truncated before review.

Findings

Status Sev Location Finding Found by
confirmed medium .github/workflows/benchmark-gpu.yml:279 GPU benchmark error message stale RAM requirement kimi
openrouter/moonshotai/kimi-k2.7-code
confirmed medium tooling/ethrex-block-converter/README.md:41 Converter README contradicts new built-fixture model kimi
openrouter/moonshotai/kimi-k2.7-code
glm
openrouter/z-ai/glm-5.2
confirmed medium tooling/ethrex-block-converter/src/main.rs:102 Duplicated build_stateless_input logic between ethrex-block-converter and ethrex-fixtures/real_block.rs nemotron
openrouter/nvidia/nemotron-3-ultra-550b-a55b
kimi
openrouter/moonshotai/kimi-k2.7-code
glm
openrouter/z-ai/glm-5.2

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-001: GPU benchmark error message stale RAM requirement
  • Status: confirmed
  • Severity: medium
  • Location: .github/workflows/benchmark-gpu.yml:279
  • Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The GPU workflow offer-query was raised to cpu_ram>=64, but the "no offer matched" error message still reports the old >=48GB RAM requirement. This misleads operators when the query comes back empty.

Evidence

Line 257 sets QUERY="... cpu_ram>=64 ..."; line 279 emits the failure with "... >=48GB RAM ... >=64GB disk ...".

Suggested fix

Update the error message to say >=64GB RAM to match the query filter.

AI-002: Converter README contradicts new built-fixture model
  • Status: confirmed
  • Severity: medium
  • Location: tooling/ethrex-block-converter/README.md:41
  • Found by: kimi:openrouter/moonshotai/kimi-k2.7-code, glm:openrouter/z-ai/glm-5.2
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The converter README's "Real-block fixture" section says the benchmark fixture is "a fetched release artifact" and instructs to "Publish the resulting SSZ artifact". This contradicts the new model where the fixture is built locally and only the source cache is hosted.

Evidence

bench_verify.sh:34 'real fetches the real-block fixture'; perf_diff.sh:29 'monolithic would need ~240 GB', :30 '158.8 s per recording', :31 '~1.2 GB of disk per bundle and ~52 GB of RAM', :52 'peaks at ~52 GB'. The Makefile and benchmark-pr.yml were updated with new numbers but these scripts were only path-referenced, not comment-updated.

Suggested fix

Update the bench_abba.sh header comments: change 'fetches' to 'builds', update 158.8 s to 125.17 s, replace the old resolution table with the measured sd values from bench-abba.yml, and fix 'ethrex 25' to 'ethrex 26'.

AI-003: Duplicated build_stateless_input logic between ethrex-block-converter and ethrex-fixtures/real_block.rs
  • Status: confirmed
  • Severity: medium
  • Location: tooling/ethrex-block-converter/src/main.rs:102
  • Found by: nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b, kimi:openrouter/moonshotai/kimi-k2.7-code, glm:openrouter/z-ai/glm-5.2
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The build_stateless_input and empty_execution_requests functions are duplicated in ethrex-block-converter/src/main.rs and tooling/ethrex-fixtures/src/bin/real_block.rs. The real_block.rs comment acknowledges this is temporary but it creates maintenance burden and risk of divergence.

Evidence

Both files contain nearly identical build_stateless_input functions (converter lines 102-183, real_block lines 33-144) and empty_execution_requests helpers. The converter version has additional validation (requests_hash, base_fee_per_gas checks) while real_block assumes fields exist from local execution.

Suggested fix

Lift build_stateless_input and empty_execution_requests into a shared module (e.g. tooling/ethrex-fixtures/src/stateless.rs) and import it from all three call sites, as the NOTE suggests.

Reviewer Lanes

Lane Model Prompt Status Findings
glm openrouter/z-ai/glm-5.2 general success 4
kimi openrouter/moonshotai/kimi-k2.7-code general success 11
minimax minimax/MiniMax-M3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
moonmath zro/minimax-m3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b general success 6

Verification Lanes

Lane Model Status Confirmed Rejected Uncertain
deepseek-verifier openrouter/deepseek/deepseek-v4-pro success 3 7 0

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (7) — rejected by the verifier
  • ethrex-block-converter rejects caches with non-empty execution requests (tooling/ethrex-block-converter/src/main.rs:165, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The README at lines 23-25 explicitly documents this as intentional design: 'Caches with non-empty requests are rejected because replay does not persist those request bodies.' The converter cannot handle non-empty requests because the replay cache format doesn't include the request body data — it's a data format limitation, not a bug. The behavior is working exactly as designed and documented.
  • real_block BLOCKHASH cache only seeds parent, not ancestor headers (tooling/ethrex-fixtures/src/bin/real_block.rs:427, found by kimi:openrouter/moonshotai/kimi-k2.7-code) — The block is built in a fresh local Amsterdam chain at the real block's height. There are no older ancestors to seed — head is the sole parent (head_number = real_block.header.number - 1), and all older witnesses the real chain carried don't apply to this custom chain. The comment at lines 411-412 says block_hash_cache 'seeds BLOCKHASH with the ancestor hashes the witness carries' but in practice only the parent exists in this chain. Seeding additional ancestors from the witness wouldn't match any stored headers. The code is correct for the use case; only the comment is slightly misleading.
  • GPU benchmark RAM requirement increased to 64GB may reduce Vast.ai offer availability (.github/workflows/benchmark-gpu.yml:172, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The comment at lines 172-178 of the diff explicitly documents this trade-off as intentional: 'It costs offer availability: fewer boxes qualify, so PRICE_CAP may need a look if the query starts coming back empty. Peak is set by the epoch size, not the block, so dropping GPU_REAL_EPOCH_LOG2 to 21 is the other lever.' The PR author is fully aware of and documents the availability impact along with two mitigation options (PRICE_CAP adjustment and epoch reduction). This is a documented, deliberate design decision.
  • Makefile fixture generation uses unconventional file rule pattern (Makefile:430, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The comment at lines 438-444 explicitly documents the pattern as intentional: 'No prerequisites on purpose. ethrex-real-block-cache is phony (so its digest is re-checked on every call), and a phony prerequisite always reads as newer than its target, which would rebuild the fixture on every single benchmark invocation.' This isn't unconventional or confusing — it's a deliberate, well-documented Makefile pattern that avoids unnecessary rebuilds.
  • bench_abba.sh adds extra cli execute run for cycle counting before benchmark (scripts/bench_abba.sh:113, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The MIN_PLAUSIBLE_CYCLES check at lines 165-189 of bench_abba.sh is a deliberately added safety guard with extensive documentation explaining exactly what it prevents: a stale/wrong fixture that the guest rejects but still produces a proof of (~496 cycles) would read as a '~99% improvement, in green, on both sides' if not caught. The comment explicitly says 'One cheap execution up front turns that class of mistake into a hard stop.' The added time is acknowledged as a necessary cost for correctness. This is not a bug but a defensive check.
  • ethrex-fixtures Cargo.toml version not bumped despite new binary and format change (tooling/ethrex-fixtures/Cargo.toml:3, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The package is pre-release 0.1.0, and the addition of default-run is a pragmatic fix for 'cargo run' ambiguity when a crate gains a second binary. Version bump conventions for pre-1.0 packages are project-specific and not a code defect. The format change (rkyv to SSZ) affects serialization output but the envelope version is still 0.1.0, which is conventional for pre-release tooling packages.
  • head.number subtraction could underflow for block 0 (tooling/ethrex-fixtures/src/bin/real_block.rs:374, found by glm:openrouter/z-ai/glm-5.2, kimi:openrouter/moonshotai/kimi-k2.7-code) — The claim itself acknowledges 'Not a practical concern for real mainnet blocks (always > 0)'. Block 0 is the genesis block and can never appear as a real block in an ethrex-replay cache. The subtraction at line 374 operates on real_block.header.number which is always >= 1 for actual mainnet blocks. The inconsistency with line 376's saturating_sub for the timestamp is stylistic — the timestamp needs saturating_sub because genesis.timestamp could reasonably be 0 (if timestamp is 12 or less), but block number 0 is impossible. This is a purely theoretical concern.

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.
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