refactor(bb): more graceful pippenger on non-powers-of-2

barretenberg/cpp/Earthfile

@@ -233,19 +233,16 @@ test-clang-format:
 test:
     ARG hardware_concurrency=""
     # prefetch
-    BUILD +test-binaries
     BUILD +preset-release-assert-test
     BUILD +test-clang-format
     BUILD ./srs_db/+build # prefetch
-    FROM +source
-    COPY --dir +test-binaries/build build
     FROM +preset-release-assert-test
     COPY --dir ./srs_db/+build/. srs_db
     # limit hardware concurrency, if provided
     IF [ "$HARDWARE_CONCURRENCY" != "" ]
         ENV HARDWARE_CONCURRENCY=$hardware_concurrency
     END
-    RUN cd build && GTEST_COLOR=1 ctest -j$(nproc) --output-on-failure
+    RUN cd build && ./bin/stdlib_client_ivc_verifier_tests --gtest_filter=ClientIVCRecursionTests.ClientTubeBase #GTEST_COLOR=1 ctest -j$(nproc) --output-on-failure
 
 vm-full-test:
     ARG hardware_concurrency=""

barretenberg/cpp/src/barretenberg/benchmark/pippenger_bench/main.cpp

@@ -72,7 +72,7 @@ int pippenger()
     scalar_multiplication::pippenger_runtime_state<curve::BN254> state(NUM_POINTS);
     std::chrono::steady_clock::time_point time_start = std::chrono::steady_clock::now();
     g1::element result = scalar_multiplication::pippenger_unsafe<curve::BN254>(
-        { &scalars[0], /*size*/ NUM_POINTS }, reference_string->get_monomial_points(), NUM_POINTS, state);
+        { &scalars[0], /*size*/ NUM_POINTS }, reference_string->get_monomial_points(), state);
     std::chrono::steady_clock::time_point time_end = std::chrono::steady_clock::now();
     std::chrono::microseconds diff = std::chrono::duration_cast<std::chrono::microseconds>(time_end - time_start);
     std::cout << "run time: " << diff.count() << "us" << std::endl;

barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp

@@ -129,6 +129,22 @@ template <typename Curve> void bench_commit_random(::benchmark::State& state)
         key->commit(polynomial);
     }
 }
+// Commit to a polynomial with dense random nonzero entries but NOT our happiest case of an exact power of 2
+// Note this used to be a 50% regression just subtracting a power of 2 by 1.
+template <typename Curve> void bench_commit_random_non_power_of_2(::benchmark::State& state)
+{
+    using Fr = typename Curve::ScalarField;
+    auto key = create_commitment_key<Curve>(MAX_NUM_POINTS);
+
+    const size_t num_points = 1 << state.range(0);
+    auto polynomial = Polynomial<Fr>(num_points - 1);
+    for (auto& coeff : polynomial) {
+        coeff = Fr::random_element();
+    }
+    for (auto _ : state) {
+        key->commit(polynomial);
+    }
+}
 
 BENCHMARK(bench_commit_zero<curve::BN254>)
     ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS)
@@ -148,6 +164,9 @@ BENCHMARK(bench_commit_sparse_random_preprocessed<curve::BN254>)
 BENCHMARK(bench_commit_random<curve::BN254>)
     ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS)
     ->Unit(benchmark::kMillisecond);
+BENCHMARK(bench_commit_random_non_power_of_2<curve::BN254>)
+    ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS)
+    ->Unit(benchmark::kMillisecond);
 
 } // namespace bb
 

barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp

@@ -9,6 +9,7 @@
 
 #include "barretenberg/common/op_count.hpp"
 #include "barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp"
+#include "barretenberg/numeric/bitop/get_msb.hpp"
 #include "barretenberg/numeric/bitop/pow.hpp"
 #include "barretenberg/polynomials/polynomial_arithmetic.hpp"
 #include "barretenberg/srs/factories/crs_factory.hpp"
@@ -34,6 +35,17 @@ template <class Curve> class CommitmentKey {
     using Fr = typename Curve::ScalarField;
     using Commitment = typename Curve::AffineElement;
     using G1 = typename Curve::AffineElement;
+    static constexpr size_t EXTRA_SRS_POINTS_FOR_ECCVM_IPA = 1;
+
+    static size_t get_num_needed_srs_points(size_t num_points)
+    {
+        // NOTE 1: Currently we must round up internal space for points as our pippenger algorithm (specifically,
+        // pippenger_unsafe_optimized_for_non_dyadic_polys) will use next power of 2. This is used to simplify the
+        // recursive halving scheme. We do, however allow the polynomial to not be fully formed. Pippenger internally
+        // will pad 0s into the runtime state.
+        // NOTE 2: We then add one for ECCVM to provide for IPA verification
+        return numeric::round_up_power_2(num_points) + EXTRA_SRS_POINTS_FOR_ECCVM_IPA;
+    }
 
   public:
     scalar_multiplication::pippenger_runtime_state<Curve> pippenger_runtime_state;
@@ -50,9 +62,9 @@ template <class Curve> class CommitmentKey {
      *
      */
     CommitmentKey(const size_t num_points)
-        : pippenger_runtime_state(num_points)
+        : pippenger_runtime_state(get_num_needed_srs_points(num_points))
         , crs_factory(srs::get_crs_factory<Curve>())
-        , srs(crs_factory->get_prover_crs(num_points))
+        , srs(crs_factory->get_prover_crs(get_num_needed_srs_points(num_points)))
     {}
 
     // Note: This constructor is to be used only by Plonk; For Honk the srs lives in the CommitmentKey
@@ -70,16 +82,17 @@ template <class Curve> class CommitmentKey {
     Commitment commit(std::span<const Fr> polynomial)
     {
         BB_OP_COUNT_TIME();
-        const size_t degree = polynomial.size();
-        if (degree > srs->get_monomial_size()) {
-            info("Attempting to commit to a polynomial of degree ",
-                 degree,
-                 " with an SRS of size ",
+        // See constructor, we must round up the number of used srs points to a power of 2.
+        const size_t consumed_srs = numeric::round_up_power_2(polynomial.size());
+        if (consumed_srs > srs->get_monomial_size()) {
+            info("Attempting to commit to a polynomial that needs ",
+                 consumed_srs,
+                 " points with an SRS of size ",
                  srs->get_monomial_size());
             ASSERT(false);
         }
-        return scalar_multiplication::pippenger_unsafe<Curve>(
-            polynomial, srs->get_monomial_points(), degree, pippenger_runtime_state);
+        return scalar_multiplication::pippenger_unsafe_optimized_for_non_dyadic_polys<Curve>(
+            polynomial, { srs->get_monomial_points(), srs->get_monomial_size() }, pippenger_runtime_state);
     };
 
     /**
@@ -145,8 +158,7 @@ template <class Curve> class CommitmentKey {
         }
 
         // Call the version of pippenger which assumes all points are distinct
-        return scalar_multiplication::pippenger_unsafe<Curve>(
-            scalars, points.data(), scalars.size(), pippenger_runtime_state);
+        return scalar_multiplication::pippenger_unsafe<Curve>(scalars, points.data(), pippenger_runtime_state);
     }
 };
 

barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp

@@ -215,13 +215,13 @@ template <typename Curve_> class IPA {
             // Step 6.a (using letters, because doxygen automaticall converts the sublist counters to letters :( )
             // L_i = < a_vec_lo, G_vec_hi > + inner_prod_L * aux_generator
             L_i = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>(
-                {&a_vec[0], /*size*/ round_size}, &G_vec_local[round_size], round_size, ck->pippenger_runtime_state);
+                {&a_vec[0], /*size*/ round_size}, &G_vec_local[round_size], ck->pippenger_runtime_state);
             L_i += aux_generator * inner_prod_L;
 
             // Step 6.b
             // R_i = < a_vec_hi, G_vec_lo > + inner_prod_R * aux_generator
             R_i = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>(
-                {&a_vec[round_size], /*size*/ round_size}, &G_vec_local[0], round_size, ck->pippenger_runtime_state);
+                {&a_vec[round_size], /*size*/ round_size}, &G_vec_local[0], ck->pippenger_runtime_state);
             R_i += aux_generator * inner_prod_R;
 
             // Step 6.c
@@ -345,7 +345,7 @@ template <typename Curve_> class IPA {
         // Step 5.
         // Compute C₀ = C' + ∑_{j ∈ [k]} u_j^{-1}L_j + ∑_{j ∈ [k]} u_jR_j
         GroupElement LR_sums = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>(
-            {&msm_scalars[0], /*size*/ pippenger_size}, &msm_elements[0], pippenger_size, vk->pippenger_runtime_state);
+            {&msm_scalars[0], /*size*/ pippenger_size}, &msm_elements[0], vk->pippenger_runtime_state);
         GroupElement C_zero = C_prime + LR_sums;
 
         //  Step 6.
@@ -394,7 +394,7 @@ template <typename Curve_> class IPA {
         // Step 8.
         // Compute G₀
         Commitment G_zero = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>(
-            {&s_vec[0], /*size*/ poly_length}, &G_vec_local[0], poly_length, vk->pippenger_runtime_state);
+            {&s_vec[0], /*size*/ poly_length}, &G_vec_local[0], vk->pippenger_runtime_state);
 
         // Step 9.
         // Receive a₀ from the prover

barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/runtime_states.hpp

@@ -64,7 +64,7 @@ constexpr size_t get_num_rounds(const size_t num_points)
 }
 
 template <typename Curve> struct affine_product_runtime_state {
-    typename Curve::AffineElement* points;
+    const typename Curve::AffineElement* points;
     typename Curve::AffineElement* point_pairs_1;
     typename Curve::AffineElement* point_pairs_2;
     typename Curve::BaseField* scratch_space;