Skip to content

Commit

Permalink
fix: new commit_sparse bug and new tests (#8649)
Browse files Browse the repository at this point in the history
This bug in commit_sparse was due to an assumption that the size of the
polynomial was a power of 2. The structured polynomials work recently
made this assumption incorrect as we can set start and end indices for a
polynomial. One note is that the bug only happens if the size is large
enough (bigger than the number of cpus from `get_num_cpus_pow2()`)
because we otherwise would use 1 thread (which divides any number).

The 3 new tests test commit_sparse on a polynomial that has a size !=
virtual_size, a start_idx != 0, and a larger test case where both are
true. This larger test case fails without the fix.
  • Loading branch information
lucasxia01 authored Sep 19, 2024
1 parent 75b7b60 commit 5818018
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,19 @@ 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 spann.
// with our polynomial span.
std::span<G1> point_table = srs->get_monomial_points().subspan(polynomial.start_index * 2);

// Define structures needed to multithread the extraction of non-zero inputs
const size_t num_threads = poly_size >= get_num_cpus_pow2() ? get_num_cpus_pow2() : 1;
const size_t block_size = poly_size / num_threads;
const size_t num_threads = calculate_num_threads(poly_size);
const size_t block_size = (poly_size + num_threads - 1) / num_threads; // round up
std::vector<std::vector<Fr>> thread_scalars(num_threads);
std::vector<std::vector<G1>> thread_points(num_threads);

// Loop over all polynomial coefficients and keep {point, scalar} pairs for which scalar != 0
parallel_for(num_threads, [&](size_t thread_idx) {
const size_t start = thread_idx * block_size;
const size_t end = (thread_idx + 1) * block_size;
const size_t end = std::min(poly_size, (thread_idx + 1) * block_size);

for (size_t idx = start; idx < end; ++idx) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,94 @@ TYPED_TEST(CommitmentKeyTest, CommitSparse)
EXPECT_EQ(sparse_commit_result, commit_result);
}

/**
* @brief Test commit_sparse on polynomial with zero start index.
*
*/
TYPED_TEST(CommitmentKeyTest, CommitSparseSmallSize)
{
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 << 12; // large enough to ensure normal pippenger logic is used
const size_t num_nonzero = 7;

// Construct a sparse random polynomial
Polynomial poly(num_nonzero, num_points, 0);
for (size_t i = 0; i < num_nonzero; ++i) {
poly.at(i) = Fr::random_element();
}

// Commit to the polynomial using both the conventional commit method and the sparse commitment method
auto key = TestFixture::template create_commitment_key<CK>(num_points);
G1 commit_result = key->commit(poly);
G1 sparse_commit_result = key->commit_sparse(poly);

EXPECT_EQ(sparse_commit_result, commit_result);
}

/**
* @brief Test commit_sparse on polynomial with nonzero start index.
*
*/
TYPED_TEST(CommitmentKeyTest, CommitSparseNonZeroStartIndex)
{
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 << 12; // large enough to ensure normal pippenger logic is used
const size_t num_nonzero = 7;
const size_t offset = 1 << 11;

// Construct a sparse random polynomial
Polynomial poly(num_nonzero, num_points, offset);
for (size_t i = offset; i < offset + num_nonzero; ++i) {
poly.at(i) = Fr::random_element();
}

// Commit to the polynomial using both the conventional commit method and the sparse commitment method
auto key = TestFixture::template create_commitment_key<CK>(num_points);
G1 commit_result = key->commit(poly);
G1 sparse_commit_result = key->commit_sparse(poly);

EXPECT_EQ(sparse_commit_result, commit_result);
}

/**
* @brief Test commit_sparse on polynomial with medium size and nonzero start index.
* @details This was used to catch a bug in commit_sparse where the number of threads was > 1 and the size was not a
* power of 2.
*/
TYPED_TEST(CommitmentKeyTest, CommitSparseMediumNonZeroStartIndex)
{
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 << 12; // large enough to ensure normal pippenger logic is used
const size_t num_nonzero = (1 << 9) + 1;
const size_t offset = 1 << 11;

// Construct a sparse random polynomial
Polynomial poly(num_nonzero, num_points, offset);
for (size_t i = offset; i < offset + num_nonzero; ++i) {
poly.at(i) = Fr::random_element();
}

// Commit to the polynomial using both the conventional commit method and the sparse commitment method
auto key = TestFixture::template create_commitment_key<CK>(num_points);
G1 commit_result = key->commit(poly);
G1 sparse_commit_result = key->commit_sparse(poly);

EXPECT_EQ(sparse_commit_result, commit_result);
}

} // namespace bb

0 comments on commit 5818018

Please sign in to comment.