Skip to content

Commit

Permalink
chore(bb): use std::span for srs (#8371)
Browse files Browse the repository at this point in the history
A bit of safety - will help me catch a bug in polynomial memory PR

Fix a breakage in tests due to bad global grumpkin CRS assumptions
  • Loading branch information
ludamad authored Sep 4, 2024
1 parent 138dc52 commit f174699
Show file tree
Hide file tree
Showing 21 changed files with 129 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ template <class Curve> class CommitmentKey {
ASSERT(false);
}
return scalar_multiplication::pippenger_unsafe_optimized_for_non_dyadic_polys<Curve>(
polynomial, { srs->get_monomial_points(), srs->get_monomial_size() }, pippenger_runtime_state);
polynomial, srs->get_monomial_points(), pippenger_runtime_state);
};

/**
Expand All @@ -113,7 +113,7 @@ 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).
G1* point_table = srs->get_monomial_points();
std::span<G1> point_table = srs->get_monomial_points();

// Define structures needed to multithread the extraction of non-zero inputs
const size_t num_threads = degree >= get_num_cpus_pow2() ? get_num_cpus_pow2() : 1;
Expand All @@ -133,6 +133,7 @@ template <class Curve> class CommitmentKey {
if (!scalar.is_zero()) {
thread_scalars[thread_idx].emplace_back(scalar);
// Save both the raw srs point and the precomputed endomorphism point from the point table
ASSERT(idx * 2 + 1 < point_table.size());
const G1& point = point_table[idx * 2];
const G1& endo_point = point_table[idx * 2 + 1];
thread_points[thread_idx].emplace_back(point);
Expand All @@ -158,7 +159,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(), pippenger_runtime_state);
return scalar_multiplication::pippenger_unsafe<Curve>(scalars, points, pippenger_runtime_state);
}
};

Expand Down
21 changes: 14 additions & 7 deletions barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "barretenberg/common/assert.hpp"
#include "barretenberg/common/container.hpp"
#include "barretenberg/common/thread.hpp"
#include "barretenberg/common/throw_or_abort.hpp"
#include "barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp"
#include "barretenberg/transcript/transcript.hpp"
#include <cstddef>
Expand Down Expand Up @@ -162,9 +163,13 @@ template <typename Curve_> class IPA {
// Step 4.
// Set initial vector a to the polynomial monomial coefficients and load vector G
auto a_vec = polynomial;
auto* srs_elements = ck->srs->get_monomial_points();
std::span<Commitment> srs_elements = ck->srs->get_monomial_points();
std::vector<Commitment> G_vec_local(poly_length);

if (poly_length * 2 > srs_elements.size()) {
throw_or_abort("potential bug: Not enough SRS points for IPA!");
}

// The SRS stored in the commitment key is the result after applying the pippenger point table so the
// values at odd indices contain the point {srs[i-1].x * beta, srs[i-1].y}, where beta is the endomorphism
// G_vec_local should use only the original SRS thus we extract only the even indices.
Expand Down Expand Up @@ -215,13 +220,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], ck->pippenger_runtime_state);
{&a_vec[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[round_size], /*size*/ round_size}, &G_vec_local[0], ck->pippenger_runtime_state);
{&a_vec[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
Expand Down Expand Up @@ -345,7 +350,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], vk->pippenger_runtime_state);
{&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.
Expand Down Expand Up @@ -377,8 +382,10 @@ template <typename Curve_> class IPA {
}
}, thread_heuristics::FF_MULTIPLICATION_COST * log_poly_degree);

auto* srs_elements = vk->get_monomial_points();

std::span<const Commitment> srs_elements = vk->get_monomial_points();
if (poly_length * 2 > srs_elements.size()) {
throw_or_abort("potential bug: Not enough SRS points for IPA!");
}
// Copy the G_vector to local memory.
std::vector<Commitment> G_vec_local(poly_length);

Expand All @@ -394,7 +401,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], vk->pippenger_runtime_state);
{&s_vec[0], /*size*/ poly_length}, {&G_vec_local[0], /*size*/ poly_length}, vk->pippenger_runtime_state);

// Step 9.
// Receive a₀ from the prover
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ TEST_F(IPATest, CommitOnManyZeroCoeffPolyWorks)
}
p[3] = Fr::one();
GroupElement commitment = this->commit(p);
auto* srs_elements = this->ck()->srs->get_monomial_points();
auto srs_elements = this->ck()->srs->get_monomial_points();
GroupElement expected = srs_elements[0] * p[0];
// The SRS stored in the commitment key is the result after applying the pippenger point table so the
// values at odd indices contain the point {srs[i-1].x * beta, srs[i-1].y}, where beta is the endomorphism
Expand Down Expand Up @@ -196,7 +196,7 @@ TEST_F(IPATest, Commit)
constexpr size_t n = 128;
auto poly = this->random_polynomial(n);
GroupElement commitment = this->commit(poly);
auto* srs_elements = this->ck()->srs->get_monomial_points();
auto srs_elements = this->ck()->srs->get_monomial_points();
GroupElement expected = srs_elements[0] * poly[0];
// The SRS stored in the commitment key is the result after applying the pippenger point table so the
// values at odd indices contain the point {srs[i-1].x * beta, srs[i-1].y}, where beta is the endomorphism
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@ template <> class VerifierCommitmentKey<curve::BN254> {
using GroupElement = typename Curve::Element;
using Commitment = typename Curve::AffineElement;

VerifierCommitmentKey()
{
srs::init_crs_factory("../srs_db/ignition");
srs = srs::get_crs_factory<Curve>()->get_verifier_crs();
};
VerifierCommitmentKey() { srs = srs::get_crs_factory<Curve>()->get_verifier_crs(); };

Commitment get_g1_identity() { return srs->get_g1_identity(); }

Expand Down Expand Up @@ -88,13 +84,12 @@ template <> class VerifierCommitmentKey<curve::Grumpkin> {
VerifierCommitmentKey(size_t num_points)
: pippenger_runtime_state(num_points)
{
srs::init_grumpkin_crs_factory("../srs_db/grumpkin");
srs = srs::get_crs_factory<Curve>()->get_verifier_crs(num_points);
}

Commitment get_g1_identity() { return srs->get_g1_identity(); }

Commitment* get_monomial_points() { return srs->get_monomial_points(); }
std::span<const Commitment> get_monomial_points() { return srs->get_monomial_points(); }

bb::scalar_multiplication::pippenger_runtime_state<Curve> pippenger_runtime_state;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class BN254 {
using G2BaseField = typename bb::fq2;
using TargetField = bb::fq12;

static constexpr const char* name = "BN254";
// TODO(#673): This flag is temporary. It is needed in the verifier classes (GeminiVerifier, etc.) while these
// classes are instantiated with "native" curve types. Eventually, the verifier classes will be instantiated only
// with stdlib types, and "native" verification will be acheived via a simulated builder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Grumpkin {
using Element = typename Group::element;
using AffineElement = typename Group::affine_element;

static constexpr const char* name = "Grumpkin";
// TODO(#673): This flag is temporary. It is needed in the verifier classes (GeminiVerifier, etc.) while these
// classes are instantiated with "native" curve types. Eventually, the verifier classes will be instantiated only
// with stdlib types, and "native" verification will be acheived via a simulated builder.
Expand Down
Loading

0 comments on commit f174699

Please sign in to comment.