From 5616be3c13026ebb48d62bd40cd8c37da3e6074a Mon Sep 17 00:00:00 2001 From: ludamad Date: Thu, 29 Aug 2024 20:19:14 +0000 Subject: [PATCH 01/23] remove redundant pippenger param --- barretenberg/cpp/src/CMakeLists.txt | 2 +- .../benchmark/pippenger_bench/main.cpp | 2 +- .../commitment_schemes/commitment_key.hpp | 5 ++- .../commitment_schemes/ipa/ipa.hpp | 8 ++--- .../scalar_multiplication.cpp | 21 +++--------- .../scalar_multiplication.hpp | 3 -- .../plonk/proof_system/verifier/verifier.cpp | 3 +- .../proof_system/verifier/verifier.test.cpp | 1 - .../plonk/work_queue/work_queue.cpp | 2 +- .../polynomials/legacy_polynomials.bench.cpp | 20 ++++++------ .../srs/scalar_multiplication.test.cpp | 32 +++++++------------ 11 files changed, 37 insertions(+), 62 deletions(-) diff --git a/barretenberg/cpp/src/CMakeLists.txt b/barretenberg/cpp/src/CMakeLists.txt index a2c8ba9eb158..6abbf3b0d867 100644 --- a/barretenberg/cpp/src/CMakeLists.txt +++ b/barretenberg/cpp/src/CMakeLists.txt @@ -12,7 +12,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) # Specifying `-Wno-${ERROR_NAME}` will silence the error completely. # To preserve the warning, but prevent them from causing the build to fail, # use the flag `-Wno-error=${ERROR_NAME}` -add_compile_options(-Werror -Wall -Wextra -Wconversion -Wsign-conversion -Wfatal-errors) +add_compile_options(-Werror -Wall -Wextra -Wconversion -Wsign-conversion) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-fcolor-diagnostics -fconstexpr-steps=100000000) diff --git a/barretenberg/cpp/src/barretenberg/benchmark/pippenger_bench/main.cpp b/barretenberg/cpp/src/barretenberg/benchmark/pippenger_bench/main.cpp index 2b6e2caf4a03..e16cdbaee356 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/pippenger_bench/main.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/pippenger_bench/main.cpp @@ -72,7 +72,7 @@ int pippenger() scalar_multiplication::pippenger_runtime_state state(NUM_POINTS); std::chrono::steady_clock::time_point time_start = std::chrono::steady_clock::now(); g1::element result = scalar_multiplication::pippenger_unsafe( - { &scalars[0], /*size*/ NUM_POINTS }, reference_string->get_monomial_points(), NUM_POINTS, state); + { &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(time_end - time_start); std::cout << "run time: " << diff.count() << "us" << std::endl; diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 4509ba3e4059..db900c3ac50a 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -79,7 +79,7 @@ template class CommitmentKey { ASSERT(false); } return scalar_multiplication::pippenger_unsafe( - polynomial, srs->get_monomial_points(), degree, pippenger_runtime_state); + polynomial, srs->get_monomial_points(), pippenger_runtime_state); }; /** @@ -145,8 +145,7 @@ template class CommitmentKey { } // Call the version of pippenger which assumes all points are distinct - return scalar_multiplication::pippenger_unsafe( - scalars, points.data(), scalars.size(), pippenger_runtime_state); + return scalar_multiplication::pippenger_unsafe(scalars, points.data(), pippenger_runtime_state); } }; diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp index 10e8bfbe80f4..fa0ed6e5f96d 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp @@ -215,13 +215,13 @@ template 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( - {&a_vec[0], /*size*/ round_size}, &G_vec_local[round_size], round_size, ck->pippenger_runtime_state); + {&a_vec[0], /*size*/ round_size}, &G_vec_local[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( - {&a_vec[round_size], /*size*/ round_size}, &G_vec_local[0], round_size, ck->pippenger_runtime_state); + {&a_vec[round_size], /*size*/ round_size}, &G_vec_local[0], ck->pippenger_runtime_state); R_i += aux_generator * inner_prod_R; // Step 6.c @@ -345,7 +345,7 @@ template 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( - {&msm_scalars[0], /*size*/ pippenger_size}, &msm_elements[0], pippenger_size, vk->pippenger_runtime_state); + {&msm_scalars[0], /*size*/ pippenger_size}, &msm_elements[0], vk->pippenger_runtime_state); GroupElement C_zero = C_prime + LR_sums; // Step 6. @@ -394,7 +394,7 @@ template class IPA { // Step 8. // Compute G₀ Commitment G_zero = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points( - {&s_vec[0], /*size*/ poly_length}, &G_vec_local[0], poly_length, vk->pippenger_runtime_state); + {&s_vec[0], /*size*/ poly_length}, &G_vec_local[0], vk->pippenger_runtime_state); // Step 9. // Receive a₀ from the prover 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 329b8138a1e6..cb94afe49b73 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -873,7 +873,6 @@ typename Curve::Element pippenger_internal(typename Curve::AffineElement* points template typename Curve::Element pippenger(std::span scalars, typename Curve::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state, bool handle_edge_cases) { @@ -886,7 +885,7 @@ typename Curve::Element pippenger(std::span s // For 8 threads, this neatly coincides with the threshold where Strauss scalar multiplication outperforms // Pippenger const size_t threshold = get_num_cpus_pow2() * 8; - + size_t num_initial_points = scalars.size(); if (num_initial_points == 0) { Element out = Group::one; out.self_set_infinity(); @@ -911,10 +910,8 @@ typename Curve::Element pippenger(std::span s Element result = pippenger_internal(points, scalars, num_slice_points, state, handle_edge_cases); if (num_slice_points != num_initial_points) { - const uint64_t leftover_points = num_initial_points - num_slice_points; return result + pippenger(scalars.subspan(num_slice_points), points + static_cast(num_slice_points * 2), - static_cast(leftover_points), state, handle_edge_cases); } @@ -939,22 +936,20 @@ typename Curve::Element pippenger(std::span s template typename Curve::Element pippenger_unsafe(std::span scalars, typename Curve::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state) { - return pippenger(scalars, points, num_initial_points, state, false); + return pippenger(scalars, points, state, false); } template typename Curve::Element pippenger_without_endomorphism_basis_points( std::span scalars, typename Curve::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state) { - std::vector G_mod(num_initial_points * 2); - bb::scalar_multiplication::generate_pippenger_point_table(points, &G_mod[0], num_initial_points); - return pippenger(scalars, &G_mod[0], num_initial_points, state, false); + std::vector G_mod(scalars.size() * 2); + bb::scalar_multiplication::generate_pippenger_point_table(points, &G_mod[0], scalars.size()); + return pippenger(scalars, &G_mod[0], state, false); } // Explicit instantiation @@ -994,19 +989,16 @@ template curve::BN254::AffineElement* reduce_buckets(affine_produc template curve::BN254::Element pippenger(std::span scalars, curve::BN254::AffineElement* points, - const size_t num_points, pippenger_runtime_state& state, bool handle_edge_cases = true); template curve::BN254::Element pippenger_unsafe(std::span scalars, curve::BN254::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state); template curve::BN254::Element pippenger_without_endomorphism_basis_points( std::span scalars, curve::BN254::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state); // Grumpkin @@ -1046,20 +1038,17 @@ template curve::Grumpkin::AffineElement* reduce_buckets( template curve::Grumpkin::Element pippenger(std::span scalars, curve::Grumpkin::AffineElement* points, - const size_t num_points, pippenger_runtime_state& state, bool handle_edge_cases = true); template curve::Grumpkin::Element pippenger_unsafe( std::span scalars, curve::Grumpkin::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state); template curve::Grumpkin::Element pippenger_without_endomorphism_basis_points( std::span scalars, curve::Grumpkin::AffineElement* points, - const size_t num_initial_points, pippenger_runtime_state& state); } // namespace bb::scalar_multiplication diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp index f178023b3251..4312e50e28a1 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp @@ -154,21 +154,18 @@ typename Curve::AffineElement* reduce_buckets(affine_product_runtime_state typename Curve::Element pippenger(std::span scalars, typename Curve::AffineElement* points, - size_t num_initial_points, pippenger_runtime_state& state, bool handle_edge_cases = true); template typename Curve::Element pippenger_unsafe(std::span scalars, typename Curve::AffineElement* points, - size_t num_initial_points, pippenger_runtime_state& state); template typename Curve::Element pippenger_without_endomorphism_basis_points( std::span scalars, typename Curve::AffineElement* points, - size_t num_initial_points, pippenger_runtime_state& state); // Explicit instantiation diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp index dd8c4d2a6f34..79a5afb4fb05 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp @@ -182,8 +182,7 @@ template bool VerifierBase::verify g1::element P[2]; - P[0] = bb::scalar_multiplication::pippenger( - { &scalars[0], num_elements }, &elements[0], num_elements, state); + P[0] = bb::scalar_multiplication::pippenger({ &scalars[0], num_elements }, &elements[0], state); P[1] = -(g1::element(PI_Z_OMEGA) * separator_challenge + PI_Z); if (key->contains_recursive_proof) { diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.test.cpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.test.cpp index 427108062ec7..87c1037625e2 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.test.cpp @@ -36,7 +36,6 @@ plonk::Verifier generate_verifier(std::shared_ptr circuit_proving_k commitments[i] = g1::affine_element(scalar_multiplication::pippenger( { poly_coefficients[i].get(), circuit_proving_key->circuit_size }, circuit_proving_key->reference_string->get_monomial_points(), - circuit_proving_key->circuit_size, state)); } diff --git a/barretenberg/cpp/src/barretenberg/plonk/work_queue/work_queue.cpp b/barretenberg/cpp/src/barretenberg/plonk/work_queue/work_queue.cpp index e6308a8343f3..2b5eef768b50 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/work_queue/work_queue.cpp +++ b/barretenberg/cpp/src/barretenberg/plonk/work_queue/work_queue.cpp @@ -214,7 +214,7 @@ void work_queue::process_queue() // Run pippenger multi-scalar multiplication. auto runtime_state = bb::scalar_multiplication::pippenger_runtime_state(msm_size); bb::g1::affine_element result(bb::scalar_multiplication::pippenger_unsafe( - { item.mul_scalars.get(), msm_size }, srs_points, msm_size, runtime_state)); + { item.mul_scalars.get(), msm_size }, srs_points, runtime_state)); transcript->add_element(item.tag, result.to_buffer()); diff --git a/barretenberg/cpp/src/barretenberg/polynomials/legacy_polynomials.bench.cpp b/barretenberg/cpp/src/barretenberg/polynomials/legacy_polynomials.bench.cpp index 716c79d86000..abad771e6262 100644 --- a/barretenberg/cpp/src/barretenberg/polynomials/legacy_polynomials.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/polynomials/legacy_polynomials.bench.cpp @@ -124,7 +124,7 @@ void pippenger_bench(State& state) noexcept state.ResumeTiming(); // uint64_t before = rdtsc(); scalar_multiplication::pippenger( - { &globals.scalars[0], /*size*/ num_points }, &globals.monomials[0], num_points, run_state); + { &globals.scalars[0], /*size*/ num_points }, &globals.monomials[0], run_state); // uint64_t after = rdtsc(); // count += (after - before); // ++i; @@ -169,23 +169,23 @@ void new_plonk_scalar_multiplications_bench(State& state) noexcept uint64_t before = rdtsc(); g1::element a = scalar_multiplication::pippenger( - { &globals.scalars[0], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[0], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element b = scalar_multiplication::pippenger( - { &globals.scalars[1], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[1], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element c = scalar_multiplication::pippenger( - { &globals.scalars[2], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[2], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element d = scalar_multiplication::pippenger( - { &globals.scalars[3], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[3], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element e = scalar_multiplication::pippenger( - { &globals.scalars[4], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[4], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element f = scalar_multiplication::pippenger( - { &globals.scalars[5], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[5], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element g = scalar_multiplication::pippenger( - { &globals.scalars[6], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[6], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element h = scalar_multiplication::pippenger( - { &globals.scalars[7], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[7], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); g1::element i = scalar_multiplication::pippenger( - { &globals.scalars[8], /*size*/ MAX_GATES }, &globals.monomials[0], MAX_GATES, run_state); + { &globals.scalars[8], /*size*/ MAX_GATES }, &globals.monomials[0], run_state); uint64_t after = rdtsc(); count += (after - before); ++k; diff --git a/barretenberg/cpp/src/barretenberg/srs/scalar_multiplication.test.cpp b/barretenberg/cpp/src/barretenberg/srs/scalar_multiplication.test.cpp index 660339f674ed..247911339ae6 100644 --- a/barretenberg/cpp/src/barretenberg/srs/scalar_multiplication.test.cpp +++ b/barretenberg/cpp/src/barretenberg/srs/scalar_multiplication.test.cpp @@ -681,8 +681,7 @@ TYPED_TEST(ScalarMultiplicationTests, OversizedInputs) } scalar_multiplication::pippenger_runtime_state state(target_degree); - Element first = - scalar_multiplication::pippenger({ scalars, /*size*/ target_degree }, monomials, target_degree, state); + Element first = scalar_multiplication::pippenger({ scalars, /*size*/ target_degree }, monomials, state); first = first.normalize(); for (size_t i = 0; i < target_degree; ++i) { @@ -690,8 +689,7 @@ TYPED_TEST(ScalarMultiplicationTests, OversizedInputs) } scalar_multiplication::pippenger_runtime_state state_2(target_degree); - Element second = - scalar_multiplication::pippenger({ scalars, /*size*/ target_degree }, monomials, target_degree, state_2); + Element second = scalar_multiplication::pippenger({ scalars, /*size*/ target_degree }, monomials, state_2); second = second.normalize(); EXPECT_EQ((first.z == second.z), true); @@ -734,8 +732,7 @@ TYPED_TEST(ScalarMultiplicationTests, UndersizedInputs) scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, num_points, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, state); result = result.normalize(); aligned_free(scalars); @@ -772,8 +769,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerSmall) scalar_multiplication::generate_pippenger_point_table(points, points, num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, num_points, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, state); result = result.normalize(); aligned_free(scalars); @@ -812,8 +808,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerEdgeCaseDbl) } scalar_multiplication::generate_pippenger_point_table(points, points, num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, num_points, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, state); result = result.normalize(); aligned_free(scalars); @@ -871,8 +866,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerShortInputs) scalar_multiplication::generate_pippenger_point_table(points.get(), points.get(), num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points.get(), num_points, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points.get(), state); result = result.normalize(); aligned_free(scalars); @@ -908,8 +902,8 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerUnsafe) scalar_multiplication::generate_pippenger_point_table(points.get(), points.get(), num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = scalar_multiplication::pippenger_unsafe( - { scalars, /*size*/ num_points }, points.get(), num_points, state); + Element result = + scalar_multiplication::pippenger_unsafe({ scalars, /*size*/ num_points }, points.get(), state); result = result.normalize(); aligned_free(scalars); @@ -966,8 +960,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerUnsafeShortInputs) scalar_multiplication::generate_pippenger_point_table(points, points, num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger_unsafe({ scalars, /*size*/ num_points }, points, num_points, state); + Element result = scalar_multiplication::pippenger_unsafe({ scalars, /*size*/ num_points }, points, state); result = result.normalize(); aligned_free(scalars); @@ -1004,8 +997,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerOne) scalar_multiplication::generate_pippenger_point_table(points, points, num_points); scalar_multiplication::pippenger_runtime_state state(num_points); - Element result = - scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, num_points, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ num_points }, points, state); result = result.normalize(); aligned_free(scalars); @@ -1026,7 +1018,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerZeroPoints) AffineElement* points = (AffineElement*)aligned_alloc(32, sizeof(AffineElement) * (2 + 1)); scalar_multiplication::pippenger_runtime_state state(0); - Element result = scalar_multiplication::pippenger({ scalars, /*size*/ 0 }, points, 0, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ 0 }, points, state); aligned_free(scalars); aligned_free(points); @@ -1051,7 +1043,7 @@ TYPED_TEST(ScalarMultiplicationTests, PippengerMulByZero) scalar_multiplication::generate_pippenger_point_table(points, points, 1); scalar_multiplication::pippenger_runtime_state state(1); - Element result = scalar_multiplication::pippenger({ scalars, /*size*/ 1 }, points, 1, state); + Element result = scalar_multiplication::pippenger({ scalars, /*size*/ 1 }, points, state); aligned_free(scalars); aligned_free(points); From 57d4d817bc5847c59f808f57f05d5cd473b072c1 Mon Sep 17 00:00:00 2001 From: ludamad Date: Thu, 29 Aug 2024 21:58:39 +0000 Subject: [PATCH 02/23] speed up non powers of 2 in pippenger --- .../scalar_multiplication.cpp | 72 ++++++++++++------- 1 file changed, 46 insertions(+), 26 deletions(-) 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 cb94afe49b73..2dfd94579557 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -127,7 +127,7 @@ void generate_pippenger_point_table(typename Curve::AffineElement* points, *points we're multiplying. Once we have the number of rounds, `m`, we need to split our scalar into `m` bit-slices. *Each pippenger round will work on one bit-slice. * - * Pippenger's algorithm works by, for each round, iterating over the points we're multplying. For each point, we + * Pippenger's algorithm works by, for each round, iterating over the points we're multiplying. For each point, we *examing the point's scalar multiplier and extract the bit-slice associated with the current pippenger round (we start *with the most significant slice). We then use the bit-slice to index a 'bucket', which we add the point into. For *example, if the bit slice is 01101, we add the corresponding point into bucket[13]. @@ -193,7 +193,7 @@ void generate_pippenger_point_table(typename Curve::AffineElement* points, * @param input_skew_table Pointer to the output array with all skews * @param round_counts The number of points in each round * @param scalars The pointer to the region with initial scalars that need to be converted into WNAF - * @param num_initial_points The number of points before the endomorphism split + * @param num_initial_points The number of points before the endomorphism split. A rounded up power of 2. **/ template void compute_wnaf_states(uint64_t* point_schedule, @@ -220,30 +220,46 @@ void compute_wnaf_states(uint64_t* point_schedule, } parallel_for(num_threads, [&](size_t i) { - Fr T0; uint64_t* wnaf_table = &point_schedule[(2 * i) * num_initial_points_per_thread]; - const Fr* thread_scalars = &scalars[i * num_initial_points_per_thread]; bool* skew_table = &input_skew_table[(2 * i) * num_initial_points_per_thread]; - uint64_t offset = i * num_points_per_thread; - - for (uint64_t j = 0; j < num_initial_points_per_thread; ++j) { - T0 = thread_scalars[j].from_montgomery_form(); - Fr::split_into_endomorphism_scalars(T0, T0, *(Fr*)&T0.data[2]); - - wnaf::fixed_wnaf_with_counts(&T0.data[0], - &wnaf_table[(j << 1UL)], - skew_table[j << 1ULL], + // Our offsets for this thread + const uint64_t point_offset = i * num_points_per_thread; + const uint64_t scalar_offset = i * num_initial_points_per_thread; + + // How many defined scalars are there? + const size_t defined_extent = std::min(scalar_offset + num_initial_points_per_thread, scalars.size()); + const size_t defined_scalars = defined_extent > scalar_offset ? defined_extent - scalar_offset : 0; + + auto wnaf_first_half = [&](const uint64_t* scalar, size_t j) { + wnaf::fixed_wnaf_with_counts(scalar, + &wnaf_table[j * 2], + skew_table[j * 2], &thread_round_counts[i][0], - ((j << 1ULL) + offset) << 32ULL, + (j * 2 + point_offset) << 32, num_points, wnaf_bits); - wnaf::fixed_wnaf_with_counts(&T0.data[2], - &wnaf_table[(j << 1UL) + 1], - skew_table[(j << 1UL) + 1], + }; + auto wnaf_second_half = [&](const uint64_t* scalar, size_t j) { + wnaf::fixed_wnaf_with_counts(scalar, + &wnaf_table[j * 2 + 1], + skew_table[j * 2 + 1], &thread_round_counts[i][0], - ((j << 1UL) + offset + 1) << 32UL, + (j * 2 + point_offset + 1) << 32, num_points, wnaf_bits); + }; + for (uint64_t j = 0; j < defined_scalars; j++) { + Fr T0 = scalars[scalar_offset + j].from_montgomery_form(); + Fr::split_into_endomorphism_scalars(T0, T0, *(Fr*)&T0.data[2]); + + wnaf_first_half(&T0.data[0], j); + wnaf_second_half(&T0.data[2], j); + } + for (uint64_t j = defined_scalars; j < num_initial_points_per_thread; j++) { + // If we are trying to use a non-power-of-2 + static const uint64_t PADDING_ZEROES[] = { 0, 0 }; + wnaf_first_half(PADDING_ZEROES, j); + wnaf_second_half(PADDING_ZEROES, j); } }); @@ -908,14 +924,18 @@ typename Curve::Element pippenger(std::span s const auto slice_bits = static_cast(numeric::get_msb(static_cast(num_initial_points))); const auto num_slice_points = static_cast(1ULL << slice_bits); - Element result = pippenger_internal(points, scalars, num_slice_points, state, handle_edge_cases); - if (num_slice_points != num_initial_points) { - return result + pippenger(scalars.subspan(num_slice_points), - points + static_cast(num_slice_points * 2), - state, - handle_edge_cases); - } - return result; + return pippenger_internal(points, + scalars, + num_slice_points == scalars.size() ? num_slice_points : num_slice_points * 2, + state, + handle_edge_cases); + // if (num_slice_points != num_initial_points) { + // return result + pippenger(scalars.subspan(num_slice_points), + // points + static_cast(num_slice_points * 2), + // state, + // handle_edge_cases); + // } + // return result; } /** From bb6b7cccc4c55c79d64b4af6f4a55f6b6dc8cb49 Mon Sep 17 00:00:00 2001 From: ludamad Date: Thu, 29 Aug 2024 23:52:00 +0000 Subject: [PATCH 03/23] fix commitmentkey: rounding up to power of 2 We were reading SRS points out of bounds. Round up to power of 2. --- .../commitment_schemes/commit.bench.cpp | 49 ++++++++++++------- .../commitment_schemes/commitment_key.hpp | 18 ++++--- .../barretenberg/numeric/bitop/get_msb.hpp | 6 +++ .../barretenberg/numeric/random/engine.cpp | 6 +-- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp index f0b20c0a03c8..e973b77f935d 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp @@ -28,7 +28,7 @@ template Polynomial sparse_random_poly(const size_t size, cons } constexpr size_t MIN_LOG_NUM_POINTS = 16; -constexpr size_t MAX_LOG_NUM_POINTS = 20; +constexpr size_t MAX_LOG_NUM_POINTS = 22; constexpr size_t MAX_NUM_POINTS = 1 << MAX_LOG_NUM_POINTS; constexpr size_t SPARSE_NUM_NONZERO = 100; @@ -129,25 +129,40 @@ template void bench_commit_random(::benchmark::State& state) key->commit(polynomial); } } +// Commit to a polynomial with dense random nonzero entries but NOT our happiest case of an exact power of 2 +// Note this used to be a 50% regression just subtracting a power of 2 by 1. +template void bench_commit_random_non_power_of_2(::benchmark::State& state) +{ + using Fr = typename Curve::ScalarField; + auto key = create_commitment_key(MAX_NUM_POINTS); + + const size_t num_points = 1 << state.range(0); + auto polynomial = Polynomial(num_points - 1); + for (auto& coeff : polynomial) { + coeff = Fr::random_element(); + } + for (auto _ : state) { + key->commit(polynomial); + } +} BENCHMARK(bench_commit_zero) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) - ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_sparse) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) - ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_sparse_preprocessed) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) - ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_sparse_random) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) - ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_sparse_random_preprocessed) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) - ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_random) - ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->DenseRange(MIN_LOG_NUM_POINTS / 2, MAX_LOG_NUM_POINTS / 2) ->Unit(benchmark::kMillisecond); +// BENCHMARK(bench_commit_sparse) +// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) +// ->Unit(benchmark::kMillisecond); +// BENCHMARK(bench_commit_sparse_preprocessed) +// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) +// ->Unit(benchmark::kMillisecond); +// BENCHMARK(bench_commit_sparse_random) +// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) +// ->Unit(benchmark::kMillisecond); +// BENCHMARK(bench_commit_sparse_random_preprocessed) +// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) +// ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_random)->DenseRange(22, 22)->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_random_non_power_of_2)->DenseRange(22, 22)->Unit(benchmark::kMillisecond); } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index db900c3ac50a..f60e9abd3ed4 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -9,6 +9,7 @@ #include "barretenberg/common/op_count.hpp" #include "barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp" +#include "barretenberg/numeric/bitop/get_msb.hpp" #include "barretenberg/numeric/bitop/pow.hpp" #include "barretenberg/polynomials/polynomial_arithmetic.hpp" #include "barretenberg/srs/factories/crs_factory.hpp" @@ -50,10 +51,11 @@ template class CommitmentKey { * */ CommitmentKey(const size_t num_points) - : pippenger_runtime_state(num_points) + : pippenger_runtime_state(numeric::round_up_power_2(num_points)) , crs_factory(srs::get_crs_factory()) - , srs(crs_factory->get_prover_crs(num_points)) - {} + , srs(crs_factory->get_prover_crs(numeric::round_up_power_2(num_points))) + { + } // Note: This constructor is to be used only by Plonk; For Honk the srs lives in the CommitmentKey CommitmentKey(const size_t num_points, std::shared_ptr> prover_crs) @@ -70,11 +72,11 @@ template class CommitmentKey { Commitment commit(std::span polynomial) { BB_OP_COUNT_TIME(); - const size_t degree = polynomial.size(); - if (degree > srs->get_monomial_size()) { - info("Attempting to commit to a polynomial of degree ", - degree, - " with an SRS of size ", + const size_t consumed_srs = numeric::round_up_power_2(polynomial.size()); + if (consumed_srs > srs->get_monomial_size()) { + info("Attempting to commit to a polynomial that needs ", + consumed_srs, + " points with an SRS of size ", srs->get_monomial_size()); ASSERT(false); } diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp index fc456a68a8e0..c67bd69a7627 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp @@ -43,4 +43,10 @@ template constexpr inline T get_msb(const T in) return (sizeof(T) <= 4) ? get_msb32(in) : get_msb64(in); } +template constexpr inline T round_up_power_2(const T in) +{ + auto lower_bound = T(1) << get_msb(in); + return lower_bound == in ? in : lower_bound * 2; +} + } // namespace bb::numeric \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp index 0c3604cb92a6..e52f265fdd13 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp @@ -131,9 +131,9 @@ RNG& get_debug_randomness(bool reset, std::uint_fast64_t seed) */ RNG& get_randomness() { - // return get_debug_randomness(); - static RandomEngine engine; - return engine; + return get_debug_randomness(); + // static RandomEngine engine; + // return engine; } } // namespace bb::numeric From d6d08d0d3d3397bc4307a60dd34be06c1f33891b Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:03:00 +0000 Subject: [PATCH 04/23] comment --- .../src/barretenberg/commitment_schemes/commitment_key.hpp | 5 +++-- 1 file changed, 3 insertions(+), 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 f60e9abd3ed4..fb6a4702be01 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -51,11 +51,12 @@ template class CommitmentKey { * */ CommitmentKey(const size_t num_points) + // Currently we must round up internal space for points as pippenger will use next power of 2. + // This is used to simplify the recursive halving scheme. : pippenger_runtime_state(numeric::round_up_power_2(num_points)) , crs_factory(srs::get_crs_factory()) , srs(crs_factory->get_prover_crs(numeric::round_up_power_2(num_points))) - { - } + {} // Note: This constructor is to be used only by Plonk; For Honk the srs lives in the CommitmentKey CommitmentKey(const size_t num_points, std::shared_ptr> prover_crs) From 4a871b2443efdeb6f69152d15c3f3d10f491b809 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:28:52 +0000 Subject: [PATCH 05/23] comment and use new pippenger --- .../commitment_schemes/commitment_key.hpp | 10 +++-- .../scalar_multiplication.cpp | 38 +++++++++++++------ .../scalar_multiplication.hpp | 7 ++++ 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index fb6a4702be01..19d08b2a5439 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -51,8 +51,10 @@ template class CommitmentKey { * */ CommitmentKey(const size_t num_points) - // Currently we must round up internal space for points as pippenger will use next power of 2. - // This is used to simplify the recursive halving scheme. + // Currently we must round up internal space for points as our pippenger algorithm (specifically, + // pippenger_unsafe_optimized_for_non_dyadic_polys) will use next power of 2. This is used to simplify the + // recursive halving scheme. We do, however allow the polynomial to not be fully formed. Pippenger internally + // will pad 0s into the runtime state. : pippenger_runtime_state(numeric::round_up_power_2(num_points)) , crs_factory(srs::get_crs_factory()) , srs(crs_factory->get_prover_crs(numeric::round_up_power_2(num_points))) @@ -73,6 +75,7 @@ template class CommitmentKey { Commitment commit(std::span polynomial) { BB_OP_COUNT_TIME(); + // See constructor, we must round up the number of used srs points to a power of 2. const size_t consumed_srs = numeric::round_up_power_2(polynomial.size()); if (consumed_srs > srs->get_monomial_size()) { info("Attempting to commit to a polynomial that needs ", @@ -81,7 +84,8 @@ template class CommitmentKey { srs->get_monomial_size()); ASSERT(false); } - return scalar_multiplication::pippenger_unsafe( + // NOTE: pippenger_unsafe_optimized_for_non_dyadic_polys requires dyadic SRS. + return scalar_multiplication::pippenger_unsafe_optimized_for_non_dyadic_polys( polynomial, srs->get_monomial_points(), pippenger_runtime_state); }; 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 2dfd94579557..0f99c872cff6 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -924,18 +924,32 @@ typename Curve::Element pippenger(std::span s const auto slice_bits = static_cast(numeric::get_msb(static_cast(num_initial_points))); const auto num_slice_points = static_cast(1ULL << slice_bits); - return pippenger_internal(points, - scalars, - num_slice_points == scalars.size() ? num_slice_points : num_slice_points * 2, - state, - handle_edge_cases); - // if (num_slice_points != num_initial_points) { - // return result + pippenger(scalars.subspan(num_slice_points), - // points + static_cast(num_slice_points * 2), - // state, - // handle_edge_cases); - // } - // return result; + Element result = pippenger_internal(points, scalars, num_slice_points, state, handle_edge_cases); + if (num_slice_points != num_initial_points) { + return result + pippenger(scalars.subspan(num_slice_points), + points + static_cast(num_slice_points * 2), + state, + handle_edge_cases); + } + return result; +} + +/* @brief Used for commits. +The main reason for this existing alone as this has one assumption: +The number of points is a dyadic (power of 2) size and scalars is not. +Pippenger above can behavely poorly with numbers with many bits set.*/ + +template +typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( + std::span scalars, + std::span points, + pippenger_runtime_state& state) +{ + BB_OP_COUNT_TIME(); + // We round up SRS points. + ASSERT(numeric::round_up_power_2(points.size()) == points.size()) + // We do not optimize for the small case at all. + return pippenger_internal(points.begin(), scalars, numeric::round_up_power_2(scalars.size()), state, false); } /** diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp index 4312e50e28a1..0ba7b390318f 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp @@ -168,6 +168,13 @@ typename Curve::Element pippenger_without_endomorphism_basis_points( typename Curve::AffineElement* points, pippenger_runtime_state& state); +// NOTE: pippenger_unsafe_optimized_for_non_dyadic_polys requires dyadic SRS. +template +typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( + std::span scalars, + std::span points, + pippenger_runtime_state& state); + // Explicit instantiation // BN254 From cf62795122b034d7a5d28a82ca151fae17c0f250 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:37:27 +0000 Subject: [PATCH 06/23] comment and use new pippenger --- .../commitment_schemes/commitment_key.hpp | 5 ++--- .../scalar_multiplication.cpp | 18 ++++++++++++++---- .../scalar_multiplication.hpp | 3 ++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 19d08b2a5439..989f0f96b8e2 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -77,16 +77,15 @@ template class CommitmentKey { BB_OP_COUNT_TIME(); // See constructor, we must round up the number of used srs points to a power of 2. const size_t consumed_srs = numeric::round_up_power_2(polynomial.size()); - if (consumed_srs > srs->get_monomial_size()) { + if (consumed_srs >= srs->get_monomial_size()) { info("Attempting to commit to a polynomial that needs ", consumed_srs, " points with an SRS of size ", srs->get_monomial_size()); ASSERT(false); } - // NOTE: pippenger_unsafe_optimized_for_non_dyadic_polys requires dyadic SRS. return scalar_multiplication::pippenger_unsafe_optimized_for_non_dyadic_polys( - polynomial, srs->get_monomial_points(), pippenger_runtime_state); + polynomial, { srs->get_monomial_points(), srs->get_monomial_size() }, pippenger_runtime_state); }; /** 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 0f99c872cff6..81a78f3957e4 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -12,6 +12,7 @@ #include "barretenberg/common/op_count.hpp" #include "barretenberg/common/thread.hpp" #include "barretenberg/common/throw_or_abort.hpp" +#include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/ecc/groups/wnaf.hpp" #include "barretenberg/numeric/bitop/get_msb.hpp" @@ -936,7 +937,7 @@ typename Curve::Element pippenger(std::span s /* @brief Used for commits. The main reason for this existing alone as this has one assumption: -The number of points is a dyadic (power of 2) size and scalars is not. +The number of points is equal to or larger than #scalars rounded up to next power of 2. Pippenger above can behavely poorly with numbers with many bits set.*/ template @@ -946,10 +947,10 @@ typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( pippenger_runtime_state& state) { BB_OP_COUNT_TIME(); - // We round up SRS points. - ASSERT(numeric::round_up_power_2(points.size()) == points.size()) + // We need a padding of scalars. + ASSERT(numeric::round_up_power_2(scalars.size()) <= points.size()) // We do not optimize for the small case at all. - return pippenger_internal(points.begin(), scalars, numeric::round_up_power_2(scalars.size()), state, false); + return pippenger_internal(&points[0], scalars, numeric::round_up_power_2(scalars.size()), state, false); } /** @@ -1030,6 +1031,11 @@ template curve::BN254::Element pippenger_unsafe(std::span& state); +template curve::BN254::Element pippenger_unsafe_optimized_for_non_dyadic_polys( + std::span scalars, + std::span points, + pippenger_runtime_state& state); + template curve::BN254::Element pippenger_without_endomorphism_basis_points( std::span scalars, curve::BN254::AffineElement* points, @@ -1079,6 +1085,10 @@ template curve::Grumpkin::Element pippenger_unsafe( std::span scalars, curve::Grumpkin::AffineElement* points, pippenger_runtime_state& state); +template curve::Grumpkin::Element pippenger_unsafe_optimized_for_non_dyadic_polys( + std::span scalars, + std::span points, + pippenger_runtime_state& state); template curve::Grumpkin::Element pippenger_without_endomorphism_basis_points( std::span scalars, diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp index 0ba7b390318f..47d1bcb72087 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp @@ -168,7 +168,8 @@ typename Curve::Element pippenger_without_endomorphism_basis_points( typename Curve::AffineElement* points, pippenger_runtime_state& state); -// NOTE: pippenger_unsafe_optimized_for_non_dyadic_polys requires dyadic SRS. +// NOTE: pippenger_unsafe_optimized_for_non_dyadic_polys requires SRS to have #scalars +// rounded up to nearest power of 2 or above points. template typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( std::span scalars, From 01efa1f1e13bb9c1e7ad2225199e8834c1c92f9c Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:44:08 +0000 Subject: [PATCH 07/23] working --- .../cpp/src/barretenberg/commitment_schemes/commitment_key.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 989f0f96b8e2..12e0ba390ff4 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -77,7 +77,7 @@ template class CommitmentKey { BB_OP_COUNT_TIME(); // See constructor, we must round up the number of used srs points to a power of 2. const size_t consumed_srs = numeric::round_up_power_2(polynomial.size()); - if (consumed_srs >= srs->get_monomial_size()) { + if (consumed_srs > srs->get_monomial_size()) { info("Attempting to commit to a polynomial that needs ", consumed_srs, " points with an SRS of size ", From 5c8418ae3a180d6c7bc6c1e5d611bf1bf97abf54 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:49:14 +0000 Subject: [PATCH 08/23] revert --- .../commitment_schemes/commit.bench.cpp | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp index e973b77f935d..c2cd8ec68b37 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commit.bench.cpp @@ -28,7 +28,7 @@ template Polynomial sparse_random_poly(const size_t size, cons } constexpr size_t MIN_LOG_NUM_POINTS = 16; -constexpr size_t MAX_LOG_NUM_POINTS = 22; +constexpr size_t MAX_LOG_NUM_POINTS = 20; constexpr size_t MAX_NUM_POINTS = 1 << MAX_LOG_NUM_POINTS; constexpr size_t SPARSE_NUM_NONZERO = 100; @@ -147,22 +147,26 @@ template void bench_commit_random_non_power_of_2(::benchmark::S } BENCHMARK(bench_commit_zero) - ->DenseRange(MIN_LOG_NUM_POINTS / 2, MAX_LOG_NUM_POINTS / 2) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_sparse) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_sparse_preprocessed) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_sparse_random) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_sparse_random_preprocessed) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_random) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) + ->Unit(benchmark::kMillisecond); +BENCHMARK(bench_commit_random_non_power_of_2) + ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) ->Unit(benchmark::kMillisecond); -// BENCHMARK(bench_commit_sparse) -// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) -// ->Unit(benchmark::kMillisecond); -// BENCHMARK(bench_commit_sparse_preprocessed) -// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) -// ->Unit(benchmark::kMillisecond); -// BENCHMARK(bench_commit_sparse_random) -// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) -// ->Unit(benchmark::kMillisecond); -// BENCHMARK(bench_commit_sparse_random_preprocessed) -// ->DenseRange(MIN_LOG_NUM_POINTS, MAX_LOG_NUM_POINTS) -// ->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_random)->DenseRange(22, 22)->Unit(benchmark::kMillisecond); -BENCHMARK(bench_commit_random_non_power_of_2)->DenseRange(22, 22)->Unit(benchmark::kMillisecond); } // namespace bb From cb8f5e60871d584f42b3bf31a96506c39ff8d3d0 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:51:27 +0000 Subject: [PATCH 09/23] revert --- barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp index e52f265fdd13..0c3604cb92a6 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp @@ -131,9 +131,9 @@ RNG& get_debug_randomness(bool reset, std::uint_fast64_t seed) */ RNG& get_randomness() { - return get_debug_randomness(); - // static RandomEngine engine; - // return engine; + // return get_debug_randomness(); + static RandomEngine engine; + return engine; } } // namespace bb::numeric From 536ebbecb0ba0bb54174dd21b528bf81ce44a76f Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:52:00 +0000 Subject: [PATCH 10/23] std span --- .../src/barretenberg/plonk/proof_system/verifier/verifier.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp index 79a5afb4fb05..cea9ea412d2a 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/verifier/verifier.cpp @@ -182,7 +182,8 @@ template bool VerifierBase::verify g1::element P[2]; - P[0] = bb::scalar_multiplication::pippenger({ &scalars[0], num_elements }, &elements[0], state); + P[0] = + bb::scalar_multiplication::pippenger({ &scalars[0], /*size*/ num_elements }, &elements[0], state); P[1] = -(g1::element(PI_Z_OMEGA) * separator_challenge + PI_Z); if (key->contains_recursive_proof) { From 528142a364767f78199edf20321415515329cf09 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 00:55:15 +0000 Subject: [PATCH 11/23] revert --- barretenberg/cpp/src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/CMakeLists.txt b/barretenberg/cpp/src/CMakeLists.txt index 6abbf3b0d867..a2c8ba9eb158 100644 --- a/barretenberg/cpp/src/CMakeLists.txt +++ b/barretenberg/cpp/src/CMakeLists.txt @@ -12,7 +12,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) # Specifying `-Wno-${ERROR_NAME}` will silence the error completely. # To preserve the warning, but prevent them from causing the build to fail, # use the flag `-Wno-error=${ERROR_NAME}` -add_compile_options(-Werror -Wall -Wextra -Wconversion -Wsign-conversion) +add_compile_options(-Werror -Wall -Wextra -Wconversion -Wsign-conversion -Wfatal-errors) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-fcolor-diagnostics -fconstexpr-steps=100000000) From c371e372e993fa0273af9ff2482df9d5e41921cd Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 08:21:54 -0400 Subject: [PATCH 12/23] Update scalar_multiplication.cpp --- .../ecc/scalar_multiplication/scalar_multiplication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 81a78f3957e4..a3e111b9423a 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -948,7 +948,7 @@ typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( { BB_OP_COUNT_TIME(); // We need a padding of scalars. - ASSERT(numeric::round_up_power_2(scalars.size()) <= points.size()) + ASSERT(numeric::round_up_power_2(scalars.size()) <= points.size()); // We do not optimize for the small case at all. return pippenger_internal(&points[0], scalars, numeric::round_up_power_2(scalars.size()), state, false); } From 94e6df345fc37a68c2afc183f64ac4bf68eeb4ad Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 09:04:03 -0400 Subject: [PATCH 13/23] Update scalar_multiplication.cpp --- .../ecc/scalar_multiplication/scalar_multiplication.cpp | 1 + 1 file changed, 1 insertion(+) 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 a3e111b9423a..46247451086b 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include "./process_buckets.hpp" From 7dc942be17e3c04c9d6289f5041e4bfa82b25606 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 09:04:22 -0400 Subject: [PATCH 14/23] Update scalar_multiplication.cpp --- .../ecc/scalar_multiplication/scalar_multiplication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 46247451086b..637ba1538108 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -1,8 +1,8 @@ +#include #include #include #include #include -#include #include #include "./process_buckets.hpp" From 25f64be6f5d866ad830d1953009801718cbf4ab0 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 09:11:01 -0400 Subject: [PATCH 15/23] Update scalar_multiplication.cpp --- .../scalar_multiplication.cpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) 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 637ba1538108..66eeab3b5e93 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -103,7 +103,7 @@ namespace bb::scalar_multiplication { * to use the curve endomorphism for faster scalar multiplication. See below for more details. */ template -void generate_pippenger_point_table(typename Curve::AffineElement* points, +void generate_pippenger_point_table(const typename Curve::AffineElement* points, typename Curve::AffineElement* table, size_t num_points) { @@ -226,7 +226,7 @@ void compute_wnaf_states(uint64_t* point_schedule, bool* skew_table = &input_skew_table[(2 * i) * num_initial_points_per_thread]; // Our offsets for this thread const uint64_t point_offset = i * num_points_per_thread; - const uint64_t scalar_offset = i * num_initial_points_per_thread; + const size_t scalar_offset = i * num_initial_points_per_thread; // How many defined scalars are there? const size_t defined_extent = std::min(scalar_offset + num_initial_points_per_thread, scalars.size()); @@ -237,7 +237,7 @@ void compute_wnaf_states(uint64_t* point_schedule, &wnaf_table[j * 2], skew_table[j * 2], &thread_round_counts[i][0], - (j * 2 + point_offset) << 32, + (j * 2ULL + point_offset) << 32ULL, num_points, wnaf_bits); }; @@ -246,18 +246,18 @@ void compute_wnaf_states(uint64_t* point_schedule, &wnaf_table[j * 2 + 1], skew_table[j * 2 + 1], &thread_round_counts[i][0], - (j * 2 + point_offset + 1) << 32, + (j * 2ULL + point_offset + 1ULL) << 32ULL, num_points, wnaf_bits); }; - for (uint64_t j = 0; j < defined_scalars; j++) { + for (size_t j = 0; j < defined_scalars; j++) { Fr T0 = scalars[scalar_offset + j].from_montgomery_form(); Fr::split_into_endomorphism_scalars(T0, T0, *(Fr*)&T0.data[2]); wnaf_first_half(&T0.data[0], j); wnaf_second_half(&T0.data[2], j); } - for (uint64_t j = defined_scalars; j < num_initial_points_per_thread; j++) { + for (size_t j = defined_scalars; j < num_initial_points_per_thread; j++) { // If we are trying to use a non-power-of-2 static const uint64_t PADDING_ZEROES[] = { 0, 0 }; wnaf_first_half(PADDING_ZEROES, j); @@ -758,7 +758,7 @@ uint32_t construct_addition_chains(affine_product_runtime_state& state, b template typename Curve::Element evaluate_pippenger_rounds(pippenger_runtime_state& state, - typename Curve::AffineElement* points, + const typename Curve::AffineElement* points, const size_t num_points, bool handle_edge_cases) { @@ -846,7 +846,7 @@ typename Curve::Element evaluate_pippenger_rounds(pippenger_runtime_state if (i == (num_rounds - 1)) { const size_t num_points_per_thread = num_points / num_threads; bool* skew_table = &state.skew_table[j * num_points_per_thread]; - AffineElement* point_table = &points[j * num_points_per_thread]; + const AffineElement* point_table = &points[j * num_points_per_thread]; AffineElement addition_temporary; for (size_t k = 0; k < num_points_per_thread; ++k) { if (skew_table[k]) { @@ -990,7 +990,7 @@ typename Curve::Element pippenger_without_endomorphism_basis_points( // Explicit instantiation // BN254 -template void generate_pippenger_point_table(curve::BN254::AffineElement* points, +template void generate_pippenger_point_table(const curve::BN254::AffineElement* points, curve::BN254::AffineElement* table, size_t num_points); @@ -998,7 +998,7 @@ template uint32_t construct_addition_chains(affine_product_runtime bool empty_bucket_counts = true); template void add_affine_points(curve::BN254::AffineElement* points, - const size_t num_points, + size_t num_points, curve::BN254::BaseField* scratch_space); template void add_affine_points_with_edge_cases(curve::BN254::AffineElement* points, @@ -1015,7 +1015,7 @@ template curve::BN254::Element pippenger_internal(curve::BN254::Af bool handle_edge_cases); template curve::BN254::Element evaluate_pippenger_rounds(pippenger_runtime_state& state, - curve::BN254::AffineElement* points, + const curve::BN254::AffineElement* points, const size_t num_points, bool handle_edge_cases = false); @@ -1043,7 +1043,7 @@ template curve::BN254::Element pippenger_without_endomorphism_basis_points& state); // Grumpkin -template void generate_pippenger_point_table(curve::Grumpkin::AffineElement* points, +template void generate_pippenger_point_table(const curve::Grumpkin::AffineElement* points, curve::Grumpkin::AffineElement* table, size_t num_points); @@ -1070,7 +1070,7 @@ template curve::Grumpkin::Element pippenger_internal( template curve::Grumpkin::Element evaluate_pippenger_rounds( pippenger_runtime_state& state, - curve::Grumpkin::AffineElement* points, + const curve::Grumpkin::AffineElement* points, const size_t num_points, bool handle_edge_cases = false); From bf21caf256be56b07bc0ca7fbeea3990b38c88d6 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 09:12:49 -0400 Subject: [PATCH 16/23] Update scalar_multiplication.hpp --- .../ecc/scalar_multiplication/scalar_multiplication.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp index 47d1bcb72087..a4604f652abd 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp @@ -93,7 +93,7 @@ void compute_wnaf_states(uint64_t* point_schedule, size_t num_initial_points); template -void generate_pippenger_point_table(typename Curve::AffineElement* points, +void generate_pippenger_point_table(const typename Curve::AffineElement* points, typename Curve::AffineElement* table, size_t num_points); @@ -142,7 +142,7 @@ typename Curve::Element pippenger_internal(typename Curve::AffineElement* points template typename Curve::Element evaluate_pippenger_rounds(pippenger_runtime_state& state, - typename Curve::AffineElement* points, + const typename Curve::AffineElement* points, size_t num_points, bool handle_edge_cases = false); From 1b16007f6dd5d82a348fb2e518909e9ae84c77ef Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 13:19:41 +0000 Subject: [PATCH 17/23] compile fix --- .../barretenberg/ecc/scalar_multiplication/runtime_states.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/runtime_states.hpp b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/runtime_states.hpp index 19f686f2f3bb..d6b659bd25f7 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/runtime_states.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/runtime_states.hpp @@ -64,7 +64,7 @@ constexpr size_t get_num_rounds(const size_t num_points) } template struct affine_product_runtime_state { - typename Curve::AffineElement* points; + const typename Curve::AffineElement* points; typename Curve::AffineElement* point_pairs_1; typename Curve::AffineElement* point_pairs_2; typename Curve::BaseField* scratch_space; From 39a952566c456824bbca6f890029f4bfbdf2b244 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 14:23:20 +0000 Subject: [PATCH 18/23] fix scalar mul edge cases. woops, tried to be too clever --- .../ecc/scalar_multiplication/scalar_multiplication.cpp | 7 +++++++ 1 file changed, 7 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 66eeab3b5e93..3bd52afd5633 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.cpp @@ -948,6 +948,13 @@ typename Curve::Element pippenger_unsafe_optimized_for_non_dyadic_polys( pippenger_runtime_state& state) { BB_OP_COUNT_TIME(); + + // our windowed non-adjacent form algorthm requires that each thread can work on at least 8 points. + const size_t threshold = get_num_cpus_pow2() * 8; + // Delegate edge-cases to normal pippenger_unsafe(). + if (scalars.size() <= threshold) { + return pippenger_unsafe(scalars, &points[0], state); + } // We need a padding of scalars. ASSERT(numeric::round_up_power_2(scalars.size()) <= points.size()); // We do not optimize for the small case at all. From c3e37c52574f588b0446515e091aacb5cd556c14 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 16:10:28 +0000 Subject: [PATCH 19/23] speculative fix --- .../cpp/scripts/benchmark_client_ivc.sh | 2 +- .../client_ivc_bench/client_ivc.bench.cpp | 22 ++++++++++--------- .../barretenberg/numeric/bitop/get_msb.hpp | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/scripts/benchmark_client_ivc.sh b/barretenberg/cpp/scripts/benchmark_client_ivc.sh index 0841f4429ba3..88786d0ee5b5 100755 --- a/barretenberg/cpp/scripts/benchmark_client_ivc.sh +++ b/barretenberg/cpp/scripts/benchmark_client_ivc.sh @@ -4,7 +4,7 @@ set -eu TARGET=${1:-"client_ivc_bench"} if [ "$TARGET" = "client_ivc_bench" ]; then - BENCHMARK="ClientIVCBench/Full/6" + BENCHMARK="ClientIVCBench/Full/8" elif [ "$TARGET" = "aztec_ivc_bench" ]; then BENCHMARK="AztecIVCBench/FullStructured/6" else diff --git a/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp b/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp index 00eb07d75495..ce4338886f0b 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp @@ -150,18 +150,20 @@ class ClientIVCBench : public benchmark::Fixture { */ BENCHMARK_DEFINE_F(ClientIVCBench, Full)(benchmark::State& state) { - ClientIVC ivc; - - auto num_circuits = static_cast(state.range(0)); - auto precomputed_vks = precompute_verification_keys(ivc, num_circuits); - for (auto _ : state) { BB_REPORT_OP_COUNT_IN_BENCH(state); - // Perform a specified number of iterations of function/kernel accumulation - perform_ivc_accumulation_rounds(num_circuits, ivc, precomputed_vks); - - // Construct IVC scheme proof (fold, decider, merge, eccvm, translator) - ivc.prove(); + for (size_t i = 0; i < 10; i++) { + state.PauseTiming(); + ClientIVC ivc; + auto num_circuits = static_cast(state.range(0)); + auto precomputed_vks = precompute_verification_keys(ivc, num_circuits); + state.ResumeTiming(); + // Perform a specified number of iterations of function/kernel accumulation + perform_ivc_accumulation_rounds(num_circuits, ivc, precomputed_vks); + + // Construct IVC scheme proof (fold, decider, merge, eccvm, translator) + ivc.prove(); + } } } diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp index c67bd69a7627..3bf24ca9e70b 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp @@ -46,7 +46,7 @@ template constexpr inline T get_msb(const T in) template constexpr inline T round_up_power_2(const T in) { auto lower_bound = T(1) << get_msb(in); - return lower_bound == in ? in : lower_bound * 2; + return (lower_bound == in || lower_bound == 1) ? in : lower_bound * 2; } } // namespace bb::numeric \ No newline at end of file From 7dae7500549e004fbceabe80317d1a5a9810d677 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 17:13:06 +0000 Subject: [PATCH 20/23] better error --- barretenberg/cpp/src/barretenberg/srs/io.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/srs/io.hpp b/barretenberg/cpp/src/barretenberg/srs/io.hpp index 893f5bdd4438..f9c4a008517c 100644 --- a/barretenberg/cpp/src/barretenberg/srs/io.hpp +++ b/barretenberg/cpp/src/barretenberg/srs/io.hpp @@ -87,7 +87,7 @@ template class IO { file.read((char*)&manifest, sizeof(Manifest)); if (!file) { ptrdiff_t read = file.gcount(); - throw_or_abort(format("Only read ", read, " bytes from file but expected ", sizeof(Manifest), ".")); + throw_or_abort(format("Only read ", read, " bytes from manifest file ", filename, " but expected ", sizeof(Manifest), ".")); } file.close(); @@ -130,7 +130,7 @@ template class IO { file.read(buffer, (int)size); if (!file) { ptrdiff_t read = file.gcount(); - throw_or_abort(format("Only read ", read, " bytes from file but expected ", size, ".")); + throw_or_abort(format("Only read ", read, " bytes from transcript file ", filename, " but expected ", size, ".")); } file.close(); From a9c2617fd10fa5c29032cebf566412016a2075fb Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 18:42:58 +0000 Subject: [PATCH 21/23] fix edge case causing srs size too big --- barretenberg/cpp/Earthfile | 5 +---- .../commitment_schemes/commitment_key.hpp | 19 +++++++++++++------ .../cpp/src/barretenberg/flavor/flavor.hpp | 16 ++++++++-------- .../srs/factories/file_crs_factory.cpp | 5 ++--- .../factories/mem_grumpkin_crs_factory.cpp | 11 +++++++++-- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/barretenberg/cpp/Earthfile b/barretenberg/cpp/Earthfile index bc9cbfa26708..557f7c7ca3e1 100644 --- a/barretenberg/cpp/Earthfile +++ b/barretenberg/cpp/Earthfile @@ -233,19 +233,16 @@ test-clang-format: test: ARG hardware_concurrency="" # prefetch - BUILD +test-binaries BUILD +preset-release-assert-test BUILD +test-clang-format BUILD ./srs_db/+build # prefetch - FROM +source - COPY --dir +test-binaries/build build FROM +preset-release-assert-test COPY --dir ./srs_db/+build/. srs_db # limit hardware concurrency, if provided IF [ "$HARDWARE_CONCURRENCY" != "" ] ENV HARDWARE_CONCURRENCY=$hardware_concurrency END - RUN cd build && GTEST_COLOR=1 ctest -j$(nproc) --output-on-failure + RUN cd build && ./bin/stdlib_client_ivc_verifier_tests --gtest_filter=ClientIVCRecursionTests.ClientTubeBase #GTEST_COLOR=1 ctest -j$(nproc) --output-on-failure vm-full-test: ARG hardware_concurrency="" diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 12e0ba390ff4..746ebbe37865 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -35,6 +35,17 @@ template class CommitmentKey { using Fr = typename Curve::ScalarField; using Commitment = typename Curve::AffineElement; using G1 = typename Curve::AffineElement; + static constexpr size_t EXTRA_SRS_POINTS_FOR_ECCVM_IPA = 1; + + static size_t get_num_needed_srs_points(size_t num_points) + { + // NOTE 1: Currently we must round up internal space for points as our pippenger algorithm (specifically, + // pippenger_unsafe_optimized_for_non_dyadic_polys) will use next power of 2. This is used to simplify the + // recursive halving scheme. We do, however allow the polynomial to not be fully formed. Pippenger internally + // will pad 0s into the runtime state. + // NOTE 2: We then add one for ECCVM to provide for IPA verification + return numeric::round_up_power_2(num_points) + EXTRA_SRS_POINTS_FOR_ECCVM_IPA; + } public: scalar_multiplication::pippenger_runtime_state pippenger_runtime_state; @@ -51,13 +62,9 @@ template class CommitmentKey { * */ CommitmentKey(const size_t num_points) - // Currently we must round up internal space for points as our pippenger algorithm (specifically, - // pippenger_unsafe_optimized_for_non_dyadic_polys) will use next power of 2. This is used to simplify the - // recursive halving scheme. We do, however allow the polynomial to not be fully formed. Pippenger internally - // will pad 0s into the runtime state. - : pippenger_runtime_state(numeric::round_up_power_2(num_points)) + : pippenger_runtime_state(get_num_needed_srs_points(num_points)) , crs_factory(srs::get_crs_factory()) - , srs(crs_factory->get_prover_crs(numeric::round_up_power_2(num_points))) + , srs(crs_factory->get_prover_crs(get_num_needed_srs_points(num_points))) {} // Note: This constructor is to be used only by Plonk; For Honk the srs lives in the CommitmentKey diff --git a/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp b/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp index 2d38473aac0b..945902109e70 100644 --- a/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp @@ -130,7 +130,7 @@ template class ProvingKey_ { { if (commitment_key == nullptr) { ZoneScopedN("init commitment key"); - this->commitment_key = std::make_shared(circuit_size + 1); + this->commitment_key = std::make_shared(circuit_size); } else { // Don't create another commitment key if we already have one this->commitment_key = commitment_key; @@ -412,13 +412,13 @@ concept IsPlonkFlavor = IsAnyOf concept IsUltraPlonkFlavor = IsAnyOf; -template +template concept IsUltraPlonkOrHonk = IsAnyOf; -template +template concept IsHonkFlavor = IsAnyOf; -template +template concept IsUltraFlavor = IsAnyOf; template @@ -439,7 +439,7 @@ MegaRecursiveFlavor_, TranslatorRecursiveFlavor_, TranslatorRecursiveFlavor_, TranslatorRecursiveFlavor_, -ECCVMRecursiveFlavor_, +ECCVMRecursiveFlavor_, AvmRecursiveFlavor_>; template concept IsECCVMRecursiveFlavor = IsAnyOf>; @@ -451,9 +451,9 @@ template concept IsFoldingFlavor = IsAnyOf, - UltraRecursiveFlavor_, + MegaFlavor, + UltraRecursiveFlavor_, + UltraRecursiveFlavor_, UltraRecursiveFlavor_, MegaRecursiveFlavor_, MegaRecursiveFlavor_, MegaRecursiveFlavor_>; diff --git a/barretenberg/cpp/src/barretenberg/srs/factories/file_crs_factory.cpp b/barretenberg/cpp/src/barretenberg/srs/factories/file_crs_factory.cpp index 823c3ebdc0c9..bb4ddcf5f420 100644 --- a/barretenberg/cpp/src/barretenberg/srs/factories/file_crs_factory.cpp +++ b/barretenberg/cpp/src/barretenberg/srs/factories/file_crs_factory.cpp @@ -55,8 +55,7 @@ FileCrsFactory::FileCrsFactory(std::string path, size_t initial_degree) template std::shared_ptr> FileCrsFactory::get_prover_crs(size_t degree) { - if (degree != degree_ || !prover_crs_) { - ZoneScopedN("get_prover_crs"); + if (degree_ < degree || !prover_crs_) { prover_crs_ = std::make_shared>(degree, path_); degree_ = degree; } @@ -66,7 +65,7 @@ std::shared_ptr> FileCrsFactory::get template std::shared_ptr> FileCrsFactory::get_verifier_crs(size_t degree) { - if (degree != degree_ || !verifier_crs_) { + if (degree_ < degree || !verifier_crs_) { verifier_crs_ = std::make_shared>(path_, degree); degree_ = degree; } diff --git a/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp b/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp index bbec0dccf0d0..a6812c41fb01 100644 --- a/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp +++ b/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp @@ -1,5 +1,6 @@ #include "mem_grumpkin_crs_factory.hpp" #include "../io.hpp" +#include "barretenberg/common/throw_or_abort.hpp" #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp" #include "barretenberg/ecc/scalar_multiplication/point_table.hpp" #include "barretenberg/ecc/scalar_multiplication/scalar_multiplication.hpp" @@ -42,13 +43,19 @@ MemGrumpkinCrsFactory::MemGrumpkinCrsFactory(std::vector(points)) {} -std::shared_ptr> MemGrumpkinCrsFactory::get_prover_crs(size_t) +std::shared_ptr> MemGrumpkinCrsFactory::get_prover_crs(size_t degree) { + if (prover_crs_->get_monomial_size() < degree) { + throw_or_abort("prover trying to get too many points in MemGrumpkinCrsFactory!"); + } return prover_crs_; } -std::shared_ptr> MemGrumpkinCrsFactory::get_verifier_crs(size_t) +std::shared_ptr> MemGrumpkinCrsFactory::get_verifier_crs(size_t degree) { + if (prover_crs_->get_monomial_size() < degree) { + throw_or_abort("verifier trying to get too many points in MemGrumpkinCrsFactory!"); + } return verifier_crs_; } From 2be10cdd7956b53efecab579334935f365f5e88a Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 19:51:13 +0000 Subject: [PATCH 22/23] revert --- .../cpp/scripts/benchmark_client_ivc.sh | 2 +- .../client_ivc_bench/client_ivc.bench.cpp | 22 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/barretenberg/cpp/scripts/benchmark_client_ivc.sh b/barretenberg/cpp/scripts/benchmark_client_ivc.sh index 88786d0ee5b5..0841f4429ba3 100755 --- a/barretenberg/cpp/scripts/benchmark_client_ivc.sh +++ b/barretenberg/cpp/scripts/benchmark_client_ivc.sh @@ -4,7 +4,7 @@ set -eu TARGET=${1:-"client_ivc_bench"} if [ "$TARGET" = "client_ivc_bench" ]; then - BENCHMARK="ClientIVCBench/Full/8" + BENCHMARK="ClientIVCBench/Full/6" elif [ "$TARGET" = "aztec_ivc_bench" ]; then BENCHMARK="AztecIVCBench/FullStructured/6" else diff --git a/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp b/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp index ce4338886f0b..00eb07d75495 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp @@ -150,20 +150,18 @@ class ClientIVCBench : public benchmark::Fixture { */ BENCHMARK_DEFINE_F(ClientIVCBench, Full)(benchmark::State& state) { + ClientIVC ivc; + + auto num_circuits = static_cast(state.range(0)); + auto precomputed_vks = precompute_verification_keys(ivc, num_circuits); + for (auto _ : state) { BB_REPORT_OP_COUNT_IN_BENCH(state); - for (size_t i = 0; i < 10; i++) { - state.PauseTiming(); - ClientIVC ivc; - auto num_circuits = static_cast(state.range(0)); - auto precomputed_vks = precompute_verification_keys(ivc, num_circuits); - state.ResumeTiming(); - // Perform a specified number of iterations of function/kernel accumulation - perform_ivc_accumulation_rounds(num_circuits, ivc, precomputed_vks); - - // Construct IVC scheme proof (fold, decider, merge, eccvm, translator) - ivc.prove(); - } + // Perform a specified number of iterations of function/kernel accumulation + perform_ivc_accumulation_rounds(num_circuits, ivc, precomputed_vks); + + // Construct IVC scheme proof (fold, decider, merge, eccvm, translator) + ivc.prove(); } } From 70f03c590d199c3fd31fdc9ed024a2b260bc3430 Mon Sep 17 00:00:00 2001 From: ludamad Date: Fri, 30 Aug 2024 20:19:33 +0000 Subject: [PATCH 23/23] format --- .../barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp | 2 +- barretenberg/cpp/src/barretenberg/srs/io.hpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp b/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp index a6812c41fb01..8d0741cc920a 100644 --- a/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp +++ b/barretenberg/cpp/src/barretenberg/srs/factories/mem_grumpkin_crs_factory.cpp @@ -46,7 +46,7 @@ MemGrumpkinCrsFactory::MemGrumpkinCrsFactory(std::vector> MemGrumpkinCrsFactory::get_prover_crs(size_t degree) { if (prover_crs_->get_monomial_size() < degree) { - throw_or_abort("prover trying to get too many points in MemGrumpkinCrsFactory!"); + throw_or_abort("prover trying to get too many points in MemGrumpkinCrsFactory!"); } return prover_crs_; } diff --git a/barretenberg/cpp/src/barretenberg/srs/io.hpp b/barretenberg/cpp/src/barretenberg/srs/io.hpp index f9c4a008517c..bd0b8a1444ca 100644 --- a/barretenberg/cpp/src/barretenberg/srs/io.hpp +++ b/barretenberg/cpp/src/barretenberg/srs/io.hpp @@ -87,7 +87,8 @@ template class IO { file.read((char*)&manifest, sizeof(Manifest)); if (!file) { ptrdiff_t read = file.gcount(); - throw_or_abort(format("Only read ", read, " bytes from manifest file ", filename, " but expected ", sizeof(Manifest), ".")); + throw_or_abort(format( + "Only read ", read, " bytes from manifest file ", filename, " but expected ", sizeof(Manifest), ".")); } file.close(); @@ -130,7 +131,8 @@ template class IO { file.read(buffer, (int)size); if (!file) { ptrdiff_t read = file.gcount(); - throw_or_abort(format("Only read ", read, " bytes from transcript file ", filename, " but expected ", size, ".")); + throw_or_abort( + format("Only read ", read, " bytes from transcript file ", filename, " but expected ", size, ".")); } file.close();