Skip to content

Commit e53d114

Browse files
lucasxia01AztecBot
authored and
AztecBot
committed
fix: Reallocate commitment key to avoid pippenger error (#11249)
Fixes recent bug introduced by the SmallSubgroupIPA work which added an edge case where we always commit to polynomials of some fixed degree (of 259 or whatever). Pippenger was set up to work for circuit_size amount of points, which could be lower than the SmallSubgroupIPA poly sizes, so it led to buffer overflows. Fixes it by reallocating commitment key if necessary in SmallSubgroupIPA prover. Also adds an assert to commit() to check for any potential overflows.
1 parent 46e0275 commit e53d114

File tree

3 files changed

+22
-1
lines changed

3 files changed

+22
-1
lines changed

cpp/src/barretenberg/commitment_schemes/commitment_key.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ template <class Curve> class CommitmentKey {
7777
CommitmentKey(const size_t num_points, std::shared_ptr<srs::factories::ProverCrs<Curve>> prover_crs)
7878
: pippenger_runtime_state(num_points)
7979
, srs(prover_crs)
80+
, dyadic_size(get_num_needed_srs_points(num_points))
8081
{}
8182

8283
/**
@@ -90,6 +91,7 @@ template <class Curve> class CommitmentKey {
9091
PROFILE_THIS_NAME("commit");
9192
// We must have a power-of-2 SRS points *after* subtracting by start_index.
9293
size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size());
94+
ASSERT(dyadic_poly_size <= dyadic_size && "Polynomial size exceeds commitment key size.");
9395
// Because pippenger prefers a power-of-2 size, we must choose a starting index for the points so that we don't
9496
// exceed the dyadic_circuit_size. The actual start index of the points will be the smallest it can be so that
9597
// the window of points is a power of 2 and still contains the scalars. The best we can do is pick a start index
@@ -211,6 +213,7 @@ template <class Curve> class CommitmentKey {
211213
{
212214
PROFILE_THIS_NAME("commit_structured");
213215
ASSERT(polynomial.end_index() <= srs->get_monomial_size());
216+
ASSERT(polynomial.end_index() <= dyadic_size && "Polynomial size exceeds commitment key size.");
214217

215218
// Percentage of nonzero coefficients beyond which we resort to the conventional commit method
216219
constexpr size_t NONZERO_THRESHOLD = 75;

cpp/src/barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa.hpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ template <typename Flavor> class SmallSubgroupIPAProver {
123123
const std::vector<FF>& multivariate_challenge,
124124
const FF claimed_ipa_eval,
125125
std::shared_ptr<typename Flavor::Transcript> transcript,
126-
std::shared_ptr<typename Flavor::CommitmentKey> commitment_key)
126+
std::shared_ptr<typename Flavor::CommitmentKey>& commitment_key)
127127
: interpolation_domain(zk_sumcheck_data.interpolation_domain)
128128
, concatenated_polynomial(zk_sumcheck_data.libra_concatenated_monomial_form)
129129
, libra_concatenated_lagrange_form(zk_sumcheck_data.libra_concatenated_lagrange_form)
@@ -135,6 +135,11 @@ template <typename Flavor> class SmallSubgroupIPAProver {
135135
, batched_quotient(QUOTIENT_LENGTH)
136136

137137
{
138+
// Reallocate the commitment key if necessary. This is an edge case with SmallSubgroupIPA since it has
139+
// polynomials that may exceed the circuit size.
140+
if (commitment_key->dyadic_size < SUBGROUP_SIZE + 3) {
141+
commitment_key = std::make_shared<typename Flavor::CommitmentKey>(SUBGROUP_SIZE + 3);
142+
}
138143
// Extract the evaluation domain computed by ZKSumcheckData
139144
if constexpr (std::is_same_v<Curve, curve::BN254>) {
140145
bn_evaluation_domain = std::move(zk_sumcheck_data.bn_evaluation_domain);

cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,9 @@ typename Curve::Element pippenger_internal(std::span<const typename Curve::Affin
905905
bool handle_edge_cases)
906906
{
907907
PROFILE_THIS();
908+
909+
ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 &&
910+
"Pippenger runtime state is too small to support this many points");
908911
// multiplication_runtime_state state;
909912
compute_wnaf_states<Curve>(state.point_schedule, state.skew_table, state.round_counts, scalars, num_initial_points);
910913
organize_buckets(state.point_schedule, num_initial_points * 2);
@@ -923,6 +926,9 @@ typename Curve::Element pippenger(PolynomialSpan<const typename Curve::ScalarFie
923926
using Group = typename Curve::Group;
924927
using Element = typename Curve::Element;
925928

929+
ASSERT(scalars_.start_index + scalars_.size() <= state.num_points / 2 &&
930+
"Pippenger runtime state is too small to support this many points");
931+
926932
// our windowed non-adjacent form algorthm requires that each thread can work on at least 8 points.
927933
// If we fall below this theshold, fall back to the traditional scalar multiplication algorithm.
928934
// For 8 threads, this neatly coincides with the threshold where Strauss scalar multiplication outperforms
@@ -984,6 +990,9 @@ typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys(
984990
{
985991
PROFILE_THIS();
986992

993+
ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 &&
994+
"Pippenger runtime state is too small to support this many points");
995+
987996
// our windowed non-adjacent form algorthm requires that each thread can work on at least 8 points.
988997
const size_t threshold = get_num_cpus_pow2() * 8;
989998
// Delegate edge-cases to normal pippenger_unsafe().
@@ -1018,6 +1027,8 @@ typename Curve::Element pippenger_unsafe(PolynomialSpan<const typename Curve::Sc
10181027
std::span<const typename Curve::AffineElement> points,
10191028
pippenger_runtime_state<Curve>& state)
10201029
{
1030+
ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 &&
1031+
"Pippenger runtime state is too small to support this many points");
10211032
return pippenger(scalars, points, state, false);
10221033
}
10231034

@@ -1030,6 +1041,8 @@ typename Curve::Element pippenger_without_endomorphism_basis_points(
10301041
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1135): We don't need start_index more scalars here.
10311042
std::vector<typename Curve::AffineElement> G_mod((scalars.start_index + scalars.size()) * 2);
10321043
ASSERT(scalars.start_index + scalars.size() <= points.size());
1044+
ASSERT(scalars.start_index + scalars.size() <= state.num_points / 2 &&
1045+
"Pippenger runtime state is too small to support this many points");
10331046
bb::scalar_multiplication::generate_pippenger_point_table<Curve>(
10341047
points.data(), &G_mod[0], scalars.start_index + scalars.size());
10351048
return pippenger(scalars, G_mod, state, false);

0 commit comments

Comments
 (0)