bench(recursion): measure the verifier at real query counts, over real blocks#846
Conversation
|
Benchmark Results for modified programs 🚀
|
|
/ai-review |
Codex Code Review
|
|
Reviewed as a static diff review (working tree is at main, so Verified correct
FYI (not blocking, out of scope for this PR) Minor
|
|
Note: |
|
Benchmark Results for unmodified programs 🚀
|
9e9f790 to
995f971
Compare
6d8c4dd to
23a96ae
Compare
|
/ai-review |
Codex Code Review
|
Review — no significant issues foundReviewed the diff (safety/security, correctness, performance, readability). This PR is almost entirely benchmark/test/build plumbing on top of the existing recursion verifier, and it's carefully done and well-documented. Verified as correct:
Note (not a finding): the Nothing blocking. |
AI ReviewPR #846 · 13 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-003: bench_recursion_cycles.sh block-mode result cache key ignores BLOCK_TXS/BLOCK_EPOCH_LOG2
Claim In Evidence Line 197: Suggested fix Include the same block parameter suffix in the result cache key, e.g. AI-008: run_recursion_bench.sh else-branch message is misleading
Claim The "unavailable for these refs" message is shown for ANY bench_recursion_cycles.sh failure (build error, missing fixture, MEASURE_CLI failure), not just unsupported preset combos. A build error or missing fixture would be reported as "preset unavailable" instead of surfacing the real cause. Evidence
Suggested fix Inspect the captured log and print a more specific message (e.g., grep the log for the known "predates" / "missing" markers the script already emits, and otherwise fall back to a generic "failed — see workflow log"). AI-010: run_recursion_bench.sh raw-log fallback always uses the min preset log
Claim Only Evidence Line 27: Suggested fix Copy the per-preset log that actually ran last, or update the fallback in bench-verify.yml to read AI-012: test-profile-recursion aggregate target doesn't include block variant
Claim
Evidence Line 282-283: Suggested fix Add AI-019: DUMP_FEATURES word-splitting turns space-separated features into positional test filters
Claim The expansion Evidence scripts/bench_recursion_scaling.sh line ~89: Suggested fix Document/require comma-separated features (e.g. 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 (12) — rejected by the verifier
Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
…l blocks The recursion cycle benchmark only measured MIN_PROOF_OPTIONS (blowup=2, 1 FRI query) verifying a proof of the empty program - a diagnostic floor, not the production verifier cost (blowup=2 at 128-bit is 219 queries, and real inner proofs are multi-epoch continuation bundles of real blocks). Presets and guests: - Preset::Blowup2 (219 q) / Blowup4 (110 q) - the realistic base-layer regimes - alongside the existing Min/Blowup8; one guest ELF per preset. - bench_vs/lambda/recursion's `continuation` guest feature: verify a whole ContinuationProof bundle in-VM (recursion-cont-<preset>.elf), built on the continuation supplied-roots verify path and the zero-copy continuation guest API from the prior two PRs. Benchmark plumbing: - test_dump_recursion_input gains env knobs: RECURSION_DUMP_PRESET, RECURSION_DUMP_INNER_ELF/INPUT (any inner program, e.g. ethrex blocks), RECURSION_DUMP_EPOCH_LOG2 (continuation mode). - scripts/bench_recursion_cycles.sh: preset-aware dump, --release test invocations, per-preset regime labels; /bench-verify now posts min + blowup2 + blowup4 tables. - scripts/bench_recursion_scaling.sh: block-size x preset ladder over the committed ethrex fixtures (1/4/8/16 transfers). - A real-block detailed profile test (blowup=4, 4 transfers) plus a `make test-profile-recursion-block` target, since cycle counts across every preset/block size are already covered by the scripts above.
Address PR review feedback:
- build_guest_elf: pass --bin so a continuation build's superset
features (`continuation min`) don't also rebuild the plain preset's
bin into the same shared target-dir path a concurrent `make -j` job
is writing to.
- drop redundant ethrex_bench_{1,8,16}.bin fixtures (only _4 is read
by tests); bench_recursion_scaling.sh regenerates them on demand.
- bench_recursion_scaling.sh: don't let one failed cycle-count/grep
abort the whole sweep under `set -e`; clean up its temp dir.
- recursion.rs docs: list blowup4 among the fixed-preset ELFs.
- recursion profile tests: check the guest's actual committed
attestation against a trusted host recompute, not just that it ran
without crashing — covers both the monolithic and continuation
(real ethrex block) profile paths.
0bab2ac to
ed9d34f
Compare
test_recursion_profile_blowup4_block previously re-proved a real ethrex block's continuation bundle on every run (minutes of prover work) just to profile the verifier guest. Build the input once via a new recursion-profile-block-input Makefile target and read it as a fixture, with an .expected sidecar carrying the id+output ground truth so the test can verify the guest's attestation without re-deriving it.
…roof profile-recursion.yml runs test-profile-recursion-block; bench-verify.yml adds a blowup4-block regime. The dump test's proof blob is now cached by ref SHA so repeat runs skip re-proving. Also trims verbose comments.
Moves the inline shell for looping presets (min/blowup2/blowup4/ blowup4-block) into .github/scripts/run_recursion_bench.sh, matching the repo's other .github/scripts/*.sh conventions.
local a=x b=$a expands $a before the assignment lands, so under set -u this failed as "preset: unbound variable". Split into two local statements.
Cut generic-monomorphized Rust symbol names to 90 chars, hiding the type params that usually matter most when reading the profile.
…mplete continue-on-error on that step exists to protect the already-posted verifier result, not to hide a broken recursion step behind a green job. Add a final step, after the result posts, that fails the job on a non-success recursion outcome.
Codex Code Review
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review summaryBenchmark-infrastructure PR: adds Strengths
Minor
Nothing blocking. |
…rs input Result cache was keyed on $PRESET alone while the blob cache also folds in BLOCK_TXS/BLOCK_EPOCH_LOG2, so overriding those on a cached ref silently reused a stale measurement. Also validate the workflow_dispatch pairs input is numeric before the range check — a non-numeric value made both comparisons error out and pass through unchanged.
2 participants
Summary
Stacked on #845. The recursion cycle benchmark only measured
MIN_PROOF_OPTIONS(blowup=2, 1 FRI query) verifying a proof of the empty program — a diagnostic floor, not the production verifier cost (blowup=2 at 128-bit is 219 queries, and real inner proofs are multi-epoch continuation bundles of real blocks).Presets and guests:
Preset::Blowup2(219 q) /Blowup4(110 q) — the realistic base-layer regimes — alongside the existing Min/Blowup8; one guest ELF per preset.bench_vs/lambda/recursion'scontinuationguest feature: verify a wholeContinuationProofbundle in-VM (recursion-cont-<preset>.elf), built on feat(recursion): supply DECODE/global-memory-genesis roots via private input for continuation verify #844/perf(prover): verify continuation proofs in place via rkyv #845's continuation supplied-roots verify path and zero-copy guest API.Benchmark plumbing:
test_dump_recursion_inputgains env knobs:RECURSION_DUMP_PRESET,RECURSION_DUMP_INNER_ELF/INPUT(any inner program, e.g. ethrex blocks),RECURSION_DUMP_EPOCH_LOG2(continuation mode).scripts/bench_recursion_cycles.sh: preset-aware dump,--releasetest invocations, per-preset regime labels;/bench-verifynow posts min + blowup2 + blowup4 tables.scripts/bench_recursion_scaling.sh: block-size × preset ladder over the committed ethrex fixtures (1/4/8/16 transfers).make test-profile-recursion-blocktarget, since cycle counts across every preset/block size are already covered by the scripts above.Note: this branch currently includes a cherry-picked copy of #843's commit (
fix(syscalls): raise the private-input clamp to 512 MiB) so the real-block tests are runnable now (real blocks exceed the old 64 MiB cap). That commit will disappear from this diff once #843 merges and this branch is rebased onto the updated base.Test plan
cargo test -p lambda-vm-prover --lib(516 passed)test_recursion_profile_blowup4_blockrun end-to-end against a real 4-tx ethrex block (not just compiled) — produced the full cycle/step/function breakdowncargo clippy --all-targets -- -D warnings/cargo fmt --checkmake -n test-profile-recursion-blockresolves the full build graph correctly