Skip to content

fix(prover): emit missing marker on continuation verify paths - #855

Open
Oppen wants to merge 1 commit into
mainfrom
fix/continuation-profile-markers
Open

fix(prover): emit missing marker on continuation verify paths#855
Oppen wants to merge 1 commit into
mainfrom
fix/continuation-profile-markers

Conversation

@Oppen

@Oppen Oppen commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • verify_epoch's per-epoch verify and verify_global (continuation.rs) call Verifier::multi_verify_views directly, without the STEP_AIRS_AND_BUS_BALANCE_DONE marker that prover/lib.rs's monolithic verify_proof_parts emits before its own multi_verify_views call.
  • The recursion-block profile test (test_recursion_profile_blowup4_block) buckets cycles by the latest marker observed, so on the continuation path the "multi_verify setup (transcript replay phase A/B, per-table fork)" step reported a flat 0% — its real cost was silently folded into whichever bucket was already active (airs_and_bus_balance for the first epoch, step4:openings carried over for later epochs/the global proof).
  • Adds the same marker call at both continuation call sites, matching the monolithic path.

Test plan

  • cargo check -p lambda-vm-prover --lib
  • make test-profile-recursion-block — step 2 now reports 17,041,080 cycles (1.16%) instead of 0

…on verify paths

verify_epoch and verify_global call multi_verify_views directly, without
the marker prover/lib.rs's monolithic verify_proof_parts emits before its
own multi_verify_views call. The recursion-block profile test buckets
cycles by the latest marker observed, so on the continuation path
"multi_verify setup (transcript replay phase A/B, per-table fork)"
cycles were silently folded into whichever bucket was already active
(airs_and_bus_balance for the first epoch, step4:openings carried over
for later epochs), reporting the step at a flat 0.

@MauroToscano MauroToscano left a comment

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.

⚠️ AI-generated review. Produced by an automated agent and posted from this account — not written by hand. Findings below are leads to verify, not confirmed conclusions.

Adds STEP_AIRS_AND_BUS_BALANCE_DONE at the two continuation multi_verify_views call sites so the block profile stops reporting a flat 0% for "multi_verify setup". Placement matches verify_proof_parts (after compute_expected_commit_bus_balance_view, before the call); step_marker is addi x0, x0, N gated on cfg(all(feature = "profile-markers", target_arch = "riscv64")), so there is no host-side or semantic surface — no transcript touch, no new panic path, nothing driven by proof data. seed() and global_transcript(...) are call arguments, so they evaluate after the marker and land in bucket 2 as on the monolithic path. Correct as far as it goes, but the attribution fix is only half complete.

prover/src/continuation.rs:809 — the marker reclaims the [2 -> 3] window, but everything in verify_epoch before it is still billed to the latched bucket, which for every epoch after the first is bucket 6. verifier.rs:610 leaves STEP_VERIFY_TRACE_AND_COMPOSITION_OPENINGS set when multi_verify_views returns, and the epoch loop (continuation.rs:1271-1295) emits nothing between iterations. So for epochs 1..n-1, build_epoch_airs — including register::compute_precomputed_commitment_with_fini, an FFT+Merkle recompute per epoch — plus the first seed() (a full-ELF keccak inside absorb_statement) and the phase-A replay in compute_expected_commit_bus_balance_view are all charged to "6. step4:openings". Only epoch 0's pre-work reaches bucket 1, via the guest's single STEP_DECODE_DONE. The 17.0M cycles reclaimed here are themselves dominated by the second seed() keccak, so the residual is the same order of magnitude: "1. airs_and_bus_balance" under-reports by roughly a factor of n and "step4:openings" is inflated by the same amount — on exactly the numbers used to prioritize verifier work. Cheap follow-up: emit a bucket-1 marker at the top of verify_epoch; a dedicated STEP_EPOCH_SETUP is preferable to reusing STEP_DECODE_DONE, whose name would then be wrong. verify_global:971 has the same shape but is cheap on the profiled path, since the guest supplies page_commitments and global_memory_configs_classify_only skips the per-page genesis recompute.

Nits:

  • No test guards the two instrumented paths: run_profile_from (prover/src/tests/recursion_smoke_test.rs:409) asserts the attestation but nothing about buckets, and the marker-order regression test drives only recursion-min (setup_guest_run -> Preset::artifact_stem()), so a future drop reappears as the same silent 0% row — asserting buckets[1..=6] are all nonzero at the end of run_profile_from holds for both guests and is preferable to a continuation-guest variant, since setup_block4_blowup4_guest_run panics outright when the block fixture is absent.
  • prover/src/tests/recursion_smoke_test.rs:778 — the documented convention ("1 -> 2 once each, then N repetitions of 3,4,5,6", transitions {1->2, 2->3, 3->4, 4->5, 5->6, 6->3}) is now monolithic-only: on the continuation guest marker 2 fires n+1 times and 6->2 is a legal edge; harmless today, but this comment is the only written record of the convention.

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