From 3e7326554059f55ae1c568c47fd6d3a01c1231cc Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Wed, 15 Jan 2025 18:53:10 +0000 Subject: [PATCH 1/7] fix --- .../small_subgroup_ipa/small_subgroup_ipa.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp index 54af7999a2ab..8c9e7f715714 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp @@ -123,7 +123,7 @@ template class SmallSubgroupIPAProver { const std::vector& multivariate_challenge, const FF claimed_ipa_eval, std::shared_ptr transcript, - std::shared_ptr commitment_key) + std::shared_ptr& commitment_key) : interpolation_domain(zk_sumcheck_data.interpolation_domain) , concatenated_polynomial(zk_sumcheck_data.libra_concatenated_monomial_form) , libra_concatenated_lagrange_form(zk_sumcheck_data.libra_concatenated_lagrange_form) @@ -135,6 +135,10 @@ template class SmallSubgroupIPAProver { , batched_quotient(QUOTIENT_LENGTH) { + // Reallocate the commitment key if necessary. This is an edge case with SmallSubgroupIPA + if (commitment_key->dyadic_size < SUBGROUP_SIZE + 3) { + commitment_key = std::make_shared(SUBGROUP_SIZE + 3); + } // Extract the evaluation domain computed by ZKSumcheckData if constexpr (std::is_same_v) { bn_evaluation_domain = std::move(zk_sumcheck_data.bn_evaluation_domain); From a10b01d5e6ce823191de5abb68fadfda355bccc0 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Wed, 15 Jan 2025 18:54:08 +0000 Subject: [PATCH 2/7] add assert to check in commit() --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 1d107e822965..3c35144c3e95 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -90,6 +90,7 @@ template class CommitmentKey { PROFILE_THIS_NAME("commit"); // We must have a power-of-2 SRS points *after* subtracting by start_index. size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size()); + ASSERT(dyadic_poly_size <= dyadic_size && "Polynomial size exceeds commitment key 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 From c53d24fc5cf0ac881856fbf43ff43f390a5ed1b2 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Wed, 15 Jan 2025 18:54:57 +0000 Subject: [PATCH 3/7] more assert --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 3c35144c3e95..6bb1e2bb6102 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -212,6 +212,7 @@ template class CommitmentKey { { PROFILE_THIS_NAME("commit_structured"); ASSERT(polynomial.end_index() <= srs->get_monomial_size()); + ASSERT(polynomial.end_index() <= dyadic_size && "Polynomial size exceeds commitment key size."); // Percentage of nonzero coefficients beyond which we resort to the conventional commit method constexpr size_t NONZERO_THRESHOLD = 75; From b44f6cbafaa29d3d2db4fea5acdd2684679d11f7 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 16 Jan 2025 15:30:13 +0000 Subject: [PATCH 4/7] initialize dyadic_size in commitment key --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 6bb1e2bb6102..be664475a083 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -77,6 +77,7 @@ template class CommitmentKey { CommitmentKey(const size_t num_points, std::shared_ptr> prover_crs) : pippenger_runtime_state(num_points) , srs(prover_crs) + , dyadic_size(get_num_needed_srs_points(num_points)) {} /** @@ -90,6 +91,8 @@ template class CommitmentKey { PROFILE_THIS_NAME("commit"); // We must have a power-of-2 SRS points *after* subtracting by start_index. size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size()); + info("dyadic_poly_size: ", dyadic_poly_size); + info("dyadic_size: ", dyadic_size); ASSERT(dyadic_poly_size <= dyadic_size && "Polynomial size exceeds commitment key 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 From 7c777c283fa4e0e6bf8845cd3c80b72d1468250c Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 16 Jan 2025 20:32:29 +0000 Subject: [PATCH 5/7] remove infos --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index be664475a083..4108a072d72c 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -91,8 +91,6 @@ template class CommitmentKey { PROFILE_THIS_NAME("commit"); // We must have a power-of-2 SRS points *after* subtracting by start_index. size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size()); - info("dyadic_poly_size: ", dyadic_poly_size); - info("dyadic_size: ", dyadic_size); ASSERT(dyadic_poly_size <= dyadic_size && "Polynomial size exceeds commitment key 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 From f2bae7ad8168a24b902f2289a3980b56101b119b Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 16 Jan 2025 20:33:23 +0000 Subject: [PATCH 6/7] update comment --- .../small_subgroup_ipa/small_subgroup_ipa.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp index 8c9e7f715714..9ca4c61dc979 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp @@ -135,7 +135,8 @@ template class SmallSubgroupIPAProver { , batched_quotient(QUOTIENT_LENGTH) { - // Reallocate the commitment key if necessary. This is an edge case with SmallSubgroupIPA + // Reallocate the commitment key if necessary. This is an edge case with SmallSubgroupIPA since it has + // polynomials that may exceed the circuit size. if (commitment_key->dyadic_size < SUBGROUP_SIZE + 3) { commitment_key = std::make_shared(SUBGROUP_SIZE + 3); } From 970c40943b1a68bce3b54f0f61a2abd5ea196509 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 16 Jan 2025 20:33:28 +0000 Subject: [PATCH 7/7] add asserts to a bunch of pippenger functions --- .../scalar_multiplication/scalar_multiplication.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp index 37995a15401a..08fc0b266e4a 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -905,6 +905,9 @@ typename Curve::Element pippenger_internal(std::span(state.point_schedule, state.skew_table, state.round_counts, scalars, num_initial_points); organize_buckets(state.point_schedule, num_initial_points * 2); @@ -923,6 +926,9 @@ typename Curve::Element pippenger(PolynomialSpan points, pippenger_runtime_state& state) { + ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 && + "Pippenger runtime state is too small to support this many points"); return pippenger(scalars, points, state, false); } @@ -1030,6 +1041,8 @@ typename Curve::Element pippenger_without_endomorphism_basis_points( // TODO(https://github.com/AztecProtocol/barretenberg/issues/1135): We don't need start_index more scalars here. std::vector G_mod((scalars.start_index + scalars.size()) * 2); ASSERT(scalars.start_index + scalars.size() <= points.size()); + ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 && + "Pippenger runtime state is too small to support this many points"); bb::scalar_multiplication::generate_pippenger_point_table( points.data(), &G_mod[0], scalars.start_index + scalars.size()); return pippenger(scalars, G_mod, state, false);