From c422de1bdb513e426bd11fcca1385187a75d529d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 24 Aug 2023 22:37:39 +0000 Subject: [PATCH 01/13] proper goblin batch mul in biggroup; tests passing --- .../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 | 12 +-- .../stdlib/primitives/biggroup/biggroup.hpp | 4 + .../primitives/biggroup/biggroup.test.cpp | 2 + .../primitives/biggroup/biggroup_goblin.hpp | 79 +++++++++++++++++++ .../verifier/ultra_recursive_verifier.cpp | 17 +++- .../verifier/ultra_recursive_verifier.hpp | 2 +- .../recursion/honk/verifier/verifier.test.cpp | 14 ++-- 10 files changed, 155 insertions(+), 115 deletions(-) create mode 100644 circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp index dbdd25061d5..50686a230ea 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp +++ b/circuits/cpp/barretenberg/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/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp index 6e229f7ba6c..860e1b056be 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp +++ b/circuits/cpp/barretenberg/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/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index d96dfce13c9..7657f445fd3 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/circuits/cpp/barretenberg/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/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index 2ff9f0eb70a..bf90e980fc5 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -706,14 +706,14 @@ template class UltraCircuitBuilder_ : public CircuitBuilderBase& points, const std::vector& scalars); + // WORKTODO: could we turn this into a class somehow? maybe it has a pointer to variables or something? maybe not + 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: diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index 43d59a7d0f0..ce2f64f9fee 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -179,6 +179,9 @@ template class element { static element batch_mul(const std::vector& points, const std::vector& scalars, const size_t max_num_bits = 0); + static element goblin_batch_mul(const std::vector& points, + const std::vector& scalars, + const size_t max_num_bits = 0); // we want to conditionally compile this method iff our curve params are the BN254 curve. // This is a bit tricky to do with `std::enable_if`, because `bn254_endo_batch_mul` is a member function of a class @@ -892,6 +895,7 @@ inline std::ostream& operator<<(std::ostream& os, element const& v #include "biggroup_batch_mul.hpp" #include "biggroup_bn254.hpp" +#include "biggroup_goblin.hpp" #include "biggroup_impl.hpp" #include "biggroup_nafs.hpp" #include "biggroup_secp256k1.hpp" diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp index 5ca3bbad35e..f4b6e7e2b2c 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp @@ -338,6 +338,8 @@ template class stdlib_biggroup : public testing::Test { EXPECT_CIRCUIT_CORRECTNESS(composer); } + // WORKTODO: add a test for goblin_batch_mul + static void test_batch_mul() { const size_t num_points = 5; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp new file mode 100644 index 00000000000..1845d1817f7 --- /dev/null +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -0,0 +1,79 @@ +#pragma once + +namespace proof_system::plonk { +namespace stdlib { + +/** + * @brief Goblin style batch multiplication + * @note (Luke): The approach of having a distinct interface for goblin style group operations is limited/flawed. The + * natural alternative is to abstract the details away from the circuit writer and to simply allow the strategy to be + * determined by the type of circuit constructor (i.e. Goblin or not) from within biggroup. Currently, the goblin-style + * circuit builder functionality has been incorporated directly into the UltraCircuitBuilder, thus there is no + * means for distinction. If we decide it is preferable to support fully flexible goblin-style group operations via the + * existing biggroup API, we will need to make an independent GoblinUltraCircuitBuilder class (plausibly via inheritance + * from UltraCircuitBuilder) and implement Goblin-style strategies for each of the operations in biggroup. + * + * @tparam C CircuitBuilder + * @tparam Fq Base field + * @tparam Fr Scalar field + * @tparam G Native group + * @param points + * @param scalars + * @param max_num_bits + * @return element + */ +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 + auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + + // Constrain decomposition of point coordinates to reconstruct original values. + // Note: may need to do point.x.assert_is_in_field() prior to the assert_eq() according to Kesha. + 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); + // point.x.assert_is_in_field(); // WORKTODO: needed? + // point.y.assert_is_in_field(); + point.x.assert_equal(point_x); + point.y.assert_equal(point_y); + + // Constrain endomorphism scalars to reconstruct scalar + 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 + 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/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp index c7dfbcec1b4..a6c17b0e058 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp @@ -35,7 +35,7 @@ UltraRecursiveVerifier_& UltraRecursiveVerifier_::operator=(Ultr * */ template -std::array UltraRecursiveVerifier_::verify_proof(const plonk::proof& proof) +std::array UltraRecursiveVerifier_::verify_proof(const plonk::proof& proof, bool use_goblin) { using FF = typename Flavor::FF; using GroupElement = typename Flavor::GroupElement; @@ -163,13 +163,22 @@ 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); + GroupElement batched_commitment_unshifted; + if (use_goblin) { + batched_commitment_unshifted = GroupElement::goblin_batch_mul(commitments.get_unshifted(), scalars_unshifted); + } else { + batched_commitment_unshifted = GroupElement::batch_mul(commitments.get_unshifted(), scalars_unshifted); + } info("Batch mul (unshifted): num gates = ", builder->get_num_gates() - prev_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 batched_commitment_to_be_shifted; + if (use_goblin) { + batched_commitment_to_be_shifted = GroupElement::goblin_batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted); + } else { + batched_commitment_to_be_shifted = GroupElement::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); prev_num_gates = builder->get_num_gates(); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp index a1eee1d4d44..2b1cde2cd64 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp @@ -25,7 +25,7 @@ template class UltraRecursiveVerifier_ { // 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. - PairingPoints verify_proof(const plonk::proof& proof); + PairingPoints verify_proof(const plonk::proof& proof, bool use_goblin = false); std::shared_ptr key; std::map commitments; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index 50c3cd77e4d..05d2bafdaab 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -15,12 +15,14 @@ namespace proof_system::plonk::stdlib::recursion::honk { -template class RecursiveVerifierTest : public testing::Test { +template class RecursiveVerifierTest : public testing::Test { + + static constexpr bool use_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>; @@ -115,7 +117,7 @@ template class RecursiveVerifierTest : public testing:: // Instantiate the recursive verifier and construct the recusive verification circuit RecursiveVerifier verifier(&outer_builder, verification_key); - auto pairing_points = verifier.verify_proof(proof_to_recursively_verify); + auto pairing_points = verifier.verify_proof(proof_to_recursively_verify, use_goblin_flag); // For testing purposes only, perform native verification and compare the result auto native_verifier = inner_composer.create_verifier(inner_circuit); @@ -220,9 +222,11 @@ template class RecursiveVerifierTest : public testing:: } }; -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) { From 0da55f9bfe71eb3f8fbd7da716f00b769631ee92 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 25 Aug 2023 16:29:11 +0000 Subject: [PATCH 02/13] convert muls in Gemini to batch mul --- .../barretenberg/honk/pcs/gemini/gemini.hpp | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 1fdbeb06353..691f7fc5db3 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp @@ -232,21 +232,26 @@ 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. + if constexpr (Curve::is_stdlib_type) { + std::vector commitments = {batched_f, batched_g}; + auto one = Fr::from_witness(r.get_context(), 1); + // Note: these batch muls are not optimal since we are performing a mul by 1. + C0_r_pos = GroupElement::batch_mul(commitments, {one, r_inv}); + C0_r_neg = GroupElement::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 }; From eb7f119f1a02e185dab7a7b33bd7b9abe87149e4 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 25 Aug 2023 17:37:03 +0000 Subject: [PATCH 03/13] convert Shplonk to batch mul --- .../barretenberg/honk/pcs/shplonk/shplonk.hpp | 129 ++++++++++-------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index f4b11815c43..7af607afa24 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp @@ -173,82 +173,103 @@ template class ShplonkVerifier_ { auto Q_commitment = transcript.template receive_from_prover("Shplonk:Q"); const Fr z_challenge = transcript.get_challenge("Shplonk:z"); + Fr evaluation_zero; // 0 \in Fr + + // [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(); + evaluation_zero = Fr::from_witness(builder, 0); + + // 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::from_witness(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; - // If recursion, store MSM inputs for batch mul, otherwise accumulate directly - if constexpr (Curve::is_stdlib_type) { + current_nu *= nu; + + // 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::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_commitment += group_one * G_commitment_constant; + // [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]; + + 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 }; From 77ca9af0f6895cd15af92c50c91d8ee99e09f361 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 25 Aug 2023 19:26:05 +0000 Subject: [PATCH 04/13] template recursive verifier on goblin flag --- .../barretenberg/honk/pcs/gemini/gemini.hpp | 6 +- .../barretenberg/honk/pcs/shplonk/shplonk.hpp | 4 +- .../stdlib/primitives/biggroup/biggroup.hpp | 1 + .../primitives/biggroup/biggroup_impl.hpp | 80 ++++++++++--------- .../verifier/ultra_recursive_verifier.cpp | 61 +++++--------- .../verifier/ultra_recursive_verifier.hpp | 18 +++-- .../recursion/honk/verifier/verifier.test.cpp | 6 +- 7 files changed, 82 insertions(+), 94 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 691f7fc5db3..94d7653b217 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/circuits/cpp/barretenberg/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; @@ -242,8 +242,8 @@ template class GeminiVerifier_ { std::vector commitments = {batched_f, batched_g}; auto one = Fr::from_witness(r.get_context(), 1); // Note: these batch muls are not optimal since we are performing a mul by 1. - C0_r_pos = GroupElement::batch_mul(commitments, {one, r_inv}); - C0_r_neg = GroupElement::batch_mul(commitments, {one, -r_inv}); + 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; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index 7af607afa24..93e2e56ac11 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/circuits/cpp/barretenberg/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; @@ -233,7 +233,7 @@ template class ShplonkVerifier_ { scalars.emplace_back(G_commitment_constant); // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1] - G_commitment = GroupElement::batch_mul(commitments, scalars); + G_commitment = GroupElement::template batch_mul(commitments, scalars); } else { evaluation_zero = Fr(0); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index ce2f64f9fee..a405d554556 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -176,6 +176,7 @@ template class element { // only works with Plookup! template static element wnaf_batch_mul(const std::vector& points, const std::vector& scalars); + template static element batch_mul(const std::vector& points, const std::vector& scalars, const size_t max_num_bits = 0); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index c6749aea6b3..92b8cbf76c4 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -597,51 +597,55 @@ 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) { - - const size_t num_points = points.size(); - ASSERT(scalars.size() == num_points); - batch_lookup_table point_table(points); - const size_t num_rounds = (max_num_bits == 0) ? Fr::modulus.get_msb() + 1 : max_num_bits; - - std::vector>> naf_entries; - for (size_t i = 0; i < num_points; ++i) { - naf_entries.emplace_back(compute_naf(scalars[i], max_num_bits)); - } - const auto offset_generators = compute_offset_generators(num_rounds); - element accumulator = - element::chain_add_end(element::chain_add(offset_generators.first, point_table.get_chain_initial_entry())); - - constexpr size_t num_rounds_per_iteration = 4; - size_t num_iterations = num_rounds / num_rounds_per_iteration; - num_iterations += ((num_iterations * num_rounds_per_iteration) == num_rounds) ? 0 : 1; - const size_t num_rounds_per_final_iteration = (num_rounds - 1) - ((num_iterations - 1) * num_rounds_per_iteration); - for (size_t i = 0; i < num_iterations; ++i) { - - std::vector> nafs(num_points); - std::vector to_add; - const size_t inner_num_rounds = - (i != num_iterations - 1) ? num_rounds_per_iteration : num_rounds_per_final_iteration; - for (size_t j = 0; j < inner_num_rounds; ++j) { - for (size_t k = 0; k < num_points; ++k) { - nafs[k] = (naf_entries[k][i * num_rounds_per_iteration + j + 1]); + if constexpr (use_goblin) { + return goblin_batch_mul(points, scalars); + } else { + const size_t num_points = points.size(); + ASSERT(scalars.size() == num_points); + batch_lookup_table point_table(points); + const size_t num_rounds = (max_num_bits == 0) ? Fr::modulus.get_msb() + 1 : max_num_bits; + + std::vector>> naf_entries; + for (size_t i = 0; i < num_points; ++i) { + naf_entries.emplace_back(compute_naf(scalars[i], max_num_bits)); + } + const auto offset_generators = compute_offset_generators(num_rounds); + element accumulator = + element::chain_add_end(element::chain_add(offset_generators.first, point_table.get_chain_initial_entry())); + + constexpr size_t num_rounds_per_iteration = 4; + size_t num_iterations = num_rounds / num_rounds_per_iteration; + num_iterations += ((num_iterations * num_rounds_per_iteration) == num_rounds) ? 0 : 1; + const size_t num_rounds_per_final_iteration = (num_rounds - 1) - ((num_iterations - 1) * num_rounds_per_iteration); + for (size_t i = 0; i < num_iterations; ++i) { + + std::vector> nafs(num_points); + std::vector to_add; + const size_t inner_num_rounds = + (i != num_iterations - 1) ? num_rounds_per_iteration : num_rounds_per_final_iteration; + for (size_t j = 0; j < inner_num_rounds; ++j) { + for (size_t k = 0; k < num_points; ++k) { + nafs[k] = (naf_entries[k][i * num_rounds_per_iteration + j + 1]); + } + to_add.emplace_back(point_table.get_chain_add_accumulator(nafs)); } - to_add.emplace_back(point_table.get_chain_add_accumulator(nafs)); + accumulator = accumulator.multiple_montgomery_ladder(to_add); } - accumulator = accumulator.multiple_montgomery_ladder(to_add); - } - for (size_t i = 0; i < num_points; ++i) { - element skew = accumulator - points[i]; - Fq out_x = accumulator.x.conditional_select(skew.x, naf_entries[i][num_rounds]); - Fq out_y = accumulator.y.conditional_select(skew.y, naf_entries[i][num_rounds]); - accumulator = element(out_x, out_y); - } - accumulator = accumulator - offset_generators.second; + for (size_t i = 0; i < num_points; ++i) { + element skew = accumulator - points[i]; + Fq out_x = accumulator.x.conditional_select(skew.x, naf_entries[i][num_rounds]); + Fq out_y = accumulator.y.conditional_select(skew.y, naf_entries[i][num_rounds]); + accumulator = element(out_x, out_y); + } + accumulator = accumulator - offset_generators.second; - return accumulator; + return accumulator; + } } /** diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp index a6c17b0e058..2df67747d69 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp @@ -7,43 +7,26 @@ #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, bool use_goblin) +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 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 VerifierCommitments = typename Flavor::VerifierCommitments; using CommitmentLabels = typename Flavor::CommitmentLabels; @@ -163,22 +146,14 @@ 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 - GroupElement batched_commitment_unshifted; - if (use_goblin) { - batched_commitment_unshifted = GroupElement::goblin_batch_mul(commitments.get_unshifted(), scalars_unshifted); - } else { - batched_commitment_unshifted = GroupElement::batch_mul(commitments.get_unshifted(), scalars_unshifted); - } + 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); prev_num_gates = builder->get_num_gates(); - GroupElement batched_commitment_to_be_shifted; - if (use_goblin) { - batched_commitment_to_be_shifted = GroupElement::goblin_batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted); - } else { - batched_commitment_to_be_shifted = GroupElement::batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted); - } + auto batched_commitment_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); prev_num_gates = builder->get_num_gates(); @@ -201,14 +176,18 @@ std::array UltraRecursiveVerifier_::ve info("Shplonk: num gates = ", builder->get_num_gates() - prev_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); + info("KZG: num gates = ", builder->get_num_gates() - prev_num_gates); + + info("Total: num gates = ", 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/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp index 2b1cde2cd64..dff4ac7ade1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp @@ -8,24 +8,26 @@ #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. - PairingPoints verify_proof(const plonk::proof& proof, bool use_goblin = false); + PairingPoints verify_proof(const plonk::proof& proof); std::shared_ptr key; std::map commitments; @@ -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/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index 05d2bafdaab..d49c8e6ac89 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -17,7 +17,7 @@ namespace proof_system::plonk::stdlib::recursion::honk { template class RecursiveVerifierTest : public testing::Test { - static constexpr bool use_goblin_flag = UseGoblinFlag::value; + static constexpr bool goblin_flag = UseGoblinFlag::value; using InnerComposer = ::proof_system::honk::UltraComposer; using InnerBuilder = typename InnerComposer::CircuitBuilder; @@ -25,7 +25,7 @@ template class RecursiveVerifierTest : public testing:: using OuterBuilder = ::proof_system::UltraCircuitBuilder; using NativeVerifier = ::proof_system::honk::UltraVerifier_<::proof_system::honk::flavor::Ultra>; - using RecursiveVerifier = UltraRecursiveVerifier_<::proof_system::honk::flavor::UltraRecursive>; + using RecursiveVerifier = UltraRecursiveVerifier_<::proof_system::honk::flavor::UltraRecursive, goblin_flag>; using VerificationKey = ::proof_system::honk::flavor::UltraRecursive::VerificationKey; using inner_curve = bn254; @@ -117,7 +117,7 @@ template class RecursiveVerifierTest : public testing:: // Instantiate the recursive verifier and construct the recusive verification circuit RecursiveVerifier verifier(&outer_builder, verification_key); - auto pairing_points = verifier.verify_proof(proof_to_recursively_verify, use_goblin_flag); + auto pairing_points = verifier.verify_proof(proof_to_recursively_verify); // For testing purposes only, perform native verification and compare the result auto native_verifier = inner_composer.create_verifier(inner_circuit); From ebee77877ecd72c1c36cd1647211296b45936b89 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 25 Aug 2023 21:57:38 +0000 Subject: [PATCH 05/13] batch mul in kzg --- .../honk/flavor/ultra_recursive.hpp | 1 - .../cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp | 34 ++++++++++++------- .../circuit_builder/ultra_circuit_builder.cpp | 2 +- .../circuit_builder/ultra_circuit_builder.hpp | 3 +- .../verifier/ultra_recursive_verifier.cpp | 5 +-- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp index 3162b860482..1484a5b5fe1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp +++ b/circuits/cpp/barretenberg/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/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp index ae0ee53a517..281aa0a67af 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp +++ b/circuits/cpp/barretenberg/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,39 @@ 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::from_witness(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); + // WORKTODO(luke): The evaluation is always zero due to the nature of shplonk. What is the proper way to + // handle this? Contraints to show scalar (evaluation) is zero? Or simply dont add anything and ensure this + // function is only used in the current context? + // if (!claim.opening_pair.evaluation.get_value().is_zero()) { + // auto ctx = verifier_transcript.builder; + // lhs -= GroupElement::one(ctx) * claim.opening_pair.evaluation; + // } } 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/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index 7657f445fd3..3446b5cef87 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -527,7 +527,7 @@ ecc_op_tuple UltraCircuitBuilder_::queue_ecc_add_accum(const barretenberg::g */ template ecc_op_tuple UltraCircuitBuilder_::queue_ecc_mul_accum(const barretenberg::g1::affine_element& point, - const barretenberg::fr& scalar) + const barretenberg::fr& scalar) { // Add raw op to op queue op_queue.mul_accumulate(point, scalar); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index bf90e980fc5..ab242bec44d 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -705,13 +705,12 @@ template class UltraCircuitBuilder_ : public CircuitBuilderBase UltraRecursiveVerifier_; using Shplonk = ::proof_system::honk::pcs::shplonk::ShplonkVerifier_; - using PCS = typename Flavor::PCS; // note: This can only be KZG + 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 }; @@ -177,7 +178,7 @@ std::array UltraRecursiveVerifier_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); From 2566a06ea54362ef2a8b830f1742eb6efc2af927 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 25 Aug 2023 22:01:15 +0000 Subject: [PATCH 06/13] remove unneeded else in batch mul --- .../primitives/biggroup/biggroup_impl.hpp | 77 +++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index 92b8cbf76c4..efc05ef4057 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -604,48 +604,47 @@ element element::batch_mul(const std::vector>> naf_entries; - for (size_t i = 0; i < num_points; ++i) { - naf_entries.emplace_back(compute_naf(scalars[i], max_num_bits)); - } - const auto offset_generators = compute_offset_generators(num_rounds); - element accumulator = - element::chain_add_end(element::chain_add(offset_generators.first, point_table.get_chain_initial_entry())); - - constexpr size_t num_rounds_per_iteration = 4; - size_t num_iterations = num_rounds / num_rounds_per_iteration; - num_iterations += ((num_iterations * num_rounds_per_iteration) == num_rounds) ? 0 : 1; - const size_t num_rounds_per_final_iteration = (num_rounds - 1) - ((num_iterations - 1) * num_rounds_per_iteration); - for (size_t i = 0; i < num_iterations; ++i) { - - std::vector> nafs(num_points); - std::vector to_add; - const size_t inner_num_rounds = - (i != num_iterations - 1) ? num_rounds_per_iteration : num_rounds_per_final_iteration; - for (size_t j = 0; j < inner_num_rounds; ++j) { - for (size_t k = 0; k < num_points; ++k) { - nafs[k] = (naf_entries[k][i * num_rounds_per_iteration + j + 1]); - } - to_add.emplace_back(point_table.get_chain_add_accumulator(nafs)); + } + const size_t num_points = points.size(); + ASSERT(scalars.size() == num_points); + batch_lookup_table point_table(points); + const size_t num_rounds = (max_num_bits == 0) ? Fr::modulus.get_msb() + 1 : max_num_bits; + + std::vector>> naf_entries; + for (size_t i = 0; i < num_points; ++i) { + naf_entries.emplace_back(compute_naf(scalars[i], max_num_bits)); + } + const auto offset_generators = compute_offset_generators(num_rounds); + element accumulator = + element::chain_add_end(element::chain_add(offset_generators.first, point_table.get_chain_initial_entry())); + + constexpr size_t num_rounds_per_iteration = 4; + size_t num_iterations = num_rounds / num_rounds_per_iteration; + num_iterations += ((num_iterations * num_rounds_per_iteration) == num_rounds) ? 0 : 1; + const size_t num_rounds_per_final_iteration = (num_rounds - 1) - ((num_iterations - 1) * num_rounds_per_iteration); + for (size_t i = 0; i < num_iterations; ++i) { + + std::vector> nafs(num_points); + std::vector to_add; + const size_t inner_num_rounds = + (i != num_iterations - 1) ? num_rounds_per_iteration : num_rounds_per_final_iteration; + for (size_t j = 0; j < inner_num_rounds; ++j) { + for (size_t k = 0; k < num_points; ++k) { + nafs[k] = (naf_entries[k][i * num_rounds_per_iteration + j + 1]); } - accumulator = accumulator.multiple_montgomery_ladder(to_add); + to_add.emplace_back(point_table.get_chain_add_accumulator(nafs)); } - for (size_t i = 0; i < num_points; ++i) { - element skew = accumulator - points[i]; - Fq out_x = accumulator.x.conditional_select(skew.x, naf_entries[i][num_rounds]); - Fq out_y = accumulator.y.conditional_select(skew.y, naf_entries[i][num_rounds]); - accumulator = element(out_x, out_y); - } - accumulator = accumulator - offset_generators.second; - - return accumulator; + accumulator = accumulator.multiple_montgomery_ladder(to_add); } + for (size_t i = 0; i < num_points; ++i) { + element skew = accumulator - points[i]; + Fq out_x = accumulator.x.conditional_select(skew.x, naf_entries[i][num_rounds]); + Fq out_y = accumulator.y.conditional_select(skew.y, naf_entries[i][num_rounds]); + accumulator = element(out_x, out_y); + } + accumulator = accumulator - offset_generators.second; + + return accumulator; } /** From fe578f197d30469f1e6aa88fa4a134eb75dcffd4 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 28 Aug 2023 17:31:20 +0000 Subject: [PATCH 07/13] add test for goblin batch mul --- .../primitives/biggroup/biggroup.test.cpp | 2 - .../primitives/biggroup/biggroup_goblin.hpp | 21 ++--- .../biggroup/biggroup_goblin.test.cpp | 89 +++++++++++++++++++ 3 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp index f4b6e7e2b2c..5ca3bbad35e 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp @@ -338,8 +338,6 @@ template class stdlib_biggroup : public testing::Test { EXPECT_CIRCUIT_CORRECTNESS(composer); } - // WORKTODO: add a test for goblin_batch_mul - static void test_batch_mul() { const size_t num_points = 5; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index 1845d1817f7..7ec0299dffb 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -5,13 +5,14 @@ namespace stdlib { /** * @brief Goblin style batch multiplication - * @note (Luke): The approach of having a distinct interface for goblin style group operations is limited/flawed. The - * natural alternative is to abstract the details away from the circuit writer and to simply allow the strategy to be - * determined by the type of circuit constructor (i.e. Goblin or not) from within biggroup. Currently, the goblin-style - * circuit builder functionality has been incorporated directly into the UltraCircuitBuilder, thus there is no - * means for distinction. If we decide it is preferable to support fully flexible goblin-style group operations via the - * existing biggroup API, we will need to make an independent GoblinUltraCircuitBuilder class (plausibly via inheritance - * from UltraCircuitBuilder) and implement Goblin-style strategies for each of the operations in biggroup. + * + * @details In goblin-style arithmetization, the operands (points/scalars) for each mul-accumulate operation are + * decomposed into smaller components and written to an operation queue via the builder. The components are also added + * as witness variables. This function adds constraints demonstrating the fidelity of the point/scalar decompositions + * given the indices of the components in the variables array. The actual mul-accumulate operations are performed + * natively (without constraints) under the hood, and the final result is obtained by queueing an equality operation via + * the builder. The components of the result are returned as indices into the variables array from which the resulting + * accumulator point is re-constructed. * * @tparam C CircuitBuilder * @tparam Fq Base field @@ -41,7 +42,7 @@ element element::goblin_batch_mul(const std::vector< // Populate the goblin-style ecc op gates for the given mul inputs auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); - // Constrain decomposition of point coordinates to reconstruct original values. + // Adds constraints demonstrating proper decomposition of point coordinates. // Note: may need to do point.x.assert_is_in_field() prior to the assert_eq() according to Kesha. auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); @@ -54,7 +55,7 @@ element element::goblin_batch_mul(const std::vector< point.x.assert_equal(point_x); point.y.assert_equal(point_y); - // Constrain endomorphism scalars to reconstruct scalar + // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars 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(); @@ -64,7 +65,7 @@ element element::goblin_batch_mul(const std::vector< // Populate equality gates based on the internal accumulator point auto op_tuple = builder->queue_ecc_eq(); - // Reconstruct the result of the batch mul + // 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); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.test.cpp new file mode 100644 index 00000000000..3b8bf3df0cd --- /dev/null +++ b/circuits/cpp/barretenberg/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 From abfdc3401d77f82f56920f77235de7ac243b0ed2 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 28 Aug 2023 21:19:22 +0000 Subject: [PATCH 08/13] comments and cleanup --- .../circuit_builder/ultra_circuit_builder.hpp | 6 +++--- .../primitives/biggroup/biggroup_goblin.hpp | 8 +++++--- .../verifier/ultra_recursive_verifier.cpp | 20 +++++++++---------- .../recursion/honk/verifier/verifier.test.cpp | 8 ++++++++ 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index ab242bec44d..cd8594d134a 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -833,7 +833,7 @@ template class UltraCircuitBuilder_ : public CircuitBuilderBase class UltraCircuitBuilder_ : public CircuitBuilderBase UltraRecursiveVerifier_(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 @@ -124,7 +124,7 @@ std::array UltraRecursiveVerifier_get_num_gates(); // Compute batched commitments needed for input to Gemini. @@ -150,13 +150,13 @@ std::array UltraRecursiveVerifier_(commitments.get_unshifted(), scalars_unshifted); - info("Batch mul (unshifted): num gates = ", builder->get_num_gates() - prev_num_gates); + 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::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: @@ -168,21 +168,19 @@ std::array UltraRecursiveVerifier_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(); // Constuct the inputs to the final KZG pairing check auto pairing_points = KZG::compute_pairing_points(shplonk_claim, transcript); - info("KZG: num gates = ", builder->get_num_gates() - prev_num_gates); - - info("Total: num gates = ", builder->get_num_gates()); + info("KZG: num gates = ", builder->get_num_gates() - prev_num_gates, ", (total = ", builder->get_num_gates(), ")"); return pairing_points; } diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index d49c8e6ac89..b593487cfb1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -15,6 +15,13 @@ namespace proof_system::plonk::stdlib::recursion::honk { +/** + * @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; @@ -25,6 +32,7 @@ template class RecursiveVerifierTest : public testing:: using OuterBuilder = ::proof_system::UltraCircuitBuilder; using NativeVerifier = ::proof_system::honk::UltraVerifier_<::proof_system::honk::flavor::Ultra>; + // 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; From 7b27b3d0a68b726774fa3069cf00b30126de41a4 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 28 Aug 2023 23:17:26 +0000 Subject: [PATCH 09/13] todos with issue numbers in gemini --- .../cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 94d7653b217..7f12fea58d2 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp @@ -238,12 +238,14 @@ template class GeminiVerifier_ { Fr r_inv = r.invert(); // r⁻¹ // 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}; + std::vector commitments = { batched_f, batched_g }; auto one = Fr::from_witness(r.get_context(), 1); - // Note: these batch muls are not optimal since we are performing a mul by 1. - C0_r_pos = GroupElement::template batch_mul(commitments, {one, r_inv}); - C0_r_neg = GroupElement::template batch_mul(commitments, {one, -r_inv}); + // TODO(#707): these batch muls are not optimal since we are performing an unnecessary mul by 1. + 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; From ef387fbd454396a9a465b1f83a1519c2a466b407 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 31 Aug 2023 16:24:00 +0000 Subject: [PATCH 10/13] WiP alternative constraints --- .../primitives/biggroup/biggroup_goblin.hpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index 53771c9e84c..5f5ac9cad8e 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -42,21 +42,24 @@ element element::goblin_batch_mul(const std::vector< // Populate the goblin-style ecc op gates for the given mul inputs auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); - // Adds constraints demonstrating proper decomposition of point coordinates. + // Adds constraints demonstrating that the EC point coordinates can be reconstructed from their decomposition. 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); - // WORKTODO (discuss with Kesha): Kesha suggested that it may be necessary to do some assert_is_in_field here. - // All of the point coordinates being compared here have been constructed via bigfield(lo, hi) which appears to - // control number of bits but it's not clear whether it guarantees membership in Fq. Seems like it must, - // otherwise is eems we'd need to assert in field every time we construct a bigfield element from witness. Also, - // is assert_equal the right thing here? Any subtlety we should document? point.x.assert_equal(point_x); point.y.assert_equal(point_y); + // // ALTERNATIVELY: try this and compare gate counts + // point.x.assert_is_in_field() + // point.y.assert_is_in_field() + // x_lo.assert_equal(point.x.binary_basis_limbs[0] + shift_1 * point.x.binary_basis_limbs[1]); + // x_hi.assert_equal(point.x.binary_basis_limbs[2] + shift_1 * point.x.binary_basis_limbs[3]); + // y_lo.assert_equal(point.y.binary_basis_limbs[0] + shift_1 * point.y.binary_basis_limbs[1]); + // y_hi.assert_equal(point.y.binary_basis_limbs[2] + shift_1 * point.y.binary_basis_limbs[3]); + // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); From 2eebc7aa7d0d03678e8d6e83ee836ad503c7f41b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 31 Aug 2023 18:25:59 +0000 Subject: [PATCH 11/13] update constraints in batch mul --- .../primitives/biggroup/biggroup_goblin.hpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index 5f5ac9cad8e..a666611bec0 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -42,23 +42,23 @@ element element::goblin_batch_mul(const std::vector< // Populate the goblin-style ecc op gates for the given mul inputs auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); - // Adds constraints demonstrating that the EC point coordinates can be reconstructed from their decomposition. + // 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); - Fq point_x(x_lo, x_hi); - Fq point_y(y_lo, y_hi); - point.x.assert_equal(point_x); - point.y.assert_equal(point_y); - - // // ALTERNATIVELY: try this and compare gate counts - // point.x.assert_is_in_field() - // point.y.assert_is_in_field() - // x_lo.assert_equal(point.x.binary_basis_limbs[0] + shift_1 * point.x.binary_basis_limbs[1]); - // x_hi.assert_equal(point.x.binary_basis_limbs[2] + shift_1 * point.x.binary_basis_limbs[3]); - // y_lo.assert_equal(point.y.binary_basis_limbs[0] + shift_1 * point.y.binary_basis_limbs[1]); - // y_hi.assert_equal(point.y.binary_basis_limbs[2] + shift_1 * point.y.binary_basis_limbs[3]); + // 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 auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); From 9abfd42af5e66924e29ac9336c39d032cbec4d40 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 5 Sep 2023 18:59:19 +0000 Subject: [PATCH 12/13] fix unconstrained witnesses and do add accum in batch mul --- .../barretenberg/honk/pcs/gemini/gemini.hpp | 7 +++++-- .../cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp | 2 +- .../barretenberg/honk/pcs/shplonk/shplonk.hpp | 4 ++-- .../primitives/biggroup/biggroup_goblin.hpp | 21 ++++++++++++++----- .../recursion/honk/verifier/verifier.test.cpp | 5 +---- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 7f12fea58d2..f46d55ae71f 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp @@ -242,8 +242,11 @@ template class GeminiVerifier_ { // 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 one = Fr::from_witness(r.get_context(), 1); - // TODO(#707): these batch muls are not optimal since we are performing an unnecessary mul by 1. + 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 { diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp index 281aa0a67af..99bbda063bd 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp @@ -85,7 +85,7 @@ template class KZG { // evaluation is not equal to zero). if constexpr (Curve::is_stdlib_type) { auto builder = verifier_transcript.builder; - auto one = Fr::from_witness(builder, 1); + 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); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index 93e2e56ac11..f53925af24e 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp @@ -192,7 +192,7 @@ template class ShplonkVerifier_ { // using a builder Simulator. if constexpr (Curve::is_stdlib_type) { auto builder = nu.get_context(); - evaluation_zero = Fr::from_witness(builder, 0); + evaluation_zero = Fr(builder, 0); // Containers for the inputs to the final batch mul std::vector commitments; @@ -201,7 +201,7 @@ template class ShplonkVerifier_ { // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] commitments.emplace_back(Q_commitment); - scalars.emplace_back(Fr::from_witness(builder, 1)); // Fr(1) + scalars.emplace_back(Fr(builder, 1)); // Fr(1) // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ) std::vector inverse_vanishing_evals; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index a666611bec0..8217b08c2a1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -13,6 +13,9 @@ namespace stdlib { * natively (without constraints) under the hood, and the final result is obtained by queueing an equality operation via * the builder. The components of the result are returned as indices into the variables array from which the resulting * accumulator point is re-constructed. + * @note Because this is the only method for performing Goblin-style group operations (Issue #707), it is sometimes used + * in situations where one of the scalars is 1 (e.g. to perform P = P_0 + z*P_1). In this case, we perform a simple add + * accumulate instead of a mul-then_accumulate. * * @tparam C CircuitBuilder * @tparam Fq Base field @@ -40,7 +43,13 @@ element element::goblin_batch_mul(const std::vector< auto& scalar = scalars[i]; // Populate the goblin-style ecc op gates for the given mul inputs - auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + 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 @@ -61,10 +70,12 @@ element element::goblin_batch_mul(const std::vector< 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 - 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); + 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 diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index b593487cfb1..7bfc45452d1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -222,10 +222,7 @@ 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()); } }; From 05e872f53ab503a906498d6911c7267d93818c22 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 5 Sep 2023 19:26:03 +0000 Subject: [PATCH 13/13] update zero evaluation and comments --- .../cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp | 9 ++------- .../cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp | 6 +----- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp index 99bbda063bd..ab2c8d8bb7a 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp @@ -89,13 +89,8 @@ template class KZG { std::vector commitments = { claim.commitment, quotient_commitment }; std::vector scalars = { one, claim.opening_pair.challenge }; P_0 = GroupElement::template batch_mul(commitments, scalars); - // WORKTODO(luke): The evaluation is always zero due to the nature of shplonk. What is the proper way to - // handle this? Contraints to show scalar (evaluation) is zero? Or simply dont add anything and ensure this - // function is only used in the current context? - // if (!claim.opening_pair.evaluation.get_value().is_zero()) { - // auto ctx = verifier_transcript.builder; - // lhs -= GroupElement::one(ctx) * claim.opening_pair.evaluation; - // } + // Note: This implementation assumes the evaluation is zero (as is the case for shplonk). + ASSERT(claim.opening_pair.evaluation.get_value() == 0); } else { P_0 = claim.commitment; P_0 += quotient_commitment * claim.opening_pair.challenge; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index f53925af24e..17c452471dc 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp @@ -173,7 +173,6 @@ template class ShplonkVerifier_ { auto Q_commitment = transcript.template receive_from_prover("Shplonk:Q"); const Fr z_challenge = transcript.get_challenge("Shplonk:z"); - Fr evaluation_zero; // 0 \in Fr // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] @@ -192,7 +191,6 @@ template class ShplonkVerifier_ { // using a builder Simulator. if constexpr (Curve::is_stdlib_type) { auto builder = nu.get_context(); - evaluation_zero = Fr(builder, 0); // Containers for the inputs to the final batch mul std::vector commitments; @@ -236,8 +234,6 @@ template class ShplonkVerifier_ { G_commitment = GroupElement::template batch_mul(commitments, scalars); } else { - evaluation_zero = Fr(0); - // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] G_commitment = Q_commitment; @@ -272,7 +268,7 @@ template class ShplonkVerifier_ { } // 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