Skip to content

perf(prover): batch-invert COMMIT bus fingerprints; doc/dedup fixes#607

Merged
diegokingston merged 7 commits into
mainfrom
cleanup/prover-lib
May 22, 2026
Merged

perf(prover): batch-invert COMMIT bus fingerprints; doc/dedup fixes#607
diegokingston merged 7 commits into
mainfrom
cleanup/prover-lib

Conversation

@diegokingston
Copy link
Copy Markdown
Collaborator

compute_commit_bus_offset — batch inversion

  • The COMMIT-bus offset loop inverted one fingerprint per public-output byte: N separate cubic-extension inversions (each a Fermat-chain exponentiation). Collect the N fingerprints and inplace_batch_inverse instead — 1 inversion + O(3N) muls. This runs on the verify path (verify_with_optionscompute_expected_commit_bus_balance), which matters for the RISC-V recursion guest. Behaviour-identical: a zero fingerprint still yields None (batch inverse Errs on any zero).

TableCounts::total — fix copy-pasted doc

  • total()'s doc comment was validate()'s text verbatim plus the real one-liner tacked on the end. Replaced with just the accurate summary.

replay_transcript_phase_a — drop duplicated arm

  • Both branches of the if air.is_preprocessed() ended with the same append_bytes(main_merkle_root). Hoisted it out; transcript order is unchanged.

273 prover lib tests pass; lint clean.

diegokingston and others added 2 commits May 21, 2026 18:08
compute_commit_bus_offset — batch inversion
- The COMMIT-bus offset loop inverted one fingerprint per public-output
  byte: N separate cubic-extension inversions (each a Fermat-chain
  exponentiation). Collect the N fingerprints and `inplace_batch_inverse`
  instead — 1 inversion + O(3N) muls. This runs on the verify path
  (`verify_with_options` → `compute_expected_commit_bus_balance`), which
  matters for the RISC-V recursion guest. Behaviour-identical: a zero
  fingerprint still yields `None` (batch inverse `Err`s on any zero).

TableCounts::total — fix copy-pasted doc
- `total()`'s doc comment was `validate()`'s text verbatim plus the real
  one-liner tacked on the end. Replaced with just the accurate summary.

replay_transcript_phase_a — drop duplicated arm
- Both branches of the `if air.is_preprocessed()` ended with the same
  `append_bytes(main_merkle_root)`. Hoisted it out; transcript order is
  unchanged.

273 prover lib tests pass; lint clean.
@github-actions
Copy link
Copy Markdown

Codex Code Review

Found one issue.

Medium - DoS via unbounded verifier allocation

prover/src/lib.rs

compute_commit_bus_offset now allocates a Vec<FieldElement<E>> with one extension-field element per byte of VmProof.public_output. verify_with_options accepts public_output from the untrusted proof bundle and does not enforce the executor’s MAX_PUBLIC_OUTPUT_TOTAL_SIZE cap before calling this. A malicious proof with an oversized public_output can cause large memory amplification during verification, potentially OOMing the verifier.

Actionable fix: in verify_with_options, reject vm_proof.public_output.len() > MAX_PUBLIC_OUTPUT_TOTAL_SIZE before computing the bus balance, or batch-invert in bounded chunks while still enforcing the VM output cap.

No other issues found in the changed diff.

@claude
Copy link
Copy Markdown

claude Bot commented May 21, 2026

Review: batch-invert COMMIT bus fingerprints; doc/dedup fixes

No security issues, bugs, or significant concerns found.

Changes reviewed

compute_commit_bus_offset — batch inversion (security-relevant)
Zero-fingerprint handling is identical: inplace_batch_inverse returns Err on any zero (all-or-nothing per its contract), propagated via .ok()?None, same as the original per-element fingerprint.inv().ok()?. The algorithm (prefix products + back-substitution) is correct. The parallel path pre-scans for zeros before mutation, preserving the all-or-nothing invariant. Empty-slice case is handled by the pre-existing early return; the batch path also handles it correctly.

replay_transcript_phase_a — deduplicate arm
Transcript order is preserved: preprocessed AIRs still get precomputed_commitment appended before lde_trace_main_merkle_root; all AIRs get the merkle root. Behavior-identical refactor.

TableCounts::total — doc fix
The security-critical warning ("a zero count for any table would remove its constraints from verification...") is correctly preserved on validate() (lines 101–104). The erroneous copy-paste on total() is cleaned up with no information loss.

@diegokingston diegokingston added this pull request to the merge queue May 22, 2026
Merged via the queue into main with commit 4a87a27 May 22, 2026
12 checks passed
@diegokingston diegokingston deleted the cleanup/prover-lib branch May 22, 2026 21:44
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