From 3da1da1be9edaca4155b5ecd9ca58258c6238761 Mon Sep 17 00:00:00 2001 From: ludamad Date: Wed, 6 Sep 2023 21:00:58 +0000 Subject: [PATCH] git subrepo pull (merge) circuits/cpp/barretenberg subrepo: subdir: "circuits/cpp/barretenberg" merged: "d0e4f0018" upstream: origin: "https://github.com/AztecProtocol/barretenberg" branch: "master" commit: "4bb7559d0" git-subrepo: version: "0.4.6" origin: "???" commit: "???" --- cpp/CMakeLists.txt | 17 +-- cpp/src/barretenberg/common/serialize.hpp | 3 +- .../honk/flavor/ultra_recursive.hpp | 1 - .../barretenberg/honk/pcs/gemini/gemini.hpp | 38 +++-- cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp | 29 ++-- .../barretenberg/honk/pcs/shplonk/shplonk.hpp | 131 ++++++++++-------- .../arithmetization/gate_data.hpp | 4 +- .../goblin_ultra_circuit_builder.test.cpp | 66 +++------ .../circuit_builder/ultra_circuit_builder.cpp | 70 +++------- .../circuit_builder/ultra_circuit_builder.hpp | 19 ++- .../stdlib/primitives/biggroup/biggroup.hpp | 5 + .../primitives/biggroup/biggroup_goblin.hpp | 96 +++++++++++++ .../biggroup/biggroup_goblin.test.cpp | 89 ++++++++++++ .../primitives/biggroup/biggroup_impl.hpp | 5 +- .../verifier/ultra_recursive_verifier.cpp | 95 +++++++------ .../verifier/ultra_recursive_verifier.hpp | 16 ++- .../recursion/honk/verifier/verifier.test.cpp | 27 ++-- 17 files changed, 450 insertions(+), 261 deletions(-) create mode 100644 cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp create mode 100644 cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 6300f11138..e480763ed9 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -53,7 +53,7 @@ if(FUZZING) endif() if(UNDEFINED_BEHAVIOUR_SANITIZER) - set(SANITIZER_OPTIONS ${SANITIZER_OPTIONS} -fsanitize=undefined -fno-sanitize=alignment) + set(SANITIZER_OPTIONS ${SANITIZER_OPTIONS} -fsanitize=undefined -fno-sanitize=alignment) endif() add_compile_options(-fsanitize=fuzzer-no-link ${SANITIZER_OPTIONS}) @@ -81,15 +81,15 @@ set(CMAKE_CXX_STANDARD_REQUIRED TRUE) set(CMAKE_CXX_EXTENSIONS ON) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10") - message(WARNING "Clang <10 is not supported") + if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "14") + message(WARNING "Clang <14 is not supported") endif() elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10") + if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10") message(WARNING "GCC <10 is not supported") endif() else() - message(WARNING "Unsuported compiler, use Clang >10 or GCC >10") + message(WARNING "Unsuported compiler, use Clang >14 or GCC >10") endif() if(COVERAGE) @@ -105,6 +105,7 @@ if(COVERAGE) # Find llvm-profdata set(PROFDATA_EXECUTABLE_NAME "llvm-profdata-${CLANG_VERSION_MAJOR}") find_program(PROFDATA_EXECUTABLE ${PROFDATA_EXECUTABLE_NAME}) + if(PROFDATA_EXECUTABLE MATCHES "NOTFOUND") message(FATAL_ERROR "Couldn't find ${PROFDATA_EXECUTABLE_NAME}") endif() @@ -112,6 +113,7 @@ if(COVERAGE) # Find llvm-cov set(COV_EXECUTABLE_NAME "llvm-cov-${CLANG_VERSION_MAJOR}") find_program(COV_EXECUTABLE ${COV_EXECUTABLE_NAME}) + if(COV_EXECUTABLE MATCHES "NOTFOUND") message(FATAL_ERROR "Couldn't find ${COV_EXECUTABLE_NAME}") endif() @@ -121,12 +123,11 @@ if(COVERAGE) # Add a custom target for creating the report add_custom_target(create_full_coverage_report - COMMAND "${CMAKE_SOURCE_DIR}/scripts/collect_coverage_information.sh" ${PROFDATA_EXECUTABLE} ${COV_EXECUTABLE} - VERBATIM + COMMAND "${CMAKE_SOURCE_DIR}/scripts/collect_coverage_information.sh" ${PROFDATA_EXECUTABLE} ${COV_EXECUTABLE} + VERBATIM ) endif() - include(cmake/build.cmake) include(GNUInstallDirs) include(cmake/arch.cmake) diff --git a/cpp/src/barretenberg/common/serialize.hpp b/cpp/src/barretenberg/common/serialize.hpp index 6452dcb7a4..16d2c66e80 100644 --- a/cpp/src/barretenberg/common/serialize.hpp +++ b/cpp/src/barretenberg/common/serialize.hpp @@ -51,7 +51,8 @@ __extension__ using uint128_t = unsigned __int128; // NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast, cert-dcl58-cpp) // clang-format on -template concept IntegralOrEnum = std::integral || std::is_enum_v; +template +concept IntegralOrEnum = std::integral || std::is_enum_v; namespace serialize { // Forward declare derived msgpack methods diff --git a/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp b/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp index 3162b86048..1484a5b5fe 100644 --- a/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp +++ b/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp @@ -41,7 +41,6 @@ class UltraRecursive { public: using CircuitBuilder = UltraCircuitBuilder; using Curve = plonk::stdlib::bn254; - using PCS = pcs::kzg::KZG; using GroupElement = Curve::Element; using Commitment = Curve::Element; using CommitmentHandle = Curve::Element; diff --git a/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 1fdbeb0635..f46d55ae71 100644 --- a/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp @@ -111,7 +111,7 @@ template class GeminiProver_ { const Fr& r_challenge); }; // namespace proof_system::honk::pcs::gemini -template class GeminiVerifier_ { +template class GeminiVerifier_ { using Fr = typename Curve::ScalarField; using GroupElement = typename Curve::Element; using Commitment = typename Curve::AffineElement; @@ -232,21 +232,31 @@ template class GeminiVerifier_ { Fr r) { // C₀ᵣ₊ = [F] + r⁻¹⋅[G] - GroupElement C0_r_pos = batched_f; + GroupElement C0_r_pos; // C₀ᵣ₋ = [F] - r⁻¹⋅[G] - GroupElement C0_r_neg = batched_f; - Fr r_inv = r.invert(); + GroupElement C0_r_neg; + Fr r_inv = r.invert(); // r⁻¹ - // TODO(luke): reinstate some kind of !batched_g.is_point_at_infinity() check for stdlib types? This is mostly - // relevant for Gemini unit tests since in practice batched_g != zero (i.e. we will always have shifted polys). - bool batched_g_is_point_at_infinity = false; - if constexpr (!Curve::is_stdlib_type) { // Note: required for Gemini tests with no shifts - batched_g_is_point_at_infinity = batched_g.is_point_at_infinity(); - } - if (!batched_g_is_point_at_infinity) { - batched_g = batched_g * r_inv; - C0_r_pos += batched_g; - C0_r_neg -= batched_g; + // If in a recursive setting, perform a batch mul. Otherwise, accumulate directly. + // TODO(#673): The following if-else represents the stldib/native code paths. Once the "native" verifier is + // achieved through a builder Simulator, the stdlib codepath should become the only codepath. + if constexpr (Curve::is_stdlib_type) { + std::vector commitments = { batched_f, batched_g }; + auto builder = r.get_context(); + auto one = Fr(builder, 1); + // TODO(#707): these batch muls include the use of 1 as a scalar. This is handled appropriately as a non-mul + // (add-accumulate) in the goblin batch_mul but is done inefficiently as a scalar mul in the conventional + // emulated batch mul. + C0_r_pos = GroupElement::template batch_mul(commitments, { one, r_inv }); + C0_r_neg = GroupElement::template batch_mul(commitments, { one, -r_inv }); + } else { + C0_r_pos = batched_f; + C0_r_neg = batched_f; + if (!batched_g.is_point_at_infinity()) { + batched_g = batched_g * r_inv; + C0_r_pos += batched_g; + C0_r_neg -= batched_g; + } } return { C0_r_pos, C0_r_neg }; diff --git a/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp b/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp index ae0ee53a51..ab2c8d8bb7 100644 --- a/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp +++ b/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp @@ -11,7 +11,7 @@ namespace proof_system::honk::pcs::kzg { -template class KZG { +template class KZG { using CK = CommitmentKey; using VK = VerifierCommitmentKey; using Fr = typename Curve::ScalarField; @@ -72,31 +72,34 @@ template class KZG { * * @param claim OpeningClaim ({r, v}, C) * @return {P₀, P₁} where - * - P₀ = C − v⋅[1]₁ + r⋅[x]₁ - * - P₁ = [Q(x)]₁ + * - P₀ = C − v⋅[1]₁ + r⋅[W(x)]₁ + * - P₁ = [W(x)]₁ */ static std::array compute_pairing_points(const OpeningClaim& claim, auto& verifier_transcript) { auto quotient_commitment = verifier_transcript.template receive_from_prover("KZG:W"); - auto lhs = claim.commitment + (quotient_commitment * claim.opening_pair.challenge); - // Add the evaluation point contribution v⋅[1]₁. + GroupElement P_0; // Note: In the recursive setting, we only add the contribution if it is not the point at infinity (i.e. if the // evaluation is not equal to zero). - // TODO(luke): What is the proper way to handle this? Contraints to show scalar (evaluation) is zero? if constexpr (Curve::is_stdlib_type) { - if (!claim.opening_pair.evaluation.get_value().is_zero()) { - auto ctx = verifier_transcript.builder; - lhs -= GroupElement::one(ctx) * claim.opening_pair.evaluation; - } + auto builder = verifier_transcript.builder; + auto one = Fr(builder, 1); + std::vector commitments = { claim.commitment, quotient_commitment }; + std::vector scalars = { one, claim.opening_pair.challenge }; + P_0 = GroupElement::template batch_mul(commitments, scalars); + // Note: This implementation assumes the evaluation is zero (as is the case for shplonk). + ASSERT(claim.opening_pair.evaluation.get_value() == 0); } else { - lhs -= GroupElement::one() * claim.opening_pair.evaluation; + P_0 = claim.commitment; + P_0 += quotient_commitment * claim.opening_pair.challenge; + P_0 -= GroupElement::one() * claim.opening_pair.evaluation; } - auto rhs = -quotient_commitment; + auto P_1 = -quotient_commitment; - return { lhs, rhs }; + return { P_0, P_1 }; }; }; } // namespace proof_system::honk::pcs::kzg diff --git a/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index f4b11815c4..17c452471d 100644 --- a/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp @@ -145,7 +145,7 @@ template class ShplonkProver_ { * @brief Shplonk Verifier * */ -template class ShplonkVerifier_ { +template class ShplonkVerifier_ { using Fr = typename Curve::ScalarField; using GroupElement = typename Curve::Element; using Commitment = typename Curve::AffineElement; @@ -174,84 +174,101 @@ template class ShplonkVerifier_ { const Fr z_challenge = transcript.get_challenge("Shplonk:z"); + // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] + // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] + GroupElement G_commitment; + // compute simulated commitment to [G] as a linear combination of // [Q], { [fⱼ] }, [1]: // [G] = [Q] - ∑ⱼ (1/zⱼ(r))[Bⱼ] + ( ∑ⱼ (1/zⱼ(r)) Tⱼ(r) )[1] // = [Q] - ∑ⱼ (1/zⱼ(r))[Bⱼ] + G₀ [1] - // G₀ = ∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ) auto G_commitment_constant = Fr(0); - // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] - // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] - GroupElement G_commitment = Q_commitment; - - // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ) - std::vector vanishing_evals; - vanishing_evals.reserve(num_claims); - for (const auto& claim : claims) { - vanishing_evals.emplace_back(z_challenge - claim.opening_pair.challenge); - } - // If recursion, invert elements individually, otherwise batch invert. (Inversion is cheap in circuits since we - // need only prove the correctness of a known inverse, we do not emulate its computation. Hence no need for - // batch inversion). - std::vector inverse_vanishing_evals; + // TODO(#673): The recursive and non-recursive (native) logic is completely separated via the following + // conditional. Much of the logic could be shared, but I've chosen to do it this way since soon the "else" + // branch should be removed in its entirety, and "native" verification will utilize the recursive code paths + // using a builder Simulator. if constexpr (Curve::is_stdlib_type) { - for (const auto& val : vanishing_evals) { - inverse_vanishing_evals.emplace_back(val.invert()); + auto builder = nu.get_context(); + + // Containers for the inputs to the final batch mul + std::vector commitments; + std::vector scalars; + + // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] + // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] + commitments.emplace_back(Q_commitment); + scalars.emplace_back(Fr(builder, 1)); // Fr(1) + + // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ) + std::vector inverse_vanishing_evals; + inverse_vanishing_evals.reserve(num_claims); + for (const auto& claim : claims) { + // Note: no need for batch inversion; emulated inverison is cheap. (just show known inverse is valid) + inverse_vanishing_evals.emplace_back((z_challenge - claim.opening_pair.challenge).invert()); } - } else { - Fr::batch_invert(vanishing_evals); - inverse_vanishing_evals = vanishing_evals; - } - auto current_nu = Fr(1); - // Note: commitments and scalars vectors used only in recursion setting for batch mul - std::vector commitments; - std::vector scalars; - for (size_t j = 0; j < num_claims; ++j) { - // (Cⱼ, xⱼ, vⱼ) - const auto& [opening_pair, commitment] = claims[j]; + auto current_nu = Fr(1); + // Note: commitments and scalars vectors used only in recursion setting for batch mul + for (size_t j = 0; j < num_claims; ++j) { + // (Cⱼ, xⱼ, vⱼ) + const auto& [opening_pair, commitment] = claims[j]; - Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ ) + Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ ) + + // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ + G_commitment_constant += scaling_factor * opening_pair.evaluation; - // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ - G_commitment_constant += scaling_factor * opening_pair.evaluation; + current_nu *= nu; - // If recursion, store MSM inputs for batch mul, otherwise accumulate directly - if constexpr (Curve::is_stdlib_type) { + // Store MSM inputs for batch mul commitments.emplace_back(commitment); - scalars.emplace_back(scaling_factor); - } else { - // [G] -= ρʲ / ( r − xⱼ )⋅[fⱼ] - G_commitment -= commitment * scaling_factor; + scalars.emplace_back(-scaling_factor); } - current_nu *= nu; - } + commitments.emplace_back(GroupElement::one(builder)); + scalars.emplace_back(G_commitment_constant); - // If recursion, do batch mul to compute [G] -= ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] - if constexpr (Curve::is_stdlib_type) { - G_commitment -= GroupElement::batch_mul(commitments, scalars); - } + // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1] + G_commitment = GroupElement::template batch_mul(commitments, scalars); - // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1] - Fr evaluation_zero; // 0 \in Fr - GroupElement group_one; // [1] - if constexpr (Curve::is_stdlib_type) { - auto ctx = transcript.builder; - evaluation_zero = Fr::from_witness(ctx, 0); - group_one = GroupElement::one(ctx); } else { - // GroupElement sort_of_one{ x, y }; - evaluation_zero = Fr(0); - group_one = vk->srs->get_first_g1(); - } + // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] + // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] + G_commitment = Q_commitment; + + // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ) + std::vector inverse_vanishing_evals; + inverse_vanishing_evals.reserve(num_claims); + for (const auto& claim : claims) { + inverse_vanishing_evals.emplace_back(z_challenge - claim.opening_pair.challenge); + } + Fr::batch_invert(inverse_vanishing_evals); + + auto current_nu = Fr(1); + // Note: commitments and scalars vectors used only in recursion setting for batch mul + for (size_t j = 0; j < num_claims; ++j) { + // (Cⱼ, xⱼ, vⱼ) + const auto& [opening_pair, commitment] = claims[j]; - G_commitment += group_one * G_commitment_constant; + Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ ) + + // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ + G_commitment_constant += scaling_factor * opening_pair.evaluation; + + // [G] -= ρʲ / ( r − xⱼ )⋅[fⱼ] + G_commitment -= commitment * scaling_factor; + + current_nu *= nu; + } + + // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1] + G_commitment += vk->srs->get_first_g1() * G_commitment_constant; + } // Return opening pair (z, 0) and commitment [G] - return { { z_challenge, evaluation_zero }, G_commitment }; + return { { z_challenge, Fr(0) }, G_commitment }; }; }; } // namespace proof_system::honk::pcs::shplonk diff --git a/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp b/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp index dbdd25061d..50686a230e 100644 --- a/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp +++ b/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp @@ -67,8 +67,8 @@ struct ecc_op_tuple { uint32_t x_hi; uint32_t y_lo; uint32_t y_hi; - uint32_t z_lo; - uint32_t z_hi; + uint32_t z_1; + uint32_t z_2; }; template inline void read(B& buf, poly_triple_& constraint) diff --git a/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp b/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp index 6e229f7ba6..860e1b056b 100644 --- a/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp +++ b/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp @@ -18,6 +18,8 @@ namespace proof_system { */ TEST(UltraCircuitBuilder, GoblinSimple) { + const size_t CHUNK_SIZE = plonk::NUM_LIMB_BITS_IN_FIELD_SIMULATION * 2; + auto builder = UltraCircuitBuilder(); // Compute a simple point accumulation natively @@ -31,10 +33,17 @@ TEST(UltraCircuitBuilder, GoblinSimple) builder.queue_ecc_mul_accum(P2, z); // Add equality op gates based on the internal accumulator - auto P_result = builder.queue_ecc_eq(); - - // Check that value returned from internal accumulator is correct - EXPECT_EQ(P_result, P_expected); + auto eq_op_tuple = builder.queue_ecc_eq(); + + // Check that we can reconstruct the coordinates of P_expected from the data in variables + auto P_result_x_lo = uint256_t(builder.variables[eq_op_tuple.x_lo]); + auto P_result_x_hi = uint256_t(builder.variables[eq_op_tuple.x_hi]); + auto P_result_x = P_result_x_lo + (P_result_x_hi << CHUNK_SIZE); + auto P_result_y_lo = uint256_t(builder.variables[eq_op_tuple.y_lo]); + auto P_result_y_hi = uint256_t(builder.variables[eq_op_tuple.y_hi]); + auto P_result_y = P_result_y_lo + (P_result_y_hi << CHUNK_SIZE); + EXPECT_EQ(P_result_x, uint256_t(P_expected.x)); + EXPECT_EQ(P_result_y, uint256_t(P_expected.y)); // Check that the accumulator in the op queue has been reset to 0 auto accumulator = builder.op_queue.get_accumulator(); @@ -49,64 +58,23 @@ TEST(UltraCircuitBuilder, GoblinSimple) EXPECT_EQ(builder.ecc_op_wire_1[4], EccOpCode::EQUALITY); // Check that we can reconstruct the coordinates of P1 from the op_wires - auto chunk_size = plonk::NUM_LIMB_BITS_IN_FIELD_SIMULATION * 2; auto P1_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[0]]); auto P1_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[0]]); - auto P1_x = P1_x_lo + (P1_x_hi << chunk_size); + auto P1_x = P1_x_lo + (P1_x_hi << CHUNK_SIZE); EXPECT_EQ(P1_x, uint256_t(P1.x)); auto P1_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[0]]); auto P1_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[1]]); - auto P1_y = P1_y_lo + (P1_y_hi << chunk_size); + auto P1_y = P1_y_lo + (P1_y_hi << CHUNK_SIZE); EXPECT_EQ(P1_y, uint256_t(P1.y)); // Check that we can reconstruct the coordinates of P2 from the op_wires auto P2_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[2]]); auto P2_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[2]]); - auto P2_x = P2_x_lo + (P2_x_hi << chunk_size); + auto P2_x = P2_x_lo + (P2_x_hi << CHUNK_SIZE); EXPECT_EQ(P2_x, uint256_t(P2.x)); auto P2_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[2]]); auto P2_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[3]]); - auto P2_y = P2_y_lo + (P2_y_hi << chunk_size); + auto P2_y = P2_y_lo + (P2_y_hi << CHUNK_SIZE); EXPECT_EQ(P2_y, uint256_t(P2.y)); - - // Check that we can reconstruct the coordinates of P_result from the op_wires - auto P_expected_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[4]]); - auto P_expected_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[4]]); - auto P_expected_x = P_expected_x_lo + (P_expected_x_hi << chunk_size); - EXPECT_EQ(P_expected_x, uint256_t(P_expected.x)); - auto P_expected_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[4]]); - auto P_expected_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[5]]); - auto P_expected_y = P_expected_y_lo + (P_expected_y_hi << chunk_size); - EXPECT_EQ(P_expected_y, uint256_t(P_expected.y)); } - -/** - * @brief Test correctness of native ecc batch mul performed behind the scenes when adding ecc op gates for a batch mul - * - */ -TEST(UltraCircuitBuilder, GoblinBatchMul) -{ - using Point = g1::affine_element; - using Scalar = fr; - - auto builder = UltraCircuitBuilder(); - const size_t num_muls = 3; - - // Compute some random points and scalars to batch multiply - std::vector points; - std::vector scalars; - auto batched_expected = Point::infinity(); - for (size_t i = 0; i < num_muls; ++i) { - points.emplace_back(Point::random_element()); - scalars.emplace_back(Scalar::random_element()); - batched_expected = batched_expected + points[i] * scalars[i]; - } - - // Populate the batch mul operands in the op wires and natively compute the result - auto batched_result = builder.batch_mul(points, scalars); - - // Extract current accumulator point from the op queue and check the result - EXPECT_EQ(batched_result, batched_expected); -} - } // namespace proof_system \ No newline at end of file diff --git a/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index d96dfce13c..3446b5cef8 100644 --- a/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -499,87 +499,61 @@ template uint32_t UltraCircuitBuilder_::put_constant_variable( * ** Goblin Methods ** */ -/** - * @brief Add gates corresponding to a batched mul - * - * @param points - * @param scalars - * @return g1::affine_element Result of batched mul - */ -template -g1::affine_element UltraCircuitBuilder_::batch_mul(const std::vector& points, - const std::vector& scalars) -{ - // TODO(luke): Do we necessarily want to check accum == 0? Other checks? - ASSERT(op_queue.get_accumulator().is_point_at_infinity()); - - size_t num_muls = points.size(); - for (size_t idx = 0; idx < num_muls; ++idx) { - queue_ecc_mul_accum(points[idx], scalars[idx]); - } - return op_queue.get_accumulator(); -} - /** * @brief Add gates for simple point addition without scalar and compute corresponding op natively * * @param point */ -template void UltraCircuitBuilder_::queue_ecc_add_accum(const barretenberg::g1::affine_element& point) +template +ecc_op_tuple UltraCircuitBuilder_::queue_ecc_add_accum(const barretenberg::g1::affine_element& point) { // Add raw op to queue op_queue.add_accumulate(point); // Add ecc op gates - add_ecc_op_gates(EccOpCode::ADD_ACCUM, point); + auto op_tuple = make_ecc_op_tuple(EccOpCode::ADD_ACCUM, point); + populate_ecc_op_wires(op_tuple); + + return op_tuple; } /** * @brief Add gates for point mul and add and compute corresponding op natively * + * @tparam FF * @param point * @param scalar + * @return ecc_op_tuple encoding the point and scalar inputs to the mul accum */ template -void UltraCircuitBuilder_::queue_ecc_mul_accum(const barretenberg::g1::affine_element& point, - const barretenberg::fr& scalar) +ecc_op_tuple UltraCircuitBuilder_::queue_ecc_mul_accum(const barretenberg::g1::affine_element& point, + const barretenberg::fr& scalar) { // Add raw op to op queue op_queue.mul_accumulate(point, scalar); // Add ecc op gates - add_ecc_op_gates(EccOpCode::MUL_ACCUM, point, scalar); + auto op_tuple = make_ecc_op_tuple(EccOpCode::MUL_ACCUM, point, scalar); + populate_ecc_op_wires(op_tuple); + + return op_tuple; } /** * @brief Add point equality gates * - * @return point to which equality has been asserted + * @return ecc_op_tuple encoding the point to which equality has been asserted */ -template barretenberg::g1::affine_element UltraCircuitBuilder_::queue_ecc_eq() +template ecc_op_tuple UltraCircuitBuilder_::queue_ecc_eq() { // Add raw op to op queue auto point = op_queue.eq(); // Add ecc op gates - add_ecc_op_gates(EccOpCode::EQUALITY, point); - - return point; -} - -/** - * @brief Add ecc op gates given an op code and its operands - * - * @param op Op code - * @param point - * @param scalar - */ -template -void UltraCircuitBuilder_::add_ecc_op_gates(uint32_t op, const g1::affine_element& point, const fr& scalar) -{ - auto op_tuple = make_ecc_op_tuple(op, point, scalar); + auto op_tuple = make_ecc_op_tuple(EccOpCode::EQUALITY, point); + populate_ecc_op_wires(op_tuple); - record_ecc_op(op_tuple); + return op_tuple; } /** @@ -624,7 +598,7 @@ ecc_op_tuple UltraCircuitBuilder_::make_ecc_op_tuple(uint32_t op, const g1:: * num_ecc_op_gates. E.g. in the composer we can reconstruct q_ecc_op as the indicator on the first num_ecc_op_gates * indices. All other selectors are simply 0 on this domain. */ -template void UltraCircuitBuilder_::record_ecc_op(const ecc_op_tuple& in) +template void UltraCircuitBuilder_::populate_ecc_op_wires(const ecc_op_tuple& in) { ecc_op_wire_1.emplace_back(in.op); ecc_op_wire_2.emplace_back(in.x_lo); @@ -633,8 +607,8 @@ template void UltraCircuitBuilder_::record_ecc_op(const ecc_op ecc_op_wire_1.emplace_back(in.op); // TODO(luke): second op val is sort of a dummy. use "op" again? ecc_op_wire_2.emplace_back(in.y_hi); - ecc_op_wire_3.emplace_back(in.z_lo); - ecc_op_wire_4.emplace_back(in.z_hi); + ecc_op_wire_3.emplace_back(in.z_1); + ecc_op_wire_4.emplace_back(in.z_2); num_ecc_op_gates += 2; }; diff --git a/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index 2ff9f0eb70..f34a3a6dbb 100644 --- a/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -705,15 +705,14 @@ template class UltraCircuitBuilder_ : public CircuitBuilderBase& points, const std::vector& scalars); + ecc_op_tuple queue_ecc_add_accum(const g1::affine_element& point); + ecc_op_tuple queue_ecc_mul_accum(const g1::affine_element& point, const fr& scalar); + ecc_op_tuple queue_ecc_eq(); private: - void record_ecc_op(const ecc_op_tuple& in); - void add_ecc_op_gates(uint32_t op, const g1::affine_element& point, const fr& scalar = fr::zero()); + void populate_ecc_op_wires(const ecc_op_tuple& in); ecc_op_tuple make_ecc_op_tuple(uint32_t op, const g1::affine_element& point, const fr& scalar = fr::zero()); public: @@ -834,7 +833,7 @@ template class UltraCircuitBuilder_ : public CircuitBuilderBase class UltraCircuitBuilder_ : public CircuitBuilderBase + */ +template +element element::goblin_batch_mul(const std::vector& points, + const std::vector& scalars, + [[maybe_unused]] const size_t max_num_bits) +{ + auto builder = points[0].get_context(); + + // Check that the internal accumulator is zero? + ASSERT(builder->op_queue.get_accumulator().is_point_at_infinity()); + + // Loop over all points and scalars + size_t num_points = points.size(); + for (size_t i = 0; i < num_points; ++i) { + auto& point = points[i]; + auto& scalar = scalars[i]; + + // Populate the goblin-style ecc op gates for the given mul inputs + ecc_op_tuple op_tuple; + bool scalar_is_constant_equal_one = scalar.get_witness_index() == IS_CONSTANT && scalar.get_value() == 1; + if (scalar_is_constant_equal_one) { // if scalar is 1, there is no need to perform a mul + op_tuple = builder->queue_ecc_add_accum(point.get_value()); + } else { // otherwise, perform a mul-then-accumulate + op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + } + + // Add constraints demonstrating that the EC point coordinates were decomposed faithfully. In particular, show + // that the lo-hi components that have been encoded in the op wires can be reconstructed via the limbs of the + // original point coordinates. + auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); + auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); + auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); + auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); + // Note: These constraints do not assume or enforce that the coordinates of the original point have been + // asserted to be in the field, only that they are less than the smallest power of 2 greater than the field + // modulus (a la the bigfield(lo, hi) constructor with can_overflow == false). + ASSERT(uint1024_t(point.x.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); + ASSERT(uint1024_t(point.y.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); + auto shift = Fr::from_witness(builder, Fq::shift_1); + x_lo.assert_equal(point.x.binary_basis_limbs[0].element + shift * point.x.binary_basis_limbs[1].element); + x_hi.assert_equal(point.x.binary_basis_limbs[2].element + shift * point.x.binary_basis_limbs[3].element); + y_lo.assert_equal(point.y.binary_basis_limbs[0].element + shift * point.y.binary_basis_limbs[1].element); + y_hi.assert_equal(point.y.binary_basis_limbs[2].element + shift * point.y.binary_basis_limbs[3].element); + + // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars + if (!scalar_is_constant_equal_one) { + auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); + auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); + auto beta = G::subgroup_field::cube_root_of_unity(); + scalar.assert_equal(z_1 - z_2 * beta); + } + } + + // Populate equality gates based on the internal accumulator point + auto op_tuple = builder->queue_ecc_eq(); + + // Reconstruct the result of the batch mul using indices into the variables array + auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); + auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); + auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); + auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); + Fq point_x(x_lo, x_hi); + Fq point_y(y_lo, y_hi); + + return element(point_x, point_y); +} + +} // namespace stdlib +} // namespace proof_system::plonk diff --git a/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp b/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp new file mode 100644 index 0000000000..7f6ff8a5d7 --- /dev/null +++ b/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp @@ -0,0 +1,89 @@ +#include "barretenberg/common/test.hpp" +#include + +#include "../biggroup/biggroup.hpp" +#include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" + +#include "barretenberg/stdlib/primitives/curves/bn254.hpp" + +#include "barretenberg/numeric/random/engine.hpp" +#include + +namespace test_stdlib_biggroup_goblin { +namespace { +auto& engine = numeric::random::get_debug_engine(); +} + +using namespace proof_system::plonk; + +template class stdlib_biggroup_goblin : public testing::Test { + using element_ct = typename Curve::Element; + using scalar_ct = typename Curve::ScalarField; + + using fq = typename Curve::BaseFieldNative; + using fr = typename Curve::ScalarFieldNative; + using g1 = typename Curve::GroupNative; + using affine_element = typename g1::affine_element; + using element = typename g1::element; + + using Builder = typename Curve::Builder; + + static constexpr auto EXPECT_CIRCUIT_CORRECTNESS = [](Builder& builder, bool expected_result = true) { + info("builder gates = ", builder.get_num_gates()); + EXPECT_EQ(builder.check_circuit(), expected_result); + }; + + public: + /** + * @brief Test goblin-style batch mul + * @details Check that 1) Goblin-style batch mul returns correct value, and 2) resulting circuit is correct + * + */ + static void test_goblin_style_batch_mul() + { + const bool goblin_flag = true; // used to indicate goblin-style in batch_mul + const size_t num_points = 5; + Builder builder; + + std::vector points; + std::vector scalars; + for (size_t i = 0; i < num_points; ++i) { + points.push_back(affine_element(element::random_element())); + scalars.push_back(fr::random_element()); + } + + std::vector circuit_points; + std::vector circuit_scalars; + for (size_t i = 0; i < num_points; ++i) { + circuit_points.push_back(element_ct::from_witness(&builder, points[i])); + circuit_scalars.push_back(scalar_ct::from_witness(&builder, scalars[i])); + } + + element_ct result_point = element_ct::template batch_mul(circuit_points, circuit_scalars); + + element expected_point = g1::one; + expected_point.self_set_infinity(); + for (size_t i = 0; i < num_points; ++i) { + expected_point += (element(points[i]) * scalars[i]); + } + + expected_point = expected_point.normalize(); + fq result_x(result_point.x.get_value().lo); + fq result_y(result_point.y.get_value().lo); + + EXPECT_EQ(result_x, expected_point.x); + EXPECT_EQ(result_y, expected_point.y); + + EXPECT_CIRCUIT_CORRECTNESS(builder); + } +}; + +using TestTypes = testing::Types>; + +TYPED_TEST_SUITE(stdlib_biggroup_goblin, TestTypes); + +HEAVY_TYPED_TEST(stdlib_biggroup_goblin, batch_mul) +{ + TestFixture::test_goblin_style_batch_mul(); +} +} // namespace test_stdlib_biggroup_goblin diff --git a/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index c6749aea6b..efc05ef405 100644 --- a/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -597,11 +597,14 @@ std::pair, element> element::c * scalars See `bn254_endo_batch_mul` for description of algorithm **/ template +template element element::batch_mul(const std::vector& points, const std::vector& scalars, const size_t max_num_bits) { - + if constexpr (use_goblin) { + return goblin_batch_mul(points, scalars); + } const size_t num_points = points.size(); ASSERT(scalars.size() == num_points); batch_lookup_table point_table(points); diff --git a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp index c7dfbcec1b..bac2b05894 100644 --- a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp +++ b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp @@ -7,50 +7,34 @@ #include "barretenberg/numeric/bitop/get_msb.hpp" namespace proof_system::plonk::stdlib::recursion::honk { -template -UltraRecursiveVerifier_::UltraRecursiveVerifier_(Builder* builder, - std::shared_ptr verifier_key) + +template +UltraRecursiveVerifier_::UltraRecursiveVerifier_(Builder* builder, + std::shared_ptr verifier_key) : key(verifier_key) , builder(builder) {} -template -UltraRecursiveVerifier_::UltraRecursiveVerifier_(UltraRecursiveVerifier_&& other) noexcept - : key(std::move(other.key)) - , pcs_verification_key(std::move(other.pcs_verification_key)) -{} - -template -UltraRecursiveVerifier_& UltraRecursiveVerifier_::operator=(UltraRecursiveVerifier_&& other) noexcept -{ - key = other.key; - pcs_verification_key = (std::move(other.pcs_verification_key)); - commitments.clear(); - pcs_fr_elements.clear(); - return *this; -} - /** * @brief This function constructs a recursive verifier circuit for an Ultra Honk proof of a given flavor. * */ -template -std::array UltraRecursiveVerifier_::verify_proof(const plonk::proof& proof) +template +std::array UltraRecursiveVerifier_::verify_proof( + const plonk::proof& proof) { - using FF = typename Flavor::FF; - using GroupElement = typename Flavor::GroupElement; - using Commitment = typename Flavor::Commitment; using Sumcheck = ::proof_system::honk::sumcheck::SumcheckVerifier; using Curve = typename Flavor::Curve; - using Gemini = ::proof_system::honk::pcs::gemini::GeminiVerifier_; - using Shplonk = ::proof_system::honk::pcs::shplonk::ShplonkVerifier_; - using PCS = typename Flavor::PCS; // note: This can only be KZG + using Gemini = ::proof_system::honk::pcs::gemini::GeminiVerifier_; + using Shplonk = ::proof_system::honk::pcs::shplonk::ShplonkVerifier_; + using KZG = ::proof_system::honk::pcs::kzg::KZG; // note: This can only be KZG using VerifierCommitments = typename Flavor::VerifierCommitments; using CommitmentLabels = typename Flavor::CommitmentLabels; using RelationParams = ::proof_system::honk::sumcheck::RelationParameters; RelationParams relation_parameters; + info("Initial: num gates = ", builder->get_num_gates()); size_t prev_num_gates = builder->get_num_gates(); transcript = Transcript{ builder, proof.proof_data }; @@ -67,7 +51,7 @@ std::array UltraRecursiveVerifier_::ve ASSERT(static_cast(public_input_size.get_value()) == key->num_public_inputs); std::vector public_inputs; - for (size_t i = 0; i < static_cast(public_input_size.get_value()); ++i) { + for (size_t i = 0; i < key->num_public_inputs; ++i) { auto public_input_i = transcript.template receive_from_prover("public_input_" + std::to_string(i)); public_inputs.emplace_back(public_input_i); } @@ -115,11 +99,15 @@ std::array UltraRecursiveVerifier_::ve commitments.z_lookup = transcript.template receive_from_prover(commitment_labels.z_lookup); // Execute Sumcheck Verifier - auto sumcheck = Sumcheck(static_cast(circuit_size.get_value())); + auto sumcheck = Sumcheck(key->circuit_size); std::optional sumcheck_output = sumcheck.verify(relation_parameters, transcript); - info("Sumcheck: num gates = ", builder->get_num_gates() - prev_num_gates); + info("Sumcheck: num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); // If Sumcheck does not return an output, sumcheck verification has failed @@ -140,7 +128,11 @@ std::array UltraRecursiveVerifier_::ve ++evaluation_idx; } - info("Batched eval: num gates = ", builder->get_num_gates() - prev_num_gates); + info("Batched eval: num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); // Compute batched commitments needed for input to Gemini. @@ -163,15 +155,24 @@ std::array UltraRecursiveVerifier_::ve scalars_unshifted[0] = FF::from_witness(builder, 1); // Batch the commitments to the unshifted and to-be-shifted polynomials using powers of rho - auto batched_commitment_unshifted = GroupElement::batch_mul(commitments.get_unshifted(), scalars_unshifted); - - info("Batch mul (unshifted): num gates = ", builder->get_num_gates() - prev_num_gates); + auto batched_commitment_unshifted = + GroupElement::template batch_mul(commitments.get_unshifted(), scalars_unshifted); + + info("Batch mul (unshifted): num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); auto batched_commitment_to_be_shifted = - GroupElement::batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted); + GroupElement::template batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted); - info("Batch mul (to-be-shited): num gates = ", builder->get_num_gates() - prev_num_gates); + info("Batch mul (to-be-shited): num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); // Produce a Gemini claim consisting of: @@ -183,23 +184,33 @@ std::array UltraRecursiveVerifier_::ve batched_commitment_to_be_shifted, transcript); - info("Gemini: num gates = ", builder->get_num_gates() - prev_num_gates); + info("Gemini: num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); // Produce a Shplonk claim: commitment [Q] - [Q_z], evaluation zero (at random challenge z) auto shplonk_claim = Shplonk::reduce_verification(pcs_verification_key, gemini_claim, transcript); - info("Shplonk: num gates = ", builder->get_num_gates() - prev_num_gates); + info("Shplonk: num gates = ", + builder->get_num_gates() - prev_num_gates, + ", (total = ", + builder->get_num_gates(), + ")"); prev_num_gates = builder->get_num_gates(); - info("Total: num gates = ", builder->get_num_gates()); - // Constuct the inputs to the final KZG pairing check - auto pairing_points = PCS::compute_pairing_points(shplonk_claim, transcript); + auto pairing_points = KZG::compute_pairing_points(shplonk_claim, transcript); + + info("KZG: num gates = ", builder->get_num_gates() - prev_num_gates, ", (total = ", builder->get_num_gates(), ")"); return pairing_points; } -template class UltraRecursiveVerifier_; +using UltraRecursiveFlavor = proof_system::honk::flavor::UltraRecursive; +template class UltraRecursiveVerifier_; +template class UltraRecursiveVerifier_; } // namespace proof_system::plonk::stdlib::recursion::honk diff --git a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp index a1eee1d4d4..dff4ac7ade 100644 --- a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp +++ b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp @@ -8,20 +8,22 @@ #include "barretenberg/stdlib/recursion/honk/transcript/transcript.hpp" namespace proof_system::plonk::stdlib::recursion::honk { -template class UltraRecursiveVerifier_ { +template class UltraRecursiveVerifier_ { using FF = typename Flavor::FF; using Commitment = typename Flavor::Commitment; + using GroupElement = typename Flavor::GroupElement; using VerificationKey = typename Flavor::VerificationKey; using VerifierCommitmentKey = typename Flavor::VerifierCommitmentKey; using Builder = typename Flavor::CircuitBuilder; - using PairingPoints = std::array; + using PairingPoints = std::array; public: explicit UltraRecursiveVerifier_(Builder* builder, std::shared_ptr verifier_key = nullptr); - UltraRecursiveVerifier_(UltraRecursiveVerifier_&& other) noexcept; + UltraRecursiveVerifier_(UltraRecursiveVerifier_&& other) = delete; UltraRecursiveVerifier_(const UltraRecursiveVerifier_& other) = delete; UltraRecursiveVerifier_& operator=(const UltraRecursiveVerifier_& other) = delete; - UltraRecursiveVerifier_& operator=(UltraRecursiveVerifier_&& other) noexcept; + UltraRecursiveVerifier_& operator=(UltraRecursiveVerifier_&& other) = delete; + ~UltraRecursiveVerifier_() = default; // TODO(luke): Eventually this will return something like aggregation_state but I'm simplifying for now until we // determine the exact interface. Simply returns the two pairing points. @@ -35,8 +37,10 @@ template class UltraRecursiveVerifier_ { Transcript transcript; }; -extern template class UltraRecursiveVerifier_; +extern template class UltraRecursiveVerifier_; +extern template class UltraRecursiveVerifier_; -using UltraRecursiveVerifier = UltraRecursiveVerifier_; +using UltraRecursiveVerifier = + UltraRecursiveVerifier_; } // namespace proof_system::plonk::stdlib::recursion::honk diff --git a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index 50c3cd77e4..7bfc45452d 100644 --- a/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -15,15 +15,25 @@ namespace proof_system::plonk::stdlib::recursion::honk { -template class RecursiveVerifierTest : public testing::Test { +/** + * @brief Test suite for Ultra Honk Recursive Verifier + * @details Construct and check a recursive verifier circuit for an UltraHonk proof using 1) the conventional Ultra + * arithmetization, or 2) a Goblin-style Ultra arithmetization + * + * @tparam UseGoblinFlag whether or not to use goblin-style arithmetization for group operations + */ +template class RecursiveVerifierTest : public testing::Test { + + static constexpr bool goblin_flag = UseGoblinFlag::value; using InnerComposer = ::proof_system::honk::UltraComposer; using InnerBuilder = typename InnerComposer::CircuitBuilder; - using OuterBuilder = typename OuterComposer::CircuitBuilder; + using OuterBuilder = ::proof_system::UltraCircuitBuilder; using NativeVerifier = ::proof_system::honk::UltraVerifier_<::proof_system::honk::flavor::Ultra>; - using RecursiveVerifier = UltraRecursiveVerifier_<::proof_system::honk::flavor::UltraRecursive>; + // Arithmetization of group operations in recursive verifier circuit (goblin or not) is determined by goblin_flag + using RecursiveVerifier = UltraRecursiveVerifier_<::proof_system::honk::flavor::UltraRecursive, goblin_flag>; using VerificationKey = ::proof_system::honk::flavor::UltraRecursive::VerificationKey; using inner_curve = bn254; @@ -212,17 +222,16 @@ template class RecursiveVerifierTest : public testing:: // Create a recursive verification circuit for the proof of the inner circuit create_outer_circuit(inner_circuit, outer_circuit); - if (outer_circuit.failed()) { - info(outer_circuit.err()); - } - EXPECT_EQ(outer_circuit.failed(), false); + EXPECT_EQ(outer_circuit.failed(), false) << outer_circuit.err(); EXPECT_TRUE(outer_circuit.check_circuit()); } }; -using OuterCircuitTypes = testing::Types<::proof_system::honk::UltraComposer>; +// Run the recursive verifier tests twice, once without using a goblin style arithmetization of group operations and +// once with. +using UseGoblinFlag = testing::Types; -TYPED_TEST_SUITE(RecursiveVerifierTest, OuterCircuitTypes); +TYPED_TEST_SUITE(RecursiveVerifierTest, UseGoblinFlag); HEAVY_TYPED_TEST(RecursiveVerifierTest, InnerCircuit) {