feat(tooling): real ethrex block fixture - #880
Conversation
|
/ai-review |
Codex Code Review
|
Review — real ethrex block fixtureRead the whole diff statically (converter, Makefile wiring, CI, both new tests). No safety/memory-safety issues: no Medium
Low
Nits
The "verified, not assumed" framing on the precompile gate is the right standard, and the caveat about it never having rejected anything is the kind of thing reviewers usually have to dig for — thanks for stating it up front. |
AI ReviewPR #880 · 10 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-003: make test-ethrex now requires network access on first run
Claim The test-ethrex target now depends on $(ETHREX_REAL_BLOCK_FIXTURE), which transitively downloads the ethrex-replay cache from GitHub. This breaks offline builds and surprises developers who previously could run ethrex tests without network. Evidence Line 303: Suggested fix Document the network requirement in the target's comment, or add a separate AI-004: `make test-ethrex` runs the unmeasured heavy VM test that CI deliberately skips
Claim
Evidence Makefile line 303-304: Suggested fix Mirror CI in the Makefile: AI-006: CI does not cache the downloaded ethrex-replay cache (~1.5 MB)
Claim The CI step 'Generate ethrex real-block fixture' runs Evidence Line 107-108: the step runs Suggested fix Add a cache step for AI-007: Converter tool built and run twice in CI
Claim The CI runs Evidence Line 108: Suggested fix Minor optimization: in the fixture step, use 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. |
… sha256 not length, skip heavy VM test in local target, and add offline variant
…e download, and tighten the KZG canary.
…da_vm into feat/ethrex-real-block
|
Superseded by #884, which includes every commit from this branch (the branch was forked from it and later merged with current main) plus the benchmark wiring, fixture hosting, and measured calibration that grew out of the review. Closing in favor of that PR — the validation work here carries over unchanged. |
Motivation
Every ethrex benchmark uses a synthetic block of N plain ETH transfers. Measured
against a real block, that workload is inverted:
ethrex_bench_20(synthetic)The headline benchmark is ecrecover-dominated; a real block is keccak/trie dominated. The real block makes fewer ECSM calls despite 10× the gas.
Description
tooling/ethrex-real-block: reads an ethrex-replay cache JSON with our pinned ethrex types and writes the rkyvProgramInputthe guest consumes. Reading the JSON rather than ethrex-replay's own.binis deliberate — it tracks ethrexmain, whereProgramInputhas diverged (extra field, moved module, rkyv 0.8.10 vs our=0.8.16), so its binary would not deserialize in our guest.real_block_executes_under_guest_crypto(tooling/ethrex-real-block): executes the block throughLambdaVmEcsmCrypto, theCryptoimpl the guest injects.execute_blocksvalidates the witness header chain, the initial and final state roots, and per blockvalidate_gas_used,validate_receipts_rootandvalidate_requests_hash— so a missing precompile is caught on gas without needing state to diverge.no_kzg_backend_linked(tooling/ethrex-tests): the screen for "does this block need an accelerator we don't have". KZG point evaluation (0x0a) is the only precompile inethrex-cryptowhose availability is feature-gated, and that crate links no KZG backend, so a block calling 0x0a failstest_ethrex_real_block_nativethere. This canary pins that property rather than leaving it incidental to the dependency graph. Note the converter crate does not screen 0x0a — it linksc-kzgtransitively viaethrex-config.unmappable_network_is_rejected: ethrex-replay writesnetwork: "LocalDevnet"for any chain outside mainnet/Hoodi/Sepolia, and that resolves to chain_id 9 with every fork active from timestamp 0. Converting under it would replay the block against invented rules and pass every check, since the witness is only validated against whichever config we chose. The converter now refuses.conversion_is_reproducible: asserts the fixture's sha256, not its length — a layout change can preserve the byte count exactly (ChainConfigis fixed-size; rkyv'sbig_endianfeature would byte-swap in place).make ethrex-real-block-fixture. Fixture is ~1 MB and gitignored; the cache URL is pinned to a commit so the input can't change under a fixed name.How to test
Using the fixture
Execute and count cycles (this is what the numbers above come from):
Cycle breakdown by function:
Proving is untested at this size — expect to need continuations for flat peak
memory, and don't assume a plain
provefits in RAM: