From 3d954764e6555e8bf0be5a6ad92e2f3f994433bb Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 28 Aug 2023 17:31:20 +0000 Subject: [PATCH] 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 0ef57df8232e..1b2ac170f855 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 @@ -337,8 +337,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 1845d1817f7b..7ec0299dffb5 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 000000000000..3b8bf3df0cdd --- /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