From a99a24431afb56deb244deda52d231b24ba577b2 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Sat, 7 Sep 2024 20:37:00 +0000 Subject: [PATCH 01/10] oink alpha challenges are now generated simultaneously instead of one alpha challenge per round --- barretenberg/sol/src/honk/Transcript.sol | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/barretenberg/sol/src/honk/Transcript.sol b/barretenberg/sol/src/honk/Transcript.sol index 1f450263a442..d90789f63100 100644 --- a/barretenberg/sol/src/honk/Transcript.sol +++ b/barretenberg/sol/src/honk/Transcript.sol @@ -151,6 +151,18 @@ library TranscriptLib { Fr unused; (alphas[NUMBER_OF_ALPHAS - 1], unused) = splitChallenge(nextPreviousChallenge); } + if (((NUMBER_OF_ALPHAS & 1) == 1) && (NUMBER_OF_ALPHAS > 2)) { + nextPreviousChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(nextPreviousChallenge)))); + Fr unused; + (alphas[NUMBER_OF_ALPHAS - 1], unused) = splitChallenge(nextPreviousChallenge); + } + // alphas[0] = FrLib.fromBytes32(keccak256(abi.encodePacked(alpha0))); + + // Fr prevChallenge = alphas[0]; + // for (uint256 i = 1; i < NUMBER_OF_ALPHAS; i++) { + // prevChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(prevChallenge)))); + // alphas[i] = prevChallenge; + // } } function generateGateChallenges(Fr previousChallenge) @@ -158,7 +170,7 @@ library TranscriptLib { view returns (Fr[CONST_PROOF_SIZE_LOG_N] memory gateChallenges, Fr nextPreviousChallenge) { - for (uint256 i = 0; i < CONST_PROOF_SIZE_LOG_N; i++) { + for (uint256 i = 0; i < CONST_PROOF_SIZE_LOG_N / 2; i++) { previousChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(previousChallenge)))); Fr unused; (gateChallenges[i], unused) = splitChallenge(previousChallenge); From 5eaab1834b761af0795a4011be68f73a38336ee2 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 5 Sep 2024 16:53:09 +0000 Subject: [PATCH 02/10] split biggroup_goblin into a distinct class and renamed `biggroup` into `inner::biggroup`. `biggroup` typedef will use either `inner::biggroup` or `biggroup_goblin` depending on the curve parametrisation and circuit builder --- .../stdlib/primitives/biggroup/biggroup.hpp | 70 ++-- .../primitives/biggroup/biggroup.test.cpp | 33 +- .../biggroup/biggroup_batch_mul.hpp | 4 +- .../primitives/biggroup/biggroup_bn254.hpp | 4 +- .../biggroup/biggroup_edgecase_handling.hpp | 4 +- .../primitives/biggroup/biggroup_goblin.hpp | 307 +++++++++++++----- .../biggroup/biggroup_goblin_impl.hpp | 102 ++++++ .../primitives/biggroup/biggroup_impl.hpp | 162 ++++----- .../primitives/biggroup/biggroup_nafs.hpp | 4 +- .../biggroup/biggroup_secp256k1.hpp | 4 +- .../primitives/biggroup/biggroup_tables.hpp | 4 +- .../primitives/biggroup/goblin_field.hpp | 100 ++++++ .../stdlib/primitives/curves/bn254.hpp | 4 +- .../stdlib/primitives/databus/databus.hpp | 21 ++ .../primitives/field/field_conversion.hpp | 49 ++- .../protogalaxy_recursive_verifier.test.cpp | 2 +- .../recursive_decider_verification_key.hpp | 2 + .../translator_recursive_verifier.cpp | 4 +- .../translator_recursive_verifier.hpp | 5 +- 19 files changed, 623 insertions(+), 262 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp create mode 100644 barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index 83ce2a6c7de5..ceee35fe9cda 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -6,24 +6,32 @@ #include "../field/field.hpp" #include "../memory/rom_table.hpp" #include "../memory/twin_rom_table.hpp" +#include "./goblin_field.hpp" #include "barretenberg/ecc/curves/bn254/g1.hpp" #include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" - +#include "barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp" // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) If using a a circuit builder with Goblin, which is // designed to have efficient bb::g1 operations, a developer might accidentally write inefficient circuits // using biggroup functions that do not use the OpQueue. We use this concept to prevent compilation of such functions. +namespace bb::stdlib { template concept IsNotGoblinInefficiencyTrap = !(IsMegaBuilder && std::same_as); -namespace bb::stdlib { +template +concept IsGoblinBigGroup = + IsMegaBuilder && std::same_as> && + std::same_as> && std::same_as; + +} // namespace bb::stdlib +namespace bb::stdlib::element_inner { // ( ͡° ͜ʖ ͡°) template class element { public: using bool_ct = stdlib::bool_t; using biggroup_tag = element; // Facilitates a constexpr check IsBigGroup - + using BaseField = Fq; struct secp256k1_wnaf { std::vector> wnaf; field_t positive_skew; @@ -43,6 +51,13 @@ template class element { element(const element& other); element(element&& other) noexcept; + // static element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) + // { + // // only valid usecase of this method is with a goblin builder + // ASSERT(IsMegaBuilder); + // element out; + // if (in) + // } static element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) { element out; @@ -58,7 +73,11 @@ template class element { out.y = y; } out.set_point_at_infinity(witness_t(ctx, input.is_point_at_infinity())); - out.validate_on_curve(); + // info(" Num gates before validating on curve: ", ctx->num_gates); + if constexpr (!IsMegaBuilder) { + out.validate_on_curve(); + } + // info(" Num gates after validating on curve: ", ctx->num_gates); return out; } @@ -177,22 +196,13 @@ template class element { * We can chain repeated point additions together, where we only require 2 non-native field multiplications per * point addition, instead of 3 **/ - static chain_add_accumulator chain_add_start(const element& p1, const element& p2) - requires(IsNotGoblinInefficiencyTrap); - static chain_add_accumulator chain_add(const element& p1, const chain_add_accumulator& accumulator) - requires(IsNotGoblinInefficiencyTrap); - static element chain_add_end(const chain_add_accumulator& accumulator) - requires(IsNotGoblinInefficiencyTrap); - - element montgomery_ladder(const element& other) const - requires(IsNotGoblinInefficiencyTrap); - element montgomery_ladder(const chain_add_accumulator& accumulator) - requires(IsNotGoblinInefficiencyTrap); - element multiple_montgomery_ladder(const std::vector& to_add) const - requires(IsNotGoblinInefficiencyTrap); - - element quadruple_and_add(const std::vector& to_add) const - requires(IsNotGoblinInefficiencyTrap); + static chain_add_accumulator chain_add_start(const element& p1, const element& p2); + static chain_add_accumulator chain_add(const element& p1, const chain_add_accumulator& accumulator); + static element chain_add_end(const chain_add_accumulator& accumulator); + element montgomery_ladder(const element& other) const; + element montgomery_ladder(const chain_add_accumulator& accumulator); + element multiple_montgomery_ladder(const std::vector& to_add) const; + element quadruple_and_add(const std::vector& to_add) const; typename NativeGroup::affine_element get_value() const { @@ -222,12 +232,6 @@ template class element { const size_t max_num_bits = 0, const bool with_edgecases = false); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) max_num_bits is unused; could implement and use - // this to optimize other operations. - 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 // template @@ -938,16 +942,22 @@ template class element { typename std::conditional, batch_lookup_table_plookup<>, batch_lookup_table_base>::type; }; -template -concept IsBigGroup = std::is_same_v; - template inline std::ostream& operator<<(std::ostream& os, element const& v) { return os << "{ " << v.x << " , " << v.y << " }"; } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_inner +namespace bb::stdlib { +template +concept IsBigGroup = std::is_same_v; + +template +using element = std::conditional_t, + goblin_element, Fr, G>, + element_inner::element>; +} // namespace bb::stdlib #include "biggroup_batch_mul.hpp" #include "biggroup_bn254.hpp" #include "biggroup_goblin.hpp" diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp index 84359b972149..2c4a5d9640e0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.test.cpp @@ -16,6 +16,9 @@ namespace { auto& engine = numeric::get_debug_randomness(); } +template +concept HasGoblinBuilder = IsMegaBuilder; + // One can only define a TYPED_TEST with a single template paramter. // Our workaround is to pass parameters of the following type. template struct TestType { @@ -1191,9 +1194,6 @@ using TestTypes = testing::Types -concept HasGoblinBuilder = IsMegaBuilder; - TYPED_TEST(stdlib_biggroup, add) { @@ -1304,7 +1304,7 @@ HEAVY_TYPED_TEST(stdlib_biggroup, multiple_montgomery_ladder) HEAVY_TYPED_TEST(stdlib_biggroup, compute_naf) { // ULTRATODO: make this work for secp curves - if constexpr (TypeParam::Curve::type == CurveType::BN254) { + if constexpr ((TypeParam::Curve::type == CurveType::BN254) && !HasGoblinBuilder) { size_t num_repetitions = 1; for (size_t i = 0; i < num_repetitions; i++) { TestFixture::test_compute_naf(); @@ -1318,8 +1318,8 @@ HEAVY_TYPED_TEST(stdlib_biggroup, compute_naf) HEAVY_TYPED_TEST(stdlib_biggroup, wnaf_batch_mul) { if constexpr (HasPlookup) { - if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + if constexpr (TypeParam::Curve::type == CurveType::BN254 && HasGoblinBuilder) { + GTEST_SKIP(); } else { TestFixture::test_compute_wnaf(); }; @@ -1332,8 +1332,8 @@ HEAVY_TYPED_TEST(stdlib_biggroup, wnaf_batch_mul) HEAVY_TYPED_TEST(stdlib_biggroup, wnaf_batch_mul_edge_cases) { if constexpr (HasPlookup) { - if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + if constexpr (TypeParam::Curve::type == CurveType::BN254 && HasGoblinBuilder) { + GTEST_SKIP(); } else { TestFixture::test_compute_wnaf(); }; @@ -1346,7 +1346,8 @@ HEAVY_TYPED_TEST(stdlib_biggroup, wnaf_batch_mul_edge_cases) case where Fr is a bigfield. */ HEAVY_TYPED_TEST(stdlib_biggroup, compute_wnaf) { - if constexpr (!HasPlookup && TypeParam::use_bigfield) { + if constexpr ((!HasPlookup && TypeParam::use_bigfield) || + (TypeParam::Curve::type == CurveType::BN254 && HasGoblinBuilder)) { GTEST_SKIP(); } else { TestFixture::test_compute_wnaf(); @@ -1360,8 +1361,8 @@ HEAVY_TYPED_TEST(stdlib_biggroup, batch_mul_short_scalars) if constexpr (TypeParam::use_bigfield) { GTEST_SKIP(); } else { - if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + if constexpr (TypeParam::Curve::type == CurveType::BN254 && HasGoblinBuilder) { + GTEST_SKIP(); } else { TestFixture::test_batch_mul_short_scalars(); }; @@ -1372,8 +1373,8 @@ HEAVY_TYPED_TEST(stdlib_biggroup, wnaf_batch_mul_128_bit) if constexpr (TypeParam::use_bigfield) { GTEST_SKIP(); } else { - if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + if constexpr (TypeParam::Curve::type == CurveType::BN254 && HasGoblinBuilder) { + GTEST_SKIP(); } else { TestFixture::test_wnaf_batch_mul_128_bit(); }; @@ -1393,7 +1394,7 @@ HEAVY_TYPED_TEST(stdlib_biggroup, bn254_endo_batch_mul) { if constexpr (TypeParam::Curve::type == CurveType::BN254 && !TypeParam::use_bigfield) { if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + GTEST_SKIP(); } else { TestFixture::test_bn254_endo_batch_mul(); }; @@ -1405,7 +1406,7 @@ HEAVY_TYPED_TEST(stdlib_biggroup, mixed_mul_bn254_endo) { if constexpr (TypeParam::Curve::type == CurveType::BN254 && !TypeParam::use_bigfield) { if constexpr (HasGoblinBuilder) { - GTEST_SKIP() << "https://github.com/AztecProtocol/barretenberg/issues/707"; + GTEST_SKIP(); } else { TestFixture::test_mixed_mul_bn254_endo(); }; @@ -1438,4 +1439,4 @@ HEAVY_TYPED_TEST(stdlib_biggroup, ecdsa_mul_secp256k1) } else { GTEST_SKIP(); } -} +} \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp index f9b0598854d8..b3ee2c4728fa 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp @@ -2,7 +2,7 @@ #include "barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp" #include -namespace bb::stdlib { +namespace bb::stdlib::element_inner { /** * @brief Multiscalar multiplication that utilizes 4-bit wNAF lookup tables. @@ -62,4 +62,4 @@ element element::wnaf_batch_mul(const std::vector element::bn254_endo_batch_mul(const std::vec // Return our scalar mul output return accumulator; } -} // namespace bb::stdlib \ No newline at end of file +} // namespace bb::stdlib::element_inner \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp index 8dcfc6e1e3ad..a0e0da2115ea 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp @@ -1,7 +1,7 @@ #pragma once #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_inner { /** * @brief Compute an offset generator for use in biggroup tables @@ -100,4 +100,4 @@ std::pair>, std::vector> element concept IsNotGoblinInefficiencyTrap = +// !(IsMegaBuilder && std::same_as); + namespace bb::stdlib { -/** - * @brief Goblin style batch multiplication - * - * @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. - * @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 - * @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 - 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()); +// ( ͡° ͜ʖ ͡°) +template class goblin_element { + public: + using element = goblin_element; + using BaseField = Fq; + using bool_ct = stdlib::bool_t; + using biggroup_tag = goblin_element; // Facilitates a constexpr check IsBigGroup + + goblin_element() = default; + goblin_element(const typename NativeGroup::affine_element& input) + : x(input.x) + , y(input.y) + , _is_infinity(input.is_point_at_infinity()) + {} + goblin_element(const Fq& x, const Fq& y) + : x(x) + , y(y) + , _is_infinity(false) + {} + + goblin_element(const element& other) = default; + goblin_element(element&& other) noexcept = default; + goblin_element& operator=(const element& other) = default; + goblin_element& operator=(element&& other) = default; + // static element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) + // { + // // only valid usecase of this method is with a goblin builder + // ASSERT(IsMegaBuilder); + // element out; + // if (in) + // } + static element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) + { + element out; + if (input.is_point_at_infinity()) { + Fq x = Fq::from_witness(ctx, NativeGroup::affine_one.x); + Fq y = Fq::from_witness(ctx, NativeGroup::affine_one.y); + out.x = x; + out.y = y; + } else { + Fq x = Fq::from_witness(ctx, input.x); + Fq y = Fq::from_witness(ctx, input.y); + out.x = x; + out.y = y; } + out.set_point_at_infinity(witness_t(ctx, input.is_point_at_infinity())); + return out; + } + + void validate_on_curve() const + { + // happens in goblin eccvm + } + + static element one(Builder* ctx) + { + uint256_t x = uint256_t(NativeGroup::one.x); + uint256_t y = uint256_t(NativeGroup::one.y); + Fq x_fq(ctx, x); + Fq y_fq(ctx, y); + return element(x_fq, y_fq); + } + + // byte_array to_byte_array() const + // { + // byte_array result(get_context()); + // result.write(y.to_byte_array()); + // result.write(x.to_byte_array()); + // return result; + // } + + element checked_unconditional_add(const element& other) const { return element::operator+(*this, other); } + element checked_unconditional_subtract(const element& other) const { return element::operator-(*this, other); } - // Add constraints demonstrating that the EC point coordinates were decomposed faithfully. In particular, show - // that the lo-hi components that have been encoded in the op wires can be reconstructed via the limbs of the - // original point coordinates. - auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); - auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); - auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); - auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); - // Note: These constraints do not assume or enforce that the coordinates of the original point have been - // asserted to be in the field, only that they are less than the smallest power of 2 greater than the field - // modulus (a la the bigfield(lo, hi) constructor with can_overflow == false). - ASSERT(uint1024_t(point.x.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); - ASSERT(uint1024_t(point.y.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); - auto shift = Fr::from_witness(builder, Fq::shift_1); - x_lo.assert_equal(point.x.binary_basis_limbs[0].element + shift * point.x.binary_basis_limbs[1].element); - x_hi.assert_equal(point.x.binary_basis_limbs[2].element + shift * point.x.binary_basis_limbs[3].element); - y_lo.assert_equal(point.y.binary_basis_limbs[0].element + shift * point.y.binary_basis_limbs[1].element); - y_hi.assert_equal(point.y.binary_basis_limbs[2].element + shift * point.y.binary_basis_limbs[3].element); - - // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars - if (!scalar_is_constant_equal_one) { - auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); - auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); - auto beta = G::subgroup_field::cube_root_of_unity(); - scalar.assert_equal(z_1 - z_2 * beta); + element operator+(const element& other) const + { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize + // Current gate count: 6398 + return batch_mul({ *this, other }, { Fr(1), Fr(1) }); + } + element operator-(const element& other) const + { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize + std::vector points{ *this, other }; + return batch_mul({ *this, other }, { Fr(1), -Fr(1) }); + } + element operator-() const + { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize + return batch_mul({ *this }, { -Fr(1) }); + } + element operator+=(const element& other) + { + *this = *this + other; + return *this; + } + element operator-=(const element& other) + { + *this = *this - other; + return *this; + } + std::array checked_unconditional_add_sub(const element& other) const + { + return std::array{ *this + other, *this - other }; + } + + element operator*(const Fr& scalar) const { return batch_mul({ *this }, { scalar }); } + + element conditional_negate(const bool_ct& predicate) const + { + element negated = -(*this); + element result(*this); + result.y = Fq::conditional_assign(predicate, negated.y, result.y); + return result; + } + + element normalize() const + { + // no need to normalize, all goblin eccvm operations are returned normalized + return *this; + } + + element reduce() const + { + // no need to reduce, all goblin eccvm operations are returned normalized + return *this; + } + + element dbl() const { return batch_mul({ *this }, { 2 }); } + + // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) max_num_bits is unused; could implement and + // use this to optimize other operations. interface compatible with biggroup.hpp, the final parameter + // handle_edge_cases is not needed as this is always done in the eccvm + static element batch_mul(const std::vector& points, + const std::vector& scalars, + const size_t max_num_bits = 0, + const bool handle_edge_cases = false); + + // we use this data structure to add together a sequence of points. + // By tracking the previous values of x_1, y_1, \lambda, we can avoid + + typename NativeGroup::affine_element get_value() const + { + bb::fq x_val = x.get_value().lo; + bb::fq y_val = y.get_value().lo; + auto result = typename NativeGroup::affine_element(x_val, y_val); + if (is_point_at_infinity().get_value()) { + result.self_set_infinity(); } + return result; } - // Populate equality gates based on the internal accumulator point - auto op_tuple = builder->queue_ecc_eq(); + Builder* get_context() const + { + if (x.get_context() != nullptr) { + return x.get_context(); + } + if (y.get_context() != nullptr) { + return y.get_context(); + } + return nullptr; + } - // Reconstruct the result of the batch mul using indices into the variables array - auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); - auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); - auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); - auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); - Fq point_x(x_lo, x_hi); - Fq point_y(y_lo, y_hi); - element result = element(point_x, point_y); - if (op_tuple.return_is_infinity) { - result.set_point_at_infinity(bool_ct(builder, true)); - }; + Builder* get_context(const element& other) const + { + if (x.get_context() != nullptr) { + return x.get_context(); + } + if (y.get_context() != nullptr) { + return y.get_context(); + } + if (other.x.get_context() != nullptr) { + return other.x.get_context(); + } + if (other.y.get_context() != nullptr) { + return other.y.get_context(); + } + return nullptr; + } - return result; -} + bool_ct is_point_at_infinity() const { return _is_infinity; } + void set_point_at_infinity(const bool_ct& is_infinity) { _is_infinity = is_infinity; } + /** + * @brief Enforce x and y coordinates of a point to be (0,0) in the case of point at infinity + * + * @details We need to have a standard witness in Noir and the point at infinity can have non-zero random + * coefficients when we get it as output from our optimised algorithms. This function returns a (0,0) point, if + * it is a point at infinity + */ + element get_standard_form() const + { + const bool_ct is_infinity = is_point_at_infinity(); + element result(*this); + const Fq zero = Fq::zero(); + result.x = Fq::conditional_assign(is_infinity, zero, result.x); + result.y = Fq::conditional_assign(is_infinity, zero, result.y); + return result; + } + Fq x; + Fq y; + + private: + bool_ct _is_infinity; +}; + +template +inline std::ostream& operator<<(std::ostream& os, goblin_element const& v) +{ + return os << "{ " << v.x << " , " << v.y << " }"; +} } // namespace bb::stdlib + +#include "biggroup_goblin_impl.hpp" diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp new file mode 100644 index 000000000000..9889262d2677 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp @@ -0,0 +1,102 @@ +#pragma once + +#include "barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp" +namespace bb::stdlib { + +/** + * @brief Goblin style batch multiplication + * + * @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. + * @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 + * @tparam Fr Scalar field + * @tparam G Native group + * @param points + * @param scalars + * @param max_num_bits + * @return element + */ +template +goblin_element goblin_element::batch_mul(const std::vector& points, + const std::vector& scalars, + [[maybe_unused]] const size_t max_num_bits, + [[maybe_unused]] const bool handle_edge_cases) +{ + auto builder = points[0].get_context(); + + // Check that the internal accumulator is zero? + ASSERT(builder->op_queue->get_accumulator().is_point_at_infinity()); + + // Loop over all points and scalars + size_t num_points = points.size(); + for (size_t i = 0; i < num_points; ++i) { + auto& point = points[i]; + auto& scalar = scalars[i]; + + // Populate the goblin-style ecc op gates for the given mul inputs + ecc_op_tuple op_tuple; + bool scalar_is_constant_equal_one = scalar.get_witness_index() == IS_CONSTANT && scalar.get_value() == 1; + if (scalar_is_constant_equal_one) { // if scalar is 1, there is no need to perform a mul + op_tuple = builder->queue_ecc_add_accum(point.get_value()); + } else { // otherwise, perform a mul-then-accumulate + op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + } + + // Add constraints demonstrating that the EC point coordinates were decomposed faithfully. In particular, show + // that the lo-hi components that have been encoded in the op wires can be reconstructed via the limbs of the + // original point coordinates. + auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); + auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); + auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); + auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); + // Note: These constraints do not assume or enforce that the coordinates of the original point have been + // asserted to be in the field, only that they are less than the smallest power of 2 greater than the field + // modulus (a la the bigfield(lo, hi) constructor with can_overflow == false). + ASSERT(uint1024_t(point.x.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); + ASSERT(uint1024_t(point.y.get_maximum_value()) <= Fq::DEFAULT_MAXIMUM_REMAINDER); + x_lo.assert_equal(point.x.limbs[0]); + x_hi.assert_equal(point.x.limbs[1]); + y_lo.assert_equal(point.y.limbs[0]); + y_hi.assert_equal(point.y.limbs[1]); + + // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars + if (!scalar_is_constant_equal_one) { + auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); + auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); + auto beta = G::subgroup_field::cube_root_of_unity(); + scalar.assert_equal(z_1 - z_2 * beta); + } + } + + // Populate equality gates based on the internal accumulator point + auto op_tuple = builder->queue_ecc_eq(); + + // Reconstruct the result of the batch mul using indices into the variables array + auto x_lo = Fr::from_witness_index(builder, op_tuple.x_lo); + auto x_hi = Fr::from_witness_index(builder, op_tuple.x_hi); + auto y_lo = Fr::from_witness_index(builder, op_tuple.y_lo); + auto y_hi = Fr::from_witness_index(builder, op_tuple.y_hi); + Fq point_x(x_lo, x_hi); + Fq point_y(y_lo, y_hi); + element result = element(point_x, point_y); + + // NOTE: we can have an `if` statement here under the strict assumption that `return_is_infinity` + // is produced from `eq_and_reset` opcode + if (op_tuple.return_is_infinity) { + result.set_point_at_infinity(bool_ct(builder, true)); + }; + + return result; +} + +} // namespace bb::stdlib diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index 38d2c020c044..4d7492351021 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -4,7 +4,7 @@ #include "../circuit_builders/circuit_builders.hpp" #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_inner { template element::element() @@ -68,14 +68,6 @@ element& element::operator=(element&& other) noexcep template element element::operator+(const element& other) const { - // return checked_unconditional_add(other); - if constexpr (IsMegaBuilder && std::same_as) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - // Current gate count: 6398 - std::vector points{ *this, other }; - std::vector scalars{ 1, 1 }; - return batch_mul(points, scalars); - } // Adding in `x_coordinates_match` ensures that lambda will always be well-formed // Our curve has the form y^2 = x^3 + b. @@ -149,14 +141,6 @@ element element::get_standard_form() const template element element::operator-(const element& other) const { - // return checked_unconditional_add(other); - if constexpr (IsMegaBuilder && std::same_as) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - // Current gate count: 6398 - std::vector points{ *this, other }; - std::vector scalars{ 1, -Fr(1) }; - return batch_mul(points, scalars); - } // if x_coordinates match, lambda triggers a divide by zero error. // Adding in `x_coordinates_match` ensures that lambda will always be well-formed @@ -208,14 +192,6 @@ element element::operator-(const element& other) con template element element::checked_unconditional_add(const element& other) const { - if constexpr (IsMegaBuilder && std::same_as) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - // Current gate count: 6398 - std::vector points{ *this, other }; - std::vector scalars{ 1, 1 }; - return goblin_batch_mul(points, scalars); - } - other.x.assert_is_not_equal(x); const Fq lambda = Fq::div_without_denominator_check({ other.y, -y }, (other.x - x)); const Fq x3 = lambda.sqradd({ -other.x, -x }); @@ -226,12 +202,6 @@ element element::checked_unconditional_add(const ele template element element::checked_unconditional_subtract(const element& other) const { - if constexpr (IsMegaBuilder && std::same_as) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - std::vector points{ *this, other }; - std::vector scalars{ 1, -Fr(1) }; - return goblin_batch_mul(points, scalars); - } other.x.assert_is_not_equal(x); const Fq lambda = Fq::div_without_denominator_check({ other.y, y }, (other.x - x)); @@ -259,9 +229,6 @@ element element::checked_unconditional_subtract(cons template std::array, 2> element::checked_unconditional_add_sub(const element& other) const { - if constexpr (IsMegaBuilder && std::same_as) { - return { *this + other, *this - other }; - } // TODO(https://github.com/AztecProtocol/barretenberg/issues/971): This will fail when the two elements are the same // even in the case of a valid circuit @@ -320,7 +287,6 @@ template element element template typename element::chain_add_accumulator element::chain_add_start(const element& p1, const element& p2) - requires(IsNotGoblinInefficiencyTrap) { chain_add_accumulator output; output.x1_prev = p1.x; @@ -338,7 +304,6 @@ typename element::chain_add_accumulator element::cha template typename element::chain_add_accumulator element::chain_add(const element& p1, const chain_add_accumulator& acc) - requires(IsNotGoblinInefficiencyTrap) { // use `chain_add_start` to start an addition chain (i.e. if acc has a y-coordinate) if (acc.is_element) { @@ -382,7 +347,6 @@ typename element::chain_add_accumulator element::cha **/ template element element::chain_add_end(const chain_add_accumulator& acc) - requires(IsNotGoblinInefficiencyTrap) { if (acc.is_element) { return element(acc.x3_prev, acc.y3_prev); @@ -436,7 +400,6 @@ element element::chain_add_end(const chain_add_accum **/ template element element::montgomery_ladder(const element& other) const - requires(IsNotGoblinInefficiencyTrap) { other.x.assert_is_not_equal(x); const Fq lambda_1 = Fq::div_without_denominator_check({ other.y - y }, (other.x - x)); @@ -473,7 +436,6 @@ element element::montgomery_ladder(const element& ot **/ template element element::montgomery_ladder(const chain_add_accumulator& to_add) - requires(IsNotGoblinInefficiencyTrap) { if (to_add.is_element) { throw_or_abort("An accumulator expected"); @@ -517,7 +479,6 @@ element element::montgomery_ladder(const chain_add_a */ template element element::quadruple_and_add(const std::vector& to_add) const - requires(IsNotGoblinInefficiencyTrap) { const Fq two_x = x + x; Fq x_1; @@ -613,7 +574,6 @@ element element::quadruple_and_add(const std::vector template element element::multiple_montgomery_ladder( const std::vector& add) const - requires(IsNotGoblinInefficiencyTrap) { struct composite_y { std::vector mul_left; @@ -803,55 +763,51 @@ element element::batch_mul(const std::vector && std::same_as) { - return goblin_batch_mul(points, scalars); - } else { - if (with_edgecases) { - std::tie(points, scalars) = mask_points(points, scalars); - } - const size_t num_points = points.size(); - ASSERT(scalars.size() == num_points); + if (with_edgecases) { + std::tie(points, scalars) = mask_points(points, scalars); + } + 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; + 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)); + 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); - } - 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); + to_add.emplace_back(point_table.get_chain_add_accumulator(nafs)); } - 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; } } @@ -887,32 +843,26 @@ element element::operator*(const Fr& scalar) const * **/ - if constexpr (IsMegaBuilder && std::same_as) { - std::vector points{ *this }; - std::vector scalars{ scalar }; - return goblin_batch_mul(points, scalars); - } else { - constexpr uint64_t num_rounds = Fr::modulus.get_msb() + 1; + constexpr uint64_t num_rounds = Fr::modulus.get_msb() + 1; - std::vector naf_entries = compute_naf(scalar); + std::vector naf_entries = compute_naf(scalar); - const auto offset_generators = compute_offset_generators(num_rounds); + const auto offset_generators = compute_offset_generators(num_rounds); - element accumulator = *this + offset_generators.first; + element accumulator = *this + offset_generators.first; - for (size_t i = 1; i < num_rounds; ++i) { - bool_ct predicate = naf_entries[i]; - bigfield y_test = y.conditional_negate(predicate); - element to_add(x, y_test); - accumulator = accumulator.montgomery_ladder(to_add); - } + for (size_t i = 1; i < num_rounds; ++i) { + bool_ct predicate = naf_entries[i]; + bigfield y_test = y.conditional_negate(predicate); + element to_add(x, y_test); + accumulator = accumulator.montgomery_ladder(to_add); + } - element skew_output = accumulator - (*this); + element skew_output = accumulator - (*this); - Fq out_x = accumulator.x.conditional_select(skew_output.x, naf_entries[num_rounds]); - Fq out_y = accumulator.y.conditional_select(skew_output.y, naf_entries[num_rounds]); + Fq out_x = accumulator.x.conditional_select(skew_output.x, naf_entries[num_rounds]); + Fq out_y = accumulator.y.conditional_select(skew_output.y, naf_entries[num_rounds]); - return element(out_x, out_y) - element(offset_generators.second); - } + return element(out_x, out_y) - element(offset_generators.second); } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_inner diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp index 40bd689f9fca..f4f2afe404bb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp @@ -2,7 +2,7 @@ #include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_inner { /** * Split a secp256k1 Fr element into two 129 bit scalars `klo, khi`, where `scalar = klo + \lambda * khi mod n` @@ -583,4 +583,4 @@ std::vector> element::compute_naf(const Fr& scalar, cons } return naf_entries; } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_inner diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp index 15ff2a9afba4..ccb3c83281f0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp @@ -6,7 +6,7 @@ * **/ #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_inner { template template @@ -140,4 +140,4 @@ element element::secp256k1_ecdsa_mul(const element& return accumulator; } -} // namespace bb::stdlib \ No newline at end of file +} // namespace bb::stdlib::element_inner \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp index 14effea1de38..c49a9a62745b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp @@ -2,7 +2,7 @@ #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" #include "barretenberg/stdlib/primitives/memory/twin_rom_table.hpp" #include "barretenberg/stdlib_circuit_builders/plookup_tables/types.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_inner { using plookup::MultiTableId; @@ -644,4 +644,4 @@ element element::lookup_table_base::get( } return element::one(bits[0].get_context()); } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_inner diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp new file mode 100644 index 000000000000..22ffa1ac582a --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp @@ -0,0 +1,100 @@ +#pragma once +#include "../bigfield/bigfield.hpp" +#include "../circuit_builders/circuit_builders_fwd.hpp" +#include "../field/field.hpp" + +namespace bb::stdlib { +template class goblin_field { + public: + static constexpr uint1024_t DEFAULT_MAXIMUM_REMAINDER = + bigfield::DEFAULT_MAXIMUM_REMAINDER; + static constexpr size_t NUM_LIMBS = bigfield::NUM_LIMBS; + static constexpr size_t NUM_LIMB_BITS = bigfield::NUM_LIMB_BITS; + static constexpr size_t NUM_LAST_LIMB_BITS = bigfield::NUM_LAST_LIMB_BITS; + + using field_ct = stdlib::field_t; + using bool_ct = stdlib::bool_t; + std::array limbs; + + goblin_field() + : limbs{ 0, 0 } + {} + + goblin_field(Builder* parent_context, const uint256_t& value) + { + (*this) = goblin_field(bb::fq(value)); + limbs[0].context = parent_context; + limbs[1].context = parent_context; + } + goblin_field(bb::fq input) + { + uint256_t converted(input); + uint256_t lo_v = converted.slice(0, 136); + uint256_t hi_v = converted.slice(136, 254); + limbs = { bb::fr(lo_v), bb::fr(hi_v) }; + } + goblin_field(field_ct lo, field_ct hi) + : limbs{ lo, hi } + {} + + // N.B. this method is because AggregationState expects group element coordinates to be split into 4 slices + // (we could update to only use 2 for Mega but that feels complex) + goblin_field(field_ct lolo, field_ct lohi, field_ct hilo, field_ct hihi, [[maybe_unused]] bool can_overflow = false) + : limbs{ lolo + lohi * (uint256_t(1) << 68), hilo + hihi * (uint256_t(1) << 68) } + {} + + void assert_equal(const goblin_field& other) const + { + limbs[0].assert_equal(other.limbs[0]); + limbs[1].assert_equal(other.limbs[1]); + } + static goblin_field zero() { return goblin_field{ 0, 0 }; } + + static goblin_field from_witness(Builder* ctx, bb::fq input) + { + uint256_t converted(input); + uint256_t lo_v = converted.slice(0, 136); + uint256_t hi_v = converted.slice(136, 254); + field_ct lo = field_ct::from_witness(ctx, lo_v); + field_ct hi = field_ct::from_witness(ctx, hi_v); + return goblin_field(lo, hi); + } + + static goblin_field conditional_assign(const bool_ct& predicate, const goblin_field& lhs, goblin_field& rhs) + { + goblin_field result; + result.limbs = { + field_ct::conditional_assign(predicate, lhs.limbs[0], rhs.limbs[0]), + field_ct::conditional_assign(predicate, lhs.limbs[1], rhs.limbs[1]), + }; + return result; + } + + // matches the interface for bigfield + uint512_t get_value() const + { + uint256_t lo = limbs[0].get_value(); + uint256_t hi = limbs[1].get_value(); + uint256_t result = lo + (hi << 136); + return result; + } + + // matches the interface for bigfield + uint512_t get_maximum_value() const { return (*this).get_value(); } + + Builder* get_context() const + { + if (limbs[0].get_context()) { + return limbs[0].get_context(); + } + return limbs[1].get_context(); + } + + // done in the translator circuit + void assert_is_in_field(){}; +}; +template inline std::ostream& operator<<(std::ostream& os, goblin_field const& v) +{ + return os << "{ " << v.limbs[0] << " , " << v.limbs[1] << " }"; +} +} // namespace bb::stdlib \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/bn254.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/bn254.hpp index 83b8f3b70b13..c15d39d586d3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/bn254.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/curves/bn254.hpp @@ -25,8 +25,8 @@ template struct bn254 { // Note: its useful to have these type names match the native analog exactly so that components that digest a Curve // (e.g. Gemini) can be agnostic as to whether they're operating on native or stdlib types. using ScalarField = field_t; - using BaseField = bigfield; - using Group = element; + using Group = element, ScalarField, GroupNative>; + using BaseField = Group::BaseField; using Element = Group; using AffineElement = Group; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp index b8bc065e463e..b39ac2db717b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp @@ -224,6 +224,7 @@ template class DataBusDepot { * @return std::array */ std::array get_witness_indices_for_commitment(const Commitment& point) + requires(!IsMegaBuilder) { return { point.x.binary_basis_limbs[0].element.normalize().witness_index, point.x.binary_basis_limbs[1].element.normalize().witness_index, @@ -234,6 +235,26 @@ template class DataBusDepot { point.y.binary_basis_limbs[2].element.normalize().witness_index, point.y.binary_basis_limbs[3].element.normalize().witness_index }; } + + std::array get_witness_indices_for_commitment(const Commitment& point) + requires(IsMegaBuilder) + { + using BigFq = stdlib::bigfield; + const auto to_bigfield = [](Fr lo, Fr hi) { + BigFq r(lo, hi); + return std::array{ + r.binary_basis_limbs[0].element.normalize().witness_index, + r.binary_basis_limbs[1].element.normalize().witness_index, + r.binary_basis_limbs[2].element.normalize().witness_index, + r.binary_basis_limbs[3].element.normalize().witness_index, + }; + }; + auto x = to_bigfield(point.x.limbs[0], point.x.limbs[1]); + auto y = to_bigfield(point.y.limbs[0], point.y.limbs[1]); + return { + x[0], x[1], x[2], x[3], y[0], y[1], y[2], y[3], + }; + } }; } // namespace bb::stdlib \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index a1ca8703c6ba..85cf813cf2d2 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -2,6 +2,7 @@ #include "barretenberg/polynomials/univariate.hpp" #include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp" +#include "barretenberg/stdlib/primitives/biggroup/goblin_field.hpp" #include "barretenberg/stdlib/primitives/curves/bn254.hpp" #include "barretenberg/stdlib/primitives/field/field.hpp" #include "barretenberg/stdlib/primitives/group/cycle_group.hpp" @@ -27,6 +28,15 @@ template inline T convert_challenge(Builder& buil } } +template +inline std::vector> convert_goblin_fr_to_bn254_frs(const goblin_field& input) +{ + std::vector> result(2); + result[0] = input.limbs[0]; + result[1] = input.limbs[1]; + return result; +} + template inline std::vector> convert_grumpkin_fr_to_bn254_frs(const fq& input) { fr shift(static_cast(1) << NUM_LIMB_BITS); @@ -48,10 +58,11 @@ template constexpr size_t calc_num_bn254_frs() { if constexpr (IsAnyOf>) { return Bn254FrParams::NUM_BN254_SCALARS; - } else if constexpr (IsAnyOf>) { + } else if constexpr (IsAnyOf> || IsAnyOf>) { return Bn254FqParams::NUM_BN254_SCALARS; } else if constexpr (IsAnyOf>) { - return 2 * calc_num_bn254_frs>(); + using BaseField = bn254_element::BaseField; + return 2 * calc_num_bn254_frs(); } else if constexpr (IsAnyOf>) { return 2 * calc_num_bn254_frs>(); } else { @@ -82,18 +93,42 @@ template T convert_from_bn254_frs(Builder& builde ASSERT(fr_vec.size() == 2); fq result(fr_vec[0], fr_vec[1]); return result; + } else if constexpr (IsAnyOf>) { + ASSERT(fr_vec.size() == 2); + goblin_field result(fr_vec[0], fr_vec[1]); + return result; } else if constexpr (IsAnyOf>) { - using BaseField = fq; + using BaseField = bn254_element::BaseField; constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); ASSERT(fr_vec.size() == 2 * BASE_FIELD_SCALAR_SIZE); bn254_element result; + result.x = convert_from_bn254_frs(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); result.y = convert_from_bn254_frs( builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); - result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && - fr_vec[3].is_zero()); + auto sum = fr_vec[0].add_two(fr_vec[1], fr_vec[2]); + sum = sum + fr_vec[3]; + result.set_point_at_infinity(sum.is_zero()); return result; + + // we know that all Field elements are constrained to 136 bits. + // therefore, if we add them all together, the only way the sum is zero is if all are zero + // 19 gates to 6 gates + // is zero + + // A * is_zero = 0 + // (1 - is_zero) * (A * I - 1) = 0 + // AI - 1 - is_zero * AI + is_zero = 0 + + // AZ + AI - 1 - ZAI + Z = 0 + + // Z + I - ZI = t0 + // A * t0 + Z - 1 = 0 + // we can reduce to 3 gates (2.25 if we fix bool checks) + // result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && + // fr_vec[3].is_zero()); + // return result; } else if constexpr (IsAnyOf>) { using BaseField = fr; constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); @@ -136,8 +171,10 @@ template std::vector> convert_to_bn25 return fr_vec; } else if constexpr (IsAnyOf>) { return convert_grumpkin_fr_to_bn254_frs(val); + } else if constexpr (IsAnyOf>) { + return convert_goblin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf>) { - using BaseField = fq; + using BaseField = bn254_element::BaseField; auto fr_vec_x = convert_to_bn254_frs(val.x); auto fr_vec_y = convert_to_bn254_frs(val.y); std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp index dfcc4690a34f..6e784dceea8a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp @@ -63,7 +63,7 @@ template class ProtogalaxyRecursiveTests : public tes static void create_function_circuit(InnerBuilder& builder, size_t log_num_gates = 10) { using fr_ct = typename InnerCurve::ScalarField; - using fq_ct = typename InnerCurve::BaseField; + using fq_ct = stdlib::bigfield; using public_witness_ct = typename InnerCurve::public_witness_ct; using witness_ct = typename InnerCurve::witness_ct; using byte_array_ct = typename InnerCurve::byte_array_ct; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp index 9f08c708adce..625a63db57bc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp @@ -4,6 +4,8 @@ #include "barretenberg/relations/relation_parameters.hpp" #include "barretenberg/ultra_honk/decider_verification_key.hpp" +#include "barretenberg/stdlib/primitives/field/field_conversion.hpp" +#include "barretenberg/sumcheck/instance/verifier_instance.hpp" namespace bb::stdlib::recursion::honk { /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.cpp index ea06cfbdbeb9..234122e1ec32 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.cpp @@ -134,7 +134,9 @@ std::array TranslatorRecursiveVerifier_ bool TranslatorRecursiveVerifier_::verify_translation( - const TranslationEvaluations_& translation_evaluations) + const TranslationEvaluations_< + typename stdlib::bigfield, + typename Flavor::FF>& translation_evaluations) { const auto reconstruct_from_array = [&](const auto& arr) { const BF reconstructed = BF(arr[0], arr[1], arr[2], arr[3]); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp index d221bec73603..364e63c63de6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp @@ -10,14 +10,15 @@ namespace bb { template class TranslatorRecursiveVerifier_ { public: using FF = typename Flavor::FF; - using BF = typename Flavor::BF; using NativeBF = typename Flavor::Curve::BaseFieldNative; + using Builder = typename Flavor::CircuitBuilder; + using BF = typename stdlib::bigfield; + using Commitment = typename Flavor::Commitment; using GroupElement = typename Flavor::GroupElement; using VerificationKey = typename Flavor::VerificationKey; using NativeVerificationKey = typename Flavor::NativeVerificationKey; using VerifierCommitmentKey = typename Flavor::VerifierCommitmentKey; - using Builder = typename Flavor::CircuitBuilder; using RelationSeparator = typename Flavor::RelationSeparator; using PairingPoints = std::array; using TranslationEvaluations = TranslationEvaluations_; From a18be4fb987134bc0f950f7ae0def2819d7cdb6d Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 5 Sep 2024 17:49:56 +0000 Subject: [PATCH 03/10] compiler fixes, namespace changes --- .../stdlib/primitives/biggroup/biggroup.hpp | 12 +-- .../biggroup/biggroup_batch_mul.hpp | 4 +- .../primitives/biggroup/biggroup_bn254.hpp | 37 +++++---- .../biggroup/biggroup_edgecase_handling.hpp | 16 ++-- .../primitives/biggroup/biggroup_goblin.hpp | 79 ++++++++++--------- .../biggroup/biggroup_goblin_impl.hpp | 6 +- .../primitives/biggroup/biggroup_impl.hpp | 34 ++++---- .../primitives/biggroup/biggroup_nafs.hpp | 32 ++++---- .../biggroup/biggroup_secp256k1.hpp | 17 ++-- .../primitives/biggroup/biggroup_tables.hpp | 4 +- .../circuit_simulator.hpp | 1 + 11 files changed, 128 insertions(+), 114 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index ceee35fe9cda..8125da0e2c83 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -24,7 +24,7 @@ concept IsGoblinBigGroup = std::same_as> && std::same_as; } // namespace bb::stdlib -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { // ( ͡° ͜ʖ ͡°) template class element { @@ -233,8 +233,8 @@ template class element { const bool with_edgecases = false); // 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 - // template + // This is a bit tricky to do with `std::enable_if`, because `bn254_endo_batch_mul` is a member function of a + // class template // && the compiler can't perform partial template specialization on member functions of class templates // => our template parameter cannot be a value but must instead by a type // Our input to `std::enable_if` is a comparison between two types (NativeGroup and bb::g1), which @@ -947,7 +947,7 @@ inline std::ostream& operator<<(std::ostream& os, element const& v { return os << "{ " << v.x << " , " << v.y << " }"; } -} // namespace bb::stdlib::element_inner +} // namespace bb::stdlib::element_default namespace bb::stdlib { template @@ -955,8 +955,8 @@ concept IsBigGroup = std::is_same_v; template using element = std::conditional_t, - goblin_element, Fr, G>, - element_inner::element>; + element_goblin::goblin_element, Fr, G>, + element_default::element>; } // namespace bb::stdlib #include "biggroup_batch_mul.hpp" #include "biggroup_bn254.hpp" diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp index b3ee2c4728fa..31d7e77f1b60 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_batch_mul.hpp @@ -2,7 +2,7 @@ #include "barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp" #include -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { /** * @brief Multiscalar multiplication that utilizes 4-bit wNAF lookup tables. @@ -62,4 +62,4 @@ element element::wnaf_batch_mul(const std::vector element::bn254_endo_batch_mul_with_generator }; // Perform multiple rounds of the montgomery ladder algoritm per "iteration" of our main loop. - // This is in order to reduce the number of field reductions required when calling `multiple_montgomery_ladder` + // This is in order to reduce the number of field reductions required when calling + // `multiple_montgomery_ladder` constexpr size_t num_rounds_per_iteration = 4; // we require that we perform max of one generator per iteration @@ -213,8 +214,8 @@ element element::bn254_endo_batch_mul_with_generator * small_points : group elements we will multiply by short scalar mutipliers whose max value will be (1 << *max_num_small_bits) small_scalars : short scalar mutipliers whose max value will be (1 << max_num_small_bits) * max_num_small_bits : MINIMUM value must be 128 bits - * (we will be splitting `big_scalars` into two 128-bit scalars, we assume all scalars after this transformation are 128 - *bits) + * (we will be splitting `big_scalars` into two 128-bit scalars, we assume all scalars after this transformation are + *128 bits) **/ template template @@ -311,10 +312,10 @@ element element::bn254_endo_batch_mul(const std::vec /** * Compute scalar multiplier NAFs * - * A Non Adjacent Form is a representation of an integer where each 'bit' is either +1 OR -1, i.e. each bit entry is - *non-zero. This is VERY useful for biggroup operations, as this removes the need to conditionally add points - *depending on whether the scalar mul bit is +1 or 0 (instead we multiply the y-coordinate by the NAF value, which - *is cheaper) + * A Non Adjacent Form is a representation of an integer where each 'bit' is either +1 OR -1, i.e. each bit + *entry is non-zero. This is VERY useful for biggroup operations, as this removes the need to conditionally add + *points depending on whether the scalar mul bit is +1 or 0 (instead we multiply the y-coordinate by the NAF + *value, which is cheaper) * * The vector `naf_entries` tracks the `naf` set for each point, where each `naf` set is a vector of bools * if `naf[i][j] = 0` this represents a NAF value of -1 @@ -328,14 +329,15 @@ element element::bn254_endo_batch_mul(const std::vec } /** - * Initialize accumulator point with an offset generator. See `compute_offset_generators` for detailed explanation + * Initialize accumulator point with an offset generator. See `compute_offset_generators` for detailed + *explanation **/ const auto offset_generators = compute_offset_generators(num_rounds); /** * Get the initial entry of our point table. This is the same as point_table.get_accumulator for the most - *significant NAF entry. HOWEVER, we know the most significant NAF value is +1 because our scalar muls are positive. - * `get_initial_entry` handles this special case as it's cheaper than `point_table.get_accumulator` + *significant NAF entry. HOWEVER, we know the most significant NAF value is +1 because our scalar muls are + *positive. `get_initial_entry` handles this special case as it's cheaper than `point_table.get_accumulator` **/ element accumulator = offset_generators.first + point_table.get_initial_entry(); @@ -349,10 +351,11 @@ element element::bn254_endo_batch_mul(const std::vec * 3. Repeat the above 2 steps but for bit `2 * i` (`add_2`) * 4. Compute `accumulator = 4 * accumulator + 2 * add_1 + add_2` using `multiple_montgomery_ladder` method * - * The purpose of the above is to minimize the number of required range checks (vs a simple double and add algo). + * The purpose of the above is to minimize the number of required range checks (vs a simple double and add + *algo). * - * When computing repeated iterations of the montgomery ladder algorithm, we can neglect computing the y-coordinate - *of each ladder output. See `multiple_montgomery_ladder` for more details. + * When computing repeated iterations of the montgomery ladder algorithm, we can neglect computing the + *y-coordinate of each ladder output. See `multiple_montgomery_ladder` for more details. **/ for (size_t i = 1; i < num_rounds / 2; ++i) { // `nafs` tracks the naf value for each point for the current round @@ -370,8 +373,8 @@ element element::bn254_endo_batch_mul(const std::vec * * This is represented using the `chain_add_accumulator` type. See the type declaration for more details * - * (this is cheaper than regular additions iff point_table.get_accumulator require 2 or more point additions. - * Cost is the same as `point_table.get_accumulator` if 1 or 0 point additions are required) + * (this is cheaper than regular additions iff point_table.get_accumulator require 2 or more point + *additions. Cost is the same as `point_table.get_accumulator` if 1 or 0 point additions are required) **/ element::chain_add_accumulator add_1 = point_table.get_chain_add_accumulator(nafs); for (size_t j = 0; j < points.size(); ++j) { @@ -424,4 +427,4 @@ element element::bn254_endo_batch_mul(const std::vec // Return our scalar mul output return accumulator; } -} // namespace bb::stdlib::element_inner \ No newline at end of file +} // namespace bb::stdlib::element_default \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp index a0e0da2115ea..15a73c800861 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp @@ -1,14 +1,14 @@ #pragma once #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { /** * @brief Compute an offset generator for use in biggroup tables * *@details Sometimes the points from which we construct the tables are going to be dependent in such a way that - *combining them for constructing the table is not possible without handling the edgecases such as the point at infinity - *and doubling. To avoid handling those we add multiples of this offset generator to the points. + *combining them for constructing the table is not possible without handling the edgecases such as the point at + *infinity and doubling. To avoid handling those we add multiples of this offset generator to the points. * * @param num_rounds */ @@ -22,11 +22,11 @@ typename G::affine_element element::compute_table_offset_generator } /** - * @brief Given two lists of points that need to be multiplied by scalars, create a new list of length +1 with original - * points masked, but the same scalar product sum + * @brief Given two lists of points that need to be multiplied by scalars, create a new list of length +1 with + * original points masked, but the same scalar product sum * @details Add +1G, +2G, +4G etc to the original points and adds a new point 2ⁿ⋅G and scalar x to the lists. By - * doubling the point every time, we ensure that no +-1 combination of 6 sequential elements run into edgecases, unless - * the points are deliberately constructed to trigger it. + * doubling the point every time, we ensure that no +-1 combination of 6 sequential elements run into edgecases, + * unless the points are deliberately constructed to trigger it. */ template std::pair>, std::vector> element::mask_points( @@ -100,4 +100,4 @@ std::pair>, std::vector> element concept IsNotGoblinInefficiencyTrap = // !(IsMegaBuilder && std::same_as); -namespace bb::stdlib { +namespace bb::stdlib::element_goblin { // ( ͡° ͜ʖ ͡°) template class goblin_element { public: - using element = goblin_element; using BaseField = Fq; using bool_ct = stdlib::bool_t; using biggroup_tag = goblin_element; // Facilitates a constexpr check IsBigGroup @@ -38,20 +37,20 @@ template class goblin_ele , _is_infinity(false) {} - goblin_element(const element& other) = default; - goblin_element(element&& other) noexcept = default; - goblin_element& operator=(const element& other) = default; - goblin_element& operator=(element&& other) = default; - // static element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) + goblin_element(const goblin_element& other) = default; + goblin_element(goblin_element&& other) noexcept = default; + goblin_element& operator=(const goblin_element& other) = default; + goblin_element& operator=(goblin_element&& other) = default; + // static goblin_element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) // { // // only valid usecase of this method is with a goblin builder // ASSERT(IsMegaBuilder); - // element out; + // goblin_element out; // if (in) // } - static element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) + static goblin_element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) { - element out; + goblin_element out; if (input.is_point_at_infinity()) { Fq x = Fq::from_witness(ctx, NativeGroup::affine_one.x); Fq y = Fq::from_witness(ctx, NativeGroup::affine_one.y); @@ -72,13 +71,13 @@ template class goblin_ele // happens in goblin eccvm } - static element one(Builder* ctx) + static goblin_element one(Builder* ctx) { uint256_t x = uint256_t(NativeGroup::one.x); uint256_t y = uint256_t(NativeGroup::one.y); Fq x_fq(ctx, x); Fq y_fq(ctx, y); - return element(x_fq, y_fq); + return goblin_element(x_fq, y_fq); } // byte_array to_byte_array() const @@ -89,72 +88,78 @@ template class goblin_ele // return result; // } - element checked_unconditional_add(const element& other) const { return element::operator+(*this, other); } - element checked_unconditional_subtract(const element& other) const { return element::operator-(*this, other); } + goblin_element checked_unconditional_add(const goblin_element& other) const + { + return goblin_element::operator+(*this, other); + } + goblin_element checked_unconditional_subtract(const goblin_element& other) const + { + return goblin_element::operator-(*this, other); + } - element operator+(const element& other) const + goblin_element operator+(const goblin_element& other) const { // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize // Current gate count: 6398 return batch_mul({ *this, other }, { Fr(1), Fr(1) }); } - element operator-(const element& other) const + goblin_element operator-(const goblin_element& other) const { // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - std::vector points{ *this, other }; + std::vector points{ *this, other }; return batch_mul({ *this, other }, { Fr(1), -Fr(1) }); } - element operator-() const + goblin_element operator-() const { // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize return batch_mul({ *this }, { -Fr(1) }); } - element operator+=(const element& other) + goblin_element operator+=(const goblin_element& other) { *this = *this + other; return *this; } - element operator-=(const element& other) + goblin_element operator-=(const goblin_element& other) { *this = *this - other; return *this; } - std::array checked_unconditional_add_sub(const element& other) const + std::array checked_unconditional_add_sub(const goblin_element& other) const { - return std::array{ *this + other, *this - other }; + return std::array{ *this + other, *this - other }; } - element operator*(const Fr& scalar) const { return batch_mul({ *this }, { scalar }); } + goblin_element operator*(const Fr& scalar) const { return batch_mul({ *this }, { scalar }); } - element conditional_negate(const bool_ct& predicate) const + goblin_element conditional_negate(const bool_ct& predicate) const { - element negated = -(*this); - element result(*this); + goblin_element negated = -(*this); + goblin_element result(*this); result.y = Fq::conditional_assign(predicate, negated.y, result.y); return result; } - element normalize() const + goblin_element normalize() const { // no need to normalize, all goblin eccvm operations are returned normalized return *this; } - element reduce() const + goblin_element reduce() const { // no need to reduce, all goblin eccvm operations are returned normalized return *this; } - element dbl() const { return batch_mul({ *this }, { 2 }); } + goblin_element dbl() const { return batch_mul({ *this }, { 2 }); } // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) max_num_bits is unused; could implement and // use this to optimize other operations. interface compatible with biggroup.hpp, the final parameter // handle_edge_cases is not needed as this is always done in the eccvm - static element batch_mul(const std::vector& points, - const std::vector& scalars, - const size_t max_num_bits = 0, - const bool handle_edge_cases = false); + static goblin_element batch_mul(const std::vector& points, + const std::vector& scalars, + const size_t max_num_bits = 0, + const bool handle_edge_cases = false); // we use this data structure to add together a sequence of points. // By tracking the previous values of x_1, y_1, \lambda, we can avoid @@ -181,7 +186,7 @@ template class goblin_ele return nullptr; } - Builder* get_context(const element& other) const + Builder* get_context(const goblin_element& other) const { if (x.get_context() != nullptr) { return x.get_context(); @@ -207,10 +212,10 @@ template class goblin_ele * coefficients when we get it as output from our optimised algorithms. This function returns a (0,0) point, if * it is a point at infinity */ - element get_standard_form() const + goblin_element get_standard_form() const { const bool_ct is_infinity = is_point_at_infinity(); - element result(*this); + goblin_element result(*this); const Fq zero = Fq::zero(); result.x = Fq::conditional_assign(is_infinity, zero, result.x); result.y = Fq::conditional_assign(is_infinity, zero, result.y); @@ -229,6 +234,6 @@ inline std::ostream& operator<<(std::ostream& os, goblin_element c { return os << "{ " << v.x << " , " << v.y << " }"; } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_goblin #include "biggroup_goblin_impl.hpp" diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp index 9889262d2677..97210ebdd3d8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin_impl.hpp @@ -1,7 +1,7 @@ #pragma once #include "barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp" -namespace bb::stdlib { +namespace bb::stdlib::element_goblin { /** * @brief Goblin style batch multiplication @@ -88,7 +88,7 @@ goblin_element goblin_element::batch_mul(const std:: 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); - element result = element(point_x, point_y); + goblin_element result = goblin_element(point_x, point_y); // NOTE: we can have an `if` statement here under the strict assumption that `return_is_infinity` // is produced from `eq_and_reset` opcode @@ -99,4 +99,4 @@ goblin_element goblin_element::batch_mul(const std:: return result; } -} // namespace bb::stdlib +} // namespace bb::stdlib::element_goblin diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index 4d7492351021..3369e14daacd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -4,7 +4,7 @@ #include "../circuit_builders/circuit_builders.hpp" #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { template element::element() @@ -122,9 +122,9 @@ element element::operator+(const element& other) con /** * @brief Enforce x and y coordinates of a point to be (0,0) in the case of point at infinity * - * @details We need to have a standard witness in Noir and the point at infinity can have non-zero random coefficients - * when we get it as output from our optimised algorithms. This function returns a (0,0) point, if it is a point at - * infinity + * @details We need to have a standard witness in Noir and the point at infinity can have non-zero random + * coefficients when we get it as output from our optimised algorithms. This function returns a (0,0) point, if it + * is a point at infinity */ template element element::get_standard_form() const @@ -230,8 +230,8 @@ template std::array, 2> element::checked_unconditional_add_sub(const element& other) const { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/971): This will fail when the two elements are the same - // even in the case of a valid circuit + // TODO(https://github.com/AztecProtocol/barretenberg/issues/971): This will fail when the two elements are the + // same even in the case of a valid circuit other.x.assert_is_not_equal(x); const Fq denominator = other.x - x; @@ -629,8 +629,8 @@ element element::multiple_montgomery_ladder( // We can avoid computing y_4, instead substituting the expression `minus_lambda_2 * (x_4 - x) - y` where // needed. This is cheaper, because we can evaluate two field multiplications (or a field multiplication + a - // field division) with only one non-native field reduction. E.g. evaluating (a * b) + (c * d) = e mod p only - // requires 1 quotient and remainder, which is the major cost of a non-native field multiplication + // field division) with only one non-native field reduction. E.g. evaluating (a * b) + (c * d) = e mod p + // only requires 1 quotient and remainder, which is the major cost of a non-native field multiplication Fq lambda2; if (i == 0) { lambda2 = Fq::div_without_denominator_check({ y + y }, (previous_x - x_3)) - lambda1; @@ -657,8 +657,8 @@ element element::multiple_montgomery_ladder( y_4.is_negative = !previous_y.is_negative; y_4.mul_left.emplace_back(lambda2); y_4.mul_right.emplace_back(previous_y.is_negative ? previous_x - x_4 : x_4 - previous_x); - // append terms in previous_y to y_4. We want to make sure the terms above are added into the start of y_4. - // This is to ensure they are cached correctly when + // append terms in previous_y to y_4. We want to make sure the terms above are added into the start of + // y_4. This is to ensure they are cached correctly when // `builder::evaluate_partial_non_native_field_multiplication` is called. // (the 1st mul_left, mul_right elements will trigger builder::evaluate_non_native_field_multiplication // when Fq::mult_madd is called - this term cannot be cached so we want to make sure it is unique) @@ -697,15 +697,15 @@ element element::multiple_montgomery_ladder( * Instead of handling the edge case (which is expensive!) we instead FORBID it from happening by * requiring x2 != x1 (other.x.assert_is_not_equal(x) will be present in all group operation methods) * - * This means it is essential we ensure an honest prover will NEVER run into this edge case, or our circuit will lack - * completeness! + * This means it is essential we ensure an honest prover will NEVER run into this edge case, or our circuit will + * lack completeness! * * To ensure an honest prover will not fall foul of this edge case when performing a SCALAR MULTIPLICATION, * we init the accumulator with an `offset_generator` point. * This point is a generator point that is not equal to the regular generator point for this curve. * - * When adding points into the accumulator, the probability that an honest prover will find a collision is now ~ 1 in - * 2^128 + * When adding points into the accumulator, the probability that an honest prover will find a collision is now ~ 1 + * in 2^128 * * We init `accumulator = generator` and then perform an n-bit scalar mul. * The output accumulator will contain a term `2^{n-1} * generator` that we need to subtract off. @@ -729,8 +729,8 @@ std::pair, element> element::c /** * @brief Generic batch multiplication that works for all elliptic curve types. * - * @details Implementation is identical to `bn254_endo_batch_mul` but WITHOUT the endomorphism transforms OR support for - * short scalars See `bn254_endo_batch_mul` for description of algorithm. + * @details Implementation is identical to `bn254_endo_batch_mul` but WITHOUT the endomorphism transforms OR support + * for short scalars See `bn254_endo_batch_mul` for description of algorithm. * * @tparam C The circuit builder type. * @tparam Fq The field of definition of the points in `_points`. @@ -865,4 +865,4 @@ element element::operator*(const Fr& scalar) const return element(out_x, out_y) - element(offset_generators.second); } -} // namespace bb::stdlib::element_inner +} // namespace bb::stdlib::element_default diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp index f4f2afe404bb..e6922787dcfb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp @@ -2,7 +2,7 @@ #include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { /** * Split a secp256k1 Fr element into two 129 bit scalars `klo, khi`, where `scalar = klo + \lambda * khi mod n` @@ -96,8 +96,8 @@ template typename element::secp256k1_wnaf_pair element::compute_secp256k1_endo_wnaf(const Fr& scalar) { /** - * The staggered offset describes the number of bits we want to remove from the input scalar before computing our - * wnaf slices. This is to enable us to make repeated calls to the montgomery ladder algo when computing a + * The staggered offset describes the number of bits we want to remove from the input scalar before computing + * our wnaf slices. This is to enable us to make repeated calls to the montgomery ladder algo when computing a * multi-scalar multiplication e.g. Consider an example with 2 points (A, B), using a 2-bit WNAF The typical * approach would be to perfomr a double-and-add algorithm, adding points into an accumulator ACC: * @@ -139,7 +139,8 @@ typename element::secp256k1_wnaf_pair element::compu const bool is_lo = false) { // The number of rounds is the minimal required to cover the whole scalar with wnaf_size windows constexpr size_t num_rounds = ((num_bits + wnaf_size - 1) / wnaf_size); - // Stagger mask is needed to retrieve the lowest bits that will not be used in montgomery ladder directly + // Stagger mask is needed to retrieve the lowest bits that will not be used in montgomery ladder + // directly const uint64_t stagger_mask = (1ULL << stagger) - 1; // Stagger scalar represents the lower "staggered" bits that are not used in the ladder const uint64_t stagger_scalar = k.data[0] & stagger_mask; @@ -179,14 +180,15 @@ typename element::secp256k1_wnaf_pair element::compu if (is_negative) { fragment = -fragment; } - // If the value is positive and there is a skew in wnaf, subtract 2ˢᵗᵃᵍᵍᵉʳ. If negative and there is - // skew, then add + // If the value is positive and there is a skew in wnaf, subtract 2ˢᵗᵃᵍᵍᵉʳ. If negative and + // there is skew, then add if (!is_negative && wnaf_skew) { fragment -= (1 << stagger); } else if (is_negative && wnaf_skew) { fragment += (1 << stagger); } - // If the lowest bit is zero, then set final skew to 1 and add 1 to the absolute value of the fragment + // If the lowest bit is zero, then set final skew to 1 and add 1 to the absolute value of the + // fragment bool output_skew = (fragment_u64 % 2) == 0; if (!is_negative && output_skew) { fragment += 1; @@ -220,8 +222,9 @@ typename element::secp256k1_wnaf_pair element::compu // Predicate == sign of current wnaf value bool predicate = bool((wnaf_values[i] >> 31U) & 1U); uint64_t offset_entry; - // If the signs of current entry and the whole scalar are the same, then add the lowest bits of current - // wnaf value to the windows size to form an entry. Otherwise, subract the lowest bits along with 1 + // If the signs of current entry and the whole scalar are the same, then add the lowest bits of + // current wnaf value to the windows size to form an entry. Otherwise, subract the lowest bits + // along with 1 if ((!predicate && !is_negative) || (predicate && is_negative)) { // TODO: Why is this mask fixed? offset_entry = wnaf_window_size + (wnaf_values[i] & 0xffffff); @@ -432,14 +435,15 @@ std::vector> element::compute_wnaf(const Fr& scalar) // We add the remaining wnaf entries into a 'high' accumulator // We can then directly construct a Fr element from the accumulators. // However we cannot underflow our accumulators, and our wnafs represent negative and positive values - // The raw value of each wnaf value is contained in the range [0, 15], however these values represent integers + // The raw value of each wnaf value is contained in the range [0, 15], however these values represent + // integers // [-15, -13, -11, ..., 13, 15] // // To map from the raw value to the actual value, we must compute `value * 2 - 15` // However, we do not subtract off the -15 term when constructing our low and high accumulators. Instead of - // multiplying by two when accumulating we simply add the accumulated value to itself. This way it automatically - // updates multiplicative constants without computing new witnesses. This ensures the low accumulator will not - // underflow + // multiplying by two when accumulating we simply add the accumulated value to itself. This way it + // automatically updates multiplicative constants without computing new witnesses. This ensures the low + // accumulator will not underflow // // Once we hvae reconstructed an Fr element out of our accumulators, // we ALSO construct an Fr element from the constant offset terms we left out @@ -583,4 +587,4 @@ std::vector> element::compute_naf(const Fr& scalar, cons } return naf_entries; } -} // namespace bb::stdlib::element_inner +} // namespace bb::stdlib::element_default diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp index ccb3c83281f0..a5a562a87aff 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp @@ -6,7 +6,7 @@ * **/ #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { template template @@ -25,9 +25,9 @@ element element::secp256k1_ecdsa_mul(const element& * Covert `u1_lo` and `u1_hi` into an 8-bit sliding window NAF. Our base point is the G1 generator. * We have a precomputed size-256 plookup table of the generator point, multiplied by all possible wNAF values * - * We also split scalar `u2` using the secp256k1 endomorphism. Convert short scalars into 4-bit sliding window NAFs. - * We will store the lookup table of all possible base-point wNAF states in a ROM table - * (it's variable-base scalar multiplication in a SNARK with a lookup table! ho ho ho) + * We also split scalar `u2` using the secp256k1 endomorphism. Convert short scalars into 4-bit sliding window + * NAFs. We will store the lookup table of all possible base-point wNAF states in a ROM table (it's + * variable-base scalar multiplication in a SNARK with a lookup table! ho ho ho) * * The wNAFs `u1_lo_wnaf, u1_hi_wnaf, u2_lo_wnaf, u2_hi_wnaf` are each offset by 1 bit relative to each other. * i.e. we right-shift `u2_hi` by 1 bit before computing its wNAF @@ -39,8 +39,8 @@ element element::secp256k1_ecdsa_mul(const element& * double-and-add scalar multiplication. It is more efficient to use the montgomery ladder algorithm, * compared against doubling an accumulator and adding points into it. * - * The bits removed by the right-shifts are stored in the wnaf's respective `least_significant_wnaf_fragment` member - * variable + * The bits removed by the right-shifts are stored in the wnaf's respective `least_significant_wnaf_fragment` + * member variable */ const auto [u1_lo_wnaf, u1_hi_wnaf] = compute_secp256k1_endo_wnaf<8, 2, 3>(u1); const auto [u2_lo_wnaf, u2_hi_wnaf] = compute_secp256k1_endo_wnaf<4, 0, 1>(u2); @@ -126,7 +126,8 @@ element element::secp256k1_ecdsa_mul(const element& to_add.y = to_add.y.conditional_negate(negative_skew_bool); element result = accumulator + to_add; - // when computing the wNAF we have already validated that positive_skew and negative_skew cannot both be true + // when computing the wNAF we have already validated that positive_skew and negative_skew cannot both be + // true bool_ct skew_combined = positive_skew_bool ^ negative_skew_bool; result.x = accumulator.x.conditional_select(result.x, skew_combined); result.y = accumulator.y.conditional_select(result.y, skew_combined); @@ -140,4 +141,4 @@ element element::secp256k1_ecdsa_mul(const element& return accumulator; } -} // namespace bb::stdlib::element_inner \ No newline at end of file +} // namespace bb::stdlib::element_default \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp index c49a9a62745b..821779acbf66 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_tables.hpp @@ -2,7 +2,7 @@ #include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" #include "barretenberg/stdlib/primitives/memory/twin_rom_table.hpp" #include "barretenberg/stdlib_circuit_builders/plookup_tables/types.hpp" -namespace bb::stdlib::element_inner { +namespace bb::stdlib::element_default { using plookup::MultiTableId; @@ -644,4 +644,4 @@ element element::lookup_table_base::get( } return element::one(bits[0].get_context()); } -} // namespace bb::stdlib::element_inner +} // namespace bb::stdlib::element_default diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp index 7734a0d90d53..1d24b8238b9b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp @@ -43,6 +43,7 @@ class CircuitSimulatorBN254 { using EmbeddedCurve = std::conditional_t, curve::BN254, curve::Grumpkin>; static constexpr CircuitType CIRCUIT_TYPE = CircuitType::ULTRA; static constexpr std::string_view NAME_STRING = "SIMULATOR"; + using EmbeddedCurve = std::conditional_t, curve::BN254, curve::Grumpkin>; bool contains_recursive_proof = false; static constexpr size_t UINT_LOG2_BASE = 2; // Would be 6 for UltraPlonk static constexpr size_t DEFAULT_PLOOKUP_RANGE_BITNUM = 1028; From 7b5d5915409621317acf308de727dc7f19bd7a50 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 5 Sep 2024 18:52:48 +0000 Subject: [PATCH 04/10] comments, changes --- .../{biggroup => bigfield}/goblin_field.hpp | 26 +++++++--- .../stdlib/primitives/biggroup/biggroup.hpp | 23 ++++---- .../primitives/biggroup/biggroup_goblin.hpp | 52 +++++++------------ .../primitives/field/field_conversion.hpp | 29 +++-------- 4 files changed, 58 insertions(+), 72 deletions(-) rename barretenberg/cpp/src/barretenberg/stdlib/primitives/{biggroup => bigfield}/goblin_field.hpp (69%) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp similarity index 69% rename from barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp rename to barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp index 22ffa1ac582a..b1d57a479b1c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/goblin_field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp @@ -4,6 +4,20 @@ #include "../field/field.hpp" namespace bb::stdlib { + +/** + * @brief goblin_field wraps x/y coordinates of bn254 group elements when using goblin + * @details this class exists because we do not want to parametrise goblin bn254 coordinates with bigfield. + * bigfield generates a large number of constraints to apply checks that are not needed for goblin coordinates + * This is because, in the goblin context we can apply the following heuristics: + * 1. goblin coordinate field elements are range-constrained in the Translator circuit (no need to range + * constrain here) + * 2. field elements that come out of the ECCVM are well-formed, we do not need to call `assert_is_in_field` + * 3. there should be no need to apply arithmetic to goblin coordinate field elements in-circuit + * Having a distinct class for `goblin_field` allows us to harvest these optimisations without a proliferation + * of edge cases and bloated logic in other classes + * @tparam Builder + */ template class goblin_field { public: static constexpr uint1024_t DEFAULT_MAXIMUM_REMAINDER = @@ -16,10 +30,10 @@ template class goblin_field { using bool_ct = stdlib::bool_t; std::array limbs; + // constructors mirror bigfield constructors goblin_field() : limbs{ 0, 0 } {} - goblin_field(Builder* parent_context, const uint256_t& value) { (*this) = goblin_field(bb::fq(value)); @@ -29,8 +43,8 @@ template class goblin_field { goblin_field(bb::fq input) { uint256_t converted(input); - uint256_t lo_v = converted.slice(0, 136); - uint256_t hi_v = converted.slice(136, 254); + uint256_t lo_v = converted.slice(0, NUM_LIMBS * 2); + uint256_t hi_v = converted.slice(NUM_LIMBS * 2, NUM_LIMBS * 3 + NUM_LAST_LIMB_BITS); limbs = { bb::fr(lo_v), bb::fr(hi_v) }; } goblin_field(field_ct lo, field_ct hi) @@ -40,7 +54,7 @@ template class goblin_field { // N.B. this method is because AggregationState expects group element coordinates to be split into 4 slices // (we could update to only use 2 for Mega but that feels complex) goblin_field(field_ct lolo, field_ct lohi, field_ct hilo, field_ct hihi, [[maybe_unused]] bool can_overflow = false) - : limbs{ lolo + lohi * (uint256_t(1) << 68), hilo + hihi * (uint256_t(1) << 68) } + : limbs{ lolo + lohi * (uint256_t(1) << NUM_LIMBS), hilo + hihi * (uint256_t(1) << NUM_LIMB_BITS) } {} void assert_equal(const goblin_field& other) const @@ -53,8 +67,8 @@ template class goblin_field { static goblin_field from_witness(Builder* ctx, bb::fq input) { uint256_t converted(input); - uint256_t lo_v = converted.slice(0, 136); - uint256_t hi_v = converted.slice(136, 254); + uint256_t lo_v = converted.slice(0, NUM_LIMBS * 2); + uint256_t hi_v = converted.slice(NUM_LIMBS * 2, NUM_LIMBS * 3 + NUM_LAST_LIMB_BITS); field_ct lo = field_ct::from_witness(ctx, lo_v); field_ct hi = field_ct::from_witness(ctx, hi_v); return goblin_field(lo, hi); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index 8125da0e2c83..ff538386143d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -1,29 +1,17 @@ #pragma once #include "../bigfield/bigfield.hpp" +#include "../bigfield/goblin_field.hpp" #include "../byte_array/byte_array.hpp" #include "../circuit_builders/circuit_builders_fwd.hpp" #include "../field/field.hpp" #include "../memory/rom_table.hpp" #include "../memory/twin_rom_table.hpp" -#include "./goblin_field.hpp" #include "barretenberg/ecc/curves/bn254/g1.hpp" #include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" #include "barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp" -// TODO(https://github.com/AztecProtocol/barretenberg/issues/707) If using a a circuit builder with Goblin, which is -// designed to have efficient bb::g1 operations, a developer might accidentally write inefficient circuits -// using biggroup functions that do not use the OpQueue. We use this concept to prevent compilation of such functions. -namespace bb::stdlib { -template -concept IsNotGoblinInefficiencyTrap = !(IsMegaBuilder && std::same_as); - -template -concept IsGoblinBigGroup = - IsMegaBuilder && std::same_as> && - std::same_as> && std::same_as; -} // namespace bb::stdlib namespace bb::stdlib::element_default { // ( ͡° ͜ʖ ͡°) @@ -953,6 +941,15 @@ namespace bb::stdlib { template concept IsBigGroup = std::is_same_v; +template +concept IsGoblinBigGroup = + IsMegaBuilder && std::same_as> && + std::same_as> && std::same_as; + +/** + * @brief element wraps either element_default::element or element_goblin::goblin_element depending on parametrisation + * @details if C = MegaBuilder, G = bn254, Fq = bigfield, Fr = field_t then we're cooking + */ template using element = std::conditional_t, element_goblin::goblin_element, Fr, G>, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index 274ea94059ab..96fa5b85997a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -1,24 +1,33 @@ #pragma once #include "../bigfield/bigfield.hpp" +#include "../bigfield/goblin_field.hpp" #include "../byte_array/byte_array.hpp" #include "../circuit_builders/circuit_builders_fwd.hpp" #include "../field/field.hpp" #include "../memory/rom_table.hpp" #include "../memory/twin_rom_table.hpp" -#include "./goblin_field.hpp" #include "barretenberg/ecc/curves/bn254/g1.hpp" #include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" -// // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) If using a a circuit builder with Goblin, which is -// // designed to have efficient bb::g1 operations, a developer might accidentally write inefficient circuits -// // using biggroup functions that do not use the OpQueue. We use this concept to prevent compilation of such -// functions. template concept IsNotGoblinInefficiencyTrap = -// !(IsMegaBuilder && std::same_as); namespace bb::stdlib::element_goblin { -// ( ͡° ͜ʖ ͡°) +/** + * @brief Custom element class for when using goblin + * @details When using goblin (builder = MEGA and element = bn254), the assumptions and heuristics + * we apply vary considerably to the "default" case, justifying a separate class + * (we use a `using` declaration to make `element` map to `goblin_element` if the correct parametrisation is + * used, see the `IsGoblinBigGroup` concept for details) Differences between goblin and regular biggroup elements: + * 1. state model is different (x/y coordinates are 2 136-bit field_t members instead of 4 68-bit field_t + * members) + * 2. on-curve checks are not applied in-circuit (they are applied in the ECCVM circuit) + * 3. we do not need to range-constrain the coordinates to be 136-bits (applied in the Translator circuit) + * @tparam Builder + * @tparam Fq + * @tparam Fr + * @tparam NativeGroup + */ template class goblin_element { public: using BaseField = Fq; @@ -36,18 +45,12 @@ template class goblin_ele , y(y) , _is_infinity(false) {} - goblin_element(const goblin_element& other) = default; goblin_element(goblin_element&& other) noexcept = default; goblin_element& operator=(const goblin_element& other) = default; - goblin_element& operator=(goblin_element&& other) = default; - // static goblin_element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) - // { - // // only valid usecase of this method is with a goblin builder - // ASSERT(IsMegaBuilder); - // goblin_element out; - // if (in) - // } + goblin_element& operator=(goblin_element&& other) noexcept = default; + ~goblin_element() = default; + static goblin_element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) { goblin_element out; @@ -80,14 +83,6 @@ template class goblin_ele return goblin_element(x_fq, y_fq); } - // byte_array to_byte_array() const - // { - // byte_array result(get_context()); - // result.write(y.to_byte_array()); - // result.write(x.to_byte_array()); - // return result; - // } - goblin_element checked_unconditional_add(const goblin_element& other) const { return goblin_element::operator+(*this, other); @@ -99,21 +94,14 @@ template class goblin_ele goblin_element operator+(const goblin_element& other) const { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - // Current gate count: 6398 return batch_mul({ *this, other }, { Fr(1), Fr(1) }); } goblin_element operator-(const goblin_element& other) const { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize std::vector points{ *this, other }; return batch_mul({ *this, other }, { Fr(1), -Fr(1) }); } - goblin_element operator-() const - { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/707) Optimize - return batch_mul({ *this }, { -Fr(1) }); - } + goblin_element operator-() const { return batch_mul({ *this }, { -Fr(1) }); } goblin_element operator+=(const goblin_element& other) { *this = *this + other; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 85cf813cf2d2..6be2d2990bc8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -2,7 +2,7 @@ #include "barretenberg/polynomials/univariate.hpp" #include "barretenberg/stdlib/primitives/bigfield/bigfield.hpp" -#include "barretenberg/stdlib/primitives/biggroup/goblin_field.hpp" +#include "barretenberg/stdlib/primitives/bigfield/goblin_field.hpp" #include "barretenberg/stdlib/primitives/curves/bn254.hpp" #include "barretenberg/stdlib/primitives/field/field.hpp" #include "barretenberg/stdlib/primitives/group/cycle_group.hpp" @@ -107,28 +107,15 @@ template T convert_from_bn254_frs(Builder& builde result.y = convert_from_bn254_frs( builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); - auto sum = fr_vec[0].add_two(fr_vec[1], fr_vec[2]); - sum = sum + fr_vec[3]; + // We have a convention that the group element is at infinity if both x/y coordinates are 0. + // We also know that all bn254 field elements are 136-bit scalars. + // Therefore we can do a cheap "iszero" check by checking the vector sum is 0 + fr sum; + for (size_t i = 0; i < BASE_FIELD_SCALAR_SIZE; i += 1) { + sum = sum.add_two(fr_vec[2 * i], fr_vec[2 * i + 1]); + } result.set_point_at_infinity(sum.is_zero()); return result; - - // we know that all Field elements are constrained to 136 bits. - // therefore, if we add them all together, the only way the sum is zero is if all are zero - // 19 gates to 6 gates - // is zero - - // A * is_zero = 0 - // (1 - is_zero) * (A * I - 1) = 0 - // AI - 1 - is_zero * AI + is_zero = 0 - - // AZ + AI - 1 - ZAI + Z = 0 - - // Z + I - ZI = t0 - // A * t0 + Z - 1 = 0 - // we can reduce to 3 gates (2.25 if we fix bool checks) - // result.set_point_at_infinity(fr_vec[0].is_zero() && fr_vec[1].is_zero() && fr_vec[2].is_zero() && - // fr_vec[3].is_zero()); - // return result; } else if constexpr (IsAnyOf>) { using BaseField = fr; constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); From 33f2e5a008111881cb0aba16da69df53b9115783 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 20:43:09 +0000 Subject: [PATCH 05/10] fix rebase error --- barretenberg/sol/src/honk/Transcript.sol | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/barretenberg/sol/src/honk/Transcript.sol b/barretenberg/sol/src/honk/Transcript.sol index d90789f63100..dabaa6743ca8 100644 --- a/barretenberg/sol/src/honk/Transcript.sol +++ b/barretenberg/sol/src/honk/Transcript.sol @@ -151,18 +151,6 @@ library TranscriptLib { Fr unused; (alphas[NUMBER_OF_ALPHAS - 1], unused) = splitChallenge(nextPreviousChallenge); } - if (((NUMBER_OF_ALPHAS & 1) == 1) && (NUMBER_OF_ALPHAS > 2)) { - nextPreviousChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(nextPreviousChallenge)))); - Fr unused; - (alphas[NUMBER_OF_ALPHAS - 1], unused) = splitChallenge(nextPreviousChallenge); - } - // alphas[0] = FrLib.fromBytes32(keccak256(abi.encodePacked(alpha0))); - - // Fr prevChallenge = alphas[0]; - // for (uint256 i = 1; i < NUMBER_OF_ALPHAS; i++) { - // prevChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(prevChallenge)))); - // alphas[i] = prevChallenge; - // } } function generateGateChallenges(Fr previousChallenge) From c7571901bfde10240ff58532892d465365b524a2 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 20:50:36 +0000 Subject: [PATCH 06/10] rebase fixes --- .../recursive_decider_verification_key.hpp | 3 +-- .../barretenberg/stdlib_circuit_builders/circuit_simulator.hpp | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp index 625a63db57bc..96a716a25b6c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp @@ -2,10 +2,9 @@ #include "barretenberg/commitment_schemes/verification_key.hpp" #include "barretenberg/flavor/flavor.hpp" #include "barretenberg/relations/relation_parameters.hpp" +#include "barretenberg/stdlib/primitives/field/field_conversion.hpp" #include "barretenberg/ultra_honk/decider_verification_key.hpp" -#include "barretenberg/stdlib/primitives/field/field_conversion.hpp" -#include "barretenberg/sumcheck/instance/verifier_instance.hpp" namespace bb::stdlib::recursion::honk { /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp index 1d24b8238b9b..7734a0d90d53 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_simulator.hpp @@ -43,7 +43,6 @@ class CircuitSimulatorBN254 { using EmbeddedCurve = std::conditional_t, curve::BN254, curve::Grumpkin>; static constexpr CircuitType CIRCUIT_TYPE = CircuitType::ULTRA; static constexpr std::string_view NAME_STRING = "SIMULATOR"; - using EmbeddedCurve = std::conditional_t, curve::BN254, curve::Grumpkin>; bool contains_recursive_proof = false; static constexpr size_t UINT_LOG2_BASE = 2; // Would be 6 for UltraPlonk static constexpr size_t DEFAULT_PLOOKUP_RANGE_BITNUM = 1028; From bd708abcd50b5a63649cd60a8ffce839afa7cf98 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 21:02:26 +0000 Subject: [PATCH 07/10] comments, more rebase fixes --- .../cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp | 2 ++ .../protogalaxy_verifier/recursive_decider_verification_key.hpp | 1 - .../translator_vm_verifier/translator_recursive_verifier.hpp | 1 - barretenberg/sol/src/honk/Transcript.sol | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp index b39ac2db717b..62fd8bae2a79 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.hpp @@ -239,6 +239,8 @@ template class DataBusDepot { std::array get_witness_indices_for_commitment(const Commitment& point) requires(IsMegaBuilder) { + // If using a goblin-plonk compatible builder, goblin element coordinates are stored as 2 field elements not 4. + // We convert to stdlib::bigfield elements so data is stored in the databus uniformly regardless of flavor using BigFq = stdlib::bigfield; const auto to_bigfield = [](Fr lo, Fr hi) { BigFq r(lo, hi); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp index 96a716a25b6c..9f08c708adce 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/recursive_decider_verification_key.hpp @@ -2,7 +2,6 @@ #include "barretenberg/commitment_schemes/verification_key.hpp" #include "barretenberg/flavor/flavor.hpp" #include "barretenberg/relations/relation_parameters.hpp" -#include "barretenberg/stdlib/primitives/field/field_conversion.hpp" #include "barretenberg/ultra_honk/decider_verification_key.hpp" namespace bb::stdlib::recursion::honk { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp index 364e63c63de6..39742bb65ccf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.hpp @@ -13,7 +13,6 @@ template class TranslatorRecursiveVerifier_ { using NativeBF = typename Flavor::Curve::BaseFieldNative; using Builder = typename Flavor::CircuitBuilder; using BF = typename stdlib::bigfield; - using Commitment = typename Flavor::Commitment; using GroupElement = typename Flavor::GroupElement; using VerificationKey = typename Flavor::VerificationKey; diff --git a/barretenberg/sol/src/honk/Transcript.sol b/barretenberg/sol/src/honk/Transcript.sol index dabaa6743ca8..1f450263a442 100644 --- a/barretenberg/sol/src/honk/Transcript.sol +++ b/barretenberg/sol/src/honk/Transcript.sol @@ -158,7 +158,7 @@ library TranscriptLib { view returns (Fr[CONST_PROOF_SIZE_LOG_N] memory gateChallenges, Fr nextPreviousChallenge) { - for (uint256 i = 0; i < CONST_PROOF_SIZE_LOG_N / 2; i++) { + for (uint256 i = 0; i < CONST_PROOF_SIZE_LOG_N; i++) { previousChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(Fr.unwrap(previousChallenge)))); Fr unused; (gateChallenges[i], unused) = splitChallenge(previousChallenge); From f8875a85f3cb4b6c171b0f28fc53f529a78ff9c5 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 22:04:37 +0000 Subject: [PATCH 08/10] fixed goblin_field error where wrong constant was being used --- .../stdlib/primitives/bigfield/goblin_field.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp index b1d57a479b1c..8e3885e46a60 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/goblin_field.hpp @@ -43,8 +43,8 @@ template class goblin_field { goblin_field(bb::fq input) { uint256_t converted(input); - uint256_t lo_v = converted.slice(0, NUM_LIMBS * 2); - uint256_t hi_v = converted.slice(NUM_LIMBS * 2, NUM_LIMBS * 3 + NUM_LAST_LIMB_BITS); + uint256_t lo_v = converted.slice(0, NUM_LIMB_BITS * 2); + uint256_t hi_v = converted.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3 + NUM_LAST_LIMB_BITS); limbs = { bb::fr(lo_v), bb::fr(hi_v) }; } goblin_field(field_ct lo, field_ct hi) @@ -54,7 +54,7 @@ template class goblin_field { // N.B. this method is because AggregationState expects group element coordinates to be split into 4 slices // (we could update to only use 2 for Mega but that feels complex) goblin_field(field_ct lolo, field_ct lohi, field_ct hilo, field_ct hihi, [[maybe_unused]] bool can_overflow = false) - : limbs{ lolo + lohi * (uint256_t(1) << NUM_LIMBS), hilo + hihi * (uint256_t(1) << NUM_LIMB_BITS) } + : limbs{ lolo + lohi * (uint256_t(1) << NUM_LIMB_BITS), hilo + hihi * (uint256_t(1) << NUM_LIMB_BITS) } {} void assert_equal(const goblin_field& other) const @@ -67,8 +67,8 @@ template class goblin_field { static goblin_field from_witness(Builder* ctx, bb::fq input) { uint256_t converted(input); - uint256_t lo_v = converted.slice(0, NUM_LIMBS * 2); - uint256_t hi_v = converted.slice(NUM_LIMBS * 2, NUM_LIMBS * 3 + NUM_LAST_LIMB_BITS); + uint256_t lo_v = converted.slice(0, NUM_LIMB_BITS * 2); + uint256_t hi_v = converted.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3 + NUM_LAST_LIMB_BITS); field_ct lo = field_ct::from_witness(ctx, lo_v); field_ct hi = field_ct::from_witness(ctx, hi_v); return goblin_field(lo, hi); From 6de4b0407d818dad8aaea3f5818f8e71b329e287 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 23:10:35 +0000 Subject: [PATCH 09/10] cleanup --- .../stdlib/primitives/biggroup/biggroup.hpp | 13 +-------- .../primitives/biggroup/biggroup_bn254.hpp | 18 ++++++------ .../biggroup/biggroup_edgecase_handling.hpp | 12 ++++---- .../primitives/biggroup/biggroup_impl.hpp | 26 ++++++++--------- .../primitives/biggroup/biggroup_nafs.hpp | 28 ++++++++----------- .../biggroup/biggroup_secp256k1.hpp | 13 ++++----- .../protogalaxy_recursive_verifier.test.cpp | 4 +-- 7 files changed, 48 insertions(+), 66 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index ff538386143d..a4f6bb109616 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -39,13 +39,6 @@ template class element { element(const element& other); element(element&& other) noexcept; - // static element from_witness_unsafe(Builder* ctx, const typename NativeGroup::affine_element& input) - // { - // // only valid usecase of this method is with a goblin builder - // ASSERT(IsMegaBuilder); - // element out; - // if (in) - // } static element from_witness(Builder* ctx, const typename NativeGroup::affine_element& input) { element out; @@ -61,11 +54,7 @@ template class element { out.y = y; } out.set_point_at_infinity(witness_t(ctx, input.is_point_at_infinity())); - // info(" Num gates before validating on curve: ", ctx->num_gates); - if constexpr (!IsMegaBuilder) { - out.validate_on_curve(); - } - // info(" Num gates after validating on curve: ", ctx->num_gates); + out.validate_on_curve(); return out; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_bn254.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_bn254.hpp index f3fd3f1ff0bc..ab744cf03cd5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_bn254.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_bn254.hpp @@ -120,8 +120,7 @@ element element::bn254_endo_batch_mul_with_generator }; // Perform multiple rounds of the montgomery ladder algoritm per "iteration" of our main loop. - // This is in order to reduce the number of field reductions required when calling - // `multiple_montgomery_ladder` + // This is in order to reduce the number of field reductions required when calling `multiple_montgomery_ladder` constexpr size_t num_rounds_per_iteration = 4; // we require that we perform max of one generator per iteration @@ -214,8 +213,8 @@ element element::bn254_endo_batch_mul_with_generator * small_points : group elements we will multiply by short scalar mutipliers whose max value will be (1 << *max_num_small_bits) small_scalars : short scalar mutipliers whose max value will be (1 << max_num_small_bits) * max_num_small_bits : MINIMUM value must be 128 bits - * (we will be splitting `big_scalars` into two 128-bit scalars, we assume all scalars after this transformation are - *128 bits) + * (we will be splitting `big_scalars` into two 128-bit scalars, we assume all scalars after this transformation are 128 + *bits) **/ template template @@ -351,11 +350,10 @@ element element::bn254_endo_batch_mul(const std::vec * 3. Repeat the above 2 steps but for bit `2 * i` (`add_2`) * 4. Compute `accumulator = 4 * accumulator + 2 * add_1 + add_2` using `multiple_montgomery_ladder` method * - * The purpose of the above is to minimize the number of required range checks (vs a simple double and add - *algo). + * The purpose of the above is to minimize the number of required range checks (vs a simple double and add algo). * - * When computing repeated iterations of the montgomery ladder algorithm, we can neglect computing the - *y-coordinate of each ladder output. See `multiple_montgomery_ladder` for more details. + * When computing repeated iterations of the montgomery ladder algorithm, we can neglect computing the y-coordinate + *of each ladder output. See `multiple_montgomery_ladder` for more details. **/ for (size_t i = 1; i < num_rounds / 2; ++i) { // `nafs` tracks the naf value for each point for the current round @@ -373,8 +371,8 @@ element element::bn254_endo_batch_mul(const std::vec * * This is represented using the `chain_add_accumulator` type. See the type declaration for more details * - * (this is cheaper than regular additions iff point_table.get_accumulator require 2 or more point - *additions. Cost is the same as `point_table.get_accumulator` if 1 or 0 point additions are required) + * (this is cheaper than regular additions iff point_table.get_accumulator require 2 or more point additions. + * Cost is the same as `point_table.get_accumulator` if 1 or 0 point additions are required) **/ element::chain_add_accumulator add_1 = point_table.get_chain_add_accumulator(nafs); for (size_t j = 0; j < points.size(); ++j) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp index 15a73c800861..69dbf0a43f6d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_edgecase_handling.hpp @@ -7,8 +7,8 @@ namespace bb::stdlib::element_default { * @brief Compute an offset generator for use in biggroup tables * *@details Sometimes the points from which we construct the tables are going to be dependent in such a way that - *combining them for constructing the table is not possible without handling the edgecases such as the point at - *infinity and doubling. To avoid handling those we add multiples of this offset generator to the points. + *combining them for constructing the table is not possible without handling the edgecases such as the point at infinity + *and doubling. To avoid handling those we add multiples of this offset generator to the points. * * @param num_rounds */ @@ -22,11 +22,11 @@ typename G::affine_element element::compute_table_offset_generator } /** - * @brief Given two lists of points that need to be multiplied by scalars, create a new list of length +1 with - * original points masked, but the same scalar product sum + * @brief Given two lists of points that need to be multiplied by scalars, create a new list of length +1 with original + * points masked, but the same scalar product sum * @details Add +1G, +2G, +4G etc to the original points and adds a new point 2ⁿ⋅G and scalar x to the lists. By - * doubling the point every time, we ensure that no +-1 combination of 6 sequential elements run into edgecases, - * unless the points are deliberately constructed to trigger it. + * doubling the point every time, we ensure that no +-1 combination of 6 sequential elements run into edgecases, unless + * the points are deliberately constructed to trigger it. */ template std::pair>, std::vector> element::mask_points( diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index 3369e14daacd..aa667bf25950 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -122,9 +122,9 @@ element element::operator+(const element& other) con /** * @brief Enforce x and y coordinates of a point to be (0,0) in the case of point at infinity * - * @details We need to have a standard witness in Noir and the point at infinity can have non-zero random - * coefficients when we get it as output from our optimised algorithms. This function returns a (0,0) point, if it - * is a point at infinity + * @details We need to have a standard witness in Noir and the point at infinity can have non-zero random coefficients + * when we get it as output from our optimised algorithms. This function returns a (0,0) point, if it is a point at + * infinity */ template element element::get_standard_form() const @@ -629,8 +629,8 @@ element element::multiple_montgomery_ladder( // We can avoid computing y_4, instead substituting the expression `minus_lambda_2 * (x_4 - x) - y` where // needed. This is cheaper, because we can evaluate two field multiplications (or a field multiplication + a - // field division) with only one non-native field reduction. E.g. evaluating (a * b) + (c * d) = e mod p - // only requires 1 quotient and remainder, which is the major cost of a non-native field multiplication + // field division) with only one non-native field reduction. E.g. evaluating (a * b) + (c * d) = e mod p only + // requires 1 quotient and remainder, which is the major cost of a non-native field multiplication Fq lambda2; if (i == 0) { lambda2 = Fq::div_without_denominator_check({ y + y }, (previous_x - x_3)) - lambda1; @@ -657,8 +657,8 @@ element element::multiple_montgomery_ladder( y_4.is_negative = !previous_y.is_negative; y_4.mul_left.emplace_back(lambda2); y_4.mul_right.emplace_back(previous_y.is_negative ? previous_x - x_4 : x_4 - previous_x); - // append terms in previous_y to y_4. We want to make sure the terms above are added into the start of - // y_4. This is to ensure they are cached correctly when + // append terms in previous_y to y_4. We want to make sure the terms above are added into the start of y_4. + // This is to ensure they are cached correctly when // `builder::evaluate_partial_non_native_field_multiplication` is called. // (the 1st mul_left, mul_right elements will trigger builder::evaluate_non_native_field_multiplication // when Fq::mult_madd is called - this term cannot be cached so we want to make sure it is unique) @@ -697,15 +697,15 @@ element element::multiple_montgomery_ladder( * Instead of handling the edge case (which is expensive!) we instead FORBID it from happening by * requiring x2 != x1 (other.x.assert_is_not_equal(x) will be present in all group operation methods) * - * This means it is essential we ensure an honest prover will NEVER run into this edge case, or our circuit will - * lack completeness! + * This means it is essential we ensure an honest prover will NEVER run into this edge case, or our circuit will lack + * completeness! * * To ensure an honest prover will not fall foul of this edge case when performing a SCALAR MULTIPLICATION, * we init the accumulator with an `offset_generator` point. * This point is a generator point that is not equal to the regular generator point for this curve. * - * When adding points into the accumulator, the probability that an honest prover will find a collision is now ~ 1 - * in 2^128 + * When adding points into the accumulator, the probability that an honest prover will find a collision is now ~ 1 in + * 2^128 * * We init `accumulator = generator` and then perform an n-bit scalar mul. * The output accumulator will contain a term `2^{n-1} * generator` that we need to subtract off. @@ -729,8 +729,8 @@ std::pair, element> element::c /** * @brief Generic batch multiplication that works for all elliptic curve types. * - * @details Implementation is identical to `bn254_endo_batch_mul` but WITHOUT the endomorphism transforms OR support - * for short scalars See `bn254_endo_batch_mul` for description of algorithm. + * @details Implementation is identical to `bn254_endo_batch_mul` but WITHOUT the endomorphism transforms OR support for + * short scalars See `bn254_endo_batch_mul` for description of algorithm. * * @tparam C The circuit builder type. * @tparam Fq The field of definition of the points in `_points`. diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp index e6922787dcfb..58675cac0def 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp @@ -96,8 +96,8 @@ template typename element::secp256k1_wnaf_pair element::compute_secp256k1_endo_wnaf(const Fr& scalar) { /** - * The staggered offset describes the number of bits we want to remove from the input scalar before computing - * our wnaf slices. This is to enable us to make repeated calls to the montgomery ladder algo when computing a + * The staggered offset describes the number of bits we want to remove from the input scalar before computing our + * wnaf slices. This is to enable us to make repeated calls to the montgomery ladder algo when computing a * multi-scalar multiplication e.g. Consider an example with 2 points (A, B), using a 2-bit WNAF The typical * approach would be to perfomr a double-and-add algorithm, adding points into an accumulator ACC: * @@ -139,8 +139,7 @@ typename element::secp256k1_wnaf_pair element::compu const bool is_lo = false) { // The number of rounds is the minimal required to cover the whole scalar with wnaf_size windows constexpr size_t num_rounds = ((num_bits + wnaf_size - 1) / wnaf_size); - // Stagger mask is needed to retrieve the lowest bits that will not be used in montgomery ladder - // directly + // Stagger mask is needed to retrieve the lowest bits that will not be used in montgomery ladder directly const uint64_t stagger_mask = (1ULL << stagger) - 1; // Stagger scalar represents the lower "staggered" bits that are not used in the ladder const uint64_t stagger_scalar = k.data[0] & stagger_mask; @@ -180,15 +179,14 @@ typename element::secp256k1_wnaf_pair element::compu if (is_negative) { fragment = -fragment; } - // If the value is positive and there is a skew in wnaf, subtract 2ˢᵗᵃᵍᵍᵉʳ. If negative and - // there is skew, then add + // If the value is positive and there is a skew in wnaf, subtract 2ˢᵗᵃᵍᵍᵉʳ. If negative and there is + // skew, then add if (!is_negative && wnaf_skew) { fragment -= (1 << stagger); } else if (is_negative && wnaf_skew) { fragment += (1 << stagger); } - // If the lowest bit is zero, then set final skew to 1 and add 1 to the absolute value of the - // fragment + // If the lowest bit is zero, then set final skew to 1 and add 1 to the absolute value of the fragment bool output_skew = (fragment_u64 % 2) == 0; if (!is_negative && output_skew) { fragment += 1; @@ -222,9 +220,8 @@ typename element::secp256k1_wnaf_pair element::compu // Predicate == sign of current wnaf value bool predicate = bool((wnaf_values[i] >> 31U) & 1U); uint64_t offset_entry; - // If the signs of current entry and the whole scalar are the same, then add the lowest bits of - // current wnaf value to the windows size to form an entry. Otherwise, subract the lowest bits - // along with 1 + // If the signs of current entry and the whole scalar are the same, then add the lowest bits of current + // wnaf value to the windows size to form an entry. Otherwise, subract the lowest bits along with 1 if ((!predicate && !is_negative) || (predicate && is_negative)) { // TODO: Why is this mask fixed? offset_entry = wnaf_window_size + (wnaf_values[i] & 0xffffff); @@ -435,15 +432,14 @@ std::vector> element::compute_wnaf(const Fr& scalar) // We add the remaining wnaf entries into a 'high' accumulator // We can then directly construct a Fr element from the accumulators. // However we cannot underflow our accumulators, and our wnafs represent negative and positive values - // The raw value of each wnaf value is contained in the range [0, 15], however these values represent - // integers + // The raw value of each wnaf value is contained in the range [0, 15], however these values represent integers // [-15, -13, -11, ..., 13, 15] // // To map from the raw value to the actual value, we must compute `value * 2 - 15` // However, we do not subtract off the -15 term when constructing our low and high accumulators. Instead of - // multiplying by two when accumulating we simply add the accumulated value to itself. This way it - // automatically updates multiplicative constants without computing new witnesses. This ensures the low - // accumulator will not underflow + // multiplying by two when accumulating we simply add the accumulated value to itself. This way it automatically + // updates multiplicative constants without computing new witnesses. This ensures the low accumulator will not + // underflow // // Once we hvae reconstructed an Fr element out of our accumulators, // we ALSO construct an Fr element from the constant offset terms we left out diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp index a5a562a87aff..b2aa899f6b15 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_secp256k1.hpp @@ -25,9 +25,9 @@ element element::secp256k1_ecdsa_mul(const element& * Covert `u1_lo` and `u1_hi` into an 8-bit sliding window NAF. Our base point is the G1 generator. * We have a precomputed size-256 plookup table of the generator point, multiplied by all possible wNAF values * - * We also split scalar `u2` using the secp256k1 endomorphism. Convert short scalars into 4-bit sliding window - * NAFs. We will store the lookup table of all possible base-point wNAF states in a ROM table (it's - * variable-base scalar multiplication in a SNARK with a lookup table! ho ho ho) + * We also split scalar `u2` using the secp256k1 endomorphism. Convert short scalars into 4-bit sliding window NAFs. + * We will store the lookup table of all possible base-point wNAF states in a ROM table + * (it's variable-base scalar multiplication in a SNARK with a lookup table! ho ho ho) * * The wNAFs `u1_lo_wnaf, u1_hi_wnaf, u2_lo_wnaf, u2_hi_wnaf` are each offset by 1 bit relative to each other. * i.e. we right-shift `u2_hi` by 1 bit before computing its wNAF @@ -39,8 +39,8 @@ element element::secp256k1_ecdsa_mul(const element& * double-and-add scalar multiplication. It is more efficient to use the montgomery ladder algorithm, * compared against doubling an accumulator and adding points into it. * - * The bits removed by the right-shifts are stored in the wnaf's respective `least_significant_wnaf_fragment` - * member variable + * The bits removed by the right-shifts are stored in the wnaf's respective `least_significant_wnaf_fragment` member + * variable */ const auto [u1_lo_wnaf, u1_hi_wnaf] = compute_secp256k1_endo_wnaf<8, 2, 3>(u1); const auto [u2_lo_wnaf, u2_hi_wnaf] = compute_secp256k1_endo_wnaf<4, 0, 1>(u2); @@ -126,8 +126,7 @@ element element::secp256k1_ecdsa_mul(const element& to_add.y = to_add.y.conditional_negate(negative_skew_bool); element result = accumulator + to_add; - // when computing the wNAF we have already validated that positive_skew and negative_skew cannot both be - // true + // when computing the wNAF we have already validated that positive_skew and negative_skew cannot both be true bool_ct skew_combined = positive_skew_bool ^ negative_skew_bool; result.x = accumulator.x.conditional_select(result.x, skew_combined); result.y = accumulator.y.conditional_select(result.y, skew_combined); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp index 6e784dceea8a..cb5012bbaec2 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/protogalaxy_verifier/protogalaxy_recursive_verifier.test.cpp @@ -205,7 +205,7 @@ template class ProtogalaxyRecursiveTests : public tes auto verifier = FoldingRecursiveVerifier{ &folding_circuit, recursive_decider_vk_1, { recursive_decider_vk_2 } }; verifier.verify_folding_proof(stdlib_proof); - info("Folding Recursive Verifier: num gates = ", folding_circuit.num_gates); + info("Folding Recursive Verifier: num gates = ", folding_circuit.get_num_gates()); EXPECT_EQ(folding_circuit.failed(), false) << folding_circuit.err(); // Perform native folding verification and ensure it returns the same result (either true or false) as @@ -276,7 +276,7 @@ template class ProtogalaxyRecursiveTests : public tes auto recursive_verifier_accumulator = verifier.verify_folding_proof(stdlib_proof); auto native_verifier_acc = std::make_shared(recursive_verifier_accumulator->get_value()); - info("Folding Recursive Verifier: num gates = ", folding_circuit.num_gates); + info("Folding Recursive Verifier: num gates = ", folding_circuit.get_num_gates()); // Check for a failure flag in the recursive verifier circuit EXPECT_EQ(folding_circuit.failed(), false) << folding_circuit.err(); From 12aaebda1ef2591d76ec4d68ea391ff18c064b67 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 10 Sep 2024 23:12:47 +0000 Subject: [PATCH 10/10] comment fix --- .../src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index a4f6bb109616..110df1f0cbec 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -210,8 +210,8 @@ template class element { const bool with_edgecases = false); // 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 template + // This is a bit tricky to do with `std::enable_if`, because `bn254_endo_batch_mul` is a member function of a class + // template // && the compiler can't perform partial template specialization on member functions of class templates // => our template parameter cannot be a value but must instead by a type // Our input to `std::enable_if` is a comparison between two types (NativeGroup and bb::g1), which