Skip to content

Commit aa67845

Browse files
iakovenkosAztecBotfedericobarbacoviledwards2225
authored
chore: respond to veridise findings (#287)
# Veridise audit response — barretenberg recursive verifiers Addresses findings from the Veridise audit of barretenberg's recursive verifier stack (sumcheck, Gemini/Shplonk/Shplemini, small-subgroup-IPA, ECCVM/translator, pairing points). The changes are correctness hardening, defense-in-depth assertions, dead-code removal, and documentation/comment fixes. No proving behavior changes on valid inputs — the fixes are guards/asserts on edge cases, comment corrections, and removal of unreachable code — so verification keys are unaffected. The one behavioral change (1165) only affects malformed mixed-infinity pairing accumulators that honest execution never produces. A living tracking doc, `veridise-audit-issues.md`, records the per-finding and per-sub-item disposition (fixed / won't-fix / N-A / open) with rationale. ## Correctness / hardening fixes - **1212** — recursive verifier ctors guard against an empty/all-constant proof (`stdlib::Proof::get_context`). - **1215** — reject empty `multivariate_challenge` in Shplemini / SmallSubgroupIPA. - **1158** — skip G1 points at infinity (and their precomputed lines) in `reduced_ate_pairing_batch_precomputed`. - **1165** — `PairingPoints::aggregate` rejects mixed-infinity accumulators instead of treating them as uninitialized. - **1217** — remove the dead, argument-swapped `VKAndHash_` alias from the goblin recursive flavors. - **1222** — enforce ZK-sumcheck API invariants (single callable `prove()` per flavor; side-effect-free ZK-correction handler). - **1225** — remove the unreachable masking-tail machinery in the Gemini batcher (dead since masking moved to the trace top), eliminating a latent `add_scaled` precondition hazard. ## Umbrella findings - **1219** (core primitive input/range/init checks) — **resolved**: 12 fixed, 4 won't-fix, 4 N-A. Notably, `SumcheckProverRound` now owns its round state: `round_size` is private and a `round_index`/`multivariate_d` pair (derived per round) drives a new `advance_round()`, letting `compute_virtual_contribution` assert `is_virtual_round()`. Deriving `multivariate_d` per round keeps this correct for the batched translator's two differently-sized rounds. - **1220** (documentation/comment hygiene + magic literals) — 21 done, 1 won't-fix, 4 N-A. Comment/formula/doc corrections across sumcheck/relations/polynomials/PCS, plus centralizing the masking-length constant (`WITNESS_MASKING_TERM_LENGTH`) in `constants.hpp` (removing three duplicated copies that must agree). - **1221** (code-quality / API-hygiene, defense-in-depth) — most items done: `[[nodiscard]]`s, a `BatchOpeningClaim` consistency assert, `ShplonkVerifier` `finalize`/`export` mutual-exclusion guard, `has_zk`-consistency check, single-use asserts in small-subgroup-IPA, null-SRS guards, and removal of the dead default ctor + `g1_identity` member on the recursive `VerifierCommitmentKey`. ## Accepted / won't-fix (with rationale in the doc) - **1156** — verifier-internal IPA randomness is sound; the real gap was closed separately. - **1159** — `handle_edge_cases=false` is safe (random challenges make collisions negligible); doc-only. - **1214** — the fixed transcript layout gives positional binding in place of explicit label domain separation. - **1223** — `compute_effective_round_size` invariants hold at the current commit; the hot-path hardening is deferred. ## Still open (decision needed) - **1213** — whether to add a transcript-level "fully consumed" check (`num_frs_read == proof_data.size()`) or accept the existing msgpack trailing-data rejection as sufficient. (Finding 1221 #19 rides on this.) ## Cross-cutting decisions worth noting - **Consistency over local "correctness"**: transcript labels kept hardcoded (1220 #15) since tests independently pin the wire strings; `serialize_from_buffer`/`read` names kept (1221 #1, 1219 #2/#3) as the codebase-wide serialization convention. - **Assert-once vs. idempotency**: single-use small-subgroup-IPA builders assert at-most-once (fail loudly on a re-call) rather than silently recompute. - **Post-rebase N-A**: two fixes (1220 #18 Shplonk reservation, 1221 #9 IPA `assert_consistent` call site) were dropped when rebasing onto `next`, which had since refactored the code they targeted; both are documented as N-A. - Also includes a fix making the `gate_challenges.size()` sumcheck-prover assertion flavor-aware (zero for multilinear-batching flavors, `>= multivariate_d` otherwise). --------- Co-authored-by: AztecBot <tech@aztec-labs.com> Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Co-authored-by: ledwards2225 <l.edwards.d@gmail.com>
1 parent 62d09b5 commit aa67845

35 files changed

Lines changed: 379 additions & 340 deletions

INIT_3_HANDOFF.md

Lines changed: 0 additions & 167 deletions
This file was deleted.

barretenberg/cpp/src/barretenberg/chonk/batched_honk_translator/batched_honk_translator_prover.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ void BatchedHonkTranslatorProver::execute_joint_sumcheck_rounds()
155155
}
156156
zk_sumcheck_data.update_zk_sumcheck_data(u, round_idx);
157157
gate_sep.partially_evaluate(u);
158-
translator_round.round_size >>= 1;
158+
translator_round.advance_round();
159159
};
160160

161161
// Per-round helper: compute U_joint = U_MZK + α^{K_H}·U_translator from given polynomial
@@ -215,7 +215,7 @@ void BatchedHonkTranslatorProver::execute_joint_sumcheck_rounds()
215215
}
216216
}
217217
rdp.update_evaluations(u, 0);
218-
mega_zk_round.round_size >>= 1;
218+
mega_zk_round.advance_round();
219219
mega_zk_round.excluded_head_size = 2; // After round 0, disabled zone collapses to 1 edge pair
220220
update_round_state(0, u);
221221
}
@@ -238,7 +238,7 @@ void BatchedHonkTranslatorProver::execute_joint_sumcheck_rounds()
238238
}
239239
}
240240
rdp.update_evaluations(u, round_idx);
241-
mega_zk_round.round_size >>= 1;
241+
mega_zk_round.advance_round();
242242
update_round_state(round_idx, u);
243243
}
244244

barretenberg/cpp/src/barretenberg/chonk/batched_honk_translator/batched_honk_translator_verifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ typename BatchedHonkTranslatorVerifier_<Curve>::OinkResult BatchedHonkTranslator
3737
transcript->load_proof(mega_zk_proof);
3838

3939
if constexpr (IsRecursive) {
40-
builder = mega_zk_proof.back().get_context();
40+
builder = mega_zk_proof.get_context();
4141
}
4242

4343
mega_zk_verifier_instance = std::make_shared<MegaZKVerifierInstance>(mega_zk_vk_and_hash);

barretenberg/cpp/src/barretenberg/commitment_schemes/claim.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ template <typename Curve> class OpeningClaim {
6767

6868
static constexpr bool IS_GRUMPKIN =
6969
std::is_same_v<Curve, curve::Grumpkin> || std::is_same_v<Curve, stdlib::grumpkin<UltraCircuitBuilder>>;
70-
// Size of public inputs representation of an opening claim over Grumpkin: 2 * 4 + 2 = 10
70+
// Size of public inputs representation of an opening claim over Grumpkin: challenge (2) + evaluation (2) +
71+
// commitment (2) = 6
7172
static constexpr size_t PUBLIC_INPUTS_SIZE = IS_GRUMPKIN ? GRUMPKIN_OPENING_CLAIM_SIZE : INVALID_PUBLIC_INPUTS_SIZE;
7273

7374
/**

barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini.hpp

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -133,20 +133,10 @@ template <typename Curve> class GeminiProver_ {
133133
Polynomial batched_unshifted; // linear combination of unshifted polynomials
134134
Polynomial batched_to_be_shifted_by_one; // linear combination of to-be-shifted polynomials
135135

136-
// Batched tails: small polynomials covering only the tail region (e.g. last NUM_MASKED_ROWS positions).
137-
// Populated during compute_batched if tails are registered.
138-
Polynomial batched_unshifted_tail_;
139-
Polynomial batched_shifted_tail_;
140-
141136
public:
142137
RefVector<Polynomial> unshifted; // set of unshifted polynomials
143138
RefVector<Polynomial> to_be_shifted_by_one; // set of polynomials to be left shifted by 1
144139

145-
// Tails: small polynomials (e.g. masking values) to be batched with the same rho scalar
146-
// as their corresponding base polynomial. Pairs of (index in unshifted/shifted list, tail poly).
147-
std::vector<std::pair<size_t, Polynomial>> unshifted_tails_;
148-
std::vector<std::pair<size_t, Polynomial>> shifted_tails_;
149-
150140
PolynomialBatcher(const size_t full_batched_size, const size_t actual_data_size = 0)
151141
: full_batched_size(full_batched_size)
152142
, actual_data_size_(actual_data_size == 0 ? full_batched_size : actual_data_size)
@@ -161,15 +151,6 @@ template <typename Curve> class GeminiProver_ {
161151
void set_unshifted(RefVector<Polynomial> polynomials) { unshifted = polynomials; }
162152
void set_to_be_shifted_by_one(RefVector<Polynomial> polynomials) { to_be_shifted_by_one = polynomials; }
163153

164-
void add_unshifted_tail(size_t batcher_index, Polynomial&& tail)
165-
{
166-
unshifted_tails_.emplace_back(batcher_index, std::move(tail));
167-
}
168-
void add_shifted_tail(size_t batcher_index, Polynomial&& tail)
169-
{
170-
shifted_tails_.emplace_back(batcher_index, std::move(tail));
171-
}
172-
173154
/**
174155
* @brief Compute batched polynomial A₀ = F + G/X as the linear combination of all polynomials to be opened,
175156
* where F is the linear combination of the unshifted polynomials and G is the linear combination of the
@@ -200,41 +181,17 @@ template <typename Curve> class GeminiProver_ {
200181
batched, std::span<const PolynomialSpan<const Fr>>(sources), std::span<const Fr>(scalars));
201182
};
202183

203-
// Batch tails into a small accumulator with the correct rho power per tail.
204-
// Tails are small (e.g. 3 elements), kept separate to avoid extending the main batched
205-
// poly to full_batched_size. Each tail's scalar is base_scalar * rho^idx.
206-
auto batch_tails = [&](Polynomial& batched_tail,
207-
const std::vector<std::pair<size_t, Polynomial>>& tail_list,
208-
const Fr& base_scalar) {
209-
for (const auto& [idx, tail] : tail_list) {
210-
if (batched_tail.is_empty()) {
211-
batched_tail = Polynomial(tail.size(), tail.virtual_size(), tail.start_index());
212-
}
213-
batched_tail.add_scaled(tail, base_scalar * challenge.pow(idx));
214-
}
215-
};
216-
217184
Polynomial full_batched(full_batched_size);
218185

219-
Fr unshifted_base(1);
220186
if (has_unshifted()) {
221187
batch(batched_unshifted, unshifted);
222188
full_batched += batched_unshifted;
223189
}
224-
batch_tails(batched_unshifted_tail_, unshifted_tails_, unshifted_base);
225-
if (!batched_unshifted_tail_.is_empty()) {
226-
full_batched += batched_unshifted_tail_;
227-
}
228190

229-
Fr shifted_base = running_scalar;
230191
if (has_to_be_shifted_by_one()) {
231192
batch(batched_to_be_shifted_by_one, to_be_shifted_by_one);
232193
full_batched += batched_to_be_shifted_by_one.shifted();
233194
}
234-
batch_tails(batched_shifted_tail_, shifted_tails_, shifted_base);
235-
if (!batched_shifted_tail_.is_empty()) {
236-
full_batched += batched_shifted_tail_.shifted();
237-
}
238195

239196
return full_batched;
240197
}
@@ -252,11 +209,6 @@ template <typename Curve> class GeminiProver_ {
252209
if (has_unshifted()) {
253210
A_0_pos += batched_unshifted;
254211
}
255-
if (!batched_unshifted_tail_.is_empty()) {
256-
Polynomial A_0_extended(A_0_pos, full_batched_size - A_0_pos.start_index());
257-
A_0_extended += batched_unshifted_tail_;
258-
A_0_pos = std::move(A_0_extended);
259-
}
260212

261213
Polynomial A_0_neg = A_0_pos;
262214

@@ -265,10 +217,6 @@ template <typename Curve> class GeminiProver_ {
265217
A_0_pos.add_scaled(batched_to_be_shifted_by_one, r_inv);
266218
A_0_neg.add_scaled(batched_to_be_shifted_by_one, -r_inv);
267219
}
268-
if (!batched_shifted_tail_.is_empty()) {
269-
A_0_pos.add_scaled(batched_shifted_tail_, r_inv);
270-
A_0_neg.add_scaled(batched_shifted_tail_, -r_inv);
271-
}
272220

273221
return { A_0_pos, A_0_neg };
274222
};

barretenberg/cpp/src/barretenberg/commitment_schemes/gemini/gemini_impl.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ std::vector<typename GeminiProver_<Curve>::Claim> GeminiProver_<Curve>::prove(
7070
// Construct the d-1 Gemini foldings of A₀(X)
7171
std::vector<Polynomial> fold_polynomials = compute_fold_polynomials(log_n, multilinear_challenge, A_0);
7272

73-
// If virtual_log_n >= log_n, pad the fold commitments with dummy group elements [1]_1.
73+
// Commit to the virtual_log_n - 1 fold polynomials. When virtual_log_n > log_n, the trailing fold polynomials
74+
// for the virtual rounds were appended as constant polynomials by compute_fold_polynomials; their commitments
75+
// contribute nothing to the Shplonk quotient and are zeroed by the verifier.
7476
for (size_t l = 0; l < virtual_log_n - 1; l++) {
7577
std::string label = "Gemini:FOLD_" + std::to_string(l + 1);
76-
// Virtual-round fold polynomials are constant; their commitments are zeroed by the verifier.
7778
transcript->send_to_verifier(label, commitment_key.commit(fold_polynomials[l]));
7879
}
7980
const Fr r_challenge = transcript->template get_challenge<Fr>("Gemini:r");
@@ -125,7 +126,7 @@ std::vector<typename GeminiProver_<Curve>::Polynomial> GeminiProver_<Curve>::com
125126
// At minimum, the disabled head region must be covered (masking values live at rows 1..3).
126127
size_t actual_size = std::max(A_0.end_index(), static_cast<size_t>(NUM_DISABLED_ROWS_IN_SUMCHECK));
127128

128-
// Reserve and allocate space for m-1 Fold polynomials, the foldings of the full batched polynomial A₀
129+
// Reserve space for the virtual_log_n - 1 Fold polynomials, the foldings of the full batched polynomial A₀
129130
std::vector<Polynomial> fold_polynomials;
130131
fold_polynomials.reserve(virtual_log_n - 1);
131132
for (size_t l = 0; l < log_n - 1; ++l) {

barretenberg/cpp/src/barretenberg/commitment_schemes/kzg/kzg.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "barretenberg/commitment_schemes/commitment_key.hpp"
1111
#include "barretenberg/commitment_schemes/pairing_points.hpp"
1212
#include "barretenberg/commitment_schemes/verification_key.hpp"
13+
#include "barretenberg/common/assert.hpp"
1314
#include "barretenberg/common/bb_bench.hpp"
1415
#include "barretenberg/stdlib/primitives/pairing_points.hpp"
1516
#include "barretenberg/transcript/transcript.hpp"
@@ -128,7 +129,7 @@ template <typename Curve_> class KZG {
128129
* - \f$ P_1 = - [W(x)]_1 \f$
129130
*/
130131
template <typename Transcript>
131-
static PairingPointsType reduce_verify_batch_opening_claim(BatchOpeningClaim<Curve>&& batch_opening_claim,
132+
static PairingPointsType reduce_verify_batch_opening_claim(BatchOpeningClaim<Curve> batch_opening_claim,
132133
const std::shared_ptr<Transcript>& transcript,
133134
const size_t expected_final_msm_size = 0)
134135
{
@@ -164,6 +165,9 @@ template <typename Curve_> class KZG {
164165
}
165166

166167
// Compute C + [W]₁ ⋅ z
168+
BB_ASSERT_EQ(batch_opening_claim.commitments.size(),
169+
batch_opening_claim.scalars.size(),
170+
"BatchOpeningClaim: commitments and scalars must have equal length");
167171
P_0 = GroupElement::batch_mul(batch_opening_claim.commitments,
168172
batch_opening_claim.scalars,
169173
/*max_num_bits=*/0,

0 commit comments

Comments
 (0)