From 9f962a8788992cd6a2c74f7a5237b66197168f54 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 28 Feb 2025 20:09:20 +0000 Subject: [PATCH 01/36] Several patches to cycle_group and cycle_group fuzzer --- .../stdlib/primitives/group/cycle_group.cpp | 75 +++++++++++++--- .../primitives/group/cycle_group.fuzzer.hpp | 56 ++++-------- .../stdlib/primitives/group/cycle_group.hpp | 24 ++++- .../primitives/group/cycle_group.test.cpp | 89 +++++++++++++++++++ 4 files changed, 190 insertions(+), 54 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 8a73645f03da..ed2ea61c15fc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -16,6 +16,7 @@ cycle_group::cycle_group(Builder* _context) , y(0) , _is_infinity(true) , _is_constant(true) + , _is_standart(true) , context(_context) {} @@ -27,11 +28,12 @@ cycle_group::cycle_group(Builder* _context) * @param is_infinity */ template -cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity) +cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bool is_standart) : x(_x.normalize()) , y(_y.normalize()) , _is_infinity(is_infinity) , _is_constant(_x.is_constant() && _y.is_constant() && is_infinity.is_constant()) + , _is_standart(is_standart) { if (_x.get_context() != nullptr) { context = _x.get_context(); @@ -61,11 +63,12 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity) * @param is_infinity */ template -cycle_group::cycle_group(const FF& _x, const FF& _y, bool is_infinity) +cycle_group::cycle_group(const FF& _x, const FF& _y, bool is_infinity, bool is_standart) : x(_x) , y(_y) , _is_infinity(is_infinity) , _is_constant(true) + , _is_standart(is_standart) , context(nullptr) { ASSERT(get_value().on_curve()); @@ -87,6 +90,7 @@ cycle_group::cycle_group(const AffineElement& _in) , y(_in.is_point_at_infinity() ? 0 : _in.y) , _is_infinity(_in.is_point_at_infinity()) , _is_constant(true) + , _is_standart(true) , context(nullptr) {} @@ -120,10 +124,20 @@ template cycle_group cycle_group::from_witness(Builder* _context, const AffineElement& _in) { cycle_group result(_context); - result.x = field_t(witness_t(_context, _in.x)); - result.y = field_t(witness_t(_context, _in.y)); + + // Point at infinity's coordinates break our arithmetic + // Since we are not using these coordinates anyway + // We can set them both to be zero + if (_in.is_point_at_infinity()) { + result.x = field_t(witness_t(_context, FF::zero())); + result.y = field_t(witness_t(_context, FF::zero())); + } else { + result.x = field_t(witness_t(_context, _in.x)); + result.y = field_t(witness_t(_context, _in.y)); + } result._is_infinity = bool_t(witness_t(_context, _in.is_point_at_infinity())); result._is_constant = false; + result._is_standart = true; result.validate_is_on_curve(); return result; } @@ -144,13 +158,24 @@ template cycle_group cycle_group::from_constant_witness(Builder* _context, const AffineElement& _in) { cycle_group result(_context); - result.x = field_t(witness_t(_context, _in.x)); - result.y = field_t(witness_t(_context, _in.y)); - result.x.assert_equal(_in.x); - result.y.assert_equal(_in.y); + + // Point at infinity's coordinates break our arithmetic + // Since we are not using these coordinates anyway + // We can set them both to be zero + if (_in.is_point_at_infinity()) { + result.x = field_t(witness_t(_context, FF::zero())); + result.y = field_t(witness_t(_context, FF::zero())); + } else { + result.x = field_t(witness_t(_context, _in.x)); + result.y = field_t(witness_t(_context, _in.y)); + } + result.x.assert_equal(result.x.get_value()); + result.y.assert_equal(result.y.get_value()); + // point at infinity is circuit constant result._is_infinity = _in.is_point_at_infinity(); result._is_constant = false; + result._is_standart = true; return result; } @@ -194,9 +219,12 @@ template void cycle_group::validate_is_on_curve() co */ template cycle_group cycle_group::get_standard_form() const { - return cycle_group(field_t::conditional_assign(_is_infinity, 0, x), - field_t::conditional_assign(_is_infinity, 0, y), - is_point_at_infinity()); + if (this->_is_standart) { + return *this; + } + cycle_group result = *this; + result.set_point_at_infinity(result.is_point_at_infinity()); + return result; } /** * @brief Evaluates a doubling. Does not use Ultra double gate @@ -231,6 +259,12 @@ cycle_group cycle_group::dbl(const std::optionalis_point_at_infinity().is_constant() && this->is_point_at_infinity().get_value()) { + modified_y = field_t::from_witness_index(context, context->put_constant_variable(1)); + } + cycle_group result; if (hint.has_value()) { auto x3 = hint.value().x; @@ -468,7 +502,11 @@ cycle_group cycle_group::checked_unconditional_add(const cycle 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"); + if (x_delta.is_constant()) { + ASSERT(x_delta.get_value() != 0); + } else { + x_delta.assert_is_not_zero("cycle_group::checked_unconditional_add, x-coordinate collision"); + } return unconditional_add(other, hint); } @@ -490,7 +528,11 @@ cycle_group cycle_group::checked_unconditional_subtract(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"); + if (x_delta.is_constant()) { + ASSERT(x_delta.get_value() != 0); + } else { + x_delta.assert_is_not_zero("cycle_group::checked_unconditional_subtract, x-coordinate collision"); + } return unconditional_subtract(other, hint); } @@ -557,6 +599,10 @@ template cycle_group cycle_group::operator+ bool_t result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity); result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); result.set_point_at_infinity(result_is_infinity); + + ASSERT(result.x.is_constant() == result.y.is_constant()); + result._is_constant = result.x.is_constant(); + return result; } @@ -614,6 +660,9 @@ template cycle_group cycle_group::operator- result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); result.set_point_at_infinity(result_is_infinity); + ASSERT(result.x.is_constant() == result.y.is_constant()); + result._is_constant = result.x.is_constant(); + return result; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 33dd28d76f33..52eec288cd28 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -6,6 +6,7 @@ #pragma clang diagnostic push // TODO(luke/kesha): Add a comment explaining why we need this ignore and what the solution is. // TODO(alex): resolve this todo in current pr +// TODO #pragma clang diagnostic ignored "-Wc99-designator" #define HAVOC_TESTING @@ -782,9 +783,8 @@ template class CycleGroupBase { ScalarField base_scalar_res = this->base_scalar + other.base_scalar; GroupElement base_res = this->base + other.base; - bool can_fail = false; if (other.cg().get_value() == this->cg().get_value()) { - uint8_t dbl_path = VarianceRNG.next() % 3; + uint8_t dbl_path = VarianceRNG.next() % 4; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(dbl_path) << " dbl path" << std::endl; #endif @@ -794,11 +794,12 @@ template class CycleGroupBase { case 1: return ExecutionHandler(base_scalar_res, base_res, other.cg().dbl()); case 2: - can_fail = true; - break; + return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); + case 3: + return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } else if (other.cg().get_value() == -this->cg().get_value()) { - uint8_t inf_path = VarianceRNG.next() % 3; + uint8_t inf_path = VarianceRNG.next() % 4; cycle_group_t res; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(inf_path) << " inf path" << std::endl; @@ -813,41 +814,30 @@ template class CycleGroupBase { res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); return ExecutionHandler(base_scalar_res, base_res, res); case 2: - can_fail = true; - break; + return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); + case 3: + return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } -#ifdef SHOW_INFORMATION - std::cout << "Edge case? " << can_fail << std::endl; -#endif - - uint8_t add_option = VarianceRNG.next() % 5; + uint8_t add_option = VarianceRNG.next() % 6; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(add_option) << " add path" << std::endl; #endif switch (add_option) { case 0: - circuit_should_fail = circuit_should_fail | can_fail; return ExecutionHandler(base_scalar_res, base_res, this->cg().unconditional_add(other.cg())); case 1: - circuit_should_fail = circuit_should_fail | can_fail; return ExecutionHandler(base_scalar_res, base_res, other.cg().unconditional_add(this->cg())); case 2: - if (!(this->cycle_group.is_constant() && other.cycle_group.is_constant())) { - circuit_should_fail = circuit_should_fail | can_fail; - return ExecutionHandler( - base_scalar_res, base_res, this->cg().checked_unconditional_add(other.cg())); - } + return ExecutionHandler(base_scalar_res, base_res, this->cg().checked_unconditional_add(other.cg())); case 3: - if (!(this->cycle_group.is_constant() && other.cycle_group.is_constant())) { - circuit_should_fail = circuit_should_fail | can_fail; - return ExecutionHandler( - base_scalar_res, base_res, other.cg().checked_unconditional_add(this->cg())); - } + return ExecutionHandler(base_scalar_res, base_res, other.cg().checked_unconditional_add(this->cg())); case 4: return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); + case 5: + return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } return {}; } @@ -857,7 +847,6 @@ template class CycleGroupBase { ScalarField base_scalar_res = this->base_scalar - other.base_scalar; GroupElement base_res = this->base - other.base; - bool can_fail = false; if (other.cg().get_value() == -this->cg().get_value()) { uint8_t dbl_path = VarianceRNG.next() % 3; #ifdef SHOW_INFORMATION @@ -870,8 +859,7 @@ template class CycleGroupBase { case 1: return ExecutionHandler(base_scalar_res, base_res, other.cg().dbl()); case 2: - can_fail = true; - break; + return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } else if (other.cg().get_value() == this->cg().get_value()) { uint8_t inf_path = VarianceRNG.next() % 3; @@ -890,13 +878,9 @@ template class CycleGroupBase { res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); return ExecutionHandler(base_scalar_res, base_res, res); case 2: - can_fail = true; - break; + return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } -#ifdef SHOW_INFORMATION - std::cout << "Edge case? " << can_fail << std::endl; -#endif uint8_t add_option = VarianceRNG.next() % 3; #ifdef SHOW_INFORMATION @@ -905,14 +889,10 @@ template class CycleGroupBase { switch (add_option) { case 0: - circuit_should_fail = circuit_should_fail | can_fail; return ExecutionHandler(base_scalar_res, base_res, this->cg().unconditional_subtract(other.cg())); case 1: - if (!(this->cycle_group.is_constant() && other.cycle_group.is_constant())) { - circuit_should_fail = circuit_should_fail | can_fail; - return ExecutionHandler( - base_scalar_res, base_res, this->cg().checked_unconditional_subtract(other.cg())); - } + return ExecutionHandler( + base_scalar_res, base_res, this->cg().checked_unconditional_subtract(other.cg())); case 2: return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } 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 ab008de22a36..38b2027c6cc7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -196,8 +196,8 @@ template class cycle_group { public: cycle_group(Builder* _context = nullptr); - cycle_group(field_t _x, field_t _y, bool_t _is_infinity); - cycle_group(const FF& _x, const FF& _y, bool _is_infinity); + cycle_group(field_t _x, field_t _y, bool_t _is_infinity, bool is_standart = false); + cycle_group(const FF& _x, const FF& _y, bool _is_infinity, bool is_standart = false); cycle_group(const AffineElement& _in); static cycle_group one(Builder* _context); static cycle_group from_witness(Builder* _context, const AffineElement& _in); @@ -207,7 +207,21 @@ template class cycle_group { AffineElement get_value() const; [[nodiscard]] bool is_constant() const { return _is_constant; } bool_t is_point_at_infinity() const { return _is_infinity; } - void set_point_at_infinity(const bool_t& is_infinity) { _is_infinity = is_infinity; } + void set_point_at_infinity(const bool_t& is_infinity) + { + // I am not sure that it handles all the cases + // when we can bump into non standard case + if (is_infinity.is_constant() && is_infinity.get_value()) { + this->x = 0; + this->y = 0; + this->_is_constant = true; + } else { + this->x = field_t::conditional_assign(is_infinity, 0, this->x); + this->y = field_t::conditional_assign(is_infinity, 0, this->y); + } + _is_infinity = is_infinity; + this->_is_standart = true; + } cycle_group get_standard_form() const; void validate_is_on_curve() const; cycle_group dbl(const std::optional hint = std::nullopt) const @@ -280,6 +294,10 @@ template class cycle_group { private: bool_t _is_infinity; bool _is_constant; + // Most of the time it is true, so we won't need to do extra conditional_assign + // during `get_standart_form` call + // However sometimes it won't be the case, so we can handle these cases using this flag + bool _is_standart; Builder* context; static batch_mul_internal_output _variable_base_batch_mul_internal(std::span scalars, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index def534437d16..5b713f2688a7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -84,6 +84,57 @@ TYPED_TEST(CycleGroupTest, TestBasicTagLogic) #endif } +/** + * @brief Checks that a point at infinity passes the constant_witness initialization + * + */ +TYPED_TEST(CycleGroupTest, TestInfConstantWintnessRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto lhs = TestFixture::generators[0] * 0; + cycle_group_ct a = cycle_group_ct::from_constant_witness(&builder, lhs); + (void)a; + EXPECT_FALSE(builder.failed()); + EXPECT_TRUE(CircuitChecker::check(builder)); +} + +/** + * @brief Checks that a point at infinity passes the witness initialization + * + */ +TYPED_TEST(CycleGroupTest, TestInfWintnessRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto lhs = TestFixture::generators[0] * 0; + cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); + (void)a; + EXPECT_FALSE(builder.failed()); + EXPECT_TRUE(CircuitChecker::check(builder)); +} + +/** + * @brief Checks that the result of adding two witness values is not constant + * + */ +TYPED_TEST(CycleGroupTest, TestWitnessSumRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto lhs = TestFixture::generators[0]; + auto rhs = TestFixture::generators[1]; + cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); + cycle_group_ct b = cycle_group_ct::from_witness(&builder, rhs); + cycle_group_ct c = a + b; + EXPECT_FALSE(c.is_constant()); + c = a - b; + EXPECT_FALSE(c.is_constant()); +} + /** * @brief Checks that a point on the curve passes the validate_is_on_curve check * @@ -138,6 +189,25 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail) EXPECT_FALSE(CircuitChecker::check(builder)); } +/** + * @brief Checks that a point that is not on the curve but *not* marked as the point at infinity fails the + * validate_is_on_curve check + * @details (1, 1) is not on the either the Grumpkin curve or the BN254 curve. + */ +TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail2) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto x = stdlib::field_t::from_witness(&builder, 1); + auto y = stdlib::field_t::from_witness(&builder, 1); + + cycle_group_ct a(x, y, /*_is_infinity=*/bool_ct(witness_ct(&builder, false))); + a.validate_is_on_curve(); + EXPECT_TRUE(builder.failed()); + EXPECT_FALSE(CircuitChecker::check(builder)); +} + TYPED_TEST(CycleGroupTest, TestStandardForm) { STDLIB_TYPE_ALIASES; @@ -151,6 +221,11 @@ TYPED_TEST(CycleGroupTest, TestStandardForm) input_b.set_point_at_infinity(true); input_d.set_point_at_infinity(true); + auto x = stdlib::field_t::from_witness(&builder, 1); + auto y = stdlib::field_t::from_witness(&builder, 1); + cycle_group_ct input_e = cycle_group_ct(x, y, true); + cycle_group_ct input_f = cycle_group_ct(x, y, bool_ct(witness_ct(&builder, true))); + // Assign different tags to all inputs input_a.set_origin_tag(submitted_value_origin_tag); input_b.set_origin_tag(challenge_origin_tag); @@ -161,11 +236,15 @@ TYPED_TEST(CycleGroupTest, TestStandardForm) auto standard_b = input_b.get_standard_form(); auto standard_c = input_c.get_standard_form(); auto standard_d = input_d.get_standard_form(); + auto standard_e = input_e.get_standard_form(); + auto standard_f = input_f.get_standard_form(); EXPECT_EQ(standard_a.is_point_at_infinity().get_value(), false); EXPECT_EQ(standard_b.is_point_at_infinity().get_value(), true); EXPECT_EQ(standard_c.is_point_at_infinity().get_value(), false); EXPECT_EQ(standard_d.is_point_at_infinity().get_value(), true); + EXPECT_EQ(standard_e.is_point_at_infinity().get_value(), true); + EXPECT_EQ(standard_f.is_point_at_infinity().get_value(), true); // Ensure that the tags in the standard form remain the same EXPECT_EQ(standard_a.get_origin_tag(), submitted_value_origin_tag); @@ -190,6 +269,12 @@ TYPED_TEST(CycleGroupTest, TestStandardForm) auto standard_d_x = standard_d.x.get_value(); auto standard_d_y = standard_d.y.get_value(); + auto standard_e_x = standard_e.x.get_value(); + auto standard_e_y = standard_e.y.get_value(); + + auto standard_f_x = standard_f.x.get_value(); + auto standard_f_y = standard_f.y.get_value(); + EXPECT_EQ(input_a_x, standard_a_x); EXPECT_EQ(input_a_y, standard_a_y); EXPECT_EQ(standard_b_x, 0); @@ -198,6 +283,10 @@ TYPED_TEST(CycleGroupTest, TestStandardForm) EXPECT_EQ(input_c_y, standard_c_y); EXPECT_EQ(standard_d_x, 0); EXPECT_EQ(standard_d_y, 0); + EXPECT_EQ(standard_e_x, 0); + EXPECT_EQ(standard_e_y, 0); + EXPECT_EQ(standard_f_x, 0); + EXPECT_EQ(standard_f_y, 0); EXPECT_TRUE(CircuitChecker::check(builder)); } From d5a6c555839f55a1ab9355499febe3709560e768 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 28 Feb 2025 20:13:20 +0000 Subject: [PATCH 02/36] standard --- .../stdlib/primitives/group/cycle_group.cpp | 18 +++++++++--------- .../stdlib/primitives/group/cycle_group.hpp | 10 +++++----- 2 files changed, 14 insertions(+), 14 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 ed2ea61c15fc..7ed5356825da 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -16,7 +16,7 @@ cycle_group::cycle_group(Builder* _context) , y(0) , _is_infinity(true) , _is_constant(true) - , _is_standart(true) + , _is_standard(true) , context(_context) {} @@ -28,12 +28,12 @@ cycle_group::cycle_group(Builder* _context) * @param is_infinity */ template -cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bool is_standart) +cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bool is_standard) : x(_x.normalize()) , y(_y.normalize()) , _is_infinity(is_infinity) , _is_constant(_x.is_constant() && _y.is_constant() && is_infinity.is_constant()) - , _is_standart(is_standart) + , _is_standard(is_standard) { if (_x.get_context() != nullptr) { context = _x.get_context(); @@ -63,12 +63,12 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bo * @param is_infinity */ template -cycle_group::cycle_group(const FF& _x, const FF& _y, bool is_infinity, bool is_standart) +cycle_group::cycle_group(const FF& _x, const FF& _y, bool is_infinity, bool is_standard) : x(_x) , y(_y) , _is_infinity(is_infinity) , _is_constant(true) - , _is_standart(is_standart) + , _is_standard(is_standard) , context(nullptr) { ASSERT(get_value().on_curve()); @@ -90,7 +90,7 @@ cycle_group::cycle_group(const AffineElement& _in) , y(_in.is_point_at_infinity() ? 0 : _in.y) , _is_infinity(_in.is_point_at_infinity()) , _is_constant(true) - , _is_standart(true) + , _is_standard(true) , context(nullptr) {} @@ -137,7 +137,7 @@ cycle_group cycle_group::from_witness(Builder* _context, const } result._is_infinity = bool_t(witness_t(_context, _in.is_point_at_infinity())); result._is_constant = false; - result._is_standart = true; + result._is_standard = true; result.validate_is_on_curve(); return result; } @@ -175,7 +175,7 @@ cycle_group cycle_group::from_constant_witness(Builder* _conte // point at infinity is circuit constant result._is_infinity = _in.is_point_at_infinity(); result._is_constant = false; - result._is_standart = true; + result._is_standard = true; return result; } @@ -219,7 +219,7 @@ template void cycle_group::validate_is_on_curve() co */ template cycle_group cycle_group::get_standard_form() const { - if (this->_is_standart) { + if (this->_is_standard) { return *this; } cycle_group result = *this; 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 38b2027c6cc7..12bbe5159e80 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -196,8 +196,8 @@ template class cycle_group { public: cycle_group(Builder* _context = nullptr); - cycle_group(field_t _x, field_t _y, bool_t _is_infinity, bool is_standart = false); - cycle_group(const FF& _x, const FF& _y, bool _is_infinity, bool is_standart = false); + cycle_group(field_t _x, field_t _y, bool_t _is_infinity, bool is_standard = false); + cycle_group(const FF& _x, const FF& _y, bool _is_infinity, bool is_standard = false); cycle_group(const AffineElement& _in); static cycle_group one(Builder* _context); static cycle_group from_witness(Builder* _context, const AffineElement& _in); @@ -220,7 +220,7 @@ template class cycle_group { this->y = field_t::conditional_assign(is_infinity, 0, this->y); } _is_infinity = is_infinity; - this->_is_standart = true; + this->_is_standard = true; } cycle_group get_standard_form() const; void validate_is_on_curve() const; @@ -295,9 +295,9 @@ template class cycle_group { bool_t _is_infinity; bool _is_constant; // Most of the time it is true, so we won't need to do extra conditional_assign - // during `get_standart_form` call + // during `get_standard_form` call // However sometimes it won't be the case, so we can handle these cases using this flag - bool _is_standart; + bool _is_standard; Builder* context; static batch_mul_internal_output _variable_base_batch_mul_internal(std::span scalars, From 01e236774617b85d7da73b00c6bd971bf15b7489 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 6 Mar 2025 14:55:46 +0000 Subject: [PATCH 03/36] few updates --- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 8 ++++++-- .../barretenberg/stdlib/primitives/group/cycle_group.hpp | 4 +--- 2 files changed, 7 insertions(+), 5 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 7ed5356825da..a74567d955e1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -105,7 +105,7 @@ template cycle_group cycle_group::one(Build { field_t x(_context, Group::one.x); field_t y(_context, Group::one.y); - return cycle_group(x, y, false); + return cycle_group(x, y, /*is_infinity=*/false, /*is_standard=*/true); } /** @@ -257,12 +257,14 @@ cycle_group cycle_group::dbl(const std::optionalis_point_at_infinity().is_constant() && this->is_point_at_infinity().get_value()) { - modified_y = field_t::from_witness_index(context, context->put_constant_variable(1)); + return *this; } cycle_group result; @@ -345,6 +347,7 @@ cycle_group cycle_group::unconditional_add( * @brief Will evaluate ECC point addition over `*this` and `other`. * Incomplete addition formula edge cases are *NOT* checked! * Only use this method if you know the x-coordinates of the operands cannot collide + * and none of the operands is a point at infinity * Ultra version that uses ecc group gate * * @tparam Builder @@ -415,6 +418,7 @@ cycle_group cycle_group::unconditional_add(const cycle_group& * @brief will evaluate ECC point subtraction over `*this` and `other`. * Incomplete addition formula edge cases are *NOT* checked! * Only use this method if you know the x-coordinates of the operands cannot collide + * and none of the operands is a point at infinity * * @tparam Builder * @param other 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 12bbe5159e80..c30075d90d1f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -209,8 +209,6 @@ template class cycle_group { bool_t is_point_at_infinity() const { return _is_infinity; } void set_point_at_infinity(const bool_t& is_infinity) { - // I am not sure that it handles all the cases - // when we can bump into non standard case if (is_infinity.is_constant() && is_infinity.get_value()) { this->x = 0; this->y = 0; @@ -317,6 +315,6 @@ template class cycle_group { template inline std::ostream& operator<<(std::ostream& os, cycle_group const& v) { - return os << v.get_value(); + return os << "{ " << v.x << ", " << v.y << " }"; } } // namespace bb::stdlib From e52d99dae98542855ddeda8b8f04d9436a8129a1 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 6 Mar 2025 14:57:11 +0000 Subject: [PATCH 04/36] not a bug --- .../stdlib/primitives/group/cycle_group.fuzzer.hpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 52eec288cd28..845d1fa37ae1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -12,6 +12,7 @@ #define HAVOC_TESTING #include "barretenberg/common/fuzzer.hpp" +// TODO: figure out how to detect w + w = c #define SHOW_INFORMATION @@ -819,8 +820,8 @@ template class CycleGroupBase { return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } - - uint8_t add_option = VarianceRNG.next() % 6; + bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || other.cycle_group.is_point_at_infinity().get_value(); + uint8_t add_option = smth_inf ? 4 + (VarianceRNG.next() % 2) : VarianceRNG.next() % 6; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(add_option) << " add path" << std::endl; #endif @@ -881,8 +882,8 @@ template class CycleGroupBase { return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } - - uint8_t add_option = VarianceRNG.next() % 3; + bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || other.cycle_group.is_point_at_infinity().get_value(); + uint8_t add_option = smth_inf ? 2: VarianceRNG.next() % 3; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(add_option) << " sub path" << std::endl; #endif From a1b8d753dd15d7cccb174cb0887048a6228ea50a Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 6 Mar 2025 22:10:36 +0000 Subject: [PATCH 05/36] Standard form related fixes --- .../stdlib/primitives/group/cycle_group.cpp | 35 +++++++++---------- .../stdlib/primitives/group/cycle_group.hpp | 20 +++++++++-- 2 files changed, 34 insertions(+), 21 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 a74567d955e1..ed6d2b6f92a6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -217,14 +217,10 @@ template void cycle_group::validate_is_on_curve() co * @brief Get point in standard form. If the point is a point at infinity, ensure the coordinates are (0,0) * */ -template cycle_group cycle_group::get_standard_form() const +template cycle_group cycle_group::get_standard_form() { - if (this->_is_standard) { - return *this; - } - cycle_group result = *this; - result.set_point_at_infinity(result.is_point_at_infinity()); - return result; + this->standardize(); + return *this; } /** * @brief Evaluates a doubling. Does not use Ultra double gate @@ -321,6 +317,7 @@ cycle_group cycle_group::dbl(const std::optional cycle_group::unconditional_add( auto lambda = y_diff.divide_no_zero_check(x_diff); auto x3 = lambda.madd(lambda, -other.x - x); auto y3 = lambda.madd(x - x3, -y); - cycle_group result(x3, y3, false); + cycle_group result(x3, y3, /*is_infinity=*/false, /*is_standard=*/true); return result; } @@ -381,9 +378,9 @@ cycle_group cycle_group::unconditional_add(const cycle_group& auto x3 = hint.value().x; auto y3 = hint.value().y; if (lhs_constant && rhs_constant) { - return cycle_group(x3, y3, false); + return cycle_group(x3, y3, /*is_infinity=*/false, /*is_standard=*/true); } - result = cycle_group(witness_t(context, x3), witness_t(context, y3), false); + result = cycle_group(witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); } else { const auto p1 = get_value(); const auto p2 = other.get_value(); @@ -396,7 +393,7 @@ cycle_group cycle_group::unconditional_add(const cycle_group& } 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); + result = cycle_group(r_x, r_y, /*is_infinity=*/false, /*is_standard=*/true); } bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), @@ -454,9 +451,9 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr auto x3 = hint.value().x; auto y3 = hint.value().y; if (lhs_constant && rhs_constant) { - return cycle_group(x3, y3, false); + return cycle_group(x3, y3, /*is_infinity=*/false, /*is_standard=*/true); } - result = cycle_group(witness_t(context, x3), witness_t(context, y3), is_point_at_infinity()); + result = cycle_group(witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); } else { auto p1 = get_value(); auto p2 = other.get_value(); @@ -469,7 +466,7 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr } 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); + result = cycle_group(r_x, r_y, /*is_infinity=*/false, /*is_standard=*/true); } bb::ecc_add_gate_ add_gate{ .x1 = x.get_witness_index(), @@ -1241,7 +1238,7 @@ template cycle_group cycle_group::straus_lo // Merge tag of table with tag of index x.set_origin_tag(OriginTag(tag, _index.get_origin_tag())); y.set_origin_tag(OriginTag(tag, _index.get_origin_tag())); - return cycle_group(x, y, false); + return cycle_group(x, y, /*is_infinity=*/false, /*is_standard=*/true); } field_t x = _index * (point_table[1].x - point_table[0].x) + point_table[0].x; field_t y = _index * (point_table[1].y - point_table[0].y) + point_table[0].y; @@ -1249,7 +1246,7 @@ template cycle_group cycle_group::straus_lo // Merge tag of table with tag of index x.set_origin_tag(OriginTag(tag, _index.get_origin_tag())); y.set_origin_tag(OriginTag(tag, _index.get_origin_tag())); - return cycle_group(x, y, false); + return cycle_group(x, y, /*is_infinity=*/false, /*is_standard=*/true); } /** @@ -1500,7 +1497,7 @@ typename cycle_group::batch_mul_internal_output cycle_group::_ for (size_t j = 0; j < lookup_data[ColumnIdx::C2].size(); ++j) { const auto x = lookup_data[ColumnIdx::C2][j]; const auto y = lookup_data[ColumnIdx::C3][j]; - lookup_points.emplace_back(cycle_group(x, y, false)); + lookup_points.emplace_back(cycle_group(x, y, /*is_infinity=*/false, /*is_standard=*/true)); } std::optional offset_1 = @@ -1844,8 +1841,10 @@ template bool_t cycle_group::operator==(con } template -void cycle_group::assert_equal(const cycle_group& other, std::string const& msg) const +void cycle_group::assert_equal(cycle_group& other, std::string const& msg) { + this->standardize(); + other.standardize(); x.assert_equal(other.x, msg); y.assert_equal(other.y, msg); } 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 c30075d90d1f..83e8900c8910 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -220,7 +220,21 @@ template class cycle_group { _is_infinity = is_infinity; this->_is_standard = true; } - cycle_group get_standard_form() const; + void standardize(){ + if(this->_is_standard){ + return; + } + if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { + this->x = 0; + this->y = 0; + this->_is_constant = true; + } else { + this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); + this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); + } + this->_is_standard = true; + } + cycle_group get_standard_form(); void validate_is_on_curve() const; cycle_group dbl(const std::optional hint = std::nullopt) const requires IsUltraArithmetic; @@ -261,7 +275,7 @@ template class cycle_group { cycle_group operator*(const BigScalarField& scalar) const; cycle_group& operator*=(const BigScalarField& scalar); bool_t operator==(const cycle_group& other) const; - void assert_equal(const cycle_group& other, std::string const& msg = "cycle_group::assert_equal") const; + void assert_equal(cycle_group& other, std::string const& msg = "cycle_group::assert_equal"); static cycle_group conditional_assign(const bool_t& predicate, const cycle_group& lhs, const cycle_group& rhs); cycle_group operator/(const cycle_group& other) const; @@ -293,7 +307,7 @@ template class cycle_group { bool_t _is_infinity; bool _is_constant; // Most of the time it is true, so we won't need to do extra conditional_assign - // during `get_standard_form` call + // during `get_standard_form` or `assert_equal` call // However sometimes it won't be the case, so we can handle these cases using this flag bool _is_standard; Builder* context; From 552214fa37a382ad54357cc3fd3afe7f279903f1 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 6 Mar 2025 22:11:11 +0000 Subject: [PATCH 06/36] Fixed subtraction + new assert_equal --- .../stdlib/primitives/group/cycle_group.fuzzer.hpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 845d1fa37ae1..cfe12f5a4586 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -752,6 +752,9 @@ template class CycleGroupBase { * will use the context of another input parameter */ const bool predicate_has_ctx = static_cast(VarianceRNG.next() % 2); +#ifdef SHOW_INFORMATION + std::cout << "Constant predicate? " << predicate_has_ctx << std::endl; +#endif return bool_t(predicate_has_ctx ? builder : nullptr, predicate); } @@ -858,7 +861,7 @@ template class CycleGroupBase { case 0: return ExecutionHandler(base_scalar_res, base_res, this->cg().dbl()); case 1: - return ExecutionHandler(base_scalar_res, base_res, other.cg().dbl()); + return ExecutionHandler(base_scalar_res, base_res, -other.cg().dbl()); case 2: return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } @@ -967,12 +970,10 @@ template class CycleGroupBase { // Assert equal does nothing in this case return; } - auto to_add = cycle_group_t::from_witness(builder, AffineElement(this->base - other.base)); - this->cycle_group.assert_equal(other.cg() + to_add); - } else { - auto to_add = cycle_group_t::from_witness(builder, AffineElement(this->base - other.base)); - this->cg().assert_equal(other.cg() + to_add); } + auto to_add = cycle_group_t::from_witness(builder, AffineElement(this->base - other.base)); + auto to_ae = other.cg() + to_add; + this->cg().assert_equal(to_ae); } /* Explicit re-instantiation using the various cycle_group_t constructors */ From 03479d43db6be3429efbde15bac32f6c282d6c92 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 15:05:19 +0000 Subject: [PATCH 07/36] negative operator fix --- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 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 ed6d2b6f92a6..b4f1b5cf898f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -502,7 +502,7 @@ template cycle_group cycle_group::checked_unconditional_add(const cycle_group& other, const std::optional hint) const { - field_t x_delta = x - other.x; + field_t x_delta = this->x - other.x; if (x_delta.is_constant()) { ASSERT(x_delta.get_value() != 0); } else { @@ -528,7 +528,7 @@ template cycle_group cycle_group::checked_unconditional_subtract(const cycle_group& other, const std::optional hint) const { - field_t x_delta = x - other.x; + field_t x_delta = this->x - other.x; if (x_delta.is_constant()) { ASSERT(x_delta.get_value() != 0); } else { @@ -677,7 +677,7 @@ template cycle_group cycle_group::operator- template cycle_group cycle_group::operator-() const { cycle_group result(*this); - result.y = -y; + result.y = (-y).normalize(); return result; } From 9112e0fa53277a5330e8d4bc754e81127d60ec14 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 15:06:00 +0000 Subject: [PATCH 08/36] Non constant predicates --- .../stdlib/primitives/group/cycle_group.fuzzer.hpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index cfe12f5a4586..ca6c1330a2e9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -751,11 +751,15 @@ template class CycleGroupBase { * in that case, the function that handles the predicate * will use the context of another input parameter */ - const bool predicate_has_ctx = static_cast(VarianceRNG.next() % 2); + const bool predicate_is_const = static_cast(VarianceRNG.next() & 1); #ifdef SHOW_INFORMATION - std::cout << "Constant predicate? " << predicate_has_ctx << std::endl; + std::cout << "Constant predicate? " << predicate_is_const << std::endl; #endif - return bool_t(predicate_has_ctx ? builder : nullptr, predicate); + if(predicate_is_const){ + const bool predicate_has_ctx = static_cast(VarianceRNG.next() % 2); + return bool_t(predicate_has_ctx ? builder : nullptr, predicate); + } + return bool_t(witness_t(builder, predicate)); } cycle_group_t cg() const From 81e911a9af8ce224d54f77f6870bffb6fd3a424c Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 15:17:06 +0000 Subject: [PATCH 09/36] regression --- .../primitives/group/cycle_group.test.cpp | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index 5b713f2688a7..6c70922b51db 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -135,6 +135,26 @@ TYPED_TEST(CycleGroupTest, TestWitnessSumRegression) EXPECT_FALSE(c.is_constant()); } +/** + * @brief Checks that adding operator-(value) to an existing value does not result into error + * + */ +TYPED_TEST(CycleGroupTest, TestOperatorNegRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto lhs = TestFixture::generators[0]; + auto rhs = TestFixture::generators[1]; + cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); + cycle_group_ct b = cycle_group_ct::from_witness(&builder, rhs); + b = -b; + cycle_group_ct c = a.unconditional_add(b); + (void)c; + EXPECT_FALSE(builder.failed()); + EXPECT_TRUE(CircuitChecker::check(builder)); +} + /** * @brief Checks that a point on the curve passes the validate_is_on_curve check * From 1aa208100d5c0802ee48e381d37101458b0131d2 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 16:49:02 +0000 Subject: [PATCH 10/36] Handling const set inf + == --- .../stdlib/primitives/group/cycle_group.hpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 83e8900c8910..cf54516f6aac 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -213,9 +213,13 @@ template class cycle_group { this->x = 0; this->y = 0; this->_is_constant = true; - } else { + } else if(!is_infinity.is_constant()) { this->x = field_t::conditional_assign(is_infinity, 0, this->x); this->y = field_t::conditional_assign(is_infinity, 0, this->y); + this->_is_constant = false; + if(this->context == nullptr){ + this->context = is_infinity.get_context(); + } } _is_infinity = is_infinity; this->_is_standard = true; @@ -228,9 +232,13 @@ template class cycle_group { this->x = 0; this->y = 0; this->_is_constant = true; - } else { + } else if (!this->_is_infinity.is_constant()){ this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); + this->_is_constant = false; + if(this->context == nullptr){ + this->context = this->_is_infinity.get_context(); + } } this->_is_standard = true; } @@ -274,7 +282,7 @@ template class cycle_group { cycle_group& operator*=(const cycle_scalar& scalar); cycle_group operator*(const BigScalarField& scalar) const; cycle_group& operator*=(const BigScalarField& scalar); - bool_t operator==(const cycle_group& other) const; + bool_t operator==(cycle_group& other); void assert_equal(cycle_group& other, std::string const& msg = "cycle_group::assert_equal"); static cycle_group conditional_assign(const bool_t& predicate, const cycle_group& lhs, const cycle_group& rhs); cycle_group operator/(const cycle_group& other) const; From 96c0656d5198ca681c0ae5ffc542470bdaccf7f6 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 16:49:27 +0000 Subject: [PATCH 11/36] new == --- .../stdlib/primitives/group/cycle_group.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 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 b4f1b5cf898f..17d88c2db38d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -1832,12 +1832,13 @@ template cycle_group& cycle_group::operator return *this; } -template bool_t cycle_group::operator==(const cycle_group& other) const +template bool_t cycle_group::operator==(cycle_group& other) { - const auto equal_and_not_infinity = - (x == other.x) && (y == other.y) && !is_point_at_infinity() && !other.is_point_at_infinity(); - const auto both_infinity = is_point_at_infinity() && other.is_point_at_infinity(); - return equal_and_not_infinity || both_infinity; + this->standardize(); + other.standardize(); + const auto equal = + (x == other.x) && (y == other.y); + return equal; } template From 4ed0510a8cffe004d43bb8071d93b4cd298f35b7 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 16:50:44 +0000 Subject: [PATCH 12/36] New features --- .../primitives/group/cycle_group.fuzzer.hpp | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index ca6c1330a2e9..5df03cf386a5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -14,7 +14,7 @@ #include "barretenberg/common/fuzzer.hpp" // TODO: figure out how to detect w + w = c -#define SHOW_INFORMATION +//#define SHOW_INFORMATION #ifdef SHOW_INFORMATION #define PRINT_SINGLE_ARG_INSTRUCTION(first_index, vector, operation_name, preposition) \ @@ -100,7 +100,7 @@ template class CycleGroupBase { SUBTRACT, NEG, DBL, - MUL, + MULTIPLY, BATCH_MUL, RANDOMSEED, _LAST @@ -233,7 +233,7 @@ template class CycleGroupBase { .arguments.fourArgs.in2 = in2, .arguments.fourArgs.in3 = in3, .arguments.fourArgs.out = out }; - case OPCODE::MUL: + case OPCODE::MULTIPLY: in = static_cast(rng.next() & 0xff); out = static_cast(rng.next() & 0xff); return { .id = instruction_opcode, @@ -527,7 +527,7 @@ template class CycleGroupBase { PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.twoArgs.in); PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.twoArgs.out); break; - case OPCODE::MUL: + case OPCODE::MULTIPLY: PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.mulArgs.in); PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.mulArgs.out); if (rng.next() & 1) { @@ -599,7 +599,7 @@ template class CycleGroupBase { static constexpr size_t ADD = 3; static constexpr size_t SUBTRACT = 3; static constexpr size_t COND_ASSIGN = 4; - static constexpr size_t MUL = sizeof(typename Instruction::MulArgs); + static constexpr size_t MULTIPLY = sizeof(typename Instruction::MulArgs); static constexpr size_t BATCH_MUL = sizeof(typename Instruction::BatchMulArgs); static constexpr size_t RANDOMSEED = sizeof(uint32_t); }; @@ -623,7 +623,7 @@ template class CycleGroupBase { static constexpr size_t NEG = 1; static constexpr size_t COND_ASSIGN = 1; - static constexpr size_t MUL = 2; + static constexpr size_t MULTIPLY = 2; static constexpr size_t ASSERT_EQUAL = 2; static constexpr size_t SET_INF = 2; @@ -670,7 +670,7 @@ template class CycleGroupBase { case Instruction::OPCODE::COND_ASSIGN: instr.arguments.fourArgs = { .in1 = *Data, .in2 = *(Data + 1), .in3 = *(Data + 2), .out = *(Data + 3) }; break; - case Instruction::OPCODE::MUL: + case Instruction::OPCODE::MULTIPLY: instr.arguments.mulArgs.in = *Data; instr.arguments.mulArgs.out = *(Data + 1); instr.arguments.mulArgs.scalar = ScalarField::serialize_from_buffer(Data + 2); @@ -723,7 +723,7 @@ template class CycleGroupBase { *(Data + 3) = instruction.arguments.fourArgs.in3; *(Data + 4) = instruction.arguments.fourArgs.out; return; - case Instruction::OPCODE::MUL: + case Instruction::OPCODE::MULTIPLY: *(Data + 1) = instruction.arguments.mulArgs.in; *(Data + 2) = instruction.arguments.mulArgs.out; ScalarField::serialize_to_buffer(instruction.arguments.mulArgs.scalar, Data + 3); @@ -786,7 +786,7 @@ template class CycleGroupBase { , cycle_group(w_g) {} - ExecutionHandler operator+(const ExecutionHandler& other) + ExecutionHandler operator_add(Builder* builder, const ExecutionHandler& other) { ScalarField base_scalar_res = this->base_scalar + other.base_scalar; GroupElement base_res = this->base + other.base; @@ -815,11 +815,11 @@ template class CycleGroupBase { switch (inf_path) { case 0: res = this->cg(); - res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); + res.set_point_at_infinity(this->construct_predicate(builder, true)); return ExecutionHandler(base_scalar_res, base_res, res); case 1: res = other.cg(); - res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); + res.set_point_at_infinity(this->construct_predicate(builder, true)); return ExecutionHandler(base_scalar_res, base_res, res); case 2: return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); @@ -850,7 +850,7 @@ template class CycleGroupBase { return {}; } - ExecutionHandler operator-(const ExecutionHandler& other) + ExecutionHandler operator_sub(Builder* builder, const ExecutionHandler& other) { ScalarField base_scalar_res = this->base_scalar - other.base_scalar; GroupElement base_res = this->base - other.base; @@ -879,11 +879,11 @@ template class CycleGroupBase { switch (inf_path) { case 0: res = this->cg(); - res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); + res.set_point_at_infinity(this->construct_predicate(builder, true)); return ExecutionHandler(base_scalar_res, base_res, res); case 1: res = other.cg(); - res.set_point_at_infinity(this->construct_predicate(this->cycle_group.get_context(), true)); + res.set_point_at_infinity(this->construct_predicate(builder, true)); return ExecutionHandler(base_scalar_res, base_res, res); case 2: return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); @@ -1019,7 +1019,8 @@ template class CycleGroupBase { ExecutionHandler set_inf(Builder* builder) { auto res = this->set(builder); - res.set_point_at_infinity(this->construct_predicate(builder, true)); + const bool set_inf = static_cast(VarianceRNG.next() & 1); + res.set_point_at_infinity(this->construct_predicate(builder, set_inf)); return res; } @@ -1282,7 +1283,7 @@ template class CycleGroupBase { PRINT_TWO_ARG_INSTRUCTION(first_index, second_index, stack, "Adding", "+") ExecutionHandler result; - result = stack[first_index] + stack[second_index]; + result = stack[first_index].operator_add(builder, stack[second_index]); // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) @@ -1317,7 +1318,7 @@ template class CycleGroupBase { PRINT_TWO_ARG_INSTRUCTION(first_index, second_index, stack, "Subtracting", "-") ExecutionHandler result; - result = stack[first_index] - stack[second_index]; + result = stack[first_index].operator_sub(builder, stack[second_index]); // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) From b50c80e5eac7dcd8491225fa9bf7be25e8db575a Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Fri, 7 Mar 2025 16:57:11 +0000 Subject: [PATCH 13/36] Better handling of const case --- .../stdlib/primitives/group/cycle_group.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 17d88c2db38d..15e324e22fea 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -599,10 +599,10 @@ template cycle_group cycle_group::operator+ // yes = lhs_infinity && rhs_infinity bool_t result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity); result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); - result.set_point_at_infinity(result_is_infinity); - ASSERT(result.x.is_constant() == result.y.is_constant()); - result._is_constant = result.x.is_constant(); + // need to set this before set_point_at_infinity call + result._is_constant = this->_is_constant & other._is_constant; + result.set_point_at_infinity(result_is_infinity); return result; } @@ -659,10 +659,10 @@ template cycle_group cycle_group::operator- // n.b. can likely optimize this bool_t result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity); result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); - result.set_point_at_infinity(result_is_infinity); - ASSERT(result.x.is_constant() == result.y.is_constant()); - result._is_constant = result.x.is_constant(); + // need to set this before set_point_at_infinity call + result._is_constant = this->_is_constant & other._is_constant; + result.set_point_at_infinity(result_is_infinity); return result; } From 6816467acd546add6f1ca637369c699f20356950 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Tue, 11 Mar 2025 21:40:39 +0000 Subject: [PATCH 14/36] Mixup bug --- .../stdlib/primitives/group/cycle_group.cpp | 13 +++++++++++ .../primitives/group/cycle_group.test.cpp | 23 +++++++++++++++++++ 2 files changed, 36 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 15e324e22fea..c6e70a03f9ad 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -549,6 +549,12 @@ cycle_group cycle_group::checked_unconditional_subtract(const */ template cycle_group cycle_group::operator+(const cycle_group& other) const { + if(this->_is_infinity.is_constant() && this->_is_infinity.get_value()){ + return other; + } + if(other._is_infinity.is_constant() && other._is_infinity.get_value()){ + return *this; + } Builder* context = get_context(other); const bool_t x_coordinates_match = (x == other.x); @@ -619,6 +625,13 @@ template cycle_group cycle_group::operator+ */ template cycle_group cycle_group::operator-(const cycle_group& other) const { + if(this->_is_infinity.is_constant() && this->_is_infinity.get_value()){ + return -other; + } + if(other._is_infinity.is_constant() && other._is_infinity.get_value()){ + return *this; + } + Builder* context = get_context(other); const bool_t x_coordinates_match = (x == other.x); const bool_t y_coordinates_match = (y == other.y); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index 6c70922b51db..5db770031f27 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -155,6 +155,29 @@ TYPED_TEST(CycleGroupTest, TestOperatorNegRegression) EXPECT_TRUE(CircuitChecker::check(builder)); } +/** + * @brief Checks the mixup bad behavior found by fuzzer. + * + */ +TYPED_TEST(CycleGroupTest, TestConstantWitnessMixupRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto c1 = cycle_group_ct(AffineElement::one()); + auto cw8 = cycle_group_ct::from_constant_witness(&builder, AffineElement::one() * 0); + auto w11 = cycle_group_ct::from_witness(&builder, TestFixture::generators[0]); + + auto w9 = cw8 + c1; // mixup happens here due to _is_infinity being a constant + auto w26 = w9 + w11; // and here the circuit checker crashes + + auto w10 = cw8 - c1; + auto w27 = w10 - w11; // and here + (void)w26; + (void)w27; + EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway +} + /** * @brief Checks that a point on the curve passes the validate_is_on_curve check * From 9e3c662b4e8a82e17b81637c46f25c64d3c7b08c Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Tue, 11 Mar 2025 21:41:29 +0000 Subject: [PATCH 15/36] Added pretty prints and defines to disable mul --- .../primitives/group/cycle_group.fuzzer.hpp | 271 ++++++++++++++++-- 1 file changed, 244 insertions(+), 27 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 5df03cf386a5..86ed05cc69d6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -14,7 +14,9 @@ #include "barretenberg/common/fuzzer.hpp" // TODO: figure out how to detect w + w = c -//#define SHOW_INFORMATION +// #define SHOW_INFORMATION +#define SHOW_PRETTY_INFORMATION +#define DISABLE_MULTIPLICATION #ifdef SHOW_INFORMATION #define PRINT_SINGLE_ARG_INSTRUCTION(first_index, vector, operation_name, preposition) \ @@ -49,13 +51,32 @@ } #else - #define PRINT_SINGLE_ARG_INSTRUCTION(first_index, vector, operation_name, preposition) #define PRINT_TWO_ARG_INSTRUCTION(first_index, second_index, vector, operation_name, preposition) #define PRINT_MUL_ARG_INSTRUCTION(first_index, scalar, vector, operation_name, preposition) #define PRINT_RESULT(prefix, action, index, value) #endif +#ifdef SHOW_PRETTY_INFORMATION +#define PREP_SINGLE_ARG(stack, first_index, output_index) \ + std::string rhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string out = rhs; \ + rhs += std::to_string(first_index); \ + out += std::to_string(output_index >= stack.size() ? stack.size() : output_index); \ + out = (output_index >= stack.size() ? "auto " : "") + out; + +#define PREP_TWO_ARG(stack, first_index, second_index, output_index) \ + std::string lhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string rhs = stack[second_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string out = \ + (stack[first_index].cycle_group.is_constant() && stack[second_index].cycle_group.is_constant()) ? "c" : "w"; \ + lhs += std::to_string(first_index); \ + rhs += std::to_string(second_index); \ + out += std::to_string(output_index >= stack.size() ? stack.size() : output_index); \ + out = (output_index >= stack.size() ? "auto " : "") + out; + +#endif + FastRandom VarianceRNG(0); // This is a global variable, so that the execution handling class could alter it and signal to the input tester @@ -100,8 +121,10 @@ template class CycleGroupBase { SUBTRACT, NEG, DBL, +#ifndef DISABLE_MULTIPLICATION MULTIPLY, BATCH_MUL, +#endif RANDOMSEED, _LAST }; @@ -195,7 +218,7 @@ template class CycleGroupBase { requires SimpleRng { OPCODE instruction_opcode = static_cast(rng.next() % (OPCODE::_LAST)); - uint8_t in, in1, in2, in3, out, mult_size; + uint8_t in, in1, in2, in3, out; Instruction instr; switch (instruction_opcode) { @@ -233,6 +256,7 @@ template class CycleGroupBase { .arguments.fourArgs.in2 = in2, .arguments.fourArgs.in3 = in3, .arguments.fourArgs.out = out }; +#ifndef DISABLE_MULTIPLICATION case OPCODE::MULTIPLY: in = static_cast(rng.next() & 0xff); out = static_cast(rng.next() & 0xff); @@ -240,11 +264,9 @@ template class CycleGroupBase { .arguments.mulArgs.scalar = ScalarField(Instruction::fast_log_distributed_uint256(rng)), .arguments.mulArgs.in = in, .arguments.mulArgs.out = out }; - case OPCODE::RANDOMSEED: - return { .id = instruction_opcode, .arguments.randomseed = rng.next() * rng.next() }; - case OPCODE::BATCH_MUL: - mult_size = MINIMUM_MUL_ELEMENTS + - static_cast(rng.next() % (MAXIMUM_MUL_ELEMENTS - MINIMUM_MUL_ELEMENTS)); + case OPCODE::BATCH_MUL: { + uint8_t mult_size = MINIMUM_MUL_ELEMENTS + + static_cast(rng.next() % (MAXIMUM_MUL_ELEMENTS - MINIMUM_MUL_ELEMENTS)); instr.id = instruction_opcode; instr.arguments.batchMulArgs.add_elements_count = mult_size; for (size_t i = 0; i < mult_size; i++) { @@ -256,6 +278,11 @@ template class CycleGroupBase { } instr.arguments.batchMulArgs.output_index = static_cast(rng.next() & 0xff); return instr; + } +#endif + case OPCODE::RANDOMSEED: + return { .id = instruction_opcode, .arguments.randomseed = rng.next() * rng.next() }; + default: abort(); // We missed some instructions in switch } @@ -527,6 +554,7 @@ template class CycleGroupBase { PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.twoArgs.in); PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.twoArgs.out); break; +#ifndef DISABLE_MULTIPLICATION case OPCODE::MULTIPLY: PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.mulArgs.in); PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.mulArgs.out); @@ -535,6 +563,7 @@ template class CycleGroupBase { mutateScalarElement(instruction.arguments.mulArgs.scalar, rng, havoc_config); } break; +#endif case OPCODE::ADD: case OPCODE::SUBTRACT: PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.threeArgs.in1); @@ -547,6 +576,7 @@ template class CycleGroupBase { PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.fourArgs.in3); PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.fourArgs.out); break; +#ifndef DISABLE_MULTIPLICATION case OPCODE::BATCH_MUL: if (rng.next() & 1) { instruction.arguments.batchMulArgs.add_elements_count = @@ -574,6 +604,7 @@ template class CycleGroupBase { } PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.batchMulArgs.output_index); break; +#endif case OPCODE::RANDOMSEED: instruction.arguments.randomseed = rng.next(); break; @@ -599,8 +630,10 @@ template class CycleGroupBase { static constexpr size_t ADD = 3; static constexpr size_t SUBTRACT = 3; static constexpr size_t COND_ASSIGN = 4; +#ifndef DISABLE_MULTIPLICATION static constexpr size_t MULTIPLY = sizeof(typename Instruction::MulArgs); static constexpr size_t BATCH_MUL = sizeof(typename Instruction::BatchMulArgs); +#endif static constexpr size_t RANDOMSEED = sizeof(uint32_t); }; @@ -623,11 +656,15 @@ template class CycleGroupBase { static constexpr size_t NEG = 1; static constexpr size_t COND_ASSIGN = 1; +#ifndef DISABLE_MULTIPLICATION static constexpr size_t MULTIPLY = 2; +#endif static constexpr size_t ASSERT_EQUAL = 2; static constexpr size_t SET_INF = 2; +#ifndef DISABLE_MULTIPLICATION static constexpr size_t BATCH_MUL = 4; +#endif static constexpr size_t _LIMIT = 64; }; /** @@ -670,6 +707,8 @@ template class CycleGroupBase { case Instruction::OPCODE::COND_ASSIGN: instr.arguments.fourArgs = { .in1 = *Data, .in2 = *(Data + 1), .in3 = *(Data + 2), .out = *(Data + 3) }; break; + +#ifndef DISABLE_MULTIPLICATION case Instruction::OPCODE::MULTIPLY: instr.arguments.mulArgs.in = *Data; instr.arguments.mulArgs.out = *(Data + 1); @@ -678,6 +717,7 @@ template class CycleGroupBase { case Instruction::OPCODE::BATCH_MUL: memcpy(&instr.arguments.batchMulArgs, Data, sizeof(typename Instruction::BatchMulArgs)); break; +#endif case Instruction::OPCODE::RANDOMSEED: memcpy(&instr.arguments.randomseed, Data, sizeof(uint32_t)); break; @@ -723,6 +763,7 @@ template class CycleGroupBase { *(Data + 3) = instruction.arguments.fourArgs.in3; *(Data + 4) = instruction.arguments.fourArgs.out; return; +#ifndef DISABLE_MULTIPLICATION case Instruction::OPCODE::MULTIPLY: *(Data + 1) = instruction.arguments.mulArgs.in; *(Data + 2) = instruction.arguments.mulArgs.out; @@ -731,6 +772,7 @@ template class CycleGroupBase { case Instruction::OPCODE::BATCH_MUL: memcpy(Data + 1, &instruction.arguments.batchMulArgs, sizeof(typename Instruction::BatchMulArgs)); return; +#endif case Instruction::OPCODE::RANDOMSEED: memcpy(Data + 1, &instruction.arguments.randomseed, sizeof(uint32_t)); return; @@ -755,10 +797,16 @@ template class CycleGroupBase { #ifdef SHOW_INFORMATION std::cout << "Constant predicate? " << predicate_is_const << std::endl; #endif - if(predicate_is_const){ + if (predicate_is_const) { const bool predicate_has_ctx = static_cast(VarianceRNG.next() % 2); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "bool_t(" << (predicate_has_ctx ? "&builder" : "nullptr") << (predicate ? ",true));" : ",false));"); +#endif return bool_t(predicate_has_ctx ? builder : nullptr, predicate); } +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "bool_t(witness_t(&builder, " << (predicate ? "true));" : "false));"); +#endif return bool_t(witness_t(builder, predicate)); } @@ -798,12 +846,24 @@ template class CycleGroupBase { #endif switch (dbl_path) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.dbl" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg().dbl()); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right.dbl" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg().dbl()); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left + right" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 3: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right + left" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } else if (other.cg().get_value() == -this->cg().get_value()) { @@ -814,20 +874,39 @@ template class CycleGroupBase { #endif switch (inf_path) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.set_point_at_infinity("; +#endif res = this->cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << ");" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, res); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right.set_point_at_infinity("; +#endif res = other.cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << ");" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, res); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left + right" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 3: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right + left" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } - bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || other.cycle_group.is_point_at_infinity().get_value(); + bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || + other.cycle_group.is_point_at_infinity().get_value(); uint8_t add_option = smth_inf ? 4 + (VarianceRNG.next() % 2) : VarianceRNG.next() % 6; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(add_option) << " add path" << std::endl; @@ -835,16 +914,34 @@ template class CycleGroupBase { switch (add_option) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.unconditional_add(right);" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg().unconditional_add(other.cg())); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right.unconditional_add(left);" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg().unconditional_add(this->cg())); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.checked_unconditional_add(right);" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg().checked_unconditional_add(other.cg())); case 3: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right.checked_unconditional_add(left);" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg().checked_unconditional_add(this->cg())); case 4: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left + right;" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 5: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right + left;" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } return {}; @@ -863,10 +960,19 @@ template class CycleGroupBase { switch (dbl_path) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.dbl();" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg().dbl()); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "-right.dbl();" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, -other.cg().dbl()); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left - right;" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } else if (other.cg().get_value() == this->cg().get_value()) { @@ -878,30 +984,55 @@ template class CycleGroupBase { switch (inf_path) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.set_point_at_infinity("; +#endif res = this->cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << ");" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, res); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "right.set_point_at_infinity("; +#endif res = other.cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << ");" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, res); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left - right" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } - bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || other.cycle_group.is_point_at_infinity().get_value(); - uint8_t add_option = smth_inf ? 2: VarianceRNG.next() % 3; + bool smth_inf = this->cycle_group.is_point_at_infinity().get_value() || + other.cycle_group.is_point_at_infinity().get_value(); + uint8_t add_option = smth_inf ? 2 : VarianceRNG.next() % 3; #ifdef SHOW_INFORMATION std::cout << " using " << size_t(add_option) << " sub path" << std::endl; #endif switch (add_option) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.unconditional_subtract(right);" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg().unconditional_subtract(other.cg())); case 1: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left.checked_unconditional_subtract(right);" << std::endl; +#endif return ExecutionHandler( base_scalar_res, base_res, this->cg().checked_unconditional_subtract(other.cg())); case 2: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "left - right;" << std::endl; +#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } return {}; @@ -911,7 +1042,11 @@ template class CycleGroupBase { { bool is_witness = VarianceRNG.next() & 1; #ifdef SHOW_INFORMATION - std::cout << " Mul is witness? " << is_witness << std::endl; + std::cout << "Mul is witness? " << is_witness << std::endl; +#endif +#ifdef SHOW_PRETTY_INFORMATION + std::cout << " * cycle_scalar_t" << (is_witness ? "::from_witness(&builder, " : "(") << "ScalarField(\"" + << multiplier << "\");"; #endif auto scalar = is_witness ? cycle_scalar_t(multiplier) : cycle_scalar_t::from_witness(builder, multiplier); return ExecutionHandler(this->base_scalar * multiplier, this->base * multiplier, this->cg() * scalar); @@ -935,6 +1070,10 @@ template class CycleGroupBase { bool is_witness = VarianceRNG.next() & 1; #ifdef SHOW_INFORMATION std::cout << " Mul is witness? " << is_witness << std::endl; +#endif +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "cycle_scalar_t" << (is_witness ? "::from_witness(&builder, " : "(") << "ScalarField(\"" + << to_mul[i] << "\"), "; #endif auto scalar = is_witness ? cycle_scalar_t(to_mul[i]) : cycle_scalar_t::from_witness(builder, to_mul[i]); to_mul_cs.push_back(scalar); @@ -989,9 +1128,16 @@ template class CycleGroupBase { #endif switch (switch_case) { case 0: +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "cycle_group_t(" << std::endl; +#endif /* construct via cycle_group_t */ return ExecutionHandler(this->base_scalar, this->base, cycle_group_t(this->cycle_group)); case 1: { +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "cycle_group_t::from" << (this->cycle_group.is_constant() ? "" : "_constant") + << "_witness(&builder, e.get_value());"; +#endif /* construct via AffineElement */ AffineElement e = this->cycle_group.get_value(); if (this->cycle_group.is_constant()) { @@ -1001,12 +1147,20 @@ template class CycleGroupBase { return ExecutionHandler(this->base_scalar, this->base, cycle_group_t::from_witness(builder, e)); } case 2: { +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "tmp = el;" << std::endl; + std::cout << "res = cycle_group_t(tmp);" << std::endl; +#endif /* Invoke assigment operator */ cycle_group_t cg_new(builder); cg_new = this->cg(); return ExecutionHandler(this->base_scalar, this->base, cycle_group_t(cg_new)); } case 3: { +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "tmp = el;" << std::endl; + std::cout << "res = cycle_group_t(std::move(tmp));" << std::endl; +#endif /* Invoke move constructor */ cycle_group_t cg_copy = this->cg(); return ExecutionHandler(this->base_scalar, this->base, cycle_group_t(std::move(cg_copy))); @@ -1020,6 +1174,9 @@ template class CycleGroupBase { { auto res = this->set(builder); const bool set_inf = static_cast(VarianceRNG.next() & 1); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "el.set_point_at_infinty("; +#endif res.set_point_at_infinity(this->construct_predicate(builder, set_inf)); return res; } @@ -1045,7 +1202,10 @@ template class CycleGroupBase { std::cout << "Pushed constant value " << instruction.arguments.element.value << ", " << instruction.arguments.element.scalar << " to position " << stack.size() - 1 << std::endl; #endif - +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "auto c" << stack.size() - 1 << " = cycle_group_t(ae(\"" + << instruction.arguments.element.scalar << "\"));" << std::endl; +#endif return 0; }; @@ -1068,6 +1228,10 @@ template class CycleGroupBase { #ifdef SHOW_INFORMATION std::cout << "Pushed witness value " << instruction.arguments.element.value << ", " << instruction.arguments.element.scalar << " to position " << stack.size() - 1 << std::endl; +#endif +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "auto w" << stack.size() - 1 << " = cycle_group_t::from_witness(&builder, ae(\"" + << instruction.arguments.element.scalar << "\"));" << std::endl; #endif return 0; } @@ -1093,6 +1257,10 @@ template class CycleGroupBase { #ifdef SHOW_INFORMATION std::cout << "Pushed constant witness value " << instruction.arguments.element.value << ", " << instruction.arguments.element.scalar << " to position " << stack.size() - 1 << std::endl; +#endif +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "auto cw" << stack.size() - 1 << " = cycle_group_t::from_constant_witness(&builder, ae(\"" + << instruction.arguments.element.scalar << "\"));" << std::endl; #endif return 0; } @@ -1117,7 +1285,10 @@ template class CycleGroupBase { size_t output_index = instruction.arguments.twoArgs.out; PRINT_SINGLE_ARG_INSTRUCTION(first_index, stack, "Doubling", "doubled") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_SINGLE_ARG(stack, first_index, output_index) + std::cout << out << " = " << rhs << ".dbl();" << std::endl; +#endif ExecutionHandler result; result = stack[first_index].dbl(); // If the output index is larger than the number of elements in stack, append @@ -1151,7 +1322,10 @@ template class CycleGroupBase { size_t output_index = instruction.arguments.twoArgs.out; PRINT_SINGLE_ARG_INSTRUCTION(first_index, stack, "Negating", "negated") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_SINGLE_ARG(stack, first_index, output_index) + std::cout << out << " = -" << rhs << ";" << std::endl; +#endif ExecutionHandler result; result = -stack[first_index]; // If the output index is larger than the number of elements in stack, append @@ -1187,7 +1361,10 @@ template class CycleGroupBase { #ifdef SHOW_INFORMATION std::cout << std::endl; #endif - +#ifdef SHOW_PRETTY_INFORMATION + PREP_TWO_ARG(stack, first_index, second_index, 0) + std::cout << "assert_equal(" << lhs << ", " << rhs << ", builder);" << std::endl; +#endif stack[first_index].assert_equal(builder, stack[second_index]); return 0; }; @@ -1213,14 +1390,20 @@ template class CycleGroupBase { ExecutionHandler result; PRINT_SINGLE_ARG_INSTRUCTION(first_index, stack, "Setting value", "") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_SINGLE_ARG(stack, first_index, output_index) + std::cout << out << " = "; +#endif result = stack[first_index].set(builder); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << rhs << ");" << std::endl; +#endif // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) stack.push_back(result); } else { - PRINT_RESULT("", "saved to ", stack.size(), result) + PRINT_RESULT("", "saved to ", output_index, result) stack[output_index] = result; } return 0; @@ -1247,14 +1430,17 @@ template class CycleGroupBase { ExecutionHandler result; PRINT_SINGLE_ARG_INSTRUCTION(first_index, stack, "Setting value to inf", "") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_SINGLE_ARG(stack, first_index, output_index) + std::cout << out << " = " << rhs << std::endl; +#endif result = stack[first_index].set_inf(builder); // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) stack.push_back(result); } else { - PRINT_RESULT("", "saved to ", stack.size(), result) + PRINT_RESULT("", "saved to ", output_index, result) stack[output_index] = result; } return 0; @@ -1281,7 +1467,10 @@ template class CycleGroupBase { size_t output_index = instruction.arguments.threeArgs.out; PRINT_TWO_ARG_INSTRUCTION(first_index, second_index, stack, "Adding", "+") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_TWO_ARG(stack, first_index, second_index, output_index) + std::cout << out << " = " << lhs << " + " << rhs << ";" << std::endl; +#endif ExecutionHandler result; result = stack[first_index].operator_add(builder, stack[second_index]); // If the output index is larger than the number of elements in stack, append @@ -1316,7 +1505,10 @@ template class CycleGroupBase { size_t output_index = instruction.arguments.threeArgs.out; PRINT_TWO_ARG_INSTRUCTION(first_index, second_index, stack, "Subtracting", "-") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_TWO_ARG(stack, first_index, second_index, output_index) + std::cout << out << " = " << lhs << " - " << rhs << ";" << std::endl; +#endif ExecutionHandler result; result = stack[first_index].operator_sub(builder, stack[second_index]); // If the output index is larger than the number of elements in stack, append @@ -1358,8 +1550,14 @@ template class CycleGroupBase { PRINT_TWO_ARG_INSTRUCTION( first_index, second_index, stack, "Selecting #" + std::to_string(predicate) + " from", ", ") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_TWO_ARG(stack, first_index, second_index, output_index) + std::cout << out << " = cycle_group_t::conditional_assign(" << std::endl; +#endif result = stack[first_index].conditional_assign(builder, stack[second_index], predicate); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << lhs << ", " << rhs << ");" << std::endl; +#endif // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) @@ -1392,7 +1590,10 @@ template class CycleGroupBase { ScalarField scalar = instruction.arguments.mulArgs.scalar; PRINT_MUL_ARG_INSTRUCTION(first_index, scalar, stack, "Multiplying", "*") - +#ifdef SHOW_PRETTY_INFORMATION + PREP_SINGLE_ARG(stack, first_index, output_index) + std::cout << out << " = " << rhs << std::endl; +#endif ExecutionHandler result; result = stack[first_index].mul(builder, scalar); // If the output index is larger than the number of elements in stack, append @@ -1400,7 +1601,6 @@ template class CycleGroupBase { PRINT_RESULT("", "pushed to ", stack.size(), result) stack.push_back(result); } else { - PRINT_RESULT("", "saved to ", output_index, result) stack[output_index] = result; } @@ -1445,13 +1645,30 @@ template class CycleGroupBase { } size_t output_index = (size_t)instruction.arguments.multOpArgs.output_index; +#ifdef SHOW_PRETTY_INFORMATION + std::string res = ""; + bool is_const = true; + for (size_t i = 0; i < instruction.arguments.batchMulArgs.add_elements_count; i++) { + size_t idx = instruction.arguments.batchMulArgs.inputs[i] % stack.size(); + std::string el = stack[idx].is_constant() ? "c" : "w"; + el += std::to_string(idx); + res += el + ", "; + is_const &= stack[idx].is_constant(); + } + std::string out = is_const ? "c" : "w"; + out = (output_index >= stack.size()) ? "auto " : "" + out; + out += std::to_string(output_index >= stack.size() ? stack.size() : output_index); + std::cout << out << " = cycle_group_t::batch_mul({" << res << "}, {"; +#endif auto result = ExecutionHandler::batch_mul(builder, to_add, to_mul); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << "});" << std::endl; +#endif // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { PRINT_RESULT("", "pushed to ", stack.size(), result) stack.push_back(result); } else { - PRINT_RESULT("", "saved to ", output_index, result) stack[output_index] = result; } From f260f603021b712c6898f1906be452e3507e92fb Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Tue, 11 Mar 2025 21:42:08 +0000 Subject: [PATCH 16/36] Added keywords for BATCH_MUL and COND_ASSIGN --- barretenberg/cpp/src/barretenberg/common/fuzzer.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/common/fuzzer.hpp b/barretenberg/cpp/src/barretenberg/common/fuzzer.hpp index b07860a6dde9..8ecf0be2a574 100644 --- a/barretenberg/cpp/src/barretenberg/common/fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/common/fuzzer.hpp @@ -21,7 +21,7 @@ ASSERT_EQUAL, ASSERT_NOT_EQUAL, SQR_ADD, ASSERT_EQUAL, ASSERT_NOT_EQUAL, SQR_ADD, SUBTRACT_WITH_CONSTRAINT, \ DIVIDE_WITH_CONSTRAINTS, SLICE, ASSERT_ZERO, ASSERT_NOT_ZERO, COND_NEGATE, ADD_MULTI, ASSERT_VALID, \ COND_SELECT, DOUBLE, RANDOMSEED, SELECT_IF_ZERO, SELECT_IF_EQ, REVERSE, GET_BIT, SET_BIT, SET, INVERT, AND, \ - OR, XOR, MODULO, SHL, SHR, ROL, ROR, NOT + OR, XOR, MODULO, SHL, SHR, ROL, ROR, NOT, BATCH_MUL, COND_ASSIGN struct HavocSettings { size_t GEN_LLVM_POST_MUTATION_PROB; // Controls frequency of additional mutation after structural ones From 102244469795feab76b4cfb720dee2ce9276e314 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:39:03 +0000 Subject: [PATCH 17/36] new handlers for bool --- .../barretenberg/stdlib/primitives/bool/bool.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp index 52d510d87741..3daabe87101b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp @@ -441,6 +441,19 @@ bool_t bool_t::conditional_assign(const bool_t& predi const bool_t& lhs, const bool_t& rhs) { + if (predicate.is_constant()) { + return predicate.get_value() ? lhs : rhs; + } + if (lhs.witness_index == rhs.witness_index && lhs.witness_index != IS_CONSTANT && + lhs.witness_inverted == rhs.witness_inverted) { + return lhs; + } + // TODO(alex): reference to the above todo just to not forget to change the lhs.witness_inverted == + // rhs.witness_inverted && ... to get_value() == get_value() + if (lhs.witness_index == rhs.witness_index && lhs.witness_index == IS_CONSTANT && + lhs.witness_inverted == rhs.witness_inverted && lhs.witness_bool == rhs.witness_bool) { + return lhs; + } return (predicate && lhs) || (!predicate && rhs); } @@ -530,6 +543,7 @@ template bool_t bool_t::normalize() const if (is_constant() || !witness_inverted) { return *this; } + // TODO(alex): shouldn't normalize return this->witness_value^witness_inverted in const case? bb::fr value = witness_bool ^ witness_inverted ? bb::fr::one() : bb::fr::zero(); From c0f16c65eadd5c01f9b0faf290f845b4c4bae0f8 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:39:47 +0000 Subject: [PATCH 18/36] should be here I guess --- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index 763d2bfc3a16..050700bed4d3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -645,6 +645,8 @@ void UltraCircuitBuilder_::create_ecc_dbl_gate(const ecc_dbl_gat * we can chain an ecc_add_gate + an ecc_dbl_gate if x3 y3 of previous add_gate equals x1 y1 of current gate * can also chain double gates together **/ + this->assert_valid_variables({ in.x1, in.x3, in.y1, in.y3 }); + bool previous_elliptic_gate_exists = block.size() > 0; bool can_fuse_into_previous_gate = previous_elliptic_gate_exists; if (can_fuse_into_previous_gate) { From 3284a8f09e3bc3436029b27a756a0713830687af Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:41:56 +0000 Subject: [PATCH 19/36] Added few features --- .../stdlib/primitives/group/cycle_group.cpp | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 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 c6e70a03f9ad..43d6ca873e12 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -253,7 +253,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, /*is_infinity=*/false, /*is_standard=*/true); } - result = cycle_group(witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); + result = + cycle_group(witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); } else { const auto p1 = get_value(); const auto p2 = other.get_value(); @@ -453,7 +454,8 @@ cycle_group cycle_group::unconditional_subtract(const cycle_gr if (lhs_constant && rhs_constant) { return cycle_group(x3, y3, /*is_infinity=*/false, /*is_standard=*/true); } - result = cycle_group(witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); + result = cycle_group( + witness_t(context, x3), witness_t(context, y3), /*is_infinity=*/false, /*is_standard=*/true); } else { auto p1 = get_value(); auto p2 = other.get_value(); @@ -549,10 +551,10 @@ cycle_group cycle_group::checked_unconditional_subtract(const */ template cycle_group cycle_group::operator+(const cycle_group& other) const { - if(this->_is_infinity.is_constant() && this->_is_infinity.get_value()){ + if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { return other; } - if(other._is_infinity.is_constant() && other._is_infinity.get_value()){ + if (other._is_infinity.is_constant() && other._is_infinity.get_value()) { return *this; } @@ -625,10 +627,10 @@ template cycle_group cycle_group::operator+ */ template cycle_group cycle_group::operator-(const cycle_group& other) const { - if(this->_is_infinity.is_constant() && this->_is_infinity.get_value()){ + if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { return -other; } - if(other._is_infinity.is_constant() && other._is_infinity.get_value()){ + if (other._is_infinity.is_constant() && other._is_infinity.get_value()) { return *this; } @@ -1849,13 +1851,11 @@ template bool_t cycle_group::operator==(cyc { this->standardize(); other.standardize(); - const auto equal = - (x == other.x) && (y == other.y); + const auto equal = (x == other.x) && (y == other.y); return equal; } -template -void cycle_group::assert_equal(cycle_group& other, std::string const& msg) +template void cycle_group::assert_equal(cycle_group& other, std::string const& msg) { this->standardize(); other.standardize(); @@ -1868,9 +1868,14 @@ cycle_group cycle_group::conditional_assign(const bool_t& pred const cycle_group& lhs, const cycle_group& rhs) { + bool result_standard = lhs._is_standard && rhs._is_standard; + if (predicate.is_constant()) { + result_standard = (predicate.get_value() && lhs._is_standard) && (!predicate.get_value() && rhs._is_standard); + } return { field_t::conditional_assign(predicate, lhs.x, rhs.x), field_t::conditional_assign(predicate, lhs.y, rhs.y), - bool_t::conditional_assign(predicate, lhs.is_point_at_infinity(), rhs.is_point_at_infinity()) }; + bool_t::conditional_assign(predicate, lhs.is_point_at_infinity(), rhs.is_point_at_infinity()), + result_standard }; }; template cycle_group cycle_group::operator/(const cycle_group& /*unused*/) const { From afbcab12e630e29926e402f6264853a3598edcb6 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:42:21 +0000 Subject: [PATCH 20/36] is_standard() --- .../stdlib/primitives/group/cycle_group.hpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 cf54516f6aac..8b947228145c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -213,35 +213,37 @@ template class cycle_group { this->x = 0; this->y = 0; this->_is_constant = true; - } else if(!is_infinity.is_constant()) { + } else if (!is_infinity.is_constant()) { this->x = field_t::conditional_assign(is_infinity, 0, this->x); this->y = field_t::conditional_assign(is_infinity, 0, this->y); this->_is_constant = false; - if(this->context == nullptr){ + if (this->context == nullptr) { this->context = is_infinity.get_context(); } } _is_infinity = is_infinity; this->_is_standard = true; } - void standardize(){ - if(this->_is_standard){ + void standardize() + { + if (this->_is_standard) { return; } if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { this->x = 0; this->y = 0; this->_is_constant = true; - } else if (!this->_is_infinity.is_constant()){ + } else if (!this->_is_infinity.is_constant()) { this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); this->_is_constant = false; - if(this->context == nullptr){ + if (this->context == nullptr) { this->context = this->_is_infinity.get_context(); } } this->_is_standard = true; } + bool is_standard() const { return this->_is_standard; }; cycle_group get_standard_form(); void validate_is_on_curve() const; cycle_group dbl(const std::optional hint = std::nullopt) const From af74a201e9dabc07f99ef531293466b23dca3b71 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:42:46 +0000 Subject: [PATCH 21/36] Two more tests and one still fails --- .../primitives/group/cycle_group.test.cpp | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index 5db770031f27..cc30448a02b6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -168,7 +168,7 @@ TYPED_TEST(CycleGroupTest, TestConstantWitnessMixupRegression) auto cw8 = cycle_group_ct::from_constant_witness(&builder, AffineElement::one() * 0); auto w11 = cycle_group_ct::from_witness(&builder, TestFixture::generators[0]); - auto w9 = cw8 + c1; // mixup happens here due to _is_infinity being a constant + auto w9 = cw8 + c1; // mixup happens here due to _is_infinity being a constant auto w26 = w9 + w11; // and here the circuit checker crashes auto w10 = cw8 - c1; @@ -178,6 +178,41 @@ TYPED_TEST(CycleGroupTest, TestConstantWitnessMixupRegression) EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway } +/** + * @brief Checks the bad behavior of conditional assign. + * + */ +TYPED_TEST(CycleGroupTest, TestConditionalAssignRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto c0 = cycle_group_ct(AffineElement::one() * 0); + auto c1 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, false)), c0, c0); + auto w3 = c1.dbl(); + (void)w3; + EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway +} + +/** + * @brief Checks the bad behavior of conditional assign. + * + */ +TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto c0 = cycle_group_ct(TestFixture::generators[0]); + auto c1 = cycle_group_ct(-TestFixture::generators[0]); + auto w2 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, true)), c0, c1); + EXPECT_TRUE(w2.x.is_constant()); + EXPECT_TRUE(!w2.y.is_constant()); + auto w3 = w2.dbl(); + (void)w3; + EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway +} + /** * @brief Checks that a point on the curve passes the validate_is_on_curve check * From 0a3e45d652d0ea36de0be31c0f6e0c2216430f94 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 00:44:01 +0000 Subject: [PATCH 22/36] fuzzer update --- .../primitives/group/cycle_group.fuzzer.hpp | 75 +++++++------------ 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 86ed05cc69d6..a4a931ece381 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -15,7 +15,7 @@ // TODO: figure out how to detect w + w = c // #define SHOW_INFORMATION -#define SHOW_PRETTY_INFORMATION +// #define SHOW_PRETTY_INFORMATION #define DISABLE_MULTIPLICATION #ifdef SHOW_INFORMATION @@ -59,17 +59,17 @@ #ifdef SHOW_PRETTY_INFORMATION #define PREP_SINGLE_ARG(stack, first_index, output_index) \ - std::string rhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ - std::string out = rhs; \ + std::string rhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string out = rhs; \ rhs += std::to_string(first_index); \ out += std::to_string(output_index >= stack.size() ? stack.size() : output_index); \ out = (output_index >= stack.size() ? "auto " : "") + out; #define PREP_TWO_ARG(stack, first_index, second_index, output_index) \ - std::string lhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ - std::string rhs = stack[second_index].cycle_group.is_constant() ? "c" : "w"; \ - std::string out = \ - (stack[first_index].cycle_group.is_constant() && stack[second_index].cycle_group.is_constant()) ? "c" : "w"; \ + std::string lhs = stack[first_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string rhs = stack[second_index].cycle_group.is_constant() ? "c" : "w"; \ + std::string out = \ + (stack[first_index].cycle_group.is_constant() && stack[second_index].cycle_group.is_constant()) ? "c" : "w"; \ lhs += std::to_string(first_index); \ rhs += std::to_string(second_index); \ out += std::to_string(output_index >= stack.size() ? stack.size() : output_index); \ @@ -800,7 +800,8 @@ template class CycleGroupBase { if (predicate_is_const) { const bool predicate_has_ctx = static_cast(VarianceRNG.next() % 2); #ifdef SHOW_PRETTY_INFORMATION - std::cout << "bool_t(" << (predicate_has_ctx ? "&builder" : "nullptr") << (predicate ? ",true));" : ",false));"); + std::cout << "bool_t(" << (predicate_has_ctx ? "&builder," : "nullptr,") + << (predicate ? "true);" : "false);"); #endif return bool_t(predicate_has_ctx ? builder : nullptr, predicate); } @@ -856,14 +857,8 @@ template class CycleGroupBase { #endif return ExecutionHandler(base_scalar_res, base_res, other.cg().dbl()); case 2: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left + right" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 3: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "right + left" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } else if (other.cg().get_value() == -this->cg().get_value()) { @@ -879,9 +874,6 @@ template class CycleGroupBase { #endif res = this->cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); -#ifdef SHOW_PRETTY_INFORMATION - std::cout << ");" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, res); case 1: #ifdef SHOW_PRETTY_INFORMATION @@ -889,19 +881,10 @@ template class CycleGroupBase { #endif res = other.cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); -#ifdef SHOW_PRETTY_INFORMATION - std::cout << ");" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, res); case 2: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left + right" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 3: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "right + left" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } } @@ -934,14 +917,8 @@ template class CycleGroupBase { #endif return ExecutionHandler(base_scalar_res, base_res, other.cg().checked_unconditional_add(this->cg())); case 4: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left + right;" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() + other.cg()); case 5: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "right + left;" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, other.cg() + this->cg()); } return {}; @@ -970,9 +947,6 @@ template class CycleGroupBase { #endif return ExecutionHandler(base_scalar_res, base_res, -other.cg().dbl()); case 2: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left - right;" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } else if (other.cg().get_value() == this->cg().get_value()) { @@ -989,9 +963,6 @@ template class CycleGroupBase { #endif res = this->cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); -#ifdef SHOW_PRETTY_INFORMATION - std::cout << ");" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, res); case 1: #ifdef SHOW_PRETTY_INFORMATION @@ -999,14 +970,8 @@ template class CycleGroupBase { #endif res = other.cg(); res.set_point_at_infinity(this->construct_predicate(builder, true)); -#ifdef SHOW_PRETTY_INFORMATION - std::cout << ");" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, res); case 2: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left - right" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } } @@ -1030,9 +995,6 @@ template class CycleGroupBase { return ExecutionHandler( base_scalar_res, base_res, this->cg().checked_unconditional_subtract(other.cg())); case 2: -#ifdef SHOW_PRETTY_INFORMATION - std::cout << "left - right;" << std::endl; -#endif return ExecutionHandler(base_scalar_res, base_res, this->cg() - other.cg()); } return {}; @@ -1178,6 +1140,9 @@ template class CycleGroupBase { std::cout << "el.set_point_at_infinty("; #endif res.set_point_at_infinity(this->construct_predicate(builder, set_inf)); +#ifdef SHOW_PRETTY_INFORMATION + std::cout << std::endl; +#endif return res; } @@ -1549,14 +1514,14 @@ template class CycleGroupBase { ExecutionHandler result; PRINT_TWO_ARG_INSTRUCTION( - first_index, second_index, stack, "Selecting #" + std::to_string(predicate) + " from", ", ") + second_index, first_index, stack, "Selecting #" + std::to_string(!predicate) + " from", ", ") #ifdef SHOW_PRETTY_INFORMATION PREP_TWO_ARG(stack, first_index, second_index, output_index) - std::cout << out << " = cycle_group_t::conditional_assign(" << std::endl; + std::cout << out << " = cycle_group_t::conditional_assign("; #endif result = stack[first_index].conditional_assign(builder, stack[second_index], predicate); #ifdef SHOW_PRETTY_INFORMATION - std::cout << lhs << ", " << rhs << ");" << std::endl; + std::cout << rhs << ", " << lhs << ");" << std::endl; #endif // If the output index is larger than the number of elements in stack, append if (output_index >= stack.size()) { @@ -1725,6 +1690,16 @@ template class CycleGroupBase { << std::endl; return false; } + + bool fake_standardized = element.cycle_group.is_standard(); + fake_standardized &= element.cycle_group.is_point_at_infinity().get_value(); + fake_standardized &= (element.cycle_group.x.get_value() != 0) || (element.cycle_group.y.get_value() != 0); + if (fake_standardized) { + std::cerr << "Failed at " << i << " with value claimed to be standard((0, 0)) but the actual value is {" + << element.cycle_group.x.get_value() << ", " << element.cycle_group.y.get_value() << "}" + << std::endl; + return false; + } } return true; } From 47053a091eb654b10e486bf98e9da84b05f7e97f Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 13:34:50 +0000 Subject: [PATCH 23/36] less gates on average --- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 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 43d6ca873e12..ad8330860bfd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -627,12 +627,12 @@ template cycle_group cycle_group::operator+ */ template cycle_group cycle_group::operator-(const cycle_group& other) const { - if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { - return -other; - } if (other._is_infinity.is_constant() && other._is_infinity.get_value()) { return *this; } + if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { + return -other; + } Builder* context = get_context(other); const bool_t x_coordinates_match = (x == other.x); From 55bad893a3afdbed7f47317000ed361c975be3bd Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 13:49:21 +0000 Subject: [PATCH 24/36] Coverage track --- barretenberg/cpp/CMakePresets.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/barretenberg/cpp/CMakePresets.json b/barretenberg/cpp/CMakePresets.json index 876d5772901e..e63f3546e866 100644 --- a/barretenberg/cpp/CMakePresets.json +++ b/barretenberg/cpp/CMakePresets.json @@ -270,6 +270,11 @@ "binaryDir": "build-fuzzing", "cacheVariables": { "FUZZING": "ON" + }, + "environment": { + "CFLAGS": "-fsanitize-coverage=indirect-calls,trace-pc-guard", + "CXXFLAGS": "-fsanitize-coverage=indirect-calls,trace-pc-guard", + "LDFLAGS": "-fsanitize-coverage=indirect-calls,trace-pc-guard" } }, { @@ -282,6 +287,11 @@ "FUZZING": "ON", "ENABLE_ASAN": "ON", "DISABLE_ASM": "ON" + }, + "environment": { + "CFLAGS": "-fsanitize-coverage=func", + "CXXFLAGS": "-fsanitize-coverage=func", + "LDFLAGS": "-fsanitize-coverage=func" } }, { From ea4d9ea2eb9718ded5ce89346304e133568e5e39 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 13:52:39 +0000 Subject: [PATCH 25/36] newline --- .../stdlib/primitives/group/cycle_group.fuzzer.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index a4a931ece381..6fae768c0ed3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -1874,4 +1874,5 @@ extern "C" size_t LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) return 0; } -#pragma clang diagnostic pop \ No newline at end of file +#pragma clang diagnostic pop + From 7653b8d2964e70b24637d26613d3d28b6fae4e2d Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 15:35:59 +0000 Subject: [PATCH 26/36] refactoring due to methods growth --- .../stdlib/primitives/group/cycle_group.cpp | 69 ++++++++++++++++++- .../stdlib/primitives/group/cycle_group.hpp | 38 +--------- 2 files changed, 69 insertions(+), 38 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 ad8330860bfd..aca291f4a610 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -222,6 +222,73 @@ template cycle_group cycle_group::get_stand this->standardize(); return *this; } + +/** + * @brief Set the point to the point at infinity. + * Depending on constant'ness of the predicate put the coordinates in an apropriate standard form. + * + */ +template void cycle_group::set_point_at_infinity(const bool_t& is_infinity) +{ + // No operations are performed in this case + if (is_infinity.is_constant() && !is_infinity.get_value()) { + return; + } + this->_is_standard = true; + + // TODO(alex): there can be a possible bug. Again due to montgomery arithmetic. + // But it will be confirmed only after the review of the multiplication related operations + this->x = field_t::conditional_assign(is_infinity, 0, this->x); + this->y = field_t::conditional_assign(is_infinity, 0, this->y); + + if (is_infinity.is_constant() && is_infinity.get_value()) { + this->_is_constant = true; + this->_is_infinity = true; + return; + } + + // Due to conditional_assign behavior + // Sometimes we won't create the gate here + // If this->x = 0 and this->y = 0 and both of them are constants + // This ensures that at least one of the switches was performed + this->_is_constant = this->x.is_constant() && this->y.is_constant(); + if (!this->_is_constant) { + this->_is_infinity = is_infinity; + } + + // In case we set point at infinity on a constant without an existing context + if (this->context == nullptr && !this->_is_constant) { + this->context = is_infinity.get_context(); + } +} + +/** + * @brief Get the point to the standard form. If the point is a point at infinity, ensure the coordinates are (0,0) + * If the point is already standard nothing changes + * + */ +template void cycle_group::standardize() +{ + if (this->_is_standard) { + return; + } + this->_is_standard = true; + + // TODO(alex): there can be a possible bug. Again due to montgomery arithmetic. + // But it will be confirmed only after the review of the multiplication related operations + this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); + this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); + + // Due to conditional_assign behavior + // Sometimes we won't create the gate here + // If this->x = 0 and this->y = 0 and both of them are constants + // This ensures that at least one of the switches was performed + this->_is_constant = this->x.is_constant() && this->y.is_constant(); + if (this->_is_constant) { + this->_is_infinity = this->_is_infinity.get_value(); + } +} + /** * @brief Evaluates a doubling. Does not use Ultra double gate * @@ -253,8 +320,6 @@ cycle_group cycle_group::dbl(const std::optional class cycle_group { AffineElement get_value() const; [[nodiscard]] bool is_constant() const { return _is_constant; } bool_t is_point_at_infinity() const { return _is_infinity; } - void set_point_at_infinity(const bool_t& is_infinity) - { - if (is_infinity.is_constant() && is_infinity.get_value()) { - this->x = 0; - this->y = 0; - this->_is_constant = true; - } else if (!is_infinity.is_constant()) { - this->x = field_t::conditional_assign(is_infinity, 0, this->x); - this->y = field_t::conditional_assign(is_infinity, 0, this->y); - this->_is_constant = false; - if (this->context == nullptr) { - this->context = is_infinity.get_context(); - } - } - _is_infinity = is_infinity; - this->_is_standard = true; - } - void standardize() - { - if (this->_is_standard) { - return; - } - if (this->_is_infinity.is_constant() && this->_is_infinity.get_value()) { - this->x = 0; - this->y = 0; - this->_is_constant = true; - } else if (!this->_is_infinity.is_constant()) { - this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); - this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); - this->_is_constant = false; - if (this->context == nullptr) { - this->context = this->_is_infinity.get_context(); - } - } - this->_is_standard = true; - } + void set_point_at_infinity(const bool_t& is_infinity); + void standardize(); bool is_standard() const { return this->_is_standard; }; cycle_group get_standard_form(); void validate_is_on_curve() const; From 380ec4e2a93063b2c9d6443b725efdd24b20b122 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 15:36:22 +0000 Subject: [PATCH 27/36] Leave it like that for now --- .../primitives/group/cycle_group.test.cpp | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index cc30448a02b6..26c35860ebbf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -194,24 +194,24 @@ TYPED_TEST(CycleGroupTest, TestConditionalAssignRegression) EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway } -/** - * @brief Checks the bad behavior of conditional assign. - * - */ -TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) -{ - STDLIB_TYPE_ALIASES; - Builder builder; - - auto c0 = cycle_group_ct(TestFixture::generators[0]); - auto c1 = cycle_group_ct(-TestFixture::generators[0]); - auto w2 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, true)), c0, c1); - EXPECT_TRUE(w2.x.is_constant()); - EXPECT_TRUE(!w2.y.is_constant()); - auto w3 = w2.dbl(); - (void)w3; - EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway -} +///** +// * @brief Checks the bad behavior of conditional assign. +// * +// */ +// TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) +//{ +// STDLIB_TYPE_ALIASES; +// Builder builder; +// +// auto c0 = cycle_group_ct(TestFixture::generators[0]); +// auto c1 = cycle_group_ct(-TestFixture::generators[0]); +// auto w2 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, true)), c0, c1); +// EXPECT_TRUE(w2.x.is_constant()); +// EXPECT_TRUE(!w2.y.is_constant()); +// auto w3 = w2.dbl(); +// (void)w3; +// EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway +//} /** * @brief Checks that a point on the curve passes the validate_is_on_curve check From 3348ddb18da5deaa72fa3337a33254ce19e26bbe Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 16:07:42 +0000 Subject: [PATCH 28/36] Resolve the constantness issues --- .../stdlib/primitives/group/cycle_group.cpp | 39 ++++++++++++++++--- .../primitives/group/cycle_group.test.cpp | 37 +++++++++--------- 2 files changed, 52 insertions(+), 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 aca291f4a610..174232c60ddd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -247,11 +247,21 @@ template void cycle_group::set_point_at_infinity(con return; } + if (!this->x.is_constant() && this->y.is_constant()) { + auto ctx = this->x.get_context(); + this->y = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->y.get_value())); + } + if (this->x.is_constant() && !this->y.is_constant()) { + auto ctx = this->y.get_context(); + this->x = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->x.get_value())); + } + // Due to conditional_assign behavior // Sometimes we won't create the gate here // If this->x = 0 and this->y = 0 and both of them are constants // This ensures that at least one of the switches was performed this->_is_constant = this->x.is_constant() && this->y.is_constant(); + if (!this->_is_constant) { this->_is_infinity = is_infinity; } @@ -279,6 +289,15 @@ template void cycle_group::standardize() this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); + if (!this->x.is_constant() && this->y.is_constant()) { + auto ctx = this->x.get_context(); + this->y = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->y.get_value())); + } + if (this->x.is_constant() && !this->y.is_constant()) { + auto ctx = this->y.get_context(); + this->x = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->x.get_value())); + } + // Due to conditional_assign behavior // Sometimes we won't create the gate here // If this->x = 0 and this->y = 0 and both of them are constants @@ -1933,14 +1952,22 @@ cycle_group cycle_group::conditional_assign(const bool_t& pred const cycle_group& lhs, const cycle_group& rhs) { - bool result_standard = lhs._is_standard && rhs._is_standard; + auto x_res = field_t::conditional_assign(predicate, lhs.x, rhs.x); + auto y_res = field_t::conditional_assign(predicate, lhs.y, rhs.y); + auto _is_infinity_res = + bool_t::conditional_assign(predicate, lhs.is_point_at_infinity(), rhs.is_point_at_infinity()); + + bool _is_standard_res = lhs._is_standard && rhs._is_standard; if (predicate.is_constant()) { - result_standard = (predicate.get_value() && lhs._is_standard) && (!predicate.get_value() && rhs._is_standard); + _is_standard_res = (predicate.get_value() && lhs._is_standard) && (!predicate.get_value() && rhs._is_standard); + } + + // Rare case when we bump into two constants, s.t. lhs = -rhs + if (x_res.is_constant() && !y_res.is_constant()) { + auto ctx = predicate.get_context(); + x_res = field_t::from_witness_index(ctx, ctx->put_constant_variable(x_res.get_value())); } - return { field_t::conditional_assign(predicate, lhs.x, rhs.x), - field_t::conditional_assign(predicate, lhs.y, rhs.y), - bool_t::conditional_assign(predicate, lhs.is_point_at_infinity(), rhs.is_point_at_infinity()), - result_standard }; + return { x_res, y_res, _is_infinity_res, _is_standard_res }; }; template cycle_group cycle_group::operator/(const cycle_group& /*unused*/) const { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp index 26c35860ebbf..0a4730b8a490 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp @@ -194,24 +194,25 @@ TYPED_TEST(CycleGroupTest, TestConditionalAssignRegression) EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway } -///** -// * @brief Checks the bad behavior of conditional assign. -// * -// */ -// TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) -//{ -// STDLIB_TYPE_ALIASES; -// Builder builder; -// -// auto c0 = cycle_group_ct(TestFixture::generators[0]); -// auto c1 = cycle_group_ct(-TestFixture::generators[0]); -// auto w2 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, true)), c0, c1); -// EXPECT_TRUE(w2.x.is_constant()); -// EXPECT_TRUE(!w2.y.is_constant()); -// auto w3 = w2.dbl(); -// (void)w3; -// EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway -//} +/** + * @brief Checks the bad behavior of conditional assign. + * + */ +TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) +{ + STDLIB_TYPE_ALIASES; + Builder builder; + + auto c0 = cycle_group_ct(TestFixture::generators[0]); + auto c1 = cycle_group_ct(-TestFixture::generators[0]); + auto w2 = cycle_group_ct::conditional_assign(bool_ct(witness_ct(&builder, true)), c0, c1); + EXPECT_FALSE(w2.x.is_constant()); + EXPECT_FALSE(w2.y.is_constant()); + EXPECT_TRUE(w2.is_point_at_infinity().is_constant()); + auto w3 = w2.dbl(); + (void)w3; + EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway +} /** * @brief Checks that a point on the curve passes the validate_is_on_curve check From 2c11ad050494db89ca96963feedd399b8cea2151 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 16:21:06 +0000 Subject: [PATCH 29/36] clang-format again --- .../barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index 6fae768c0ed3..e98550349092 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -1875,4 +1875,3 @@ extern "C" size_t LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) } #pragma clang diagnostic pop - From bed01b28f7a9a4591112050bc72d2721323595b4 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 16:54:15 +0000 Subject: [PATCH 30/36] fixes for ci 1 --- .../cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp index ec31de7ac8d7..0226909bd6c1 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp @@ -531,7 +531,8 @@ template class IPA { msm_scalars.emplace_back(a_zero); msm_scalars.emplace_back(generator_challenge * a_zero.madd(b_zero, {-opening_claim.opening_pair.evaluation})); GroupElement ipa_relation = GroupElement::batch_mul(msm_elements, msm_scalars); - ipa_relation.assert_equal(-opening_claim.commitment); + auto neg_commitment = -opening_claim.commitment; + ipa_relation.assert_equal(neg_commitment); // TODO(https://github.com/AztecProtocol/barretenberg/issues/1144): Add proper constraints for taking the log of a field_t. Fr stdlib_log_poly_length(static_cast(log_poly_length)); @@ -727,7 +728,8 @@ template class IPA { msm_scalars.emplace_back(a_zero); msm_scalars.emplace_back(generator_challenge * a_zero.madd(b_zero, {-opening_claim.opening_pair.evaluation})); GroupElement ipa_relation = GroupElement::batch_mul(msm_elements, msm_scalars); - ipa_relation.assert_equal(-opening_claim.commitment); + auto neg_commitment = -opening_claim.commitment; + ipa_relation.assert_equal(neg_commitment); return (ipa_relation.get_value() == -opening_claim.commitment.get_value()); } From ccb3f7975eb3a09aea7f9518292a759262b911ce Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 17:46:09 +0000 Subject: [PATCH 31/36] fixes for ci 2: --- .../examples/join_split/join_split_circuit.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split_circuit.cpp b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split_circuit.cpp index d968ae593fd9..b75c9c923da7 100644 --- a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split_circuit.cpp +++ b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split_circuit.cpp @@ -157,8 +157,11 @@ join_split_outputs join_split_circuit_component(join_split_inputs const& inputs) // When allowing chaining, ensure propagation is to one's self (and not to some other user). group_ct self = input_note_1.owner; - allow_chain_1.must_imply(output_note_1.owner == self, "inter-user chaining disallowed"); - allow_chain_2.must_imply(output_note_2.owner == self, "inter-user chaining disallowed"); + + group_ct output_note_1_owner = output_note_1.owner; + group_ct output_note_2_owner = output_note_2.owner; + allow_chain_1.must_imply(output_note_1_owner == self, "inter-user chaining disallowed"); + allow_chain_2.must_imply(output_note_2_owner == self, "inter-user chaining disallowed"); // Prevent chaining from a partial claim note. is_defi_deposit.must_imply(!allow_chain_1, "cannot chain from a partial claim note"); @@ -190,7 +193,9 @@ join_split_outputs join_split_circuit_component(join_split_inputs const& inputs) */ // Verify input notes have the same account public key and account_required. - input_note_1.owner.assert_equal(input_note_2.owner, "input note owners don't match"); + group_ct input_note_1_owner = input_note_1.owner; + group_ct input_note_2_owner = input_note_2.owner; + input_note_1_owner.assert_equal(input_note_2_owner, "input note owners don't match"); input_note_1.account_required.assert_equal(input_note_2.account_required, "input note account_required don't match"); @@ -199,7 +204,7 @@ join_split_outputs join_split_circuit_component(join_split_inputs const& inputs) inputs.account_private_key.assert_is_not_zero("account private key is zero"); auto account_public_key = group_ct(grumpkin::g1::affine_one) * group_ct::cycle_scalar::create_from_bn254_scalar(inputs.account_private_key); - account_public_key.assert_equal(input_note_1.owner, "account_private_key incorrect"); + account_public_key.assert_equal(input_note_1_owner, "account_private_key incorrect"); inputs.account_required.assert_equal(input_note_1.account_required, "account_required incorrect"); // Verify output notes creator_pubkey is either account_public_key.x or 0. From 4e0ce11f22b50a5abe2a9a84d8595efd24247939 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Wed, 12 Mar 2025 18:35:05 +0000 Subject: [PATCH 32/36] Remember origin tag --- .../barretenberg/stdlib/primitives/bool/bool.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp index 3daabe87101b..64356d40516d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp @@ -442,16 +442,17 @@ bool_t bool_t::conditional_assign(const bool_t& predi const bool_t& rhs) { if (predicate.is_constant()) { - return predicate.get_value() ? lhs : rhs; - } - if (lhs.witness_index == rhs.witness_index && lhs.witness_index != IS_CONSTANT && - lhs.witness_inverted == rhs.witness_inverted) { - return lhs; + auto result = bool_t(predicate.get_value() ? lhs : rhs); + result.set_origin_tag(OriginTag(predicate.get_origin_tag(), lhs.get_origin_tag(), rhs.get_origin_tag())); + return result; } + + bool same = (lhs.witness_index == rhs.witness_index) && (lhs.witness_inverted == rhs.witness_inverted); + bool witness_same = same && lhs.witness_index != IS_CONSTANT; + bool const_same = same && (lhs.witness_index == IS_CONSTANT) && (lhs.witness_bool == rhs.witness_bool); // TODO(alex): reference to the above todo just to not forget to change the lhs.witness_inverted == // rhs.witness_inverted && ... to get_value() == get_value() - if (lhs.witness_index == rhs.witness_index && lhs.witness_index == IS_CONSTANT && - lhs.witness_inverted == rhs.witness_inverted && lhs.witness_bool == rhs.witness_bool) { + if (witness_same || const_same) { return lhs; } return (predicate && lhs) || (!predicate && rhs); From 90bf51ddf867a1dd7c40fd9014c7a7f81903155a Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 13 Mar 2025 15:44:50 +0000 Subject: [PATCH 33/36] Addressing bool_t comments --- .../barretenberg/stdlib/primitives/bool/bool.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp index 64356d40516d..cb1920c70171 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp @@ -330,6 +330,10 @@ template bool_t bool_t::operator^(const boo template bool_t bool_t::operator!() const { bool_t result(*this); + if (result.is_constant()) { + result.witness_bool = !result.witness_bool; + return result; + } result.witness_inverted = !result.witness_inverted; return result; } @@ -447,11 +451,9 @@ bool_t bool_t::conditional_assign(const bool_t& predi return result; } - bool same = (lhs.witness_index == rhs.witness_index) && (lhs.witness_inverted == rhs.witness_inverted); - bool witness_same = same && lhs.witness_index != IS_CONSTANT; + bool same = lhs.witness_index == rhs.witness_index; + bool witness_same = same && lhs.witness_index != IS_CONSTANT && (lhs.witness_inverted == rhs.witness_inverted); bool const_same = same && (lhs.witness_index == IS_CONSTANT) && (lhs.witness_bool == rhs.witness_bool); - // TODO(alex): reference to the above todo just to not forget to change the lhs.witness_inverted == - // rhs.witness_inverted && ... to get_value() == get_value() if (witness_same || const_same) { return lhs; } @@ -541,10 +543,10 @@ template bool_t bool_t::implies_both_ways(c template bool_t bool_t::normalize() const { - if (is_constant() || !witness_inverted) { + if (is_constant()) { + ASSERT(!this->witness_inverted); return *this; } - // TODO(alex): shouldn't normalize return this->witness_value^witness_inverted in const case? bb::fr value = witness_bool ^ witness_inverted ? bb::fr::one() : bb::fr::zero(); From 507e0ab32c71963d57ec4619af3c561f9d95a7a3 Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 13 Mar 2025 15:45:32 +0000 Subject: [PATCH 34/36] Addressing CycleGroup comments --- .../stdlib/primitives/group/cycle_group.cpp | 31 +++---------------- .../stdlib/primitives/group/cycle_group.hpp | 8 +++-- 2 files changed, 11 insertions(+), 28 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 174232c60ddd..06f756fbb889 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -236,8 +236,6 @@ template void cycle_group::set_point_at_infinity(con } this->_is_standard = true; - // TODO(alex): there can be a possible bug. Again due to montgomery arithmetic. - // But it will be confirmed only after the review of the multiplication related operations this->x = field_t::conditional_assign(is_infinity, 0, this->x); this->y = field_t::conditional_assign(is_infinity, 0, this->y); @@ -283,29 +281,8 @@ template void cycle_group::standardize() return; } this->_is_standard = true; - - // TODO(alex): there can be a possible bug. Again due to montgomery arithmetic. - // But it will be confirmed only after the review of the multiplication related operations this->x = field_t::conditional_assign(this->_is_infinity, 0, this->x); this->y = field_t::conditional_assign(this->_is_infinity, 0, this->y); - - if (!this->x.is_constant() && this->y.is_constant()) { - auto ctx = this->x.get_context(); - this->y = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->y.get_value())); - } - if (this->x.is_constant() && !this->y.is_constant()) { - auto ctx = this->y.get_context(); - this->x = field_t::from_witness_index(ctx, ctx->put_constant_variable(this->x.get_value())); - } - - // Due to conditional_assign behavior - // Sometimes we won't create the gate here - // If this->x = 0 and this->y = 0 and both of them are constants - // This ensures that at least one of the switches was performed - this->_is_constant = this->x.is_constant() && this->y.is_constant(); - if (this->_is_constant) { - this->_is_infinity = this->_is_infinity.get_value(); - } } /** @@ -341,8 +318,9 @@ cycle_group cycle_group::dbl(const std::optionalis_point_at_infinity().is_constant() && this->is_point_at_infinity().get_value()) { return *this; } @@ -1935,7 +1913,7 @@ template bool_t cycle_group::operator==(cyc { this->standardize(); other.standardize(); - const auto equal = (x == other.x) && (y == other.y); + const auto equal = (x == other.x) && (y == other.y) && (this->_is_infinity == other._is_infinity); return equal; } @@ -1945,6 +1923,7 @@ template void cycle_group::assert_equal(cycle_group& other.standardize(); x.assert_equal(other.x, msg); y.assert_equal(other.y, msg); + this->_is_infinity.assert_equal(other._is_infinity); } template 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 a81e06f5df9a..02dbef727815 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -282,9 +282,13 @@ template class cycle_group { private: bool_t _is_infinity; bool _is_constant; + // The point is considered to be `standard` or in `standard form` when: + // - It's not a point at infinity, and the coordinates belong to the curve + // - It's a point at infinity and both of the coordinates are set to be 0. (0, 0) // Most of the time it is true, so we won't need to do extra conditional_assign - // during `get_standard_form` or `assert_equal` call - // However sometimes it won't be the case, so we can handle these cases using this flag + // during `get_standard_form`, `assert_equal` or `==` calls + // However sometimes it won't be the case(due to some previous design choices), + // so we can handle these cases using this flag bool _is_standard; Builder* context; From 10790afa72aafd1322a69fc154b98cac5b77e01e Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 13 Mar 2025 15:46:05 +0000 Subject: [PATCH 35/36] Addressing fuzzer comments --- .../stdlib/primitives/group/cycle_group.fuzzer.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp index e98550349092..b41fa335387f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.fuzzer.hpp @@ -4,15 +4,17 @@ #include "barretenberg/numeric/uint256/uint256.hpp" #include "barretenberg/stdlib/primitives/group/cycle_group.hpp" #pragma clang diagnostic push -// TODO(luke/kesha): Add a comment explaining why we need this ignore and what the solution is. -// TODO(alex): resolve this todo in current pr -// TODO + +// -Wc99-designator prevents us from using designators and nested designators +// in struct intializations +// such as {.in.first = a, .out = b}, since it's not a part of c++17 standard +// However the use of them in this particular file heavily increases +// the readability and conciseness of the CycleGroupBase::Instruction initializations #pragma clang diagnostic ignored "-Wc99-designator" #define HAVOC_TESTING #include "barretenberg/common/fuzzer.hpp" -// TODO: figure out how to detect w + w = c // #define SHOW_INFORMATION // #define SHOW_PRETTY_INFORMATION From 815ddbd883458197081345aaa6d8858d7fa1974a Mon Sep 17 00:00:00 2001 From: Sarkoxed Date: Thu, 13 Mar 2025 21:40:29 +0000 Subject: [PATCH 36/36] Comment on normalizations --- .../src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 3 +++ 1 file changed, 3 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 06f756fbb889..b683b82e7aab 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -754,6 +754,9 @@ template cycle_group cycle_group::operator- template cycle_group cycle_group::operator-() const { cycle_group result(*this); + // We have to normalize immediately. All the methods, related to + // elliptic curve operations, assume that the coordinates are in normalized form and + // don't perform any extra normalizations result.y = (-y).normalize(); return result; }