From 35475e22f207573369352e629ac636bfb6130b1d Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 29 Oct 2024 19:14:26 +0000 Subject: [PATCH 01/15] wip --- .../stdlib/primitives/group/cycle_group.cpp | 114 +++++++++++------- .../stdlib/primitives/group/cycle_group.hpp | 22 ++-- 2 files changed, 83 insertions(+), 53 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 789c5eb7627..7fbf698d594 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -203,7 +203,7 @@ template cycle_group cycle_group::get_stand * @return cycle_group */ template -cycle_group cycle_group::dbl() const +cycle_group cycle_group::dbl([[maybe_unused]] const std::optional /*unused*/) const requires IsNotUltraArithmetic { auto modified_y = field_t::conditional_assign(is_point_at_infinity(), 1, y); @@ -220,29 +220,36 @@ cycle_group cycle_group::dbl() const * @return cycle_group */ template -cycle_group cycle_group::dbl() const +cycle_group cycle_group::dbl(const std::optional hint) const requires IsUltraArithmetic { // ensure we use a value of y that is not zero. (only happens if point at infinity) // this costs 0 gates if `is_infinity` is a circuit constant auto modified_y = field_t::conditional_assign(is_point_at_infinity(), 1, y).normalize(); - auto x1 = x.get_value(); - auto y1 = modified_y.get_value(); - - // N.B. the formula to derive the witness value for x3 mirrors the formula in elliptic_relation.hpp - // Specifically, we derive x^4 via the Short Weierstrass curve formula `y^2 = x^3 + b` - // i.e. x^4 = x * (y^2 - b) - // We must follow this pattern exactly to support the edge-case where the input is the point at infinity. - auto y_pow_2 = y1.sqr(); - auto x_pow_4 = x1 * (y_pow_2 - Group::curve_b); - auto lambda_squared = (x_pow_4 * 9) / (y_pow_2 * 4); - auto lambda = (x1 * x1 * 3) / (y1 + y1); - auto x3 = lambda_squared - x1 - x1; - auto y3 = lambda * (x1 - x3) - y1; + + cycle_group result; + if (hint.has_value()) { + result = hint.value(); + } else { + auto x1 = x.get_value(); + auto y1 = modified_y.get_value(); + + // N.B. the formula to derive the witness value for x3 mirrors the formula in elliptic_relation.hpp + // Specifically, we derive x^4 via the Short Weierstrass curve formula `y^2 = x^3 + b` + // i.e. x^4 = x * (y^2 - b) + // We must follow this pattern exactly to support the edge-case where the input is the point at infinity. + auto y_pow_2 = y1.sqr(); + auto x_pow_4 = x1 * (y_pow_2 - Group::curve_b); + auto lambda_squared = (x_pow_4 * 9) / (y_pow_2 * 4); + auto lambda = (x1 * x1 * 3) / (y1 + y1); + auto x3 = lambda_squared - x1 - x1; + auto y3 = lambda * (x1 - x3) - y1; + + result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); + } if (is_constant()) { - return cycle_group(x3, y3, is_point_at_infinity().get_value()); + return result; } - cycle_group result(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); context->create_ecc_dbl_gate(bb::ecc_dbl_gate_{ .x1 = x.get_witness_index(), .y1 = modified_y.normalize().get_witness_index(), @@ -263,7 +270,8 @@ cycle_group cycle_group::dbl() const * @return cycle_group */ template -cycle_group cycle_group::unconditional_add(const cycle_group& other) const +cycle_group cycle_group::unconditional_add( + const cycle_group& other, [[maybe_unused]] const std::optional /*unused*/ /*unused*/) const requires IsNotUltraArithmetic { auto x_diff = other.x - x; @@ -288,7 +296,8 @@ cycle_group cycle_group::unconditional_add(const cycle_group& * @return cycle_group */ template -cycle_group cycle_group::unconditional_add(const cycle_group& other) const +cycle_group cycle_group::unconditional_add(const cycle_group& other, + const std::optional hint) const requires IsUltraArithmetic { auto context = get_context(other); @@ -303,17 +312,21 @@ cycle_group cycle_group::unconditional_add(const cycle_group& auto rhs = cycle_group::from_constant_witness(context, other.get_value()); return unconditional_add(rhs); } + cycle_group result; + if (hint.has_value()) { + result = hint.value(); + } else { - const auto p1 = get_value(); - const auto p2 = other.get_value(); - AffineElement p3(Element(p1) + Element(p2)); + const auto p1 = get_value(); + const auto p2 = other.get_value(); + AffineElement p3(Element(p1) + Element(p2)); + field_t r_x(witness_t(context, p3.x)); + field_t r_y(witness_t(context, p3.y)); + result = cycle_group(r_x, r_y, false); + } if (lhs_constant && rhs_constant) { - return cycle_group(p3); + return result; } - field_t r_x(witness_t(context, p3.x)); - field_t r_y(witness_t(context, p3.y)); - cycle_group result(r_x, r_y, false); - bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), .y1 = y.get_witness_index(), @@ -338,10 +351,11 @@ cycle_group cycle_group::unconditional_add(const cycle_group& * @return cycle_group */ template -cycle_group cycle_group::unconditional_subtract(const cycle_group& other) const +cycle_group cycle_group::unconditional_subtract(const cycle_group& other, + const std::optional hint) const { if constexpr (!IS_ULTRA) { - return unconditional_add(-other); + return unconditional_add(-other, hint); } else { auto context = get_context(other); @@ -350,22 +364,28 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr if (lhs_constant && !rhs_constant) { auto lhs = cycle_group::from_constant_witness(context, get_value()); - return lhs.unconditional_subtract(other); + return lhs.unconditional_subtract(other, hint); } if (!lhs_constant && rhs_constant) { auto rhs = cycle_group::from_constant_witness(context, other.get_value()); - return unconditional_subtract(rhs); + return unconditional_subtract(rhs, hint); + } + cycle_group result; + if (hint.has_value()) { + result = hint.value(); + } else { + + auto p1 = get_value(); + auto p2 = other.get_value(); + AffineElement p3(Element(p1) - Element(p2)); + + field_t r_x(witness_t(context, p3.x)); + field_t r_y(witness_t(context, p3.y)); + result = cycle_group(r_x, r_y, false); } - auto p1 = get_value(); - auto p2 = other.get_value(); - AffineElement p3(Element(p1) - Element(p2)); if (lhs_constant && rhs_constant) { - return cycle_group(p3); + return result; } - field_t r_x(witness_t(context, p3.x)); - field_t r_y(witness_t(context, p3.y)); - cycle_group result(r_x, r_y, false); - bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), .y1 = y.get_witness_index(), @@ -394,11 +414,12 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr * @return cycle_group */ template -cycle_group cycle_group::checked_unconditional_add(const cycle_group& other) const +cycle_group cycle_group::checked_unconditional_add(const cycle_group& other, + const std::optional hint) const { field_t x_delta = x - other.x; x_delta.assert_is_not_zero("cycle_group::checked_unconditional_add, x-coordinate collision"); - return unconditional_add(other); + return unconditional_add(other, hint); } /** @@ -414,11 +435,12 @@ cycle_group cycle_group::checked_unconditional_add(const cycle * @return cycle_group */ template -cycle_group cycle_group::checked_unconditional_subtract(const cycle_group& other) const +cycle_group cycle_group::checked_unconditional_subtract(const cycle_group& other, + const std::optional hint) const { field_t x_delta = x - other.x; x_delta.assert_is_not_zero("cycle_group::checked_unconditional_subtract, x-coordinate collision"); - return unconditional_subtract(other); + return unconditional_subtract(other, hint); } /** @@ -870,7 +892,8 @@ template cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, const cycle_group& base_point, const cycle_group& offset_generator, - size_t table_bits) + size_t table_bits, + std::optional> hints) : _table_bits(table_bits) , _context(context) { @@ -891,7 +914,8 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, field_t modded_y = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.y, base_point.y); cycle_group modded_base_point(modded_x, modded_y, false); for (size_t i = 1; i < table_size; ++i) { - auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point); + std::optional hint = hints.has_value() ? hints.value()[i - 1] : std::optional(); + auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); point_table[i] = cycle_group(x, y, false); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index 8a9b7ff0620..d50f8fcc851 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -152,7 +152,8 @@ template class cycle_group { straus_lookup_table(Builder* context, const cycle_group& base_point, const cycle_group& offset_generator, - size_t table_bits); + size_t table_bits, + std::optional> hints = std::optional>()); cycle_group read(const field_t& index); size_t _table_bits; Builder* _context; @@ -186,17 +187,22 @@ template class cycle_group { void set_point_at_infinity(const bool_t& is_infinity) { _is_infinity = is_infinity; } cycle_group get_standard_form() const; void validate_is_on_curve() const; - cycle_group dbl() const + cycle_group dbl(const std::optional hint = std::optional()) const requires IsUltraArithmetic; - cycle_group dbl() const + cycle_group dbl(const std::optional hint = std::optional()) const requires IsNotUltraArithmetic; - cycle_group unconditional_add(const cycle_group& other) const + cycle_group unconditional_add(const cycle_group& other, + const std::optional hint = std::optional()) const requires IsUltraArithmetic; - cycle_group unconditional_add(const cycle_group& other) const + cycle_group unconditional_add(const cycle_group& other, + const std::optional hint = std::optional()) const requires IsNotUltraArithmetic; - cycle_group unconditional_subtract(const cycle_group& other) const; - cycle_group checked_unconditional_add(const cycle_group& other) const; - cycle_group checked_unconditional_subtract(const cycle_group& other) const; + cycle_group unconditional_subtract(const cycle_group& other, + const std::optional hint = std::optional()) const; + cycle_group checked_unconditional_add(const cycle_group& other, + const std::optional hint = std::optional()) const; + cycle_group checked_unconditional_subtract( + const cycle_group& other, const std::optional hint = std::optional()) const; cycle_group operator+(const cycle_group& other) const; cycle_group operator-(const cycle_group& other) const; cycle_group operator-() const; From 8cb78b58a716347a802a929e251cd6fb4db8d894 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 00:55:14 +0000 Subject: [PATCH 02/15] cycle_group::batch_mul batches modular inversions during witness generation --- .../stdlib/primitives/group/cycle_group.cpp | 263 ++++++++++++++---- .../stdlib/primitives/group/cycle_group.hpp | 24 +- 2 files changed, 226 insertions(+), 61 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 7fbf698d594..3312a9baaf5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -203,7 +203,7 @@ template cycle_group cycle_group::get_stand * @return cycle_group */ template -cycle_group cycle_group::dbl([[maybe_unused]] const std::optional /*unused*/) const +cycle_group cycle_group::dbl([[maybe_unused]] const std::optional /*unused*/) const requires IsNotUltraArithmetic { auto modified_y = field_t::conditional_assign(is_point_at_infinity(), 1, y); @@ -220,7 +220,7 @@ cycle_group cycle_group::dbl([[maybe_unused]] const std::optio * @return cycle_group */ template -cycle_group cycle_group::dbl(const std::optional hint) const +cycle_group cycle_group::dbl(const std::optional hint) const requires IsUltraArithmetic { // ensure we use a value of y that is not zero. (only happens if point at infinity) @@ -229,7 +229,31 @@ cycle_group cycle_group::dbl(const std::optional cycle_group result; if (hint.has_value()) { - result = hint.value(); + auto x3 = hint.value().x; + auto y3 = hint.value().y; + if (is_constant()) { + result = cycle_group(x3, y3, is_point_at_infinity()); + return result; + } + + result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); + auto x1 = x.get_value(); + auto y1 = modified_y.get_value(); + + // N.B. the formula to derive the witness value for x3 mirrors the formula in elliptic_relation.hpp + // Specifically, we derive x^4 via the Short Weierstrass curve formula `y^2 = x^3 + b` + // i.e. x^4 = x * (y^2 - b) + // We must follow this pattern exactly to support the edge-case where the input is the point at infinity. + auto y_pow_2 = y1.sqr(); + auto x_pow_4 = x1 * (y_pow_2 - Group::curve_b); + auto lambda_squared = (x_pow_4 * 9) / (y_pow_2 * 4); + auto lambda = (x1 * x1 * 3) / (y1 + y1); + auto xx3 = lambda_squared - x1 - x1; + auto yy3 = lambda * (x1 - xx3) - y1; + + ASSERT(xx3 == result.get_value().x); + ASSERT(yy3 == result.get_value().y); + } else { auto x1 = x.get_value(); auto y1 = modified_y.get_value(); @@ -244,12 +268,13 @@ cycle_group cycle_group::dbl(const std::optional auto lambda = (x1 * x1 * 3) / (y1 + y1); auto x3 = lambda_squared - x1 - x1; auto y3 = lambda * (x1 - x3) - y1; + if (is_constant()) { + return cycle_group(x3, y3, is_point_at_infinity().get_value()); + } result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); } - if (is_constant()) { - return result; - } + context->create_ecc_dbl_gate(bb::ecc_dbl_gate_{ .x1 = x.get_witness_index(), .y1 = modified_y.normalize().get_witness_index(), @@ -271,7 +296,7 @@ cycle_group cycle_group::dbl(const std::optional */ template cycle_group cycle_group::unconditional_add( - const cycle_group& other, [[maybe_unused]] const std::optional /*unused*/ /*unused*/) const + const cycle_group& other, [[maybe_unused]] const std::optional /*unused*/) const requires IsNotUltraArithmetic { auto x_diff = other.x - x; @@ -297,7 +322,7 @@ cycle_group cycle_group::unconditional_add( */ template cycle_group cycle_group::unconditional_add(const cycle_group& other, - const std::optional hint) const + const std::optional hint) const requires IsUltraArithmetic { auto context = get_context(other); @@ -306,27 +331,38 @@ cycle_group cycle_group::unconditional_add(const cycle_group& const bool rhs_constant = other.is_constant(); if (lhs_constant && !rhs_constant) { auto lhs = cycle_group::from_constant_witness(context, get_value()); - return lhs.unconditional_add(other); + return lhs.unconditional_add(other, hint); } if (!lhs_constant && rhs_constant) { auto rhs = cycle_group::from_constant_witness(context, other.get_value()); - return unconditional_add(rhs); + return unconditional_add(rhs, hint); } cycle_group result; if (hint.has_value()) { - result = hint.value(); - } else { + auto x3 = hint.value().x; + auto y3 = hint.value().y; + if (lhs_constant && rhs_constant) { + return cycle_group(x3, y3, false); + } + result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); + + const auto p1 = get_value(); + const auto p2 = other.get_value(); + AffineElement p3(Element(p1) + Element(p2)); + ASSERT(p3.x == result.get_value().x); + ASSERT(p3.y == result.get_value().y); + } else { const auto p1 = get_value(); const auto p2 = other.get_value(); AffineElement p3(Element(p1) + Element(p2)); + if (lhs_constant && rhs_constant) { + return cycle_group(p3); + } field_t r_x(witness_t(context, p3.x)); field_t r_y(witness_t(context, p3.y)); result = cycle_group(r_x, r_y, false); } - if (lhs_constant && rhs_constant) { - return result; - } bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), .y1 = y.get_witness_index(), @@ -352,7 +388,7 @@ cycle_group cycle_group::unconditional_add(const cycle_group& */ template cycle_group cycle_group::unconditional_subtract(const cycle_group& other, - const std::optional hint) const + const std::optional hint) const { if constexpr (!IS_ULTRA) { return unconditional_add(-other, hint); @@ -372,20 +408,23 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr } cycle_group result; if (hint.has_value()) { - result = hint.value(); + auto x3 = hint.value().x; + auto y3 = hint.value().y; + if (lhs_constant && rhs_constant) { + return cycle_group(x3, y3, false); + } + result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); } else { - auto p1 = get_value(); auto p2 = other.get_value(); AffineElement p3(Element(p1) - Element(p2)); - + if (lhs_constant && rhs_constant) { + return cycle_group(p3); + } field_t r_x(witness_t(context, p3.x)); field_t r_y(witness_t(context, p3.y)); result = cycle_group(r_x, r_y, false); } - if (lhs_constant && rhs_constant) { - return result; - } bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), .y1 = y.get_witness_index(), @@ -415,7 +454,7 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr */ template cycle_group cycle_group::checked_unconditional_add(const cycle_group& other, - const std::optional hint) const + const std::optional hint) const { field_t x_delta = x - other.x; x_delta.assert_is_not_zero("cycle_group::checked_unconditional_add, x-coordinate collision"); @@ -436,7 +475,7 @@ cycle_group cycle_group::checked_unconditional_add(const cycle */ template cycle_group cycle_group::checked_unconditional_subtract(const cycle_group& other, - const std::optional hint) const + const std::optional hint) const { field_t x_delta = x - other.x; x_delta.assert_is_not_zero("cycle_group::checked_unconditional_subtract, x-coordinate collision"); @@ -798,7 +837,10 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, // convert an input cycle_scalar object into a vector of slices, each containing `table_bits` bits. // this also performs an implicit range check on the input slices const auto slice_scalar = [&](const field_t& scalar, const size_t num_bits) { - std::vector result; + std::pair, std::vector> result; + result.first.reserve(1ULL << table_bits); + result.second.reserve(1ULL << table_bits); + if (num_bits == 0) { return result; } @@ -808,11 +850,21 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, uint256_t raw_value = scalar.get_value(); for (size_t i = 0; i < num_slices; ++i) { uint64_t slice_v = static_cast(raw_value.data[0]) & table_mask; - result.push_back(field_t(slice_v)); + result.first.push_back(field_t(slice_v)); + result.second.push_back(slice_v); raw_value = raw_value >> table_bits; } return result; } + uint256_t raw_value = scalar.get_value(); + const uint64_t table_mask = (1ULL << table_bits) - 1ULL; + const size_t num_slices = (num_bits + table_bits - 1) / table_bits; + for (size_t i = 0; i < num_slices; ++i) { + uint64_t slice_v = static_cast(raw_value.data[0]) & table_mask; + result.second.push_back(slice_v); + raw_value = raw_value >> table_bits; + } + if constexpr (IS_ULTRA) { const auto slice_indices = context->decompose_into_default_range(scalar.normalize().get_witness_index(), @@ -820,26 +872,22 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, table_bits, "straus_scalar_slice decompose_into_default_range"); for (auto& idx : slice_indices) { - result.emplace_back(field_t::from_witness_index(context, idx)); + result.first.emplace_back(field_t::from_witness_index(context, idx)); } } else { - uint256_t raw_value = scalar.get_value(); - const uint64_t table_mask = (1ULL << table_bits) - 1ULL; - const size_t num_slices = (num_bits + table_bits - 1) / table_bits; for (size_t i = 0; i < num_slices; ++i) { - uint64_t slice_v = static_cast(raw_value.data[0]) & table_mask; + uint64_t slice_v = result.second[i]; field_t slice(witness_t(context, slice_v)); context->create_range_constraint( slice.get_witness_index(), table_bits, "straus_scalar_slice create_range_constraint"); - result.emplace_back(slice); - raw_value = raw_value >> table_bits; + result.first.push_back(slice); } std::vector linear_elements; FF scaling_factor = 1; for (size_t i = 0; i < num_slices; ++i) { - linear_elements.emplace_back(result[i] * scaling_factor); + linear_elements.emplace_back(result.first[i] * scaling_factor); scaling_factor += scaling_factor; } field_t::accumulate(linear_elements).assert_equal(scalar); @@ -852,8 +900,10 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, auto hi_slices = slice_scalar(scalar.hi, hi_bits); auto lo_slices = slice_scalar(scalar.lo, lo_bits); - std::copy(lo_slices.begin(), lo_slices.end(), std::back_inserter(slices)); - std::copy(hi_slices.begin(), hi_slices.end(), std::back_inserter(slices)); + std::copy(lo_slices.first.begin(), lo_slices.first.end(), std::back_inserter(slices)); + std::copy(hi_slices.first.begin(), hi_slices.first.end(), std::back_inserter(slices)); + std::copy(lo_slices.second.begin(), lo_slices.second.end(), std::back_inserter(slices_native)); + std::copy(hi_slices.second.begin(), hi_slices.second.end(), std::back_inserter(slices_native)); } /** @@ -874,6 +924,31 @@ std::optional> cycle_group::straus_scalar_slice::read( return slices[index]; } +template +std::vector::Element> cycle_group< + Builder>::straus_lookup_table::compute_straus_lookup_table_hints(const Element& base_point, + const Element& offset_generator, + size_t table_bits) +{ + const size_t table_size = 1UL << table_bits; + Element base = base_point.is_point_at_infinity() ? Group::one : base_point; + std::vector hints; + hints.emplace_back(offset_generator); + std::vector t; + t.emplace_back(offset_generator); + for (size_t i = 1; i < table_size; ++i) { + auto add_output = t[i - 1] + base; + hints.emplace_back(add_output); + // hints.emplace_back(hints[i - 1] + base); + if (base_point.is_point_at_infinity()) { + t.emplace_back(offset_generator); + } else { + t.emplace_back(add_output); + } + } + return hints; +} + /** * @brief Construct a new cycle group::straus lookup table::straus lookup table object * @@ -893,12 +968,13 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, const cycle_group& base_point, const cycle_group& offset_generator, size_t table_bits, - std::optional> hints) + std::optional> hints) : _table_bits(table_bits) , _context(context) { const size_t table_size = 1UL << table_bits; point_table.resize(table_size); + x_coordinate_checks.resize(table_size - 1); point_table[0] = offset_generator; // We want to support the case where input points are points at infinity. @@ -914,8 +990,10 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, field_t modded_y = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.y, base_point.y); cycle_group modded_base_point(modded_x, modded_y, false); for (size_t i = 1; i < table_size; ++i) { - std::optional hint = hints.has_value() ? hints.value()[i - 1] : std::optional(); - auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); + std::optional hint = + hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; + x_coordinate_checks[i - 1] = { point_table[i - 1].x, modded_base_point.x }; + auto add_output = point_table[i - 1].unconditional_add(modded_base_point, hint); field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); point_table[i] = cycle_group(x, y, false); @@ -1022,13 +1100,66 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ const size_t num_points = scalars.size(); std::vector scalar_slices; + + std::vector operation_transcript; + std::vector> native_straus_tables; + Element offset_generator_accumulator = offset_generators[0]; + { + for (size_t i = 0; i < num_points; ++i) { + std::vector native_straus_table; + native_straus_table.emplace_back(offset_generators[i + 1]); + size_t table_size = 1ULL << TABLE_BITS; + for (size_t j = 1; j < table_size; ++j) { + native_straus_table.emplace_back(native_straus_table[j - 1] + base_points[i].get_value()); + } + native_straus_tables.emplace_back(native_straus_table); + } + for (size_t i = 0; i < num_points; ++i) { + scalar_slices.emplace_back(straus_scalar_slice(context, scalars[i], TABLE_BITS)); + + auto table_transcript = straus_lookup_table::compute_straus_lookup_table_hints( + base_points[i].get_value(), offset_generators[i + 1], TABLE_BITS); + std::copy(table_transcript.begin() + 1, table_transcript.end(), std::back_inserter(operation_transcript)); + } + Element accumulator = offset_generators[0]; + + for (size_t i = 0; i < num_rounds; ++i) { + if (i != 0) { + for (size_t j = 0; j < TABLE_BITS; ++j) { + // offset_generator_accuulator is a regular Element, so dbl() won't add constraints + accumulator = accumulator.dbl(); + operation_transcript.emplace_back(accumulator); + offset_generator_accumulator = offset_generator_accumulator.dbl(); + } + } + for (size_t j = 0; j < num_points; ++j) { + + const Element point = native_straus_tables[j][scalar_slices[j].slices_native[num_rounds - i - 1]]; + + accumulator += point; + + operation_transcript.emplace_back(accumulator); + offset_generator_accumulator = offset_generator_accumulator + Element(offset_generators[j + 1]); + } + } + } + + Element::batch_normalize(&operation_transcript[0], operation_transcript.size()); + + std::vector operation_hints; + operation_hints.reserve(operation_transcript.size()); + for (auto& element : operation_transcript) { + operation_hints.emplace_back(AffineElement(element.x, element.y)); + } + std::vector point_tables; + const size_t hints_per_table = (1ULL << TABLE_BITS) - 1; for (size_t i = 0; i < num_points; ++i) { - scalar_slices.emplace_back(straus_scalar_slice(context, scalars[i], TABLE_BITS)); + std::span table_hints(&operation_hints[i * hints_per_table], hints_per_table); point_tables.emplace_back(straus_lookup_table(context, base_points[i], offset_generators[i + 1], TABLE_BITS)); } - Element offset_generator_accumulator = offset_generators[0]; + AffineElement* hint_ptr = &operation_hints[num_points * hints_per_table]; cycle_group accumulator = offset_generators[0]; // populate the set of points we are going to add into our accumulator, *before* we do any ECC operations @@ -1039,44 +1170,56 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ for (size_t i = 0; i < num_rounds; ++i) { for (size_t j = 0; j < num_points; ++j) { const std::optional scalar_slice = scalar_slices[j].read(num_rounds - i - 1); - // if we are doing a batch mul over scalars of different bit-lengths, we may not have any scalar bits for a - // given round and a given scalar + // if we are doing a batch mul over scalars of different bit-lengths, we may not have any scalar + // bits for a given round and a given scalar if (scalar_slice.has_value()) { const cycle_group point = point_tables[j].read(scalar_slice.value()); points_to_add.emplace_back(point); } } } + std::vector> x_coordinate_checks; size_t point_counter = 0; + if (!unconditional_add) { + for (size_t i = 0; i < num_points; ++i) { + std::copy(point_tables[i].x_coordinate_checks.begin(), + point_tables[i].x_coordinate_checks.end(), + std::back_inserter(x_coordinate_checks)); + } + } for (size_t i = 0; i < num_rounds; ++i) { if (i != 0) { for (size_t j = 0; j < TABLE_BITS; ++j) { - // offset_generator_accuulator is a regular Element, so dbl() won't add constraints - accumulator = accumulator.dbl(); - offset_generator_accumulator = offset_generator_accumulator.dbl(); + accumulator = accumulator.dbl(*hint_ptr); + hint_ptr++; } } for (size_t j = 0; j < num_points; ++j) { const std::optional scalar_slice = scalar_slices[j].read(num_rounds - i - 1); - // if we are doing a batch mul over scalars of different bit-lengths, we may not have a bit slice for a - // given round and a given scalar + // if we are doing a batch mul over scalars of different bit-lengths, we may not have a bit slice + // for a given round and a given scalar + ASSERT(scalar_slice.value().get_value() == scalar_slices[j].slices_native[num_rounds - i - 1]); if (scalar_slice.has_value()) { const auto& point = points_to_add[point_counter++]; if (!unconditional_add) { x_coordinate_checks.push_back({ accumulator.x, point.x }); } - accumulator = accumulator.unconditional_add(point); - offset_generator_accumulator = offset_generator_accumulator + Element(offset_generators[j + 1]); + accumulator = accumulator.unconditional_add(point, *hint_ptr); + hint_ptr++; } } } + // U cannot be zero + field_t coordinate_check_product = 1; for (auto& [x1, x2] : x_coordinate_checks) { auto x_diff = x2 - x1; - x_diff.assert_is_not_zero("_variable_base_batch_mul_internal x-coordinate collision"); + coordinate_check_product *= x_diff; } + coordinate_check_product.assert_is_not_zero("_variable_base_batch_mul_internal x-coordinate collision"); + /** * offset_generator_accumulator represents the sum of all the offset generator terms present in `accumulator`. * We don't subtract off yet, as we may be able to combine `offset_generator_accumulator` with other constant terms @@ -1143,12 +1286,28 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ ASSERT(offset_1.has_value()); offset_generator_accumulator += offset_1.value(); } + + std::vector operation_transcript; + { + Element accumulator = lookup_points[0].get_value(); + for (size_t i = 1; i < lookup_points.size(); ++i) { + accumulator = accumulator + (lookup_points[i].get_value()); + operation_transcript.emplace_back(accumulator); + } + } + Element::batch_normalize(&operation_transcript[0], operation_transcript.size()); + std::vector operation_hints; + operation_hints.reserve(operation_transcript.size()); + for (auto& element : operation_transcript) { + operation_hints.emplace_back(AffineElement(element.x, element.y)); + } + cycle_group accumulator = lookup_points[0]; // Perform all point additions sequentially. The Ultra ecc_addition relation costs 1 gate iff additions are chained // and output point of previous addition = input point of current addition. // If this condition is not met, the addition relation costs 2 gates. So it's good to do these sequentially! for (size_t i = 1; i < lookup_points.size(); ++i) { - accumulator = accumulator.unconditional_add(lookup_points[i]); + accumulator = accumulator.unconditional_add(lookup_points[i], operation_hints[i - 1]); } /** * offset_generator_accumulator represents the sum of all the offset generator terms present in `accumulator`. diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index d50f8fcc851..35709bb5b8d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -119,6 +119,7 @@ template class cycle_group { std::optional read(size_t index); size_t _table_bits; std::vector slices; + std::vector slices_native; }; /** @@ -148,16 +149,21 @@ template class cycle_group { */ struct straus_lookup_table { public: + static std::vector compute_straus_lookup_table_hints(const Element& base_point, + const Element& offset_generator, + size_t table_bits); + straus_lookup_table() = default; straus_lookup_table(Builder* context, const cycle_group& base_point, const cycle_group& offset_generator, size_t table_bits, - std::optional> hints = std::optional>()); + std::optional> hints = std::nullopt); cycle_group read(const field_t& index); size_t _table_bits; Builder* _context; std::vector point_table; + std::vector> x_coordinate_checks; size_t rom_id = 0; }; @@ -187,22 +193,22 @@ template class cycle_group { void set_point_at_infinity(const bool_t& is_infinity) { _is_infinity = is_infinity; } cycle_group get_standard_form() const; void validate_is_on_curve() const; - cycle_group dbl(const std::optional hint = std::optional()) const + cycle_group dbl(const std::optional hint = std::nullopt) const requires IsUltraArithmetic; - cycle_group dbl(const std::optional hint = std::optional()) const + cycle_group dbl(const std::optional hint = std::nullopt) const requires IsNotUltraArithmetic; cycle_group unconditional_add(const cycle_group& other, - const std::optional hint = std::optional()) const + const std::optional hint = std::nullopt) const requires IsUltraArithmetic; cycle_group unconditional_add(const cycle_group& other, - const std::optional hint = std::optional()) const + const std::optional hint = std::nullopt) const requires IsNotUltraArithmetic; cycle_group unconditional_subtract(const cycle_group& other, - const std::optional hint = std::optional()) const; + const std::optional hint = std::nullopt) const; cycle_group checked_unconditional_add(const cycle_group& other, - const std::optional hint = std::optional()) const; - cycle_group checked_unconditional_subtract( - const cycle_group& other, const std::optional hint = std::optional()) const; + const std::optional hint = std::nullopt) const; + cycle_group checked_unconditional_subtract(const cycle_group& other, + const std::optional hint = std::nullopt) const; cycle_group operator+(const cycle_group& other) const; cycle_group operator-(const cycle_group& other) const; cycle_group operator-() const; From 7b80a032e3364fede0ce70b290d305f827c7922f Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 01:02:10 +0000 Subject: [PATCH 03/15] removed x_coordinate_check from straus_lookup_table --- .../stdlib/primitives/group/cycle_group.cpp | 11 +---------- .../stdlib/primitives/group/cycle_group.hpp | 1 - 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 3312a9baaf5..2622825f572 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -974,7 +974,6 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, { const size_t table_size = 1UL << table_bits; point_table.resize(table_size); - x_coordinate_checks.resize(table_size - 1); point_table[0] = offset_generator; // We want to support the case where input points are points at infinity. @@ -992,8 +991,7 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, for (size_t i = 1; i < table_size; ++i) { std::optional hint = hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; - x_coordinate_checks[i - 1] = { point_table[i - 1].x, modded_base_point.x }; - auto add_output = point_table[i - 1].unconditional_add(modded_base_point, hint); + auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); point_table[i] = cycle_group(x, y, false); @@ -1181,13 +1179,6 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ std::vector> x_coordinate_checks; size_t point_counter = 0; - if (!unconditional_add) { - for (size_t i = 0; i < num_points; ++i) { - std::copy(point_tables[i].x_coordinate_checks.begin(), - point_tables[i].x_coordinate_checks.end(), - std::back_inserter(x_coordinate_checks)); - } - } for (size_t i = 0; i < num_rounds; ++i) { if (i != 0) { for (size_t j = 0; j < TABLE_BITS; ++j) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index 35709bb5b8d..da5b07bf66b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -163,7 +163,6 @@ template class cycle_group { size_t _table_bits; Builder* _context; std::vector point_table; - std::vector> x_coordinate_checks; size_t rom_id = 0; }; From 850db1bdff0cd88c32f30f623c02033981f39dfa Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 01:09:27 +0000 Subject: [PATCH 04/15] removed debug code --- .../stdlib/primitives/group/cycle_group.cpp | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 2622825f572..d8ba6701196 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -237,23 +237,6 @@ cycle_group cycle_group::dbl(const std::optional cycle_group::unconditional_add(const cycle_group& return cycle_group(x3, y3, false); } result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); - - const auto p1 = get_value(); - const auto p2 = other.get_value(); - AffineElement p3(Element(p1) + Element(p2)); - - ASSERT(p3.x == result.get_value().x); - ASSERT(p3.y == result.get_value().y); } else { const auto p1 = get_value(); const auto p2 = other.get_value(); From ca6da54a011b73e07301071aceb1c5771f8a44e4 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 01:20:32 +0000 Subject: [PATCH 05/15] minor optimization to remove 12 gates per mul in `batch_mul` when the base point is constant --- .../stdlib/primitives/group/cycle_group.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index d8ba6701196..1ad718cca8e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -964,14 +964,27 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, field_t modded_x = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.x, base_point.x); field_t modded_y = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.y, base_point.y); cycle_group modded_base_point(modded_x, modded_y, false); + const bool is_checked = !modded_base_point.is_constant(); + // if the input point is constant, it is cheaper to fix the point as a witness and then derive the table, than it is + // to derive the table and fix its witnesses to be constant! (due to group additions = 1 gate, and fixing x/y coords + // to be constant = 2 gates) + if (modded_base_point.is_constant()) { + modded_base_point = cycle_group::from_constant_witness(_context, modded_base_point.get_value()); + } for (size_t i = 1; i < table_size; ++i) { std::optional hint = hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; - auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); + cycle_group add_output; + if (is_checked) { + add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); + } else { + add_output = point_table[i - 1].unconditional_add(modded_base_point, hint); + } field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); point_table[i] = cycle_group(x, y, false); } + if constexpr (IS_ULTRA) { rom_id = context->create_ROM_array(table_size); for (size_t i = 0; i < table_size; ++i) { From a6f433579489ac9d77bdcee1f3203aa01302fede Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 02:17:41 +0000 Subject: [PATCH 06/15] wip --- .../commitment_schemes/ipa/ipa.hpp | 4 ++ .../eccvm_recursive_verifier.test.cpp | 2 +- .../stdlib/primitives/group/cycle_group.cpp | 37 ++++++++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp index 859bb7876ff..f6654a74af8 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp @@ -486,7 +486,9 @@ template class IPA { // Step 5. // Compute C₀ = C' + ∑_{j ∈ [k]} u_j^{-1}L_j + ∑_{j ∈ [k]} u_jR_j + std::cout << "about to batch mul LR sums" << std::endl; GroupElement LR_sums = GroupElement::batch_mul(msm_elements, msm_scalars); + std::cout << "done with sums" << std::endl; GroupElement C_zero = C_prime + LR_sums; @@ -527,7 +529,9 @@ template class IPA { // Compute G₀ // Unlike the native verification function, the verifier commitment key only containts the SRS so we can apply // batch_mul directly on it. + std::cout << "about to batch mul G sums" << std::endl; Commitment G_zero = Commitment::batch_mul(srs_elements, s_vec); + std::cout << "done with G sums" << std::endl; // Step 9. // Receive a₀ from the prover diff --git a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp index 5dc17cb76b5..43e18c956a5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp @@ -84,7 +84,7 @@ template class ECCVMRecursiveTests : public ::testing OuterBuilder outer_circuit; RecursiveVerifier verifier{ &outer_circuit, verification_key }; verifier.verify_proof(proof); - info("Recursive Verifier: num gates = ", outer_circuit.num_gates); + info("Recursive Verifier: num gates = ", outer_circuit.get_estimated_num_finalized_gates()); // Check for a failure flag in the recursive verifier circuit EXPECT_EQ(outer_circuit.failed(), false) << outer_circuit.err(); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 1ad718cca8e..2da63624f3d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -236,6 +236,7 @@ cycle_group cycle_group::dbl(const std::optional cycle_group::unconditional_add(const cycle_group& if (lhs_constant && rhs_constant) { return cycle_group(x3, y3, false); } - result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); + ASSERT(is_point_at_infinity().is_constant() == true); + result = cycle_group(witness_t(context, x3), witness_t(context, y3), false); } else { const auto p1 = get_value(); const auto p2 = other.get_value(); @@ -964,22 +966,28 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, field_t modded_x = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.x, base_point.x); field_t modded_y = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.y, base_point.y); cycle_group modded_base_point(modded_x, modded_y, false); - const bool is_checked = !modded_base_point.is_constant(); - // if the input point is constant, it is cheaper to fix the point as a witness and then derive the table, than it is - // to derive the table and fix its witnesses to be constant! (due to group additions = 1 gate, and fixing x/y coords - // to be constant = 2 gates) - if (modded_base_point.is_constant()) { - modded_base_point = cycle_group::from_constant_witness(_context, modded_base_point.get_value()); - } + // const bool is_checked = !modded_base_point.is_constant(); + // std::cout << "is checked = " << is_checked << ", is infinity constant = " << + // base_point.is_point_at_infinity().is_constant() << std::endl; if the input point is constant, it is cheaper to + // fix the point as a witness and then derive the table, than it is to derive the table and fix its witnesses to be + // constant! (due to group additions = 1 gate, and fixing x/y coords to be constant = 2 gates) if + // (modded_base_point.is_constant() && !base_point.is_point_at_infinity().get_value()) { + // modded_base_point = cycle_group::from_constant_witness(_context, modded_base_point.get_value()); + // point_table[0] = cycle_group::from_constant_witness(_context, offset_generator.get_value()); + // for (size_t i = 1; i < table_size; ++i) { + // std::optional hint = + // hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; + // cycle_group add_output; + // point_table[i] = point_table[i - 1].unconditional_add(modded_base_point, hint); + // } + // } + // else + + // A + B = C for (size_t i = 1; i < table_size; ++i) { std::optional hint = hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; - cycle_group add_output; - if (is_checked) { - add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); - } else { - add_output = point_table[i - 1].unconditional_add(modded_base_point, hint); - } + auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); point_table[i] = cycle_group(x, y, false); @@ -989,6 +997,7 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, rom_id = context->create_ROM_array(table_size); for (size_t i = 0; i < table_size; ++i) { if (point_table[i].is_constant()) { + std::cout << "fixing point table" << std::endl; auto element = point_table[i].get_value(); point_table[i] = cycle_group::from_constant_witness(_context, element); point_table[i].x.assert_equal(element.x); From 1e1668f81bc17163442f1cfcd745a99d691b7c8a Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 30 Oct 2024 02:47:33 +0000 Subject: [PATCH 07/15] minor efficiency improvements for straus_lookup_table constructor --- .../commitment_schemes/ipa/ipa.hpp | 1 - .../stdlib/primitives/group/cycle_group.cpp | 73 +++++++++---------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp index 917059de886..f877488330e 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp @@ -539,7 +539,6 @@ template class IPA { // batch_mul directly on it. const std::vector srs_elements = vk->get_monomial_points(); Commitment G_zero = Commitment::batch_mul(srs_elements, s_vec); - std::cout << "done with G sums" << std::endl; // Step 8. // Compute R = C' + ∑_{j ∈ [k]} u_j^{-1}L_j + ∑_{j ∈ [k]} u_jR_j - G₀ * a₀ - (f(\beta) + a₀ * b₀) ⋅ U diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index d9c1edf7ee9..04772fcdec3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -999,17 +999,8 @@ std::vector::Element> cycle_group< Element base = base_point.is_point_at_infinity() ? Group::one : base_point; std::vector hints; hints.emplace_back(offset_generator); - std::vector t; - t.emplace_back(offset_generator); for (size_t i = 1; i < table_size; ++i) { - auto add_output = t[i - 1] + base; - hints.emplace_back(add_output); - // hints.emplace_back(hints[i - 1] + base); - if (base_point.is_point_at_infinity()) { - t.emplace_back(offset_generator); - } else { - t.emplace_back(add_output); - } + hints.emplace_back(hints[i - 1] + base); } return hints; } @@ -1053,38 +1044,46 @@ cycle_group::straus_lookup_table::straus_lookup_table(Builder* context, field_t modded_x = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.x, base_point.x); field_t modded_y = field_t::conditional_assign(base_point.is_point_at_infinity(), fallback_point.y, base_point.y); cycle_group modded_base_point(modded_x, modded_y, false); - // const bool is_checked = !modded_base_point.is_constant(); - // std::cout << "is checked = " << is_checked << ", is infinity constant = " << - // base_point.is_point_at_infinity().is_constant() << std::endl; if the input point is constant, it is cheaper to - // fix the point as a witness and then derive the table, than it is to derive the table and fix its witnesses to be - // constant! (due to group additions = 1 gate, and fixing x/y coords to be constant = 2 gates) if - // (modded_base_point.is_constant() && !base_point.is_point_at_infinity().get_value()) { - // modded_base_point = cycle_group::from_constant_witness(_context, modded_base_point.get_value()); - // point_table[0] = cycle_group::from_constant_witness(_context, offset_generator.get_value()); - // for (size_t i = 1; i < table_size; ++i) { - // std::optional hint = - // hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; - // cycle_group add_output; - // point_table[i] = point_table[i - 1].unconditional_add(modded_base_point, hint); - // } - // } - // else - - // A + B = C - for (size_t i = 1; i < table_size; ++i) { - std::optional hint = - hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; - auto add_output = point_table[i - 1].checked_unconditional_add(modded_base_point, hint); - field_t x = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.x, add_output.x); - field_t y = field_t::conditional_assign(base_point.is_point_at_infinity(), offset_generator.y, add_output.y); - point_table[i] = cycle_group(x, y, false); - } + // if the input point is constant, it is cheaper to fix the point as a witness and then derive the table, than it is + // to derive the table and fix its witnesses to be constant! (due to group additions = 1 gate, and fixing x/y coords + // to be constant = 2 gates) + if (modded_base_point.is_constant() && !base_point.is_point_at_infinity().get_value()) { + modded_base_point = cycle_group::from_constant_witness(_context, modded_base_point.get_value()); + point_table[0] = cycle_group::from_constant_witness(_context, offset_generator.get_value()); + for (size_t i = 1; i < table_size; ++i) { + std::optional hint = + hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; + point_table[i] = point_table[i - 1].unconditional_add(modded_base_point, hint); + } + } else { + std::vector> x_coordinate_checks; + // ensure all of the ecc add gates are lined up so that we can pay 1 gate per add and not 2 + for (size_t i = 1; i < table_size; ++i) { + std::optional hint = + hints.has_value() ? std::optional(hints.value()[i - 1]) : std::nullopt; + x_coordinate_checks.emplace_back(point_table[i - 1].x, modded_base_point.x); + point_table[i] = point_table[i - 1].unconditional_add(modded_base_point, hint); + } + + // batch the x-coordinate checks together + // because `assert_is_not_zero` witness generation needs a modular inversion (expensive) + field_t coordinate_check_product = 1; + for (auto& [x1, x2] : x_coordinate_checks) { + auto x_diff = x2 - x1; + coordinate_check_product *= x_diff; + } + coordinate_check_product.assert_is_not_zero("straus_lookup_table x-coordinate collision"); + + for (size_t i = 1; i < table_size; ++i) { + point_table[i] = + cycle_group::conditional_assign(base_point.is_point_at_infinity(), offset_generator, point_table[i]); + } + } if constexpr (IS_ULTRA) { rom_id = context->create_ROM_array(table_size); for (size_t i = 0; i < table_size; ++i) { if (point_table[i].is_constant()) { - std::cout << "fixing point table" << std::endl; auto element = point_table[i].get_value(); point_table[i] = cycle_group::from_constant_witness(_context, element); point_table[i].x.assert_equal(element.x); From 96cedff7550c9d95eb77b52fa6a7e75ae04660ff Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Tue, 5 Nov 2024 23:28:25 +0000 Subject: [PATCH 08/15] disabled join split circuit change test --- .../examples/join_split/join_split.test.cpp | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp index 1ef0dfe3305..9d6a9f54ea7 100644 --- a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp +++ b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp @@ -19,12 +19,12 @@ using namespace bb::crypto::merkle_tree; /* Old join-split tests below. The value of having all of these logic tests is unclear, but we'll leave them around, at least for a while. */ -#ifdef CI -constexpr bool CIRCUIT_CHANGE_EXPECTED = false; -#else +// #ifdef CI +// constexpr bool CIRCUIT_CHANGE_EXPECTED = false; +// #else // During development, if the circuit vk hash/gate count is expected to change, set the following to true. -constexpr bool CIRCUIT_CHANGE_EXPECTED = false; -#endif +// constexpr bool CIRCUIT_CHANGE_EXPECTED = false; +// #endif using namespace bb; using namespace bb::stdlib; @@ -688,32 +688,31 @@ TEST_F(join_split_tests, test_withdraw_but_different_input_note_2_asset_id_fails // Input note combinations. Deposit/Send/Withdraw. // ************************************************************************************************************* -TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change) -{ - join_split_tx tx = zero_input_setup(); - tx.proof_id = proof_ids::DEPOSIT; - tx.public_value = 30; - tx.public_owner = fr::random_element(); - tx.output_note[0].value = 30; +// TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change) +// { +// join_split_tx tx = zero_input_setup(); +// tx.proof_id = proof_ids::DEPOSIT; +// tx.public_value = 30; +// tx.public_owner = fr::random_element(); +// tx.output_note[0].value = 30; - auto result = sign_and_verify_logic(tx, user.owner); +// auto result = sign_and_verify_logic(tx, user.owner); - EXPECT_TRUE(result.valid); +// EXPECT_TRUE(result.valid); - // The below part detects any changes in the join-split circuit - constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16; +// // The below part detects any changes in the join-split circuit - constexpr uint256_t CIRCUIT_HASH("0x2b30566e4d921ea9b0c76802d86ea5b8381ffa78ef143af1b0d0e3045862cb6b"); +// constexpr uint256_t CIRCUIT_HASH("0x2b30566e4d921ea9b0c76802d86ea5b8381ffa78ef143af1b0d0e3045862cb6b"); - const uint256_t circuit_hash = circuit.hash_circuit(); - // circuit is finalized now - if (!CIRCUIT_CHANGE_EXPECTED) { - EXPECT_LT(circuit.get_estimated_num_finalized_gates(), DYADIC_CIRCUIT_SIZE) - << "The gate count for the join-split circuit has crossed a power-of-two boundary."; - EXPECT_EQ(circuit_hash, CIRCUIT_HASH) - << "The circuit hash for the join_split circuit has changed to: " << circuit_hash; - } -} +// const uint256_t circuit_hash = circuit.hash_circuit(); +// // circuit is finalized now +// if (!CIRCUIT_CHANGE_EXPECTED) { +// EXPECT_LT(circuit.get_estimated_num_finalized_gates(), DYADIC_CIRCUIT_SIZE) +// << "The gate count for the join-split circuit has crossed a power-of-two boundary."; +// EXPECT_EQ(circuit_hash, CIRCUIT_HASH) +// << "The circuit hash for the join_split circuit has changed to: " << circuit_hash; +// } +// } // Bespoke test seeking bug. TEST_F(join_split_tests, test_0_input_notes_create_dupicate_output_notes_fails) From 2dcde3009f4fc9ffa95f705254f95cd77d95f1ab Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 6 Nov 2024 03:46:39 +0000 Subject: [PATCH 09/15] compiler fix --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 04772fcdec3..67e0ac8ddf4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -903,8 +903,8 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, // this also performs an implicit range check on the input slices const auto slice_scalar = [&](const field_t& scalar, const size_t num_bits) { std::pair, std::vector> result; - result.first.reserve(1ULL << table_bits); - result.second.reserve(1ULL << table_bits); + result.first.reserve(static_cast(1ULL) << table_bits); + result.second.reserve(static_cast(1ULL) << table_bits); if (num_bits == 0) { return result; From f841ba03a1210b4ac7313cb0e7836f2bd34bf58f Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Wed, 11 Dec 2024 15:35:12 +0000 Subject: [PATCH 10/15] fixed wasm compiler error --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index d4c7a961533..51d849ebece 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -1271,7 +1271,8 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ } for (size_t j = 0; j < num_points; ++j) { - const Element point = native_straus_tables[j][scalar_slices[j].slices_native[num_rounds - i - 1]]; + const Element point = + native_straus_tables[j][static_cast(scalar_slices[j].slices_native[num_rounds - i - 1])]; accumulator += point; From 267d8ad44cef2f642b0d956c97f5485508ed8dd8 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 12 Dec 2024 12:52:57 +0000 Subject: [PATCH 11/15] added comments and added back in join_split_test --- .../examples/join_split/join_split.test.cpp | 51 ++++++++++--------- .../stdlib/primitives/group/cycle_group.cpp | 42 +++++++++++++-- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp index 4b80b679f0e..bac099cc407 100644 --- a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp +++ b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp @@ -19,12 +19,12 @@ using namespace bb::crypto::merkle_tree; /* Old join-split tests below. The value of having all of these logic tests is unclear, but we'll leave them around, at least for a while. */ -// #ifdef CI -// constexpr bool CIRCUIT_CHANGE_EXPECTED = false; -// #else +#ifdef CI +constexpr bool CIRCUIT_CHANGE_EXPECTED = false; +#else // During development, if the circuit vk hash/gate count is expected to change, set the following to true. -// constexpr bool CIRCUIT_CHANGE_EXPECTED = false; -// #endif +constexpr bool CIRCUIT_CHANGE_EXPECTED = false; +#endif using namespace bb; using namespace bb::stdlib; @@ -688,31 +688,32 @@ TEST_F(join_split_tests, test_withdraw_but_different_input_note_2_asset_id_fails // Input note combinations. Deposit/Send/Withdraw. // ************************************************************************************************************* -// TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change) -// { -// join_split_tx tx = zero_input_setup(); -// tx.proof_id = proof_ids::DEPOSIT; -// tx.public_value = 30; -// tx.public_owner = fr::random_element(); -// tx.output_note[0].value = 30; +TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change) +{ + join_split_tx tx = zero_input_setup(); + tx.proof_id = proof_ids::DEPOSIT; + tx.public_value = 30; + tx.public_owner = fr::random_element(); + tx.output_note[0].value = 30; -// auto result = sign_and_verify_logic(tx, user.owner); + auto result = sign_and_verify_logic(tx, user.owner); -// EXPECT_TRUE(result.valid); + EXPECT_TRUE(result.valid); -// // The below part detects any changes in the join-split circuit + // The below part detects any changes in the join-split circuit + constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16; -// constexpr uint256_t CIRCUIT_HASH("0x2b30566e4d921ea9b0c76802d86ea5b8381ffa78ef143af1b0d0e3045862cb6b"); + constexpr uint256_t CIRCUIT_HASH("0x727924c3ea2266dc2b6a38f335019c26a941911d5991f25adb3ce9c288af7751"); -// const uint256_t circuit_hash = circuit.hash_circuit(); -// // circuit is finalized now -// if (!CIRCUIT_CHANGE_EXPECTED) { -// EXPECT_LT(circuit.get_estimated_num_finalized_gates(), DYADIC_CIRCUIT_SIZE) -// << "The gate count for the join-split circuit has crossed a power-of-two boundary."; -// EXPECT_EQ(circuit_hash, CIRCUIT_HASH) -// << "The circuit hash for the join_split circuit has changed to: " << circuit_hash; -// } -// } + const uint256_t circuit_hash = circuit.hash_circuit(); + // circuit is finalized now + if (!CIRCUIT_CHANGE_EXPECTED) { + EXPECT_LT(circuit.get_estimated_num_finalized_gates(), DYADIC_CIRCUIT_SIZE) + << "The gate count for the join-split circuit has crossed a power-of-two boundary."; + EXPECT_EQ(circuit_hash, CIRCUIT_HASH) + << "The circuit hash for the join_split circuit has changed to: " << circuit_hash; + } +} // Bespoke test seeking bug. TEST_F(join_split_tests, test_0_input_notes_create_dupicate_output_notes_fails) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 51d849ebece..cb356bfce93 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -314,6 +314,7 @@ cycle_group cycle_group::unconditional_add( * * @tparam Builder * @param other + * @param hint : value of output point witness, if known ahead of time (used to avoid modular inversions during witgen) * @return cycle_group */ template @@ -383,6 +384,7 @@ cycle_group cycle_group::unconditional_add(const cycle_group& * * @tparam Builder * @param other + * @param hint : value of output point witness, if known ahead of time (used to avoid modular inversions during witgen) * @return cycle_group */ template @@ -458,6 +460,7 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr * * @tparam Builder * @param other + * @param hint : value of output point witness, if known ahead of time (used to avoid modular inversions during witgen) * @return cycle_group */ template @@ -479,6 +482,7 @@ cycle_group cycle_group::checked_unconditional_add(const cycle * * @tparam Builder * @param other + * @param hint : value of output point witness, if known ahead of time (used to avoid modular inversions during witgen) * @return cycle_group */ template @@ -943,6 +947,9 @@ cycle_group::straus_scalar_slice::straus_scalar_slice(Builder* context, // convert an input cycle_scalar object into a vector of slices, each containing `table_bits` bits. // this also performs an implicit range check on the input slices const auto slice_scalar = [&](const field_t& scalar, const size_t num_bits) { + // we record the scalar slices both as field_t circuit elements and u64 values + // (u64 values are used to index arrays and we don't want to repeatedly cast a stdlib value to a numeric + // primitive as this gets expensive when repeated enough times) std::pair, std::vector> result; result.first.reserve(static_cast(1ULL) << table_bits); result.second.reserve(static_cast(1ULL) << table_bits); @@ -1036,6 +1043,19 @@ std::optional> cycle_group::straus_scalar_slice::read( return slices[index]; } +/** + * @brief Compute the output points generated when computing the Straus lookup table + * @details When performing an MSM, we first compute all the witness values as Element types (with a Z-coordinate), + * and then we batch-convert the points into affine representation `AffineElement` + * This avoids the need to compute a modular inversion for every group operation, + * which dramatically cuts witness generation times + * + * @tparam Builder + * @param base_point + * @param offset_generator + * @param table_bits + * @return std::vector::Element> + */ template std::vector::Element> cycle_group< Builder>::straus_lookup_table::compute_straus_lookup_table_hints(const Element& base_point, @@ -1238,6 +1258,12 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ std::vector scalar_slices; + /** + * Compute the witness values of the batch_mul algorithm natively, as Element types with a Z-coordinate. + * We then batch-convert to AffineElement types, and feed this points as "hints" into the cycle_group methods. + * This avoids the need to compute modular inversions for every group operation, which dramatically reduces witness + * generation times + */ std::vector operation_transcript; std::vector> native_straus_tables; Element offset_generator_accumulator = offset_generators[0]; @@ -1282,6 +1308,7 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ } } + // Normalize the computed witness points and convert into AffineElement type Element::batch_normalize(&operation_transcript[0], operation_transcript.size()); std::vector operation_hints; @@ -1312,8 +1339,8 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ for (size_t i = 0; i < num_rounds; ++i) { for (size_t j = 0; j < num_points; ++j) { const std::optional scalar_slice = scalar_slices[j].read(num_rounds - i - 1); - // if we are doing a batch mul over scalars of different bit-lengths, we may not have any scalar - // bits for a given round and a given scalar + // if we are doing a batch mul over scalars of different bit-lengths, we may not have any scalar bits for a + // given round and a given scalar if (scalar_slice.has_value()) { const cycle_group point = point_tables[j].read(scalar_slice.value()); points_to_add.emplace_back(point); @@ -1347,7 +1374,9 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ } } - // U cannot be zero + // validate that none of the x-coordinate differences are zero + // we batch the x-coordinate checks together + // because `assert_is_not_zero` witness generation needs a modular inversion (expensive) field_t coordinate_check_product = 1; for (auto& [x1, x2] : x_coordinate_checks) { auto x_diff = x2 - x1; @@ -1427,7 +1456,12 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ ASSERT(offset_1.has_value()); offset_generator_accumulator += offset_1.value(); } - + /** + * Compute the witness values of the batch_mul algorithm natively, as Element types with a Z-coordinate. + * We then batch-convert to AffineElement types, and feed this points as "hints" into the cycle_group methods. + * This avoids the need to compute modular inversions for every group operation, which dramatically reduces witness + * generation times + */ std::vector operation_transcript; { Element accumulator = lookup_points[0].get_value(); From a9f0629b85c66428d51ce70c6c7a68a2ccfd6da5 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 12 Dec 2024 12:58:18 +0000 Subject: [PATCH 12/15] added missing hint description --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index cb356bfce93..a037cd41c18 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -202,6 +202,7 @@ template cycle_group cycle_group::get_stand * @brief Evaluates a doubling. Does not use Ultra double gate * * @tparam Builder + * @param unused param is due to interface-compatibility with the UltraArithmetic version of `dbl` * @return cycle_group */ template @@ -219,6 +220,7 @@ cycle_group cycle_group::dbl([[maybe_unused]] const std::optio * @brief Evaluates a doubling. Uses Ultra double gate * * @tparam Builder + * @param hint : value of output point witness, if known ahead of time (used to avoid modular inversions during witgen) * @return cycle_group */ template From 197cf5d693aa2c9b63992f085162bc753dcb4dbc Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Thu, 12 Dec 2024 13:00:31 +0000 Subject: [PATCH 13/15] grammar --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index a037cd41c18..e0bcea1f14e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -1262,7 +1262,7 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ /** * Compute the witness values of the batch_mul algorithm natively, as Element types with a Z-coordinate. - * We then batch-convert to AffineElement types, and feed this points as "hints" into the cycle_group methods. + * We then batch-convert to AffineElement types, and feed these points as "hints" into the cycle_group methods. * This avoids the need to compute modular inversions for every group operation, which dramatically reduces witness * generation times */ @@ -1460,7 +1460,7 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ } /** * Compute the witness values of the batch_mul algorithm natively, as Element types with a Z-coordinate. - * We then batch-convert to AffineElement types, and feed this points as "hints" into the cycle_group methods. + * We then batch-convert to AffineElement types, and feed these points as "hints" into the cycle_group methods. * This avoids the need to compute modular inversions for every group operation, which dramatically reduces witness * generation times */ From 4379bfb16a1ac48a696bf62e13488da9f193b1e4 Mon Sep 17 00:00:00 2001 From: zac-williamson Date: Fri, 20 Dec 2024 15:31:51 +0000 Subject: [PATCH 14/15] PR comments --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index e0bcea1f14e..080e5a20087 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -243,7 +243,6 @@ cycle_group cycle_group::dbl(const std::optional cycle_group::unconditional_add(const cycle_group& if (lhs_constant && rhs_constant) { return cycle_group(x3, y3, false); } - ASSERT(is_point_at_infinity().is_constant() == true); result = cycle_group(witness_t(context, x3), witness_t(context, y3), false); } else { const auto p1 = get_value(); From ce032f0d984f6f148007c825bfede6c79ba32a0c Mon Sep 17 00:00:00 2001 From: Cody Gunton Date: Mon, 6 Jan 2025 15:29:46 -0500 Subject: [PATCH 15/15] Update circuit hash --- .../src/barretenberg/examples/join_split/join_split.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp index bac099cc407..b02d82664d2 100644 --- a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp +++ b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp @@ -703,7 +703,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change) // The below part detects any changes in the join-split circuit constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16; - constexpr uint256_t CIRCUIT_HASH("0x727924c3ea2266dc2b6a38f335019c26a941911d5991f25adb3ce9c288af7751"); + constexpr uint256_t CIRCUIT_HASH("0x48687216f00a81d2a0f64f0a10cce056fce2ad13c47f8329229eb3712d3f7566"); const uint256_t circuit_hash = circuit.hash_circuit(); // circuit is finalized now