Repository navigation
feat(l1,l2): zkevm execution benchmark - #6982
Conversation
…rop eth-act loader; README
…-elf; empty-ELF warn; strict tier
iovoid
added
tooling
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge; all findings are cosmetic or quality-of-life concerns that do not affect benchmark correctness or the regression gate logic. The benchmark results themselves are accurate — the AIR-cost parser is well-tested with a committed sample and an invariant assertion. The regression-detection logic correctly handles the failed-workload trap. The two notable gaps are that report metadata always claims a hardcoded zisk_version regardless of the actual installed binary, and the curate command silently skips .json.gz files even though every in-repo fixture uses that format. Neither affects the correctness of a benchmark run. tooling/zkevm_bench/src/run.rs (hardcoded version string in report metadata) and tooling/zkevm_bench/src/curate.rs (.json.gz files silently skipped)
|
| Filename | Overview |
|---|---|
| tooling/zkevm_bench/src/run.rs | Core benchmark runner; hardcodes zisk_version in report metadata regardless of the actual installed ziskemu version |
| tooling/zkevm_bench/src/curate.rs | Curation scanner filters only .json files, silently skipping .json.gz — all in-repo fixtures are .json.gz so running curate on the fixtures dir yields 0 results |
| crates/prover/src/backend/zisk_profile.rs | New parser for ziskemu COST DISTRIBUTION output; well-tested with unit tests and a committed sample fixture |
| crates/prover/src/backend/zisk.rs | Adds execute_profiled() running ziskemu -X for profiling; replaces repeat(0u8).take(n) with repeat_n |
| tooling/zkevm_bench/src/compare.rs | Regression-detection logic; correctly handles failed workloads and missing workloads; well-tested |
| tooling/zkevm_bench/src/manifest.rs | Workload manifest parsing with tier validation; unknown tier values rejected at load time |
| tooling/zkevm_bench/src/generate.rs | EEST-to-Cache fixture generator; single bad fixture is skipped without aborting the batch |
| tooling/ef_tests/blockchain/test_runner.rs | Adds blocks_and_witness_for_test() for ethrex-native witness generation; adds ZisK stub in re_run_stateless |
| tooling/zkevm_bench/src/cache.rs | Cache loader supporting both .json and .json.gz; test references a pre-existing fixture confirmed present in the repo |
| tooling/zkevm_bench/src/micro.rs | EEST zkevm fixture loader; gracefully skips test when vectors are not downloaded |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as ethrex-zkevm-bench
participant Run as run.rs
participant Manifest as manifest.rs
participant Cache as cache.rs/micro.rs
participant ZiskBackend as ZiskBackend
participant Ziskemu as ziskemu
participant Profile as zisk_profile.rs
User->>CLI: run --workloads manifest.toml --mode quick
CLI->>Run: run_bench()
Run->>Manifest: load_manifest() + filtered_for_run()
Manifest-->>Run: Vec WorkloadSpec
loop each WorkloadSpec
Run->>Cache: load input (cache or micro)
Run->>ZiskBackend: execute_profiled(ProgramInput)
ZiskBackend->>Ziskemu: ziskemu --elf ... --inputs ... -X
Ziskemu-->>ZiskBackend: COST DISTRIBUTION output
ZiskBackend->>Profile: parse_air_cost()
Profile-->>Run: ZiskAirCost
end
Run-->>User: report.json
User->>CLI: compare baseline.json head.json
CLI-->>User: deltas + exit code
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as ethrex-zkevm-bench
participant Run as run.rs
participant Manifest as manifest.rs
participant Cache as cache.rs/micro.rs
participant ZiskBackend as ZiskBackend
participant Ziskemu as ziskemu
participant Profile as zisk_profile.rs
User->>CLI: run --workloads manifest.toml --mode quick
CLI->>Run: run_bench()
Run->>Manifest: load_manifest() + filtered_for_run()
Manifest-->>Run: Vec WorkloadSpec
loop each WorkloadSpec
Run->>Cache: load input (cache or micro)
Run->>ZiskBackend: execute_profiled(ProgramInput)
ZiskBackend->>Ziskemu: ziskemu --elf ... --inputs ... -X
Ziskemu-->>ZiskBackend: COST DISTRIBUTION output
ZiskBackend->>Profile: parse_air_cost()
Profile-->>Run: ZiskAirCost
end
Run-->>User: report.json
User->>CLI: compare baseline.json head.json
CLI-->>User: deltas + exit code
Comments Outside Diff (1)
-
tooling/zkevm_bench/src/run.rs, line 1017-1037 (link)Non-deterministic stress workload ordering
std::fs::read_dirreturns entries in filesystem-defined order, which varies across platforms and is not guaranteed to be stable across runs. Sincediscover_stress_workloadsresults are appended directly tospecs, the stress section of the report JSON will have a different ordering on Linux vs macOS (or even between runs on the same machine after filesystem changes). This creates noisy diffs when version-controlling report files. A simplespecs.sort_by(|a, b| a.name.cmp(&b.name))after the loop would make the output deterministic.Prompt To Fix With AI
This is a comment left during a code review. Path: tooling/zkevm_bench/src/run.rs Line: 1017-1037 Comment: **Non-deterministic stress workload ordering** `std::fs::read_dir` returns entries in filesystem-defined order, which varies across platforms and is not guaranteed to be stable across runs. Since `discover_stress_workloads` results are appended directly to `specs`, the stress section of the report JSON will have a different ordering on Linux vs macOS (or even between runs on the same machine after filesystem changes). This creates noisy diffs when version-controlling report files. A simple `specs.sort_by(|a, b| a.name.cmp(&b.name))` after the loop would make the output deterministic. How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
tooling/zkevm_bench/src/run.rs:147-152
The `zisk_version` field is hardcoded to `"v0.16.1"` rather than queried from the installed binary. If a user upgrades ziskemu independently, every report they produce will still claim to come from v0.16.1, making cross-version comparisons misleading. Consider calling `ziskemu --version` at startup and parsing its output, or at minimum surfacing a warning when the string is used as-is.
```suggestion
meta: Meta {
zisk_version: detect_zisk_version().unwrap_or_else(|| "v0.16.1".into()),
guest_elf_sha256: guest_elf_sha256(),
generated_by: "ethrex-zkevm-bench".into(),
git_commit: std::env::var("GIT_COMMIT").ok(),
},
```
### Issue 2 of 3
tooling/zkevm_bench/src/curate.rs:115-117
The filter accepts only `.json` files, silently skipping `.json.gz`. Every in-repo fixture under `fixtures/blocks/` and `fixtures/stress/` is `.json.gz`, so running `curate` on those directories will always produce 0 rows with no diagnostic message. If the intent is to support only ethrex-replay's uncompressed caches, a comment or a clear early-exit error when 0 results are found would prevent confusion. Alternatively, accepting `.json.gz` matches the existing `load_cache` capability.
```suggestion
if !name.starts_with("cache_mainnet_")
|| !(name.ends_with(".json") || name.ends_with(".json.gz"))
{
continue;
}
```
### Issue 3 of 3
tooling/zkevm_bench/src/run.rs:1017-1037
**Non-deterministic stress workload ordering**
`std::fs::read_dir` returns entries in filesystem-defined order, which varies across platforms and is not guaranteed to be stable across runs. Since `discover_stress_workloads` results are appended directly to `specs`, the stress section of the report JSON will have a different ordering on Linux vs macOS (or even between runs on the same machine after filesystem changes). This creates noisy diffs when version-controlling report files. A simple `specs.sort_by(|a, b| a.name.cmp(&b.name))` after the loop would make the output deterministic.
Reviews (1): Last reviewed commit: "fix(zkevm-bench): compare flags failed/m..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| meta: Meta { | ||
| zisk_version: "v0.16.1".into(), | ||
| guest_elf_sha256: guest_elf_sha256(), | ||
| generated_by: "ethrex-zkevm-bench".into(), | ||
| git_commit: std::env::var("GIT_COMMIT").ok(), | ||
| }, |
There was a problem hiding this comment.
zisk_version field is hardcoded to "v0.16.1" rather than queried from the installed binary. If a user upgrades ziskemu independently, every report they produce will still claim to come from v0.16.1, making cross-version comparisons misleading. Consider calling ziskemu --version at startup and parsing its output, or at minimum surfacing a warning when the string is used as-is.
| meta: Meta { | |
| zisk_version: "v0.16.1".into(), | |
| guest_elf_sha256: guest_elf_sha256(), | |
| generated_by: "ethrex-zkevm-bench".into(), | |
| git_commit: std::env::var("GIT_COMMIT").ok(), | |
| }, | |
| meta: Meta { | |
| zisk_version: detect_zisk_version().unwrap_or_else(|| "v0.16.1".into()), | |
| guest_elf_sha256: guest_elf_sha256(), | |
| generated_by: "ethrex-zkevm-bench".into(), | |
| git_commit: std::env::var("GIT_COMMIT").ok(), | |
| }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tooling/zkevm_bench/src/run.rs
Line: 147-152
Comment:
The `zisk_version` field is hardcoded to `"v0.16.1"` rather than queried from the installed binary. If a user upgrades ziskemu independently, every report they produce will still claim to come from v0.16.1, making cross-version comparisons misleading. Consider calling `ziskemu --version` at startup and parsing its output, or at minimum surfacing a warning when the string is used as-is.
```suggestion
meta: Meta {
zisk_version: detect_zisk_version().unwrap_or_else(|| "v0.16.1".into()),
guest_elf_sha256: guest_elf_sha256(),
generated_by: "ethrex-zkevm-bench".into(),
git_commit: std::env::var("GIT_COMMIT").ok(),
},
```
How can I resolve this? If you propose a fix, please make it concise.| if !name.starts_with("cache_mainnet_") || !name.ends_with(".json") { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
.json files, silently skipping .json.gz. Every in-repo fixture under fixtures/blocks/ and fixtures/stress/ is .json.gz, so running curate on those directories will always produce 0 rows with no diagnostic message. If the intent is to support only ethrex-replay's uncompressed caches, a comment or a clear early-exit error when 0 results are found would prevent confusion. Alternatively, accepting .json.gz matches the existing load_cache capability.
| if !name.starts_with("cache_mainnet_") || !name.ends_with(".json") { | |
| continue; | |
| } | |
| if !name.starts_with("cache_mainnet_") | |
| || !(name.ends_with(".json") || name.ends_with(".json.gz")) | |
| { | |
| continue; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tooling/zkevm_bench/src/curate.rs
Line: 115-117
Comment:
The filter accepts only `.json` files, silently skipping `.json.gz`. Every in-repo fixture under `fixtures/blocks/` and `fixtures/stress/` is `.json.gz`, so running `curate` on those directories will always produce 0 rows with no diagnostic message. If the intent is to support only ethrex-replay's uncompressed caches, a comment or a clear early-exit error when 0 results are found would prevent confusion. Alternatively, accepting `.json.gz` matches the existing `load_cache` capability.
```suggestion
if !name.starts_with("cache_mainnet_")
|| !(name.ends_with(".json") || name.ends_with(".json.gz"))
{
continue;
}
```
How can I resolve this? If you propose a fix, please make it concise.…benchmark # Conflicts: # Makefile
Lines of code reportTotal lines added: Detailed view |
ElFantasma
left a comment
ElFantasma
left a comment
There was a problem hiding this comment.
Nice tooling scope — deterministic ziskemu-based benchmarking with the three-tier preset structure is a good regression-signal shape. Fixtures correctly LFS-tracked (.gitattributes pattern tooling/zkevm_bench/fixtures/**/*.json.gz). Endorse greptile's 2 P2s and add one cross-file observation, plus flag the red CI:
zisk_versionhardcoded atrun.rs:148— real bug: reports will misattribute across ziskemu upgrades. Greptile's suggesteddetect_zisk_version()viaziskemu --versionis the right shape.curate.rs:115rejects.json.gz— reinforcement for greptile:run.rs:52(discover_stress_workloads) already accepts both.jsonand.json.gzvianame.ends_with(".json.gz") || name.ends_with(".json"). Same tool, different sibling function, opposite behavior on the same file extension. Thecuratedocstring documents the.gzskip explicitly (line 100–104: ".gzarchives — are skipped"), so the design intent seems to be "curate is only for uncompressed ethrex-replay caches" — but a user pointingcurateattooling/zkevm_bench/fixtures/blocks/(where every fixture is.json.gz) will silently get 0 rows with no error. Either accept.json.gzfor consistency withdiscover_stress_workloads+load_cache(both already handle both), or add a hard error whenrows.is_empty()after the scan.- CI is red (Cargo.lock check, Build Docker, and Test failing across ubuntu-22.04 / ubuntu-22.04-arm / macos-15). Author has a merge-fixup commit
9707f1606 chore: fix Cargo.lock versions after mergeat the tip; the check failures may reflect a still-stale lockfile or the newly-addeddatatest-stable/ef_tests-blockchaindep additions inCargo.lock. Blocker for merge — worth confirming the lockfile matchesCargo.tomlcleanly and re-running.
One minor observation on run.rs:74: the empty-ELF path prints a warning to stderr and continues — every workload then fails with guest_output_ok: false. That's arguably fine for a bench tool (still produces a report), but in a CI context that stderr line is easy to miss. A --strict-elf flag (or auto-abort when ELF is empty AND no workloads succeed) would prevent someone from mistakenly using an all-false report as a regression signal.
…benchmark # Conflicts: # crates/guest-program/src/crypto/zisk.rs
|
3c3b6f4af (devnet-7) |
this client (devnet-8) | |
|---|---|---|
COLD_STORAGE_ACCESS |
3000 | 2100 |
ACCOUNT_WRITE |
8000 | 9000 |
CALL_VALUE |
10300 | 11300 |
| access-list storage key | full cold cost | cold − WARM_ACCESS |
Five of the 45 failed even while the client was still on devnet-7, so those are
skew against 3c3b6f4af itself; the other 40 appeared with the devnet-8 move.
The two groups are commented separately in EXTRA_SKIPS because they lift at
different times.
Corroborating that these are spec-base skew and not client bugs: ethrex passes
the five original cases against the tests-zkevm@v0.6.2 versions of the same
tests, and the eth-act witness dashboard has ethrex at 22829/22829 on
eels/consume-engine-witness and 19413/19413 on stateless-validator sp1 for
that bundle. Diffing witness_codes_reset_delegation across the two fixture
sets: pre and genesisBlockHeader are byte-identical and it is the same
transaction hash, yet postState, receipts, stateRoot, receiptTrie and
blockAccessListHash all differ — the receipt's cumulativeGasUsed goes
30816 → 24653. Same input, different output.
The regular vectors/ bundle has no eip8025_optional_proofs tests, so
make test-levm never sees any of this.
The pin cannot move today. eip8025_optional_proofs does not exist on the
current forks/amsterdam default branch, and 3c3b6f4af has diverged from it
(124 ahead, 315 behind), so it was never merged there. devnets/glamsterdam/8
is 315 commits behind 3c3b6f4af: it has the devnet-8 gas schedule but not the
#3278 schema, so vectors filled from it would carry the pre-#3278 69-byte
statelessOutputBytes this client cannot parse. No upstream revision has both.
Removal: bump SPEC_SHA in scripts/gen_stateless_vectors.sh once an
upstream revision carries both eip8025_optional_proofs and the devnet-8 gas
schedule, then delete the 40-entry group. Re-check the original five at the same
time; if they still fail against a matching base they are real execution bugs
and must not stay skipped.
Why this file previously claimed an Amsterdam-wide skip
parse_and_execute used to drop every network >= Fork::Amsterdam fixture from
the stateless run. Every test in the tests-zkevm bundle it read is
network: Amsterdam (23,946 of 23,946), so the stateless suite executed nothing
at all while reporting success. parse_and_execute now fails any stateless
fixture file that runs zero tests without a named skip, so a structural skip
cannot silently empty the suite again.
ZisK guest program hash changes with the unsync_cell gate
Where: crates/common/types/block.rs, transaction.rs.
The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.
The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.
This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rs → unsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.
Release signing key is an unprotected repository secret
Where: .github/workflows/tag_release.yaml.
MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.
This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:
- Move
MINISIGN_SECRET_KEY/MINISIGN_PASSWORDinto a GitHub Environment with required
reviewers, and addenvironment:to the two jobs that sign. - Add a ruleset targeting
refs/tags/v*restricting who may create release tags.
Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.
Four conflicts plus one semantic break that git could not see. `.github/actions/install-zisk/action.yml`: both sides pin the ZisK Rust toolchain instead of riding `latest`. main's #7010 is the later, dedicated fix and additionally handles ziskup >= v1.0.0-alpha resolving its install dir from XDG_CONFIG_HOME, which this branch's version predates and would regress. The branch's contribution here was purely additive, so main's side is taken whole. `tooling/ef_tests/blockchain/{Cargo.toml,test_runner.rs}`: #7115 removed the `BackendType` dispatch from the EF-test runner, so the branch's `zisk` feature and its `BackendType::ZisK` stub arm — which existed only to keep that match exhaustive and returned "not wired into ef_tests-blockchain" — no longer have anything to gate. Both take main's side, and the now-unreferenced `zisk = ["ethrex-prover/zisk"]` feature is dropped along with the `features = ["zisk"]` that `tooling/zkevm_bench` requested from it. zkevm_bench uses `blocks_and_witness_for_test`, `types::TestUnit` and `fork::Fork`, none of which were ever behind that feature. Resolved hunk-by-hunk rather than by taking main's file: this branch adds `blocks_and_witness_for_test` outside the conflict regions, and zkevm_bench imports it. `apply_fork_choice` grew a fifth `max_reorg_depth` parameter on main. That branch-new call site is the only one the merge left at four arguments; it now passes `None`, matching the three existing call sites in the same file. `tooling/Cargo.lock` and `tooling/zkevm_bench/Cargo.lock` regenerated.
#7115 redefined the L1 `ProgramInput` as `Vec<u8>` — the spec's schema-prefixed `statelessInputBytes` — so `ProgramInput::new(blocks, witness)` and the `.blocks` field this tool relied on no longer exist. Any merge of main into this branch hits it; the zisk clippy job was the only thing building the crate, which is why it was the sole failure. `build_ssz_stateless_input` is the one encoder in the tree that produces those bytes. It sits under `crates/l2/sequencer/native_rollup/` but touches nothing beyond `ethrex-common` and `NativeCrypto`, so `ethrex-l2` is taken as a dependency to reach it. That is a heavy edge for a benchmark tool — the lock goes from 468 to 920 packages — and the alternative would be to move the encoder down into `ethrex-common` next to the types it converts. `SszStatelessInput` carries a single `new_payload_request`, while a cache holds a `Vec<Block>`. Every ethrex-replay cache is one block, so the conversion destructures to exactly one and errors otherwise rather than silently benchmarking the first block of a multi-block cache. Enabling `ethrex-l2` does not disturb the input type: its `l2` feature is inert (`l2 = []`) and it depends on `ethrex-guest-program` with default features, which are `["secp256k1"]`. `ProgramInput` therefore stays `Vec<u8>` rather than flipping to the L2 rkyv struct through feature unification. Tests assert the schema-id prefix and non-empty bytes, the structural facts that survive the type change.
3 participants
Motivation
We want to benchmark the performance and memory usage of our client when executing inside a zkVM.
This is useful to detect and fix OOMs, and deterministically measure purported performance improvements
Description
This executes our zisk guest using ziskemu, giving information about cost and memory usage. The performance under ZisK should generalize well to other zkVMs.
This tool has three presets:
quick(a couple of real blocks + selected worst-case tests),medium(real blocks + EEST microbenchmarks + selected worst-case tests), andslow(everything).The workloads used are EEST tests to evaluate improvements to specific operations, resource-hogging fixtures (EEST tests-benchmark) to improve pathological cases, and real mainnet blocks to assess typical real-world performance.
The output format is JSON, for easy automated usage. Some future uses could be in the CI (to prevent regressions) and with autoresearch-like tools.
Closes #6955