fix: Reduce SRS size back to normal (#9098)

Resolves #1097.

Previously, we had to bump up SRS sizes to 1.5x the dyadic circuit size
because structured polynomials meant that we could commit starting from
the start_index of the polynomial, but because pippenger likes a power
of 2 points, that meant that we sometimes exceeded the
dyadic_circuit_size during a roundup to a power of 2.

This PR fixes this by using PolynomialSpans to store the scalars. Note
that these scalars do not necessarily represent polynomials anymore, so
maybe this object can be renamed. The PolynomialSpan allows us to store
a start_index with the scalars, where the start_index here means the
offset into the span of points that the scalars start at. For example,
if we are committing to a polynomial which starts at index 13, and has
13 length. The points we will use will now be [10, 26) instead of [13,
29) previously. The start_index here would be 3 because the scalars
start at 13, which is 3 after the points start.

The range for the points is chosen to the be the earliest power of 2
window that fits the scalars, meaning we try to shift it as left as
possible. This means that will never exceed the dyadic_circuit_size as a
result, so we can keep the old (and good) SRS sizes.

cpp/src/barretenberg/bb/main.cpp

@@ -63,10 +63,8 @@ const auto current_dir = current_path.filename().string();
  */
 void init_bn254_crs(size_t dyadic_circuit_size)
 {
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1097): tighter bound needed
-    // currently using 1.6x points in CRS because of structured polys, see notes for how to minimize
     // Must +1 for Plonk only!
-    auto bn254_g1_data = get_bn254_g1_data(CRS_PATH, dyadic_circuit_size + dyadic_circuit_size * 6 / 10 + 1);
+    auto bn254_g1_data = get_bn254_g1_data(CRS_PATH, dyadic_circuit_size + 1);
     auto bn254_g2_data = get_bn254_g2_data(CRS_PATH);
     srs::init_crs_factory(bn254_g1_data, bn254_g2_data);
 }
@@ -79,10 +77,7 @@ void init_bn254_crs(size_t dyadic_circuit_size)
  */
 void init_grumpkin_crs(size_t eccvm_dyadic_circuit_size)
 {
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1097): tighter bound needed
-    // currently using 1.6x points in CRS because of structured polys, see notes for how to minimize
-    auto grumpkin_g1_data =
-        get_grumpkin_g1_data(CRS_PATH, eccvm_dyadic_circuit_size + eccvm_dyadic_circuit_size * 6 / 10);
+    auto grumpkin_g1_data = get_grumpkin_g1_data(CRS_PATH, eccvm_dyadic_circuit_size + 1);
     srs::init_grumpkin_crs_factory(grumpkin_g1_data);
 }
 

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

@@ -72,7 +72,9 @@ 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(), state);
+        PolynomialSpan<const curve::BN254::ScalarField>{ /*start_index*/ 0, { &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;

cpp/src/barretenberg/commitment_schemes/commitment_key.hpp

@@ -87,7 +87,19 @@ template <class Curve> class CommitmentKey {
     {
         PROFILE_THIS();
         // We must have a power-of-2 SRS points *after* subtracting by start_index.
-        const size_t consumed_srs = numeric::round_up_power_2(polynomial.size()) + polynomial.start_index;
+        size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size());
+        // Because pippenger prefers a power-of-2 size, we must choose a starting index for the points so that we don't
+        // exceed the dyadic_circuit_size. The actual start index of the points will be the smallest it can be so that
+        // the window of points is a power of 2 and still contains the scalars. The best we can do is pick a start index
+        // that ends at the end of the polynomial, which would be polynomial.end_index() - dyadic_poly_size. However,
+        // our polynomial might defined too close to 0, so we set the start_index to 0 in that case.
+        size_t actual_start_index =
+            polynomial.end_index() > dyadic_poly_size ? polynomial.end_index() - dyadic_poly_size : 0;
+        // The relative start index is the offset of the scalars from the start of the points window, i.e.
+        // [actual_start_index, actual_start_index + dyadic_poly_size), so we subtract actual_start_index from the start
+        // index.
+        size_t relative_start_index = polynomial.start_index - actual_start_index;
+        const size_t consumed_srs = actual_start_index + dyadic_poly_size;
         auto srs = srs::get_crs_factory<Curve>()->get_prover_crs(consumed_srs);
         // We only need the
         if (consumed_srs > srs->get_monomial_size()) {
@@ -100,10 +112,11 @@ template <class Curve> class CommitmentKey {
         // Extract the precomputed point table (contains raw SRS points at even indices and the corresponding
         // endomorphism point (\beta*x, -y) at odd indices). We offset by polynomial.start_index * 2 to align
         // with our polynomial span.
-        std::span<G1> point_table = srs->get_monomial_points().subspan(polynomial.start_index * 2);
+
+        std::span<G1> point_table = srs->get_monomial_points().subspan(actual_start_index * 2);
         DEBUG_LOG_ALL(polynomial.span);
         Commitment point = scalar_multiplication::pippenger_unsafe_optimized_for_non_dyadic_polys<Curve>(
-            polynomial.span, point_table, pippenger_runtime_state);
+            { relative_start_index, polynomial.span }, point_table, pippenger_runtime_state);
         DEBUG_LOG(point);
         return point;
     };
@@ -173,7 +186,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, pippenger_runtime_state);
+        return scalar_multiplication::pippenger_unsafe<Curve>({ 0, scalars }, points, pippenger_runtime_state);
     }
 
     /**
@@ -230,7 +243,7 @@ template <class Curve> class CommitmentKey {
         }
 
         // Call pippenger
-        return scalar_multiplication::pippenger_unsafe<Curve>(scalars, points, pippenger_runtime_state);
+        return scalar_multiplication::pippenger_unsafe<Curve>({ 0, scalars }, points, pippenger_runtime_state);
     }
 
     /**

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

@@ -222,13 +222,13 @@ template <typename Curve_> class IPA {
             // Step 6.a (using letters, because doxygen automatically 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.at(0), /*size*/ round_size}, {&G_vec_local[round_size], /*size*/ round_size}, ck->pippenger_runtime_state);
+                {0, {&a_vec.at(0), /*size*/ round_size}}, {&G_vec_local[round_size], /*size*/ 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.at(round_size), /*size*/ round_size}, {&G_vec_local[0], /*size*/ round_size}, ck->pippenger_runtime_state);
+                {0, {&a_vec.at(round_size), /*size*/ round_size}}, {&G_vec_local[0], /*size*/ round_size}, ck->pippenger_runtime_state);
             R_i += aux_generator * inner_prod_R;
 
             // Step 6.c
@@ -352,7 +352,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], /*size*/ pippenger_size}, vk->pippenger_runtime_state);
+            {0, {&msm_scalars[0], /*size*/ pippenger_size}}, {&msm_elements[0], /*size*/ pippenger_size}, vk->pippenger_runtime_state);
         GroupElement C_zero = C_prime + LR_sums;
 
         //  Step 6.
@@ -403,7 +403,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], /*size*/ poly_length}, vk->pippenger_runtime_state);
+           {0, {&s_vec[0], /*size*/ poly_length}}, {&G_vec_local[0], /*size*/ poly_length}, vk->pippenger_runtime_state);
 
         // Step 9.
         // Receive a₀ from the prover

cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp

@@ -95,6 +95,90 @@ using Curves = ::testing::Types<curve::BN254, curve::Grumpkin>;
 
 TYPED_TEST_SUITE(CommitmentKeyTest, Curves);
 
+// Check that commit for a structured polynomial and the same polynomial but full return the same results
+TYPED_TEST(CommitmentKeyTest, CommitFull)
+{
+    using Curve = TypeParam;
+    using CK = CommitmentKey<Curve>;
+    using G1 = Curve::AffineElement;
+    using Fr = Curve::ScalarField;
+    using Polynomial = bb::Polynomial<Fr>;
+
+    const size_t num_points = 1 << 5; // large enough to ensure normal pippenger logic is used
+    const size_t start_index = 13;    // random start index
+    const size_t num_nonzero = 13;    // random size
+
+    // Construct a random structured polynomial
+    Polynomial poly{ num_nonzero, num_points, start_index };
+    for (size_t i = start_index; i < start_index + num_nonzero; ++i) {
+        poly.at(i) = Fr::random_element();
+    }
+
+    // Commit to the polynomial and the same polynomial but full
+    auto key = TestFixture::template create_commitment_key<CK>(num_points);
+    G1 commit_result = key->commit(poly);
+    auto full_poly = poly.full();
+    G1 full_commit_result = key->commit(full_poly);
+
+    EXPECT_EQ(commit_result, full_commit_result);
+}
+
+// Check that commit for a structured polynomial and the same polynomial but full return the same results
+TYPED_TEST(CommitmentKeyTest, CommitFullMedium)
+{
+    using Curve = TypeParam;
+    using CK = CommitmentKey<Curve>;
+    using G1 = Curve::AffineElement;
+    using Fr = Curve::ScalarField;
+    using Polynomial = bb::Polynomial<Fr>;
+
+    const size_t num_points = 4096;  // large enough to ensure normal pippenger logic is used
+    const size_t start_index = 1402; // random start index
+    const size_t num_nonzero = 1392; // random size
+
+    // Construct a random structured polynomial
+    Polynomial poly{ num_nonzero, num_points, start_index };
+    for (size_t i = start_index; i < start_index + num_nonzero; ++i) {
+        poly.at(i) = Fr::random_element();
+    }
+
+    // Commit to the polynomial and the same polynomial but full
+    auto key = TestFixture::template create_commitment_key<CK>(num_points);
+    G1 commit_result = key->commit(poly);
+    auto full_poly = poly.full();
+    G1 full_commit_result = key->commit(full_poly);
+
+    EXPECT_EQ(commit_result, full_commit_result);
+}
+
+// Check that commit for a structured polynomial doesn't require more SRS points beyond the size of the polynomial
+TYPED_TEST(CommitmentKeyTest, CommitSRSCheck)
+{
+    using Curve = TypeParam;
+    using CK = CommitmentKey<Curve>;
+    using G1 = Curve::AffineElement;
+    using Fr = Curve::ScalarField;
+    using Polynomial = bb::Polynomial<Fr>;
+
+    const size_t num_points = 32;  // large enough to ensure normal pippenger logic is used
+    const size_t start_index = 15; // just slightly less than half
+    const size_t num_nonzero = 17; // fill to the end of the size
+
+    // Construct a random structured polynomial
+    Polynomial poly{ num_nonzero, num_points, start_index };
+    for (size_t i = start_index; i < start_index + num_nonzero; ++i) {
+        poly.at(i) = Fr::random_element();
+    }
+
+    // Commit to the polynomial and the same polynomial but full
+    auto key = TestFixture::template create_commitment_key<CK>(num_points);
+    G1 commit_result = key->commit(poly);
+    auto full_poly = poly.full();
+    G1 full_commit_result = key->commit(full_poly);
+
+    EXPECT_EQ(commit_result, full_commit_result);
+}
+
 // Check that commit and commit_sparse return the same result for a random sparse polynomial
 TYPED_TEST(CommitmentKeyTest, CommitSparse)
 {
@@ -234,9 +318,11 @@ TYPED_TEST(CommitmentKeyTest, CommitStructuredWire)
     // Commit to the polynomial using both the conventional commit method and the sparse commitment method
     auto key = TestFixture::template create_commitment_key<CK>(polynomial.virtual_size());
 
+    auto full_poly = polynomial.full();
+    G1 actual_expected_result = key->commit(full_poly);
     G1 expected_result = key->commit(polynomial);
     G1 result = key->commit_structured(polynomial, active_range_endpoints);
-
+    EXPECT_EQ(actual_expected_result, expected_result);
     EXPECT_EQ(result, expected_result);
 }