Skip to content

perf(prover): verify continuation proofs in place via rkyv#845

Merged
MauroToscano merged 9 commits into
mainfrom
perf/continuation-rkyv-zero-copy
Jul 17, 2026
Merged

perf(prover): verify continuation proofs in place via rkyv#845
MauroToscano merged 9 commits into
mainfrom
perf/continuation-rkyv-zero-copy

Conversation

@Oppen

@Oppen Oppen commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Stacked on #844. Adapts continuation verification to the same in-place rkyv pattern verify_recursion_blob already uses for the monolithic path (#769):

  • verify_epoch/verify_global take a StarkProofView slice (owned or archived) instead of an owned MultiProof/EpochProof, so the new guest entry point (verify_continuation_and_attest_blob, via verify_continuation_archived) reads every per-epoch/global STARK proof straight out of the archive. Only small per-epoch metadata (table counts, reg_fini, l2g_root, public output) is materialized — the (large) per-epoch/global proof data is never copied into an owned MultiProof just to verify it.
  • Adds the continuation guest's wire format (ContinuationGuestInput, encode_continuation_guest_input) mirroring GuestInput's magic-prefixed rkyv layout, and verify_continuation_and_attest[_blob] mirroring verify_and_attest_blob's program_id fold.
  • replay_transcript_phase_a/compute_expected_commit_bus_balance (owned) lose their last production caller to this refactor; deleted rather than kept as compatibility wrappers now that every caller (production and test) goes through the _view variants perf(verifier): verify STARK proofs in place via rkyv #769 already introduced.

Test plan

  • cargo test -p lambda-vm-prover --lib (516 passed)
  • New test: test_recursion_continuation_blob_decodes_and_verifies_on_host — encode/decode/verify/attest contract for the continuation guest blob
  • cargo clippy --all-targets -- -D warnings / cargo fmt --check

@Oppen

Oppen commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Mediumrecursion.rs: The new continuation blob verifier is not wired into the recursion guest. The guest has no continuation feature and always calls verify_and_attest_blob, so an encoded ContinuationGuestInput fails monolithic archive validation. Add the guest feature/dispatch and corresponding artifact build path; the host-only test does not exercise the actual guest entry point.

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review: perf(prover): verify continuation proofs in place via rkyv (#845)

Solid, well-scoped refactor. verify_continuation_archived faithfully mirrors the owned verify_continuation_with_roots — same up-front bounds (num_private_input_pages, n == 0, reg_fini.len()), same per-epoch fold, same page-alignment Err gate, and the view-based verify_global / verify_l2g_commitment_binding_views preserve the length guards (final_proof_views.len() >= epoch_l2g_roots.len()), so no new index panics. The blob path's magic/version + alignment-fallback handling matches verify_recursion_blob exactly, and proof_options stays caller-supplied (never read from the untrusted blob). The _view migration in the tests and the deletion of the now-unused replay_transcript_phase_a / compute_expected_commit_bus_balance are clean.

No safety, soundness, or correctness issues found. A couple of minor notes:

  • Low — possibly-unused API. verify_continuation_and_attest (the non-blob, owned-bundle variant) has no caller or test — the new test and the guest both go through verify_continuation_and_attest_blob, and there is no monolithic non-blob verify_and_attest counterpart to mirror. If it is not an intended host-side entry point, consider dropping it (or add a test) so it does not bit-rot as untested surface area.

  • Nit (no change needed). verify_continuation_and_attest_blob re-loads and re-hashes the inner ELF in program_id_from_elf rather than sharing a digest the way the monolithic verify_and_attest_blob does via RecursionVerification. This is negligible here since the epoch/global transcripts already absorb_statement (hashing the ELF) 2·n times, so a single extra hash is lost in the noise — not worth restructuring verify_continuation_archived's return type for.

Nice work — the parity between the owned and archived paths is easy to audit.

@Oppen
Oppen force-pushed the perf/continuation-rkyv-zero-copy branch from a227d09 to 9a1239c Compare July 17, 2026 19:35
Base automatically changed from feat/continuation-precomputed-commitments to main July 17, 2026 22:02
@Oppen
Oppen force-pushed the perf/continuation-rkyv-zero-copy branch from 9a1239c to 9e9f790 Compare July 17, 2026 22:16
Oppen added 8 commits July 17, 2026 19:17
Adapts continuation verification to the same in-place rkyv pattern
verify_recursion_blob already uses for the monolithic path (#769):
verify_epoch/verify_global take a StarkProofView slice (owned or
archived) instead of an owned MultiProof/EpochProof, so the new guest
entry point (verify_continuation_and_attest_blob, via
verify_continuation_archived) reads every per-epoch/global STARK proof
straight out of the archive. Only small per-epoch metadata (table
counts, reg_fini, l2g_root, public output) is materialized - the
(large) per-epoch/global proof data is never copied into an owned
MultiProof just to verify it.

Adds the continuation guest's wire format (ContinuationGuestInput,
encode_continuation_guest_input) mirroring GuestInput's magic-prefixed
rkyv layout, and verify_continuation_and_attest[_blob] mirroring
verify_and_attest_blob's program_id fold.

replay_transcript_phase_a/compute_expected_commit_bus_balance (owned)
lose their last production caller to this refactor; deleted rather
than kept as compatibility wrappers now that every caller (production
and test) goes through the _view variants already introduced by #769.
Adds a negative test for the archived continuation path: encode a
bundle with a tampered epoch l2g_root, assert the blob verify rejects
it. Drops verify_continuation_and_attest_blob's now-unused owned-bundle
predecessor and renames the surviving function to
verify_continuation_and_attest, since the _blob suffix only existed to
disambiguate it from that owned variant. Also fixes a stale intra-doc
link left over from the deleted compute_expected_commit_bus_balance.
…wed views

verify_epoch/verify_global/verify_proof_parts had ballooned into
long parameter lists (proof_views, table_counts, runtime_page_ranges,
reg_fini, l2g_root, public_output, ...) built ad hoc at every call
site via `.iter().map(StarkProofView::Owned/Archived).collect()`, a
leftover from keeping owned and archived verify paths side by side.

Adds MultiProofView (crypto/stark) mirroring StarkProofView, and
EpochProofView/ContinuationProofView (prover) mirroring it one level
up, so callers pass one view instead of exploding a bundle into
loose fields. multi_verify_views, compute_expected_commit_bus_balance_view,
and replay_transcript_phase_a_view are now generic over a
ProofViewSource trait (impl'd for slices, Vecs, and MultiProofView)
and iterate in place - no Vec materialization anywhere in the path.

Collapses verify_continuation_with_roots/verify_continuation_archived
(two ~130-line near-duplicates) into one verify_continuation_view, and
unifies verify_l2g_commitment_binding/_views into one view-based fn.
…not per element

MultiProofView::iter() built each StarkProofView via get(i): a match
on Owned/Archived plus a bounds-checked index, redone for every
element on every one of multi_verify_views' several passes over the
same proof set, even though a given MultiProofView is homogeneously
one variant for its whole lifetime.

MultiProofViewIter now matches once, in iter() itself, and then
drives a plain slice::Iter for whichever representation was chosen -
no per-step bounds check, no re-matching against the source enum.
…ration, not per element"

This reverts commit f0cd8abb8d5b05e656072d462efae5f80e92c9a4.
Closes the ~0.04% guest-cycle regression the MultiProofView refactor
introduced (measured via scripts/bench_recursion_scaling.sh,
blowup4/txs=4): len/get/last/iter and the ProofViewSource impls'
view_len/view_iter cross a crate boundary (stark -> prover) and
apparently weren't getting inlined into multi_verify_views' hot loops.

An alternative fix - matching Owned/Archived once per MultiProofView
instead of once per element, via a hand-rolled slice::Iter-backed
enum iterator - was tried and measured WORSE (+2.3% cycles): it
defeats LLVM's specialized Range<usize>::map optimization the
closure-based iter() benefits from (see the previous two commits).
#[inline(always)] alone, keeping the closure-based iter(), measured
at parity with (very slightly better than) the pre-refactor baseline.
…inuation verify

- Add tests that splice a different run's global proof onto valid epochs
  (same shape, different L2G data) so verify_l2g_commitment_binding_view's
  own reject branch actually executes, both for the owned and archived
  verify paths. The existing tampered-root tests were caught earlier by
  verify_epoch's per-epoch root check and never reached this binding.
- Thread entry_point out of verify_continuation_archived so
  verify_continuation_and_attest can fold program_id via
  program_id_from_digest instead of re-parsing the ELF with
  program_id_from_elf.
- Extract access_recursion_archive, sharing the aligned-fallback +
  rkyv::access boilerplate between verify_recursion_blob and
  verify_continuation_and_attest.
@Oppen
Oppen force-pushed the perf/continuation-rkyv-zero-copy branch from 9e9f790 to 995f971 Compare July 17, 2026 22:17
@Oppen

Oppen commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

  • Medium — prover/src/recursion.rs:283: The continuation verifier is not wired into the recursion guest. bench_vs/lambda/recursion/src/main.rs still exclusively calls verify_and_attest_blob; no production code calls verify_continuation_and_attest. Thus continuation blobs cannot actually be verified by the intended guest, despite the host-only smoke test. Add the feature-gated guest entry-point branch or remove this currently unreachable path.

Comment thread prover/src/recursion.rs
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review — perf(prover): verify continuation proofs in place via rkyv

Reviewed the diff focused on safety/soundness, bugs, performance, and readability. This is a clean, well-scoped refactor that extends the existing owned/archived StarkProofView pattern up to the MultiProof level and to continuation verification. Verification semantics look preserved throughout:

  • multi_verify_views over ProofViewSource reproduces the old slice-based iteration exactly (len guard, three per-table passes, lookup-challenge fold), just without materializing a Vec.
  • verify_epoch new Result<bool, Error> split is sound: table_counts.validate(), the expected_proof_count check, and the final l2g_root consistency check all keep the same meaning; metadata-deserialize Err paths are effectively unreachable after rkyv::access bytecheck (defensive, fine).
  • entry_point returned from verify_continuation_archived matches program_id_from_elf/program_id (both take elf.entry_point from the same Elf::load), so the folded program_id is identical to the honest recompute — the new tests confirm this.
  • The archived path only materializes small metadata; the large per-epoch/global proof data stays in the buffer. No new unsafe, no unbounded allocation, no host/device concerns.
  • Good test coverage added for both the per-epoch root check and verify_l2g_commitment_binding_view own reject branch, over both the owned and zero-copy blob paths.

Findings:

Low (readability / doc accuracy) — verify_continuation_and_attest reimplements the prefix + alignment + rkyv::access logic that the PR own new access_recursion_archive helper exists to share; the helper doc even names this function as a caller, but it is not one. See inline comment — use the helper or fix its doc.

Nothing blocking. Nice work.

@Oppen

Oppen commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

New feedback will be addressed in a follow-up unless blocking.

@github-actions

Copy link
Copy Markdown

AI Review

PR #845 · 9 changed files

Findings

Status Sev Location Finding Found by
confirmed medium crypto/stark/src/proof/view.rs:470 MultiProofView::get panics on out-of-bounds index nemotron
openrouter/nvidia/nemotron-3-ultra-550b-a55b
confirmed low prover/src/continuation.rs:530 verify_epoch return type change propagates deserialization errors as Err nemotron
openrouter/nvidia/nemotron-3-ultra-550b-a55b
confirmed low prover/src/continuation.rs:1189 verify_continuation_view returns entry_point that only the archived caller uses moonmath
zro/minimax-m3
confirmed low prover/src/continuation.rs:1288 verify_continuation_view materializes each epoch's reg_fini twice (verify_epoch + the loop) moonmath
zro/minimax-m3
confirmed low prover/src/lib.rs:271 access_recursion_archive doc claims it is shared with verify_continuation_and_attest, but that caller duplicates the logic moonmath
zro/minimax-m3
glm
openrouter/z-ai/glm-5.2
kimi
openrouter/moonshotai/kimi-k2.7-code
confirmed low prover/src/tests/prove_elfs_tests.rs:57 Test helpers still allocate the Vec<StarkProofView> boilerplate MultiProofView was meant to eliminate glm
openrouter/z-ai/glm-5.2

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

AI-001: MultiProofView::get panics on out-of-bounds index
  • Status: confirmed
  • Severity: medium
  • Location: crypto/stark/src/proof/view.rs:470
  • Found by: nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The public get() method uses as_slice()[i] which panics if i >= len(). While internal callers (iter, last) check bounds first, the method is part of the public API and external callers could trigger a panic.

Evidence

Line 470: Self::Archived(p) =&gt; StarkProofView::Archived(&amp;p.proofs.as_slice()[i]) - direct index access without bounds check. The method is pub fn get(&amp;self, i: usize) -&gt; StarkProofView&lt;'a, F, E, PI&gt; with no Option return.

Suggested fix

Either return Option<StarkProofView> (like Vec::get) or add a debug_assert/panic with a clear message. Since this is a view type for internal use, adding debug_assert!(i &lt; self.len(), "index out of bounds"); before the match would catch misuse in debug builds while keeping the fast path in release.

AI-003: verify_epoch return type change propagates deserialization errors as Err
  • Status: confirmed
  • Severity: low
  • Location: prover/src/continuation.rs:530
  • Found by: nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

verify_epoch now returns Result<bool, Error> instead of bool. Deserialization errors from archived proofs now propagate as Err rather than returning false. This is a behavior change for callers.

Evidence

Function signature changed from fn verify_epoch(...) -&gt; bool to fn verify_epoch(...) -&gt; Result&lt;bool, Error&gt;. Callers in verify_continuation_view use ? to propagate errors. Previously both verification failure and deserialization failure returned false.

Suggested fix

This appears intentional (better error handling), but document the semantics: Err = malformed bundle (deserialization failed), Ok(false) = well-formed proof that failed verification, Ok(true) = success. Ensure all callers handle the three cases correctly.

AI-004: verify_continuation_view returns entry_point that only the archived caller uses
  • Status: confirmed
  • Severity: low
  • Location: prover/src/continuation.rs:1189
  • Found by: moonmath:zro/minimax-m3
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

verify_continuation_view returns Result&lt;Option&lt;(Vec&lt;u8&gt;, u64)&gt;, Error&gt; where the u64 is elf.entry_point. The doc on verify_continuation_archived claims the entry_point avoids a second Elf::load for the program_id fold, but the only path that actually uses the returned entry_point is verify_continuation_archived -> verify_continuation_and_attest. The owned path (verify_continuation_with_roots) discards it with Ok(result.map(|(public_output, _entry_point)| public_output)) at continuation.rs:1189, so for the owned call the second tuple element is dead — it is computed and thrown away. This is harmless but it leaks the archived-only optimization into the shared return type, which is exactly the kind of API friction the view refactor was meant to clean up.

Evidence

continuation.rs:1189 discards the entry_point in the owned wrapper. continuation.rs:1331 returns Ok(Some((public_output, elf.entry_point))) from the shared view. elf.entry_point is Elf::entry_point which was already read at continuation.rs:1261 via register::register_init_from_entry_point(elf.entry_point) — so it was computed once and is read again at the end. The savings in the archived path are real (avoids an extra Elf::load in the recursion guest); the cost in the owned path is negligible (a field copy).

Suggested fix

Keep as-is unless verify_continuation_view is exposed widely enough that the unused tuple element becomes a maintenance hazard; in that case, split into two helpers (e.g., a verify_continuation_view_with_entry_point for the recursion guest and a simpler verify_continuation_view for the owned path). Not worth doing now.

AI-005: verify_continuation_view materializes each epoch's reg_fini twice (verify_epoch + the loop)
  • Status: confirmed
  • Severity: low
  • Location: prover/src/continuation.rs:1288
  • Found by: moonmath:zro/minimax-m3
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

Per epoch, verify_epoch calls epoch.reg_fini()? to build the AIRs (continuation.rs:762), and then the loop in verify_continuation_view calls epoch.reg_fini()? again at the end of the iteration (continuation.rs:1288) to update register_init for the next epoch. Each EpochProofView::reg_fini() runs rkyv::deserialize::&lt;Vec&lt;u32&gt;, _&gt; on the archived bundle, so on the archived/zero-copy path every epoch pays for the deserialize twice.

Evidence

continuation.rs:762 (let reg_fini = epoch.reg_fini()?; inside verify_epoch) and continuation.rs:1288 (register_init = epoch.reg_fini()?; inside verify_continuation_view's loop) both rkyv-deserialize the same reg_fini Vec<u32> for the same epoch in the same call. For a multi-epoch continuation over the archived blob path, this is 2N deserializes of the same data.

Suggested fix

Have verify_epoch return the materialized reg_fini (or a struct that pairs the bool verdict with the fini), and have the loop reuse that value for the next-iteration register_init. This keeps the rkyv deserialize to once per epoch on the archived path. The owned path is unaffected (a clone() was already happening there, so the cost moves rather than disappears).

AI-006: access_recursion_archive doc claims it is shared with verify_continuation_and_attest, but that caller duplicates the logic
  • Status: confirmed
  • Severity: low
  • Location: prover/src/lib.rs:271
  • Found by: moonmath:zro/minimax-m3, glm:openrouter/z-ai/glm-5.2, kimi:openrouter/moonshotai/kimi-k2.7-code
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The doc comment on access_recursion_archive advertises it as "Shared by [verify_recursion_blob] and [crate::recursion::verify_continuation_and_attest]", but the continuation caller in prover/src/recursion.rs does not use it: it hand-rolls the same recursion_archive_bytes + alignment-fallback + rkyv::access sequence instead. The helper exists precisely for this case (the T is generic over the archived type) and the only reason it isn't reused is the helper additionally returns an archive_base pointer for the rebase step that the continuation path doesn't need — but that is just unused return data.

Evidence

The PR adds access_recursion_archive (pub(crate)) with that doc, and verify_recursion_blob calls it. But verify_continuation_and_attest (recursion.rs lines 283-305) re-implements recursion_archive_bytes + the if (archive_bytes.as_ptr()...).is_multiple_of(ALIGN) { archive_bytes } else { aligned_fallback.extend_from_slice(...); &amp;aligned_fallback } block + rkyv::access inline instead of delegating.

Suggested fix

Either (a) drop the "Shared by ... verify_continuation_and_attest" line from the helper's doc, or (b) call the helper from verify_continuation_and_attest and discard the unused archive_base (and the archive_bytes rebased lifetime isn't needed there because the function owns page_commitments and only re-borrows inner_elf long enough for verify_continuation_archived to Elf::load it). Option (b) keeps the helper genuinely shared as the doc advertises.

AI-009: Test helpers still allocate the Vec<StarkProofView> boilerplate MultiProofView was meant to eliminate
  • Status: confirmed
  • Severity: low
  • Location: prover/src/tests/prove_elfs_tests.rs:57
  • Found by: glm:openrouter/z-ai/glm-5.2
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The PR introduces MultiProofView::Owned(&multi_proof.proof) specifically to replace the proofs.iter().map(StarkProofView::Owned).collect::&lt;Vec&lt;_&gt;&gt;() boilerplate, but several test helpers in prove_elfs_tests.rs still build let views: Vec&lt;StarkProofView&lt;F,E,()&gt;&gt; = multi_proof.proofs.iter().map(StarkProofView::Owned).collect(); and pass &amp;views to compute_expected_commit_bus_balance_view / multi_verify_views.

Evidence

prove_elfs_tests.rs shows let views: Vec&lt;StarkProofView&lt;F, E, ()&gt;&gt; = multi_proof.proofs.iter().map(StarkProofView::Owned).collect(); followed by compute_expected_commit_bus_balance_view(&amp;airs.air_refs(), &amp;views, ...) and Verifier::multi_verify_views(&amp;airs.air_refs(), &amp;views, ...) in prove_and_verify_vm_minimal, verify_vm_minimal, test_prove_elfs_test_commit_4_wrong_pages_rejected, test_deep_stack_runtime_pages_roundtrip, test_deep_stack_missing_pages_rejected, test_heap_alloc_runtime_pages_roundtrip, and the epoch tests. Each could pass MultiProofView::Owned(&amp;multi_proof.proof) / MultiProofView::Owned(&amp;vm_proof.proof) directly to both calls with no Vec allocation.

Suggested fix

Replace the views Vec with MultiProofView::Owned(&amp;multi_proof.proof) (and similarly for vm_proof) and pass it to both compute_expected_commit_bus_balance_view and multi_verify_views, matching how verify_prepared and verify_epoch were migrated.

Reviewer Lanes

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

Verification Lanes

Lane Model Status Confirmed Rejected Uncertain
deepseek-verifier openrouter/deepseek/deepseek-v4-pro success 6 1 0

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

Discarded candidates (1) — rejected by the verifier
  • Missing exhaustive field-coverage guard for MultiProof (crypto/stark/src/proof/view.rs:580, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The exhaustive guard functions (lines 651-679) cover structs whose individual fields are accessed by view accessors. MultiProofView only accesses the .proofs field (len, index) and does not destructure any other field of MultiProof. Adding a new field to MultiProof wouldn't create a missing accessor in MultiProofView, so a guard for MultiProof is unnecessary. The guard pattern is for field-level view coverage, not for every archivable struct.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

@MauroToscano
MauroToscano enabled auto-merge July 17, 2026 22:31
@MauroToscano
MauroToscano added this pull request to the merge queue Jul 17, 2026
Merged via the queue into main with commit 3ea4f91 Jul 17, 2026
15 checks passed
@MauroToscano
MauroToscano deleted the perf/continuation-rkyv-zero-copy branch July 17, 2026 23:07
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