FRI early-termination review follow-ups (2/3): deduplicate the fold schedule + terminal check#787
Merged
MauroToscano merged 2 commits intoJul 6, 2026
Conversation
The early-termination fold layout (clamp, total_folds, num_committed, terminal_len, effective_k) was computed independently in three places — `commit_phase_from_evaluations` (CPU prover, from LDE size), `try_fri_commit_gpu` (GPU prover, from n0), and `fri_termination_params` (verifier, from trace bits) — with two different parameterizations whose equivalence lived only in comments. The verifier's own doc comment claimed the arithmetic was centralized "to prevent silent drift", but it was not: an edit to the clamp in one copy would break all proofs, or (worse) only GPU-produced ones, whose parity is not exercised in CI. Introduce `FriFoldLayout::new(lde_log, blowup_log, k)` in fri/terminal.rs and have all three callers derive the layout from it. The single formulation (`terminal_log = min(blowup_log + k, lde_log)`) is also overflow-safe by construction, removing the hand-rolled shift-overflow guards. As part of the same cleanup, the CPU prover now derives the terminal coset offset once as `coset_offset^(2^total_folds)` (matching the GPU and verifier) instead of tracking it by incremental squaring through the fold loop. Wire-identical: full stark prove/verify suite, FRI early-termination soundness tests, and multi-table roundtrip all pass; GPU path compiles under --features cuda.
…d loop `verify_query_and_sym_openings` checked each query against the reconstructed terminal codeword in two places — a dedicated early return for the single-fold (`total_folds == 1`) regime, and the last-iteration `else` arm inside the fold loop for the multi-fold regime. Two hand-synced copies of a soundness-critical comparison (the last-iteration arm being exactly where the padded-decommitment bypass the PR guards against would land) is a drift risk. After the fold loop, `v` and `index` already hold the query's terminal-layer value and position in every regime, so a single check hoisted after the loop covers both — deleting the `is_empty()` early return and the `i < len - 1` last-iteration branch. The per-query decommitment length checks in `step_3_verify_fri` (which pin the fold count) make this behaviour-identical. Verified by the full FRI early-termination soundness suite (empty/padded/ truncated/over-length decommitment rejection across the no-fold, single-fold, and multi-fold regimes) and the prove/verify roundtrips.
MauroToscano
force-pushed
the
review/fri-schedule-dedup
branch
from
29c6af7 to
7c39c7b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #729 (PR-A #785 already merged into this branch). Two Medium-severity refactors from the review, one commit each. No proof-format or transcript change — wire-identical.
refactor(stark): derive the FRI fold schedule in one placeThe early-termination schedule (clamp,
total_folds,num_committed,terminal_len,effective_k) was computed independently in three places — the CPU prover (commit_phase_from_evaluations, from LDE size), the GPU prover (try_fri_commit_gpu, fromn0), and the verifier (fri_termination_params, from trace bits) — with two parameterizations whose equivalence lived only in comments.fri_termination_params' own doc claimed the arithmetic was centralized "to prevent silent drift"; it wasn't. The GPU copy is the riskiest — its parity test is#[ignore], so CI never runs it.Introduces
FriSchedule::new(lde_log, blowup_log, k)infri/terminal.rs; all three callers derive from it. The single formulationterminal_log = min(blowup_log + k, lde_log)is overflow-safe by construction, removing the hand-rolled shift-overflow guards. Also folds the CPU prover's incrementalterminal_offsetsquaring into the one-linecoset_offset^(2^total_folds)the GPU and verifier already use.refactor(stark): hoist the terminal-codeword check out of the fold loopverify_query_and_sym_openingscompared each query against the terminal codeword in two hand-synced places — a single-fold early return and the fold loop's last-iterationelsearm (exactly where the padded-decommitment bypass the PR guards against would land). Sincev/indexhold the terminal-layer value/position after the loop in every regime, one hoisted check covers both, deleting theis_empty()early return and thei < len - 1branch. Behaviour-identical becausestep_3_verify_fri's decommitment-length checks pin the fold count.Testing
starklib suite (187 tests) green, including every FRI early-termination soundness negative test (empty / padded / truncated / over-length decommitment across no-fold, single-fold, multi-fold;cross_k; oversized-k clamp; blowup4; k0) and the multi-table serialized roundtrip.--features cudatype-checks (GPU prover uses the sharedFriSchedule);cargo fmt --checkandclippyclean onstark+math-cuda, default and cuda.