From 01a526ec99eeea74627a9574316d2e6c7fc462c0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 6 Nov 2025 18:42:24 +0000 Subject: [PATCH 1/3] handle coordinate constancy in cycle group constructor --- .../stdlib/primitives/group/cycle_group.cpp | 25 ++++++++++++------- .../stdlib/primitives/group/cycle_group.hpp | 4 +-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 0bb09c40dc8f..b14e06299b44 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -41,9 +41,9 @@ template 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 assert_on_curve) - : _x(_x) - , _y(_y) +cycle_group::cycle_group(const field_t& x, const field_t& y, bool_t is_infinity, bool assert_on_curve) + : _x(x) + , _y(y) , _is_infinity(is_infinity) , _is_standard(is_infinity.is_constant()) { @@ -59,9 +59,16 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bo *this = constant_infinity(this->context); } - // We don't support points with only one constant coordinate since valid use-cases are limited and it complicates - // the logic - BB_ASSERT(_x.is_constant() == _y.is_constant(), "cycle_group: Inconsistent constancy of coordinates"); + // For the simplicity of methods in this class, we ensure that the coordinates of a point always have the same + // constancy. If they don't, we convert the non-constant coordinate to a fixed witness. + if (_x.is_constant() != _y.is_constant()) { + info("Warning: cycle_group constructed with inconsistent coordinate constancy - converting to fixed witness"); + if (_x.is_constant()) { + _x.convert_constant_to_fixed_witness(context); + } else { + _y.convert_constant_to_fixed_witness(context); + } + } // Elements are always expected to be on the curve but may or may not be constrained as such. BB_ASSERT(get_value().on_curve(), "cycle_group: Point is not on curve"); @@ -83,9 +90,9 @@ cycle_group::cycle_group(field_t _x, field_t _y, bool_t is_infinity, bo * @param is_infinity */ template -cycle_group::cycle_group(const bb::fr& _x, const bb::fr& _y, bool is_infinity) - : _x(is_infinity ? 0 : _x) - , _y(is_infinity ? 0 : _y) +cycle_group::cycle_group(const bb::fr& x, const bb::fr& y, bool is_infinity) + : _x(is_infinity ? 0 : x) + , _y(is_infinity ? 0 : y) , _is_infinity(is_infinity) , _is_standard(true) , context(nullptr) 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 6b2a07be2b48..8f8f631e4bd8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -67,8 +67,8 @@ template class cycle_group { public: cycle_group(Builder* _context = nullptr); - cycle_group(field_t _x, field_t _y, bool_t _is_infinity, bool assert_on_curve); - cycle_group(const bb::fr& _x, const bb::fr& _y, bool _is_infinity); + cycle_group(const field_t& x, const field_t& y, bool_t is_infinity, bool assert_on_curve); + cycle_group(const bb::fr& x, const bb::fr& y, bool is_infinity); cycle_group(const AffineElement& _in); static cycle_group one(Builder* _context); static cycle_group constant_infinity(Builder* _context = nullptr); From aa6524f4e4f583f03fcd117f7c4b348d66962a9a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 6 Nov 2025 18:46:35 +0000 Subject: [PATCH 2/3] remove special handling for mixed constancy in cycle_group::conditional_assign --- .../barretenberg/stdlib/primitives/group/cycle_group.cpp | 7 ------- 1 file changed, 7 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 b14e06299b44..0f2ddbe3d7a0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -1230,13 +1230,6 @@ cycle_group cycle_group::conditional_assign(const bool_t& pred _is_standard_res = predicate.get_value() ? lhs._is_standard : rhs._is_standard; } - // AUDITTODO: Talk to Sasha. Comment seems to be unrelated and its not clear why the logic is needed. - // 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())); - } - cycle_group result(x_res, y_res, _is_infinity_res, /*assert_on_curve=*/false); result._is_standard = _is_standard_res; return result; From 1e5b2a67f44ebeefba06e12e93d067d79e6c6a09 Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Fri, 7 Nov 2025 10:44:22 -0700 Subject: [PATCH 3/3] Remove warning for inconsistent coordinate constancy Removed warning message for inconsistent coordinate constancy in cycle_group. --- .../cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 0f2ddbe3d7a0..7feaaf6a5c83 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -62,7 +62,6 @@ cycle_group::cycle_group(const field_t& x, const field_t& y, bool_t is_ // For the simplicity of methods in this class, we ensure that the coordinates of a point always have the same // constancy. If they don't, we convert the non-constant coordinate to a fixed witness. if (_x.is_constant() != _y.is_constant()) { - info("Warning: cycle_group constructed with inconsistent coordinate constancy - converting to fixed witness"); if (_x.is_constant()) { _x.convert_constant_to_fixed_witness(context); } else {