chore: minor fixes pt. 2 (#22138)

Fix several issues discovered by Claude

Author: iakovenkos

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

@@ -267,8 +267,7 @@ template <typename Curve> class GeminiProver_ {
 
     static std::vector<Polynomial> compute_fold_polynomials(const size_t log_n,
                                                             std::span<const Fr> multilinear_challenge,
-                                                            const Polynomial& A_0,
-                                                            const bool& has_zk = false);
+                                                            const Polynomial& A_0);
 
     static std::vector<Claim> construct_univariate_opening_claims(const size_t log_n,
                                                                   Polynomial&& A_0_pos,

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

@@ -66,12 +66,12 @@ std::vector<typename GeminiProver_<Curve>::Claim> GeminiProver_<Curve>::prove(
     Polynomial A_0 = polynomial_batcher.compute_batched(rho);
 
     // Construct the d-1 Gemini foldings of A₀(X)
-    std::vector<Polynomial> fold_polynomials = compute_fold_polynomials(log_n, multilinear_challenge, A_0, has_zk);
+    std::vector<Polynomial> fold_polynomials = compute_fold_polynomials(log_n, multilinear_challenge, A_0);
 
     // If virtual_log_n >= log_n, pad the fold commitments with dummy group elements [1]_1.
     for (size_t l = 0; l < virtual_log_n - 1; l++) {
         std::string label = "Gemini:FOLD_" + std::to_string(l + 1);
-        // When has_zk is true, we are sending commitments to 0. Seems to work, but maybe brittle.
+        // Virtual-round fold polynomials are constant; their commitments are zeroed by the verifier.
         transcript->send_to_verifier(label, commitment_key.commit(fold_polynomials[l]));
     }
     const Fr r_challenge = transcript->template get_challenge<Fr>("Gemini:r");
@@ -108,7 +108,7 @@ std::vector<typename GeminiProver_<Curve>::Claim> GeminiProver_<Curve>::prove(
  */
 template <typename Curve>
 std::vector<typename GeminiProver_<Curve>::Polynomial> GeminiProver_<Curve>::compute_fold_polynomials(
-    const size_t log_n, std::span<const Fr> multilinear_challenge, const Polynomial& A_0, const bool& has_zk)
+    const size_t log_n, std::span<const Fr> multilinear_challenge, const Polynomial& A_0)
 {
     BB_BENCH_NAME("Gemini::compute_fold_polynomials");
     BB_ASSERT_GTE(log_n, size_t(2), "Gemini folding requires at least 4-element polynomials");
@@ -156,25 +156,23 @@ std::vector<typename GeminiProver_<Curve>::Polynomial> GeminiProver_<Curve>::com
         A_l = A_l_fold;
     }
 
-    // Perform virtual rounds.
-    // After the first `log_n - 1` rounds, the prover's `fold` univariates stabilize. With ZK, the verifier multiplies
-    // the evaluations by 0, otherwise, when `virtual_log_n > log_n`, the prover honestly computes and sends the
-    // constant folds.
+    // Virtual rounds (indices log_n .. virtual_log_n - 1).
+    // After real folding, the fold polynomials are constant. Since each constant polynomial evaluates to its own
+    // value at every point, (f(X) - f(x)) / (X - x) = 0, so these contribute nothing to the Shplonk quotient Q(X).
+    // On the verifier side, padding_indicator_array zeros their contributions independently.
     const auto& last = fold_polynomials.back();
     const Fr u_last = multilinear_challenge[log_n - 1];
     const Fr final_eval = last.at(0) + u_last * (last.at(1) - last.at(0));
     Polynomial const_fold(1);
-    // Temporary fix: when we're running a zk proof, the verifier uses a `padding_indicator_array`. So the evals in
-    // rounds past `log_n - 1` will be ignored. Hence the prover also needs to ignore them, otherwise Shplonk will fail.
-    const_fold.at(0) = final_eval * Fr(static_cast<int>(!has_zk));
+    const_fold.at(0) = final_eval;
     fold_polynomials.emplace_back(const_fold);
 
     // FOLD_{log_n+1}, ..., FOLD_{d_v-1}
     Fr tail = Fr(1);
     for (size_t k = log_n; k < virtual_log_n - 1; ++k) {
         tail *= (Fr(1) - multilinear_challenge[k]); // multiply by (1 - u_k)
         Polynomial next_const(1);
-        next_const.at(0) = final_eval * tail * Fr(static_cast<int>(!has_zk));
+        next_const.at(0) = final_eval * tail;
         fold_polynomials.emplace_back(next_const);
     }
 

barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplemini.hpp

@@ -368,8 +368,10 @@ template <typename Curve, bool HasZK = false> class ShpleminiVerifier_ {
                 libra_evaluations, gemini_evaluation_challenge, multivariate_challenge, libra_univariate_evaluation);
         }
 
-        // Currently, only used in ECCVM
+        // Used in ECCVM and BatchedHonkTranslator. The nu power offset in batch_sumcheck_round_claims
+        // assumes ZK claims (NUM_SMALL_IPA_EVALUATIONS) precede sumcheck round claims in the batching order.
         if (committed_sumcheck) {
+            BB_ASSERT(HasZK, "committed sumcheck requires ZK for correct nu power indexing");
             batch_sumcheck_round_claims(commitments,
                                         scalars,
                                         constant_term_accumulator,
@@ -513,18 +515,28 @@ template <typename Curve, bool HasZK = false> class ShpleminiVerifier_ {
         }
 
         // Erase the duplicate entries (higher-index range first to preserve lower indices)
-        auto erase_range = [&](size_t start, size_t count) {
+        auto erase_range = [&](size_t duplicate_start, size_t original_start, size_t count) {
             for (size_t i = 0; i < count; ++i) {
-                scalars.erase(scalars.begin() + static_cast<std::ptrdiff_t>(start));
-                commitments.erase(commitments.begin() + static_cast<std::ptrdiff_t>(start));
+                // Verify the commitment being erased matches its original (native only).
+                // Each erase shifts elements down, so duplicate_start always points to the
+                // next duplicate; the original at original_start + i is unaffected since
+                // we erase higher-index ranges first.
+                if constexpr (!Curve::is_stdlib_type) {
+                    BB_ASSERT(commitments[duplicate_start] == commitments[original_start + i],
+                              "remove_repeated_commitments: commitment mismatch at duplicate index " +
+                                  std::to_string(duplicate_start) + " vs original index " +
+                                  std::to_string(original_start + i));
+                }
+                scalars.erase(scalars.begin() + static_cast<std::ptrdiff_t>(duplicate_start));
+                commitments.erase(commitments.begin() + static_cast<std::ptrdiff_t>(duplicate_start));
             }
         };
         if (second_duplicate_start > first_duplicate_start) {
-            erase_range(second_duplicate_start, r2.count);
-            erase_range(first_duplicate_start, r1.count);
+            erase_range(second_duplicate_start, second_original_start, r2.count);
+            erase_range(first_duplicate_start, first_original_start, r1.count);
         } else {
-            erase_range(first_duplicate_start, r1.count);
-            erase_range(second_duplicate_start, r2.count);
+            erase_range(first_duplicate_start, first_original_start, r1.count);
+            erase_range(second_duplicate_start, second_original_start, r2.count);
         }
     }
 

barretenberg/cpp/src/barretenberg/commitment_schemes/shplonk/shplonk.hpp

@@ -171,29 +171,24 @@ template <typename Curve> class ShplonkProver_ {
         // s.t. G(r) = 0
         Polynomial G(std::move(batched_quotient_Q)); // G(X) = Q(X)
 
-        // G₀ = ∑ⱼ νʲ ⋅ vⱼ / ( z − xⱼ )
         Fr current_nu = Fr::one();
-        Polynomial tmp(G.size());
         size_t idx = 0;
 
         size_t fold_idx = 0;
-        for (auto& claim : opening_claims) {
+        for (const auto& claim : opening_claims) {
 
             if (claim.gemini_fold) {
-                tmp = claim.polynomial;
-                tmp.at(0) = tmp[0] - gemini_fold_pos_evaluations[fold_idx++];
-                Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z − xⱼ )
-                // G -= νʲ ⋅ ( fⱼ(X) − vⱼ) / ( z − xⱼ )
-                G.add_scaled(tmp, -scaling_factor);
+                // G -= νʲ ⋅ ( fⱼ(X) − vⱼ₊) / ( z + xⱼ ), where vⱼ₊ is the positive fold evaluation
+                Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z + xⱼ )
+                G.add_scaled(claim.polynomial, -scaling_factor);
+                G.at(0) = G[0] + scaling_factor * gemini_fold_pos_evaluations[fold_idx++];
 
                 current_nu *= nu_challenge;
             }
-            // tmp = νʲ ⋅ ( fⱼ(X) − vⱼ) / ( z − xⱼ )
-            claim.polynomial.at(0) = claim.polynomial[0] - claim.opening_pair.evaluation;
-            Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z − xⱼ )
-
             // G -= νʲ ⋅ ( fⱼ(X) − vⱼ) / ( z − xⱼ )
+            Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z − xⱼ )
             G.add_scaled(claim.polynomial, -scaling_factor);
+            G.at(0) = G[0] + scaling_factor * claim.opening_pair.evaluation;
 
             current_nu *= nu_challenge;
         }
@@ -203,22 +198,18 @@ template <typename Curve> class ShplonkProver_ {
             current_nu = nu_challenge.pow(2 * virtual_log_n);
         }
 
-        for (auto& claim : libra_opening_claims) {
-            // Compute individual claim quotient tmp = ( fⱼ(X) − vⱼ) / ( X − xⱼ )
-            claim.polynomial.at(0) = claim.polynomial[0] - claim.opening_pair.evaluation;
+        for (const auto& claim : libra_opening_claims) {
+            // G -= νʲ ⋅ ( fⱼ(X) − vⱼ) / ( z − xⱼ )
             Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z − xⱼ )
-
-            // Add the claim quotient to the batched quotient polynomial
             G.add_scaled(claim.polynomial, -scaling_factor);
+            G.at(0) = G[0] + scaling_factor * claim.opening_pair.evaluation;
             current_nu *= nu_challenge;
         }
 
-        for (auto& claim : sumcheck_opening_claims) {
-            claim.polynomial.at(0) = claim.polynomial[0] - claim.opening_pair.evaluation;
+        for (const auto& claim : sumcheck_opening_claims) {
             Fr scaling_factor = current_nu * inverse_vanishing_evals[idx++]; // = νʲ / (z − xⱼ )
-
-            // Add the claim quotient to the batched quotient polynomial
             G.add_scaled(claim.polynomial, -scaling_factor);
+            G.at(0) = G[0] + scaling_factor * claim.opening_pair.evaluation;
             current_nu *= nu_challenge;
         }
         // Return opening pair (z, 0) and polynomial G(X) = Q(X) - Q_z(X)

barretenberg/cpp/src/barretenberg/crypto/poseidon2/sponge/sponge.hpp

@@ -17,9 +17,10 @@ namespace bb::crypto {
 
 /**
  * @brief Implements a cryptographic sponge over prime fields.
- *        Implements the sponge specification from the Community Cryptographic Specification Project
- *        see https://github.com/C2SP/C2SP/blob/792c1254124f625d459bfe34417e8f6bdd02eb28/poseidon-sponge.md
- *        (Note: this spec was not accepted into the C2SP repo, we might want to reference something else!)
+ *        Sponge construction follows the Duplex Sponge model (https://keccak.team/files/SpongeDuplex.pdf).
+ *        Domain separation uses IV = (input_length << 64) per Section 4.2 of the Poseidon paper
+ *        (https://eprint.iacr.org/2019/458.pdf). Permutation is Poseidon2
+ *        (https://eprint.iacr.org/2023/323.pdf).
  *
  *        Note: If we ever use this sponge class for more than 1 hash functions, we should move this out of `poseidon2`
  *              and into its own directory

barretenberg/cpp/src/barretenberg/sumcheck/sumcheck.test.cpp

@@ -71,9 +71,8 @@ template <typename Flavor> typename Flavor::ProverPolynomials create_satisfiable
     // For ZK flavors: add randomness to the last rows (which will be masked by row-disabling polynomial)
     // These rows don't need to satisfy the relation because they're disabled
     if constexpr (Flavor::HasZK) {
-        constexpr size_t NUM_DISABLED_ROWS = 3; // Matches the number of disabled rows in ZK sumcheck
-        if (circuit_size > NUM_DISABLED_ROWS) {
-            for (size_t i = circuit_size - NUM_DISABLED_ROWS; i < circuit_size; ++i) {
+        if (circuit_size > NUM_DISABLED_ROWS_IN_SUMCHECK) {
+            for (size_t i = circuit_size - NUM_DISABLED_ROWS_IN_SUMCHECK; i < circuit_size; ++i) {
                 full_polynomials.w_l.at(i) = FF::random_element();
                 full_polynomials.w_r.at(i) = FF::random_element();
                 full_polynomials.w_o.at(i) = FF::random_element();

barretenberg/cpp/src/barretenberg/sumcheck/sumcheck_round.hpp

@@ -284,43 +284,6 @@ template <typename Flavor> class SumcheckProverRound {
         size_t size;
     };
 
-    /**
-     * @brief Helper struct that will, given a vector of BlockOfContiguousRows, return the edge indices that correspond
-     * to the nonzero rows
-     */
-    struct RowIterator {
-        const std::vector<BlockOfContiguousRows>* blocks;
-        size_t current_block_index = 0;
-        size_t current_block_count = 0;
-        RowIterator(const std::vector<BlockOfContiguousRows>& _blocks, size_t starting_index = 0)
-            : blocks(&_blocks)
-        {
-            size_t count = 0;
-            for (size_t i = 0; i < blocks->size(); ++i) {
-                const BlockOfContiguousRows block = blocks->at(i);
-                if (count + (block.size / 2) > starting_index) {
-                    current_block_index = i;
-                    current_block_count = (starting_index - count) * 2;
-                    break;
-                }
-                count += (block.size / 2);
-            }
-        }
-
-        size_t get_next_edge()
-        {
-            const BlockOfContiguousRows& block = blocks->at(current_block_index);
-            size_t edge = block.starting_edge_idx + current_block_count;
-            if (current_block_count + 2 >= block.size) {
-                current_block_index += 1;
-                current_block_count = 0;
-            } else {
-                current_block_count += 2;
-            }
-            return edge;
-        }
-    };
-
     /**
      * @brief Compute the number of unskippable rows we must iterate over
      * @details Some circuits have a circuit size much larger than the number of used rows (ECCVM, Translator).

barretenberg/cpp/src/barretenberg/sumcheck/zk_sumcheck_data.hpp

@@ -11,6 +11,7 @@
 #include "barretenberg/polynomials/polynomial.hpp"
 #include "barretenberg/polynomials/univariate.hpp"
 #include <array>
+#include <tuple>
 #include <vector>
 
 namespace bb {
@@ -76,8 +77,7 @@ template <typename Flavor> struct ZKSumcheckData {
             transcript->send_to_verifier("Libra:concatenation_commitment", libra_commitment);
         }
         // Compute the total sum of the Libra polynomials
-        libra_scaling_factor = FF(1);
-        libra_total_sum = compute_libra_total_sum(libra_univariates, libra_scaling_factor, constant_term);
+        std::tie(libra_total_sum, libra_scaling_factor) = compute_libra_total_sum(libra_univariates, constant_term);
 
         // Send the Libra total sum to the transcript
         transcript->send_to_verifier("Libra:Sum", libra_total_sum);
@@ -106,13 +106,12 @@ template <typename Flavor> struct ZKSumcheckData {
         : constant_term(FF::random_element())
         , libra_univariates(generate_libra_univariates(multivariate_d, univariate_length))
         , log_circuit_size(multivariate_d)
-        , libra_scaling_factor(FF(1))
         , libra_challenge(FF::random_element())
-        , libra_total_sum(compute_libra_total_sum(libra_univariates, libra_scaling_factor, constant_term))
-        , libra_running_sum(libra_total_sum * libra_challenge)
         , univariate_length(univariate_length)
 
     {
+        std::tie(libra_total_sum, libra_scaling_factor) = compute_libra_total_sum(libra_univariates, constant_term);
+        libra_running_sum = libra_total_sum * libra_challenge;
         setup_auxiliary_data(libra_univariates, libra_scaling_factor, libra_challenge, libra_running_sum);
     }
     /**
@@ -137,23 +136,22 @@ template <typename Flavor> struct ZKSumcheckData {
      * the Boolean hypercube.
      *
      * @param libra_univariates
-     * @param scaling_factor
-     * @return FF
+     * @param constant_term
+     * @return A pair of (total_sum, scaling_factor), where scaling_factor = 2^{d-1}.
      */
-    static FF compute_libra_total_sum(const std::vector<Polynomial<FF>>& libra_univariates,
-                                      FF& scaling_factor,
-                                      const FF& constant_term)
+    static std::pair<FF, FF> compute_libra_total_sum(const std::vector<Polynomial<FF>>& libra_univariates,
+                                                     const FF& constant_term)
     {
         FF total_sum = 0;
-        scaling_factor *= one_half;
+        FF scaling_factor = one_half;
 
         for (auto& univariate : libra_univariates) {
             total_sum += univariate.at(0) + univariate.evaluate(FF(1));
             scaling_factor *= 2;
         }
         total_sum *= scaling_factor;
 
-        return total_sum + constant_term * (1 << libra_univariates.size());
+        return { total_sum + constant_term * (1 << libra_univariates.size()), scaling_factor };
     }
 
     /**