From 6b23fd2f2ab5b780ee99f1515a471e08627f6e15 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 12 Sep 2025 20:42:41 +0000 Subject: [PATCH 01/17] remove unused cycle_scalar interfaces and use bigfield in test suite --- .../primitives/group/cycle_group.test.cpp | 71 +++++++++++++------ .../stdlib/primitives/group/cycle_scalar.cpp | 22 ------ .../stdlib/primitives/group/cycle_scalar.hpp | 8 +-- .../stdlib/transcript/transcript.hpp | 1 + 4 files changed, 53 insertions(+), 49 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 c9a01118fac3..8efc76b45c24 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 @@ -20,7 +20,8 @@ using Group = typename Curve::Group; \ using bool_ct = stdlib::bool_t; \ using witness_ct = stdlib::witness_t; \ - using cycle_scalar_ct = cycle_group_ct::cycle_scalar; + using cycle_scalar_ct = cycle_group_ct::cycle_scalar; \ + using BigScalarField = typename cycle_scalar_ct::BigScalarField; using namespace bb; @@ -771,22 +772,26 @@ TYPED_TEST(CycleGroupTest, TestBatchMulGeneralMSM) // 1: add entry where point, scalar are witnesses expected += (element * scalar); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar1(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar1)); // 2: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(cycle_group_ct(element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar2(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar2)); // 3: add entry where point is witness, scalar is constant expected += (element * scalar); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); + BigScalarField big_scalar3(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar3)); // 4: add entry where point is constant, scalar is constant expected += (element * scalar); points.emplace_back(cycle_group_ct(element)); - scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); + BigScalarField big_scalar4(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar4)); } // Here and in the following cases assign different tags to points and scalars and get the union of them back @@ -813,10 +818,12 @@ TYPED_TEST(CycleGroupTest, TestBatchMulProducesInfinity) auto element = TestFixture::generators[0]; typename Group::Fr scalar = Group::Fr::random_element(&engine); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar1(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar1)); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, -scalar)); + BigScalarField big_scalar2(&builder, -scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar2)); const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -841,7 +848,8 @@ TYPED_TEST(CycleGroupTest, TestBatchMulMultiplyByZero) auto element = TestFixture::generators[0]; typename Group::Fr scalar = 0; points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar)); const auto expected_tag = assign_and_merge_tags(points, scalars); auto result = cycle_group_ct::batch_mul(points, scalars); @@ -869,14 +877,16 @@ TYPED_TEST(CycleGroupTest, TestBatchMulInputsAreInfinity) cycle_group_ct point = cycle_group_ct::from_witness(&builder, element); point.set_point_at_infinity(witness_ct(&builder, true)); points.emplace_back(point); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar1(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar1)); } // is_infinity = constant { cycle_group_ct point = cycle_group_ct::from_witness(&builder, element); point.set_point_at_infinity(true); points.emplace_back(point); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar2(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar2)); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -907,14 +917,16 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseInLookupTable) // 1: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(element); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar)); scalars_native.emplace_back(uint256_t(scalar)); // 2: add entry where point is constant, scalar is constant element = plookup::fixed_base::table::rhs_generator_point(); expected += (element * scalar); points.emplace_back(element); - scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); + BigScalarField big_scalar_const(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); scalars_native.emplace_back(uint256_t(scalar)); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -946,14 +958,16 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) // 1: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(element); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar)); scalars_native.emplace_back(scalar); // 2: add entry where point is constant, scalar is constant element = plookup::fixed_base::table::rhs_generator_point(); expected += (element * scalar); points.emplace_back(element); - scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); + BigScalarField big_scalar_const(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); scalars_native.emplace_back(scalar); // 3: add entry where point is constant, scalar is witness @@ -961,7 +975,8 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) element = Group::one * Group::Fr::random_element(&engine); expected += (element * scalar); points.emplace_back(element); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar2(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar2)); scalars_native.emplace_back(scalar); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -989,11 +1004,13 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseZeroScalars) // 1: add entry where point is constant, scalar is witness points.emplace_back((element)); - scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); + BigScalarField big_scalar(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar)); // 2: add entry where point is constant, scalar is constant points.emplace_back((element)); - scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); + BigScalarField big_scalar_const(&builder, scalar); + scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); } const auto expected_tag = assign_and_merge_tags(points, scalars); auto result = cycle_group_ct::batch_mul(points, scalars); @@ -1014,7 +1031,6 @@ TYPED_TEST(CycleGroupTest, TestMul) // case 1, general MSM with inputs that are combinations of constant and witnesses { cycle_group_ct point; - typename cycle_group_ct::cycle_scalar scalar; cycle_group_ct result; for (size_t i = 0; i < num_muls; ++i) { auto element = TestFixture::generators[i]; @@ -1023,7 +1039,8 @@ TYPED_TEST(CycleGroupTest, TestMul) // 1: add entry where point, scalar are witnesses point = (cycle_group_ct::from_witness(&builder, element)); - scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar)); + BigScalarField big_scalar1(&builder, native_scalar); + cycle_scalar_ct scalar = (cycle_scalar_ct(big_scalar1)); point.set_origin_tag(submitted_value_origin_tag); scalar.set_origin_tag(challenge_origin_tag); result = point * scalar; @@ -1032,16 +1049,21 @@ TYPED_TEST(CycleGroupTest, TestMul) // 2: add entry where point is constant, scalar is witness point = (cycle_group_ct(element)); - scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar)); + BigScalarField big_scalar2(&builder, native_scalar); + scalar = (cycle_scalar_ct(big_scalar2)); EXPECT_EQ((result).get_value(), (expected_result)); // 3: add entry where point is witness, scalar is constant point = (cycle_group_ct::from_witness(&builder, element)); + BigScalarField big_scalar3(&builder, native_scalar); + scalar = (cycle_scalar_ct(big_scalar3)); EXPECT_EQ((result).get_value(), (expected_result)); // 4: add entry where point is constant, scalar is constant point = (cycle_group_ct(element)); + BigScalarField big_scalar4(&builder, native_scalar); + scalar = (cycle_scalar_ct(big_scalar4)); EXPECT_EQ((result).get_value(), (expected_result)); } } @@ -1156,7 +1178,8 @@ TYPED_TEST(CycleGroupTest, MixedLengthScalarsIsNotSupported) // First scalar: 256 bits uint256_t scalar1_value = uint256_t(123456789); - scalars.push_back(cycle_group_ct::cycle_scalar::from_witness(&builder, typename Curve::ScalarField(scalar1_value))); + BigScalarField big_scalar1(&builder, typename Curve::ScalarField(scalar1_value)); + scalars.push_back(cycle_scalar_ct(big_scalar1)); // Second scalar: 128 bits uint256_t scalar2_value = uint256_t(987654321); @@ -1188,8 +1211,10 @@ TYPED_TEST(CycleGroupTest, TestFixedBaseBatchMul) auto scalar1_val = Group::Fr::random_element(&engine); auto scalar2_val = Group::Fr::random_element(&engine); - scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar1_val)); - scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar2_val)); + BigScalarField big_scalar1(&builder, scalar1_val); + BigScalarField big_scalar2(&builder, scalar2_val); + scalars.push_back(cycle_scalar_ct(big_scalar1)); + scalars.push_back(cycle_scalar_ct(big_scalar2)); points.push_back(cycle_group_ct(lhs_generator)); // constant point points.push_back(cycle_group_ct(rhs_generator)); // constant point diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index f0056c5c9e35..6df5684ad106 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -39,28 +39,6 @@ template cycle_scalar::cycle_scalar(const field_t& i hi.set_origin_tag(in.get_origin_tag()); } -template cycle_scalar::cycle_scalar(const ScalarField& in) -{ - const uint256_t value(in); - const uint256_t lo_v = value.slice(0, LO_BITS); - const uint256_t hi_v = value.slice(LO_BITS, HI_BITS); - lo = lo_v; - hi = hi_v; -} - -template -cycle_scalar cycle_scalar::from_witness(Builder* context, const ScalarField& value) -{ - const uint256_t value_u256(value); - const uint256_t lo_v = value_u256.slice(0, LO_BITS); - const uint256_t hi_v = value_u256.slice(LO_BITS, HI_BITS); - field_t lo = witness_t(context, lo_v); - field_t hi = witness_t(context, hi_v); - lo.set_free_witness_tag(); - hi.set_free_witness_tag(); - return cycle_scalar(lo, hi); -} - /** * @brief Use when we want to multiply a group element by a string of bits of known size. * N.B. using this constructor method will make our scalar multiplication methods not perform primality tests. diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp index 4c79a0202e9a..34036fc65190 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp @@ -52,7 +52,6 @@ template class cycle_scalar { // we want to validate the cycle_scalar < bn254::fr::modulus *not* grumpkin::fr::modulus bool _use_bn254_scalar_field_for_primality_test = false; - public: cycle_scalar(const field_t& _lo, const field_t& _hi, const size_t bits, @@ -63,10 +62,11 @@ template class cycle_scalar { , _num_bits(bits) , _skip_primality_test(skip_primality_test) , _use_bn254_scalar_field_for_primality_test(use_bn254_scalar_field_for_primality_test) {}; - cycle_scalar(const ScalarField& _in = 0); + + public: cycle_scalar(const field_t& _lo, const field_t& _hi); + // AUDITTODO: this is not used (aside from transcript) and should be deleted. cycle_scalar(const field_t& _in); - static cycle_scalar from_witness(Builder* context, const ScalarField& value); static cycle_scalar from_witness_bitstring(Builder* context, const uint256_t& bitstring, size_t num_bits); static cycle_scalar create_from_bn254_scalar(const field_t& _in, bool skip_primality_test = false); [[nodiscard]] bool is_constant() const; @@ -115,4 +115,4 @@ template class cycle_scalar { } }; -} // namespace bb::stdlib \ No newline at end of file +} // namespace bb::stdlib diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index 01e837830bcc..e3ddf4e7b18f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -35,6 +35,7 @@ template struct StdlibTranscriptParams { static inline std::array split_challenge(const DataType& challenge) { // use existing field-splitting code in cycle_scalar + // AUDITTODO: there should be a field_t method for handling this - we should not use cycle_scalar using cycle_scalar = typename stdlib::cycle_group::cycle_scalar; const cycle_scalar scalar = cycle_scalar(challenge); scalar.lo.create_range_constraint(cycle_scalar::LO_BITS); From 4832bd6262002c9d513814e48df785a4fe572274 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sat, 13 Sep 2025 21:41:56 +0000 Subject: [PATCH 02/17] remove option to skip primality in construct from bn254 --- .../stdlib/primitives/group/cycle_scalar.cpp | 22 ++++++++++--------- .../stdlib/primitives/group/cycle_scalar.hpp | 2 +- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index 6df5684ad106..c21b625dd8a0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -61,13 +61,14 @@ cycle_scalar cycle_scalar::from_witness_bitstring(Builder* con field_t hi = witness_t(context, hi_v); lo.set_free_witness_tag(); hi.set_free_witness_tag(); - cycle_scalar result{ lo, hi, num_bits, true, false }; - return result; + return cycle_scalar{ + lo, hi, num_bits, /*skip_primality_test=*/true, /*use_bn254_scalar_field_for_primality_test=*/false + }; } /** - * @brief Use when we want to multiply a group element by a string of bits of known size. - * N.B. using this constructor method will make our scalar multiplication methods not perform primality tests. + * @brief Construct a cycle scalar (grumpkin scalar field element) from a bn254 scalar field element + * @details This method ensures that the input is constrained to be less than the bn254 scalar field modulus. * * @tparam Builder * @param context @@ -75,15 +76,17 @@ cycle_scalar cycle_scalar::from_witness_bitstring(Builder* con * @param num_bits * @return cycle_scalar */ -template -cycle_scalar cycle_scalar::create_from_bn254_scalar(const field_t& in, const bool skip_primality_test) +template cycle_scalar cycle_scalar::create_from_bn254_scalar(const field_t& in) { const uint256_t value_u256(in.get_value()); const uint256_t lo_v = value_u256.slice(0, LO_BITS); const uint256_t hi_v = value_u256.slice(LO_BITS, HI_BITS); + const bool skip_primality_test = false; + const bool use_bn254_scalar_field_for_primality_test = true; if (in.is_constant()) { - cycle_scalar result{ field_t(lo_v), field_t(hi_v), NUM_BITS, false, true }; - return result; + return cycle_scalar{ + field_t(lo_v), field_t(hi_v), NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test + }; } field_t lo = witness_t(in.get_context(), lo_v); field_t hi = witness_t(in.get_context(), hi_v); @@ -93,8 +96,7 @@ cycle_scalar cycle_scalar::create_from_bn254_scalar(const fiel lo.set_origin_tag(in.get_origin_tag()); hi.set_origin_tag(in.get_origin_tag()); - cycle_scalar result{ lo, hi, NUM_BITS, skip_primality_test, true }; - return result; + return cycle_scalar{ lo, hi, NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test }; } /** * @brief Construct a new cycle scalar from a bigfield _value, over the same ScalarField Field. If _value is a witness, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp index 34036fc65190..6287247c67de 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp @@ -68,7 +68,7 @@ template class cycle_scalar { // AUDITTODO: this is not used (aside from transcript) and should be deleted. cycle_scalar(const field_t& _in); static cycle_scalar from_witness_bitstring(Builder* context, const uint256_t& bitstring, size_t num_bits); - static cycle_scalar create_from_bn254_scalar(const field_t& _in, bool skip_primality_test = false); + static cycle_scalar create_from_bn254_scalar(const field_t& _in); [[nodiscard]] bool is_constant() const; ScalarField get_value() const; Builder* get_context() const { return lo.get_context() != nullptr ? lo.get_context() : hi.get_context(); } From bf3eacb715f822936cd1bb3a22375c136f8b770d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sat, 13 Sep 2025 22:11:40 +0000 Subject: [PATCH 03/17] use from_witness for consistency --- .../stdlib/primitives/group/cycle_scalar.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index c21b625dd8a0..b3f9af3b3fd8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -27,8 +27,8 @@ template cycle_scalar::cycle_scalar(const field_t& i lo = lo_v; hi = hi_v; } else { - lo = witness_t(in.get_context(), lo_v); - hi = witness_t(in.get_context(), hi_v); + lo = field_t::from_witness(in.get_context(), lo_v); + hi = field_t::from_witness(in.get_context(), hi_v); (lo + hi * shift).assert_equal(in); // TODO(https://github.com/AztecProtocol/barretenberg/issues/1022): ensure lo and hi are in bb::fr modulus not // bb::fq modulus otherwise we could have two representations for in @@ -57,10 +57,8 @@ cycle_scalar cycle_scalar::from_witness_bitstring(Builder* con BB_ASSERT_LT(bitstring.get_msb(), num_bits); const uint256_t lo_v = bitstring.slice(0, LO_BITS); const uint256_t hi_v = bitstring.slice(LO_BITS, HI_BITS); - field_t lo = witness_t(context, lo_v); - field_t hi = witness_t(context, hi_v); - lo.set_free_witness_tag(); - hi.set_free_witness_tag(); + auto lo = field_t::from_witness(context, lo_v); + auto hi = field_t::from_witness(context, hi_v); return cycle_scalar{ lo, hi, num_bits, /*skip_primality_test=*/true, /*use_bn254_scalar_field_for_primality_test=*/false }; @@ -88,8 +86,8 @@ template cycle_scalar cycle_scalar::create_ field_t(lo_v), field_t(hi_v), NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test }; } - field_t lo = witness_t(in.get_context(), lo_v); - field_t hi = witness_t(in.get_context(), hi_v); + auto lo = field_t::from_witness(in.get_context(), lo_v); + auto hi = field_t::from_witness(in.get_context(), hi_v); lo.add_two(hi * (uint256_t(1) << LO_BITS), -in).assert_equal(0); // We need to manually propagate the origin tag From c6292e0358d0274564bbaf7fb1c8607768fbc086 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sun, 14 Sep 2025 20:09:38 +0000 Subject: [PATCH 04/17] add split_at_unrestricted (bad name!) --- .../stdlib/primitives/field/field.cpp | 23 +++++++++++++++++++ .../stdlib/primitives/field/field.hpp | 2 ++ 2 files changed, 25 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 4e731bd45205..ffd43624cd27 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1260,6 +1260,29 @@ template field_t field_t::accumulate(const return total.normalize(); } +template +std::pair, field_t> field_t::split_at_unrestricted(const size_t lsb_index) const +{ + const size_t max_bits = 256; // should this be 254? + ASSERT(lsb_index < max_bits); + const uint256_t value(this->get_value()); + const uint256_t lo_val = value.slice(0, lsb_index); + const uint256_t hi_val = value.slice(lsb_index, max_bits); + const uint256_t shift = uint256_t(1) << lsb_index; + if (this->is_constant()) { + return { field_t(lo_val), field_t(hi_val) }; + } + + field_t lo = from_witness(get_context(), lo_val); + field_t hi = from_witness(get_context(), hi_val); + (lo + (hi * shift)).assert_equal(*this); + + // We need to manually propagate the origin tag + lo.set_origin_tag(get_origin_tag()); + hi.set_origin_tag(get_origin_tag()); + return { lo, hi }; +} + /** * @brief Splits the field element into (lo, hi), where: * - lo contains bits [0, lsb_index) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp index 1432864718ae..b98890327679 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp @@ -388,6 +388,8 @@ template class field_t { Builder* get_context() const { return context; } + std::pair, field_t> split_at_unrestricted(const size_t lsb_index) const; + std::pair, field_t> split_at( const size_t lsb_index, const size_t num_bits = grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) const; From 14c1400cd15469d921d2a1c341d5e28a2c252f7a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sun, 14 Sep 2025 21:28:28 +0000 Subject: [PATCH 05/17] add split method to field and bring back from witness for fuzzer build --- .../stdlib/primitives/field/field.cpp | 12 ++++++-- .../stdlib/primitives/group/cycle_scalar.cpp | 29 +++++++------------ .../stdlib/primitives/group/cycle_scalar.hpp | 5 ++-- .../stdlib/transcript/transcript.hpp | 15 +++++----- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index ffd43624cd27..9f9e2ba4dec9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1260,24 +1260,30 @@ template field_t field_t::accumulate(const return total.normalize(); } +// WORKTODO: better name! template std::pair, field_t> field_t::split_at_unrestricted(const size_t lsb_index) const { - const size_t max_bits = 256; // should this be 254? + // WORKTODO: is this right? depends on how bb::fr is implemented. ideally this would be some predefined constant.. + const size_t max_bits = 254; ASSERT(lsb_index < max_bits); + const uint256_t value(this->get_value()); const uint256_t lo_val = value.slice(0, lsb_index); const uint256_t hi_val = value.slice(lsb_index, max_bits); - const uint256_t shift = uint256_t(1) << lsb_index; + + // If `this` is constant, return constants based on the native hi/lo values if (this->is_constant()) { return { field_t(lo_val), field_t(hi_val) }; } + // Otherwise, create hi/lo witnesses and constrain them to reconstruct `this` as field elements field_t lo = from_witness(get_context(), lo_val); field_t hi = from_witness(get_context(), hi_val); + const uint256_t shift = uint256_t(1) << lsb_index; (lo + (hi * shift)).assert_equal(*this); - // We need to manually propagate the origin tag + // Set the origin tag for the limbs to be that of the original field element lo.set_origin_tag(get_origin_tag()); hi.set_origin_tag(get_origin_tag()); return { lo, hi }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index b3f9af3b3fd8..e226dae7b2ba 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -17,26 +17,17 @@ cycle_scalar::cycle_scalar(const field_t& _lo, const field_t& _hi) , hi(_hi) {} -template cycle_scalar::cycle_scalar(const field_t& in) +template +cycle_scalar cycle_scalar::from_witness(Builder* context, const ScalarField& value) { - const uint256_t value(in.get_value()); - const uint256_t lo_v = value.slice(0, LO_BITS); - const uint256_t hi_v = value.slice(LO_BITS, HI_BITS); - constexpr uint256_t shift = uint256_t(1) << LO_BITS; - if (in.is_constant()) { - lo = lo_v; - hi = hi_v; - } else { - lo = field_t::from_witness(in.get_context(), lo_v); - hi = field_t::from_witness(in.get_context(), hi_v); - (lo + hi * shift).assert_equal(in); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1022): ensure lo and hi are in bb::fr modulus not - // bb::fq modulus otherwise we could have two representations for in - validate_scalar_is_in_field(); - } - // We need to manually propagate the origin tag - lo.set_origin_tag(in.get_origin_tag()); - hi.set_origin_tag(in.get_origin_tag()); + const uint256_t value_u256(value); + const uint256_t lo_v = value_u256.slice(0, LO_BITS); + const uint256_t hi_v = value_u256.slice(LO_BITS, HI_BITS); + field_t lo = witness_t(context, lo_v); + field_t hi = witness_t(context, hi_v); + lo.set_free_witness_tag(); + hi.set_free_witness_tag(); + return cycle_scalar(lo, hi); } /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp index 6287247c67de..8107755b8009 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp @@ -65,8 +65,9 @@ template class cycle_scalar { public: cycle_scalar(const field_t& _lo, const field_t& _hi); - // AUDITTODO: this is not used (aside from transcript) and should be deleted. - cycle_scalar(const field_t& _in); + // AUDITTODO: this is used only in the fuzzer. Its not inherently problematic, but perhaps the fuzzer should use a + // production entrypoint. + static cycle_scalar from_witness(Builder* context, const ScalarField& value); static cycle_scalar from_witness_bitstring(Builder* context, const uint256_t& bitstring, size_t num_bits); static cycle_scalar create_from_bn254_scalar(const field_t& _in); [[nodiscard]] bool is_constant() const; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index e3ddf4e7b18f..3e875eb36c3d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -34,13 +34,14 @@ template struct StdlibTranscriptParams { */ static inline std::array split_challenge(const DataType& challenge) { - // use existing field-splitting code in cycle_scalar - // AUDITTODO: there should be a field_t method for handling this - we should not use cycle_scalar - using cycle_scalar = typename stdlib::cycle_group::cycle_scalar; - const cycle_scalar scalar = cycle_scalar(challenge); - scalar.lo.create_range_constraint(cycle_scalar::LO_BITS); - scalar.hi.create_range_constraint(cycle_scalar::HI_BITS); - return std::array{ scalar.lo, scalar.hi }; + // WORKTODO: now that we're not using cycle_scalar, should we split differently? + const size_t low_bits = 128; + const size_t high_bits = 126; + const auto [lo, high] = challenge.split_at_unrestricted(low_bits); + // WORKTODO: are these range constraints necessary? + lo.create_range_constraint(low_bits); + high.create_range_constraint(high_bits); + return std::array{ lo, high }; } template static inline T convert_challenge(const DataType& challenge) { From fb17bdf842f74b903803c80040a4e44325214878 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sun, 14 Sep 2025 23:17:26 +0000 Subject: [PATCH 06/17] bring back constructor for fuzzer --- .../stdlib/primitives/group/cycle_scalar.cpp | 10 ++++++++++ .../stdlib/primitives/group/cycle_scalar.hpp | 2 ++ 2 files changed, 12 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index e226dae7b2ba..d46a5c66d2f3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -17,6 +17,15 @@ cycle_scalar::cycle_scalar(const field_t& _lo, const field_t& _hi) , hi(_hi) {} +template cycle_scalar::cycle_scalar(const ScalarField& in) +{ + const uint256_t value(in); + const uint256_t lo_v = value.slice(0, LO_BITS); + const uint256_t hi_v = value.slice(LO_BITS, HI_BITS); + lo = lo_v; + hi = hi_v; +} + template cycle_scalar cycle_scalar::from_witness(Builder* context, const ScalarField& value) { @@ -72,6 +81,7 @@ template cycle_scalar cycle_scalar::create_ const uint256_t hi_v = value_u256.slice(LO_BITS, HI_BITS); const bool skip_primality_test = false; const bool use_bn254_scalar_field_for_primality_test = true; + // WORKTODO: should be able to use field_t::split_at_unrestricted here if (in.is_constant()) { return cycle_scalar{ field_t(lo_v), field_t(hi_v), NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp index 8107755b8009..ac936b555803 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp @@ -64,6 +64,8 @@ template class cycle_scalar { , _use_bn254_scalar_field_for_primality_test(use_bn254_scalar_field_for_primality_test) {}; public: + // AUDITTODO: this is used only in the fuzzer. + cycle_scalar(const ScalarField& _in = 0); cycle_scalar(const field_t& _lo, const field_t& _hi); // AUDITTODO: this is used only in the fuzzer. Its not inherently problematic, but perhaps the fuzzer should use a // production entrypoint. From ce5e3da7a6a280678a8b7743b4a13db137f11bc8 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 15 Sep 2025 18:06:32 +0000 Subject: [PATCH 07/17] update plsit at naming and fix tests --- ...h_description_ultra_recursive_verifier.test.cpp | 2 +- .../eccvm_recursive_verifier.test.cpp | 2 +- .../ultra_recursive_verifier.test.cpp | 2 +- .../barretenberg/stdlib/primitives/field/field.cpp | 6 +++--- .../barretenberg/stdlib/primitives/field/field.hpp | 4 ++-- .../stdlib/primitives/field/field.test.cpp | 4 ++-- .../barretenberg/stdlib/primitives/logic/logic.cpp | 4 ++-- .../barretenberg/stdlib/transcript/transcript.hpp | 14 +++++++------- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_ultra_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_ultra_recursive_verifier.test.cpp index 85f1bc3decc5..44529b5aba38 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_ultra_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_ultra_recursive_verifier.test.cpp @@ -136,7 +136,7 @@ template class BoomerangRecursiveVerifierTest : publi outer_circuit.finalize_circuit(false); auto graph = cdg::StaticAnalyzer(outer_circuit); auto connected_components = graph.find_connected_components(); - EXPECT_EQ(connected_components.size(), 2); + EXPECT_EQ(connected_components.size(), 5); info("Connected components: ", connected_components.size()); auto variables_in_one_gate = graph.show_variables_in_one_gate(outer_circuit); EXPECT_EQ(variables_in_one_gate.size(), 2); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp index f4a1ddd09418..62c48a8a1bd5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp @@ -139,7 +139,7 @@ class ECCVMRecursiveTests : public ::testing::Test { } // Check that the size of the recursive verifier is consistent with historical expectation - uint32_t NUM_GATES_EXPECTED = 213923; + uint32_t NUM_GATES_EXPECTED = 213307; ASSERT_EQ(static_cast(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED) << "Ultra-arithmetized ECCVM Recursive verifier gate count changed! Update this value if you are sure this " "is expected."; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp index bc839029be3f..ba21acd4c7a3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp @@ -291,7 +291,7 @@ template class RecursiveVerifierTest : public testing } // Check the size of the recursive verifier if constexpr (std::same_as>) { - uint32_t NUM_GATES_EXPECTED = 796978; + uint32_t NUM_GATES_EXPECTED = 796237; ASSERT_EQ(static_cast(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED) << "MegaZKHonk Recursive verifier changed in Ultra gate count! Update this value if you " "are sure this is expected."; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 9f9e2ba4dec9..55f44aa3042b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1262,7 +1262,7 @@ template field_t field_t::accumulate(const // WORKTODO: better name! template -std::pair, field_t> field_t::split_at_unrestricted(const size_t lsb_index) const +std::pair, field_t> field_t::split_at(const size_t lsb_index) const { // WORKTODO: is this right? depends on how bb::fr is implemented. ideally this would be some predefined constant.. const size_t max_bits = 254; @@ -1295,8 +1295,8 @@ std::pair, field_t> field_t::split_at_unrestr * - hi contains bits [lsb_index, num_bits) */ template -std::pair, field_t> field_t::split_at(const size_t lsb_index, - const size_t num_bits) const +std::pair, field_t> field_t::no_wrap_split_at(const size_t lsb_index, + const size_t num_bits) const { ASSERT(lsb_index < num_bits); ASSERT(num_bits <= grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp index b98890327679..7a6ed1e36576 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp @@ -388,9 +388,9 @@ template class field_t { Builder* get_context() const { return context; } - std::pair, field_t> split_at_unrestricted(const size_t lsb_index) const; + std::pair, field_t> split_at(const size_t lsb_index) const; - std::pair, field_t> split_at( + std::pair, field_t> no_wrap_split_at( const size_t lsb_index, const size_t num_bits = grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) const; bool_t is_zero() const; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp index e551b5d7f657..f9be2d2fe3b4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp @@ -930,7 +930,7 @@ template class stdlib_field : public testing::Test { // Lambda to check split_at functionality auto check_split_at = [&](const field_ct& a, size_t start, size_t num_bits) { const uint256_t a_native = a.get_value(); - auto split_data = a.split_at(start, num_bits); + auto split_data = a.no_wrap_split_at(start, num_bits); EXPECT_EQ(split_data.first.get_value(), a_native & ((uint256_t(1) << start) - 1)); EXPECT_EQ(split_data.second.get_value(), (a_native >> start) & ((uint256_t(1) << num_bits) - 1)); @@ -1424,7 +1424,7 @@ template class stdlib_field : public testing::Test { // Split preserves tags const size_t num_bits = uint256_t(a.get_value()).get_msb() + 1; - auto split_data = a.split_at(num_bits / 2, num_bits); + auto split_data = a.no_wrap_split_at(num_bits / 2, num_bits); EXPECT_EQ(split_data.first.get_origin_tag(), submitted_value_origin_tag); EXPECT_EQ(split_data.second.get_origin_tag(), submitted_value_origin_tag); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/logic/logic.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/logic/logic.cpp index 0b21c8c18119..28d4392c9913 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/logic/logic.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/logic/logic.cpp @@ -96,8 +96,8 @@ field_t logic::create_logic_constraint( right = right >> 32; } - field_pt a_slice_other = a.split_at(num_bits).first; - field_pt b_slice_other = b.split_at(num_bits).first; + field_pt a_slice_other = a.no_wrap_split_at(num_bits).first; + field_pt b_slice_other = b.no_wrap_split_at(num_bits).first; a_slice_other.assert_equal(a_accumulator, "stdlib logic: failed to reconstruct left operand"); b_slice_other.assert_equal(b_accumulator, "stdlib logic: failed to reconstruct right operand"); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index 3e875eb36c3d..2031b59b857a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -34,14 +34,14 @@ template struct StdlibTranscriptParams { */ static inline std::array split_challenge(const DataType& challenge) { - // WORKTODO: now that we're not using cycle_scalar, should we split differently? - const size_t low_bits = 128; - const size_t high_bits = 126; - const auto [lo, high] = challenge.split_at_unrestricted(low_bits); + // Note: Current choice of bit splitting is somewhat arbitrary and based on historic use of cycle_scalar. + const size_t lo_bits = 128; + const size_t hi_bits = 126; + const auto [lo, hi] = challenge.split_at(lo_bits); // WORKTODO: are these range constraints necessary? - lo.create_range_constraint(low_bits); - high.create_range_constraint(high_bits); - return std::array{ lo, high }; + lo.create_range_constraint(lo_bits); + hi.create_range_constraint(hi_bits); + return std::array{ lo, hi }; } template static inline T convert_challenge(const DataType& challenge) { From dd1401e8c1fa65058129bf8a533a45f60e709e77 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 15 Sep 2025 19:15:15 +0000 Subject: [PATCH 08/17] use split_at in create_from_bn254_scalar --- .../stdlib/primitives/field/field.cpp | 3 +- .../stdlib/primitives/group/cycle_scalar.cpp | 31 +++++-------------- 2 files changed, 8 insertions(+), 26 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 55f44aa3042b..9ad7d490be42 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1264,8 +1264,7 @@ template field_t field_t::accumulate(const template std::pair, field_t> field_t::split_at(const size_t lsb_index) const { - // WORKTODO: is this right? depends on how bb::fr is implemented. ideally this would be some predefined constant.. - const size_t max_bits = 254; + static constexpr size_t max_bits = native::modulus.get_msb() + 1; ASSERT(lsb_index < max_bits); const uint256_t value(this->get_value()); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index d46a5c66d2f3..7104d309e703 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -69,34 +69,17 @@ cycle_scalar cycle_scalar::from_witness_bitstring(Builder* con * @details This method ensures that the input is constrained to be less than the bn254 scalar field modulus. * * @tparam Builder - * @param context - * @param value - * @param num_bits - * @return cycle_scalar + * @param in a field_t representing a bn254 scalar field element + * @return cycle_scalar a cycle_scalar representing the same value in the grumpkin scalar field */ template cycle_scalar cycle_scalar::create_from_bn254_scalar(const field_t& in) { - const uint256_t value_u256(in.get_value()); - const uint256_t lo_v = value_u256.slice(0, LO_BITS); - const uint256_t hi_v = value_u256.slice(LO_BITS, HI_BITS); - const bool skip_primality_test = false; - const bool use_bn254_scalar_field_for_primality_test = true; - // WORKTODO: should be able to use field_t::split_at_unrestricted here - if (in.is_constant()) { - return cycle_scalar{ - field_t(lo_v), field_t(hi_v), NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test - }; - } - auto lo = field_t::from_witness(in.get_context(), lo_v); - auto hi = field_t::from_witness(in.get_context(), hi_v); - lo.add_two(hi * (uint256_t(1) << LO_BITS), -in).assert_equal(0); - - // We need to manually propagate the origin tag - lo.set_origin_tag(in.get_origin_tag()); - hi.set_origin_tag(in.get_origin_tag()); - - return cycle_scalar{ lo, hi, NUM_BITS, skip_primality_test, use_bn254_scalar_field_for_primality_test }; + auto [lo, hi] = in.split_at(LO_BITS); + return cycle_scalar{ + lo, hi, NUM_BITS, /*skip_primality_test=*/false, /*use_bn254_scalar_field_for_primality_test=*/true + }; } + /** * @brief Construct a new cycle scalar from a bigfield _value, over the same ScalarField Field. If _value is a witness, * we add constraints to ensure the conversion is correct by reconstructing a bigfield from the limbs of the From 3035addf65460808b6d1a602946d93a9f6c33ff6 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 15 Sep 2025 19:34:45 +0000 Subject: [PATCH 09/17] cleanup --- .../barretenberg/stdlib/primitives/field/field.cpp | 12 ++++++++++-- .../barretenberg/stdlib/transcript/transcript.hpp | 1 - 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 9ad7d490be42..9e671ccf5568 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1260,7 +1260,12 @@ template field_t field_t::accumulate(const return total.normalize(); } -// WORKTODO: better name! +/** + * @brief Splits the field element into (lo, hi), where: + * - lo contains bits [0, lsb_index) + * - hi contains bits [lsb_index, max_bits) + * + */ template std::pair, field_t> field_t::split_at(const size_t lsb_index) const { @@ -1276,7 +1281,7 @@ std::pair, field_t> field_t::split_at(const s return { field_t(lo_val), field_t(hi_val) }; } - // Otherwise, create hi/lo witnesses and constrain them to reconstruct `this` as field elements + // Otherwise, create hi/lo witnesses and constrain them to reconstruct `this` in the field field_t lo = from_witness(get_context(), lo_val); field_t hi = from_witness(get_context(), hi_val); const uint256_t shift = uint256_t(1) << lsb_index; @@ -1292,6 +1297,9 @@ std::pair, field_t> field_t::split_at(const s * @brief Splits the field element into (lo, hi), where: * - lo contains bits [0, lsb_index) * - hi contains bits [lsb_index, num_bits) + * @details Max bits is specified as an argument, and must be <= grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH (to ensure no + * modular wrap). + * */ template std::pair, field_t> field_t::no_wrap_split_at(const size_t lsb_index, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index 2031b59b857a..e8e2f893166f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -38,7 +38,6 @@ template struct StdlibTranscriptParams { const size_t lo_bits = 128; const size_t hi_bits = 126; const auto [lo, hi] = challenge.split_at(lo_bits); - // WORKTODO: are these range constraints necessary? lo.create_range_constraint(lo_bits); hi.create_range_constraint(hi_bits); return std::array{ lo, hi }; From d9eb67614128de3efe2c1cdf64c9a293dbe41732 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 17 Sep 2025 20:09:56 +0000 Subject: [PATCH 10/17] implement split unique in field utils --- ...test_civc_standalone_vks_havent_changed.sh | 2 +- .../eccvm_recursive_verifier.test.cpp | 2 +- .../ultra_recursive_verifier.test.cpp | 2 +- .../stdlib/primitives/field/field.cpp | 33 ------ .../stdlib/primitives/field/field.hpp | 2 - .../stdlib/primitives/field/field_utils.cpp | 112 ++++++++++++++++++ .../stdlib/primitives/field/field_utils.hpp | 52 ++++++++ .../stdlib/primitives/group/cycle_scalar.cpp | 62 +++------- .../stdlib/primitives/group/cycle_scalar.hpp | 9 +- .../stdlib/transcript/transcript.hpp | 14 +-- 10 files changed, 199 insertions(+), 91 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp create mode 100644 barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.hpp diff --git a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh index 4e279bc40aff..0203d2c83540 100755 --- a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh @@ -13,7 +13,7 @@ cd .. # - Generate a hash for versioning: sha256sum bb-civc-inputs.tar.gz # - Upload the compressed results: aws s3 cp bb-civc-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[hash(0:8)].tar.gz # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0 -pinned_short_hash="d8ed4dd8" +pinned_short_hash="1176062e" pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-${pinned_short_hash}.tar.gz" function compress_and_upload { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp index 62c48a8a1bd5..f4a1ddd09418 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp @@ -139,7 +139,7 @@ class ECCVMRecursiveTests : public ::testing::Test { } // Check that the size of the recursive verifier is consistent with historical expectation - uint32_t NUM_GATES_EXPECTED = 213307; + uint32_t NUM_GATES_EXPECTED = 213923; ASSERT_EQ(static_cast(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED) << "Ultra-arithmetized ECCVM Recursive verifier gate count changed! Update this value if you are sure this " "is expected."; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp index ba21acd4c7a3..bc839029be3f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp @@ -291,7 +291,7 @@ template class RecursiveVerifierTest : public testing } // Check the size of the recursive verifier if constexpr (std::same_as>) { - uint32_t NUM_GATES_EXPECTED = 796237; + uint32_t NUM_GATES_EXPECTED = 796978; ASSERT_EQ(static_cast(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED) << "MegaZKHonk Recursive verifier changed in Ultra gate count! Update this value if you " "are sure this is expected."; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 9e671ccf5568..e072c93d378f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1260,39 +1260,6 @@ template field_t field_t::accumulate(const return total.normalize(); } -/** - * @brief Splits the field element into (lo, hi), where: - * - lo contains bits [0, lsb_index) - * - hi contains bits [lsb_index, max_bits) - * - */ -template -std::pair, field_t> field_t::split_at(const size_t lsb_index) const -{ - static constexpr size_t max_bits = native::modulus.get_msb() + 1; - ASSERT(lsb_index < max_bits); - - const uint256_t value(this->get_value()); - const uint256_t lo_val = value.slice(0, lsb_index); - const uint256_t hi_val = value.slice(lsb_index, max_bits); - - // If `this` is constant, return constants based on the native hi/lo values - if (this->is_constant()) { - return { field_t(lo_val), field_t(hi_val) }; - } - - // Otherwise, create hi/lo witnesses and constrain them to reconstruct `this` in the field - field_t lo = from_witness(get_context(), lo_val); - field_t hi = from_witness(get_context(), hi_val); - const uint256_t shift = uint256_t(1) << lsb_index; - (lo + (hi * shift)).assert_equal(*this); - - // Set the origin tag for the limbs to be that of the original field element - lo.set_origin_tag(get_origin_tag()); - hi.set_origin_tag(get_origin_tag()); - return { lo, hi }; -} - /** * @brief Splits the field element into (lo, hi), where: * - lo contains bits [0, lsb_index) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp index 7a6ed1e36576..20d2df27745b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp @@ -388,8 +388,6 @@ template class field_t { Builder* get_context() const { return context; } - std::pair, field_t> split_at(const size_t lsb_index) const; - std::pair, field_t> no_wrap_split_at( const size_t lsb_index, const size_t num_bits = grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) const; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp new file mode 100644 index 000000000000..304298459b6b --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp @@ -0,0 +1,112 @@ +// === AUDIT STATUS === +// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// external_1: { status: not started, auditors: [], date: YYYY-MM-DD } +// external_2: { status: not started, auditors: [], date: YYYY-MM-DD } +// ===================== + +#include "./field_utils.hpp" +#include "./field.hpp" +#include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" + +namespace bb::stdlib { + +template +void validate_split_in_field(const field_t& lo, + const field_t& hi, + const size_t lo_bits, + const uint256_t& field_modulus) +{ + constexpr bool IS_ULTRA = Builder::CIRCUIT_TYPE == CircuitType::ULTRA; + + const size_t hi_bits = field_modulus.get_msb() + 1 - lo_bits; + + // Split the field modulus at the same position + const uint256_t r_lo = field_modulus.slice(0, lo_bits); + const uint256_t r_hi = field_modulus.slice(lo_bits, field_modulus.get_msb() + 1); + + // Check if we need to borrow + bool need_borrow = uint256_t(lo.get_value()) > r_lo; + field_t borrow = + lo.is_constant() ? need_borrow : field_t::from_witness(lo.get_context(), need_borrow); + + // directly call `create_new_range_constraint` to avoid creating an arithmetic gate + if (!lo.is_constant()) { + // We need to manually propagate the origin tag + borrow.set_origin_tag(lo.get_origin_tag()); + if constexpr (IS_ULTRA) { + lo.get_context()->create_new_range_constraint(borrow.get_witness_index(), 1, "borrow"); + } else { + borrow.assert_equal(borrow * borrow); + } + } + + // Hi range check = r_hi - hi - borrow + // Lo range check = r_lo - lo + borrow * 2^lo_bits + field_t hi_diff = (-hi + r_hi) - borrow; + field_t lo_diff = (-lo + r_lo) + (borrow * (uint256_t(1) << lo_bits)); + + hi_diff.create_range_constraint(hi_bits); + lo_diff.create_range_constraint(lo_bits); +} + +template +std::pair, field_t> split_unique(const field_t& field, + const size_t lo_bits, + const bool skip_range_constraints) +{ + using native = typename field_t::native; + static constexpr size_t max_bits = native::modulus.get_msb() + 1; + ASSERT(lo_bits < max_bits); + + const uint256_t value(field.get_value()); + const uint256_t lo_val = value.slice(0, lo_bits); + const uint256_t hi_val = value.slice(lo_bits, max_bits); + + // If `field` is constant, return constants based on the native hi/lo values + if (field.is_constant()) { + return { field_t(lo_val), field_t(hi_val) }; + } + + // Create hi/lo witnesses + field_t lo = field_t::from_witness(field.get_context(), lo_val); + field_t hi = field_t::from_witness(field.get_context(), hi_val); + + // Component 1: Reconstruction constraint + const uint256_t shift = uint256_t(1) << lo_bits; + (lo + (hi * shift)).assert_equal(field); + + // Set the origin tag for the limbs + lo.set_origin_tag(field.get_origin_tag()); + hi.set_origin_tag(field.get_origin_tag()); + + // Component 2: Field validation against bn254 scalar field modulus + validate_split_in_field(lo, hi, lo_bits, native::modulus); + + // Component 3: Range constraints (unless skipped) + if (!skip_range_constraints) { + lo.create_range_constraint(lo_bits); + // For bn254 scalar field, hi_bits = 254 - lo_bits + const size_t hi_bits = 254 - lo_bits; + hi.create_range_constraint(hi_bits); + } + + return { lo, hi }; +} + +// Explicit instantiations for split_unique +template std::pair, field_t> split_unique( + const field_t& field, const size_t lo_bits, const bool skip_range_constraints); +template std::pair, field_t> split_unique( + const field_t& field, const size_t lo_bits, const bool skip_range_constraints); + +// Explicit instantiations for validate_split_in_field +template void validate_split_in_field(const field_t& lo, + const field_t& hi, + const size_t lo_bits, + const uint256_t& field_modulus); +template void validate_split_in_field(const field_t& lo, + const field_t& hi, + const size_t lo_bits, + const uint256_t& field_modulus); + +} // namespace bb::stdlib diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.hpp new file mode 100644 index 000000000000..9b54b750545e --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.hpp @@ -0,0 +1,52 @@ +// === AUDIT STATUS === +// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// external_1: { status: not started, auditors: [], date: YYYY-MM-DD } +// external_2: { status: not started, auditors: [], date: YYYY-MM-DD } +// ===================== + +#pragma once + +#include "./field.hpp" +#include + +namespace bb::stdlib { + +template class field_t; + +/** + * @brief Split a bn254 scalar field element into unique lo and hi limbs + * + * @details Splits `field` into a low and high limb at the given bit index with: + * 1. Reconstruction constraint: lo + hi * 2^lo_bits = field + * 2. Modulus check: lo + hi * 2^lo_bits < bn254::ScalarField::modulus + * 3. Range constraints: lo in [0, 2^lo_bits), hi in [0, 2^(254-lo_bits)) (unless skip_range_constraints = true) + * + * @note The combination of (2) and (3) establishes the uniqueness of the decomposition. + * + * @param field The bn254 scalar field element to split + * @param lo_bits Number of bits for the low limb + * @param skip_range_constraints If true, skip range constraints (use when they're implicit, e.g., in lookups) + * @return std::pair, field_t> The (lo, hi) pair + */ +template +std::pair, field_t> split_unique(const field_t& field, + const size_t lo_bits, + const bool skip_range_constraints = false); + +/** + * @brief Validates that lo + hi * 2^lo_bits < field_modulus + * @details Can be used in conjunction with range constraints on lo and hi to establish a unique decomposition of a + * field element. + * + * @param lo The low limb + * @param hi The high limb + * @param lo_bits The bit position at which the split occurred + * @param field_modulus The field modulus to validate against + */ +template +void validate_split_in_field(const field_t& lo, + const field_t& hi, + const size_t lo_bits, + const uint256_t& field_modulus); + +} // namespace bb::stdlib diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp index 17ce13dfbeb8..43df8fc41bde 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.cpp @@ -8,6 +8,7 @@ #include "./cycle_group.hpp" #include "barretenberg/common/assert.hpp" #include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" +#include "barretenberg/stdlib/primitives/field/field_utils.hpp" namespace bb::stdlib { @@ -69,9 +70,14 @@ cycle_scalar cycle_scalar::from_u256_witness(Builder* context, */ template cycle_scalar cycle_scalar::create_from_bn254_scalar(const field_t& in) { - auto [lo, hi] = in.split_at(LO_BITS); + // Use split_unique with skip_range_constraints=true since the range constraints are implicit + // in the lookup arguments used in scalar multiplication and thus do not need to be applied here. + auto [lo, hi] = split_unique(in, LO_BITS, /*skip_range_constraints=*/true); + // AUDITTODO: we skip the primality test in the constructor here since its done in split_unique. Eventually + // the skip_primality_test logic will be removed entirely and constraints will always be applied immediately on + // construction. return cycle_scalar{ - lo, hi, NUM_BITS, /*skip_primality_test=*/false, /*use_bn254_scalar_field_for_primality_test=*/true + lo, hi, NUM_BITS, /*skip_primality_test=*/true, /*use_bn254_scalar_field_for_primality_test=*/true }; } @@ -145,7 +151,7 @@ template cycle_scalar::cycle_scalar(BigScalarField& const uint64_t hi_bits = hi_max.get_msb() + 1; lo.create_range_constraint(BigScalarField::NUM_LIMB_BITS); hi.create_range_constraint(static_cast(hi_bits)); - limb0.assert_equal(lo + hi * BigScalarField::shift_1); + limb0.assert_equal(lo + (hi * BigScalarField::shift_1)); limb1 += hi; limb1_max += hi_max; @@ -177,7 +183,7 @@ template cycle_scalar::cycle_scalar(BigScalarField& // We need to propagate the origin tag to the chunks of limb1 limb_1_lo.set_origin_tag(limb1.get_origin_tag()); limb_1_hi.set_origin_tag(limb1.get_origin_tag()); - limb1.assert_equal(limb_1_hi * limb_1_hi_multiplicand + limb_1_lo); + limb1.assert_equal((limb_1_hi * limb_1_hi_multiplicand) + limb_1_lo); // Step 4: apply range constraints to validate both slices represent the expected contributions to *this.lo and // *this,hi @@ -207,50 +213,20 @@ template bool cycle_scalar::is_constant() const } /** - * @brief Checks that a cycle_scalar value is smaller than a prime field modulus when evaluated over the INTEGERS - * N.B. The prime we check can be either the SNARK curve group order or the circuit's embedded curve group order - * (i.e. BN254 or Grumpkin) - * For a canonical scalar mul, we check against the embedded curve (i.e. the curve - * cycle_group implements). - * HOWEVER: for Pedersen hashes and Pedersen commitments, the hashed/committed data will be - * native circuit field elements i.e. for a BN254 snark, cycle_group = Grumpkin and we will be committing/hashing - * BN254::ScalarField values *NOT* Grumpkin::ScalarFIeld values. - * TLDR: whether the input scalar has to be < BN254::ScalarField or < Grumpkin::ScalarField is context-dependent. + * @brief Validates that the scalar (lo + hi * 2^LO_BITS) is less than the appropriate field modulus + * @details Checks against either bn254 scalar field or grumpkin scalar field based on internal flags. + * If _skip_primality_test is true, no validation is performed. + * @note: Implies (lo + hi * 2^LO_BITS) < field_modulus as integers when combined with appropriate range constraints on + * lo and hi. * * @tparam Builder */ template void cycle_scalar::validate_scalar_is_in_field() const { - using FF = typename field_t::native; - constexpr bool IS_ULTRA = Builder::CIRCUIT_TYPE == CircuitType::ULTRA; - - // AUDITTODO: Investigate using field_t::split_at here per Sergei's suggestion - if (!is_constant() && !skip_primality_test()) { - // Check that scalar.hi * 2^LO_BITS + scalar.lo < cycle_group_modulus when evaluated over the integers - const uint256_t cycle_group_modulus = - use_bn254_scalar_field_for_primality_test() ? FF::modulus : ScalarField::modulus; - const auto [r_lo, r_hi] = decompose_into_lo_hi(cycle_group_modulus); - - bool need_borrow = uint256_t(lo.get_value()) > r_lo; - field_t borrow = lo.is_constant() ? need_borrow : field_t::from_witness(get_context(), need_borrow); - - // directly call `create_new_range_constraint` to avoid creating an arithmetic gate - if (!lo.is_constant()) { - // We need to manually propagate the origin tag - borrow.set_origin_tag(lo.get_origin_tag()); - if constexpr (IS_ULTRA) { - get_context()->create_new_range_constraint(borrow.get_witness_index(), 1, "borrow"); - } else { - borrow.assert_equal(borrow * borrow); - } - } - // Hi range check = r_hi - y_hi - borrow - // Lo range check = r_lo - y_lo + borrow * 2^{126} - field_t hi_diff = (-hi + r_hi) - borrow; - field_t lo_diff = (-lo + r_lo) + (borrow * (uint256_t(1) << LO_BITS)); - - hi_diff.create_range_constraint(HI_BITS); - lo_diff.create_range_constraint(LO_BITS); + if (!_skip_primality_test) { + const uint256_t& field_modulus = + _use_bn254_scalar_field_for_primality_test ? field_t::native::modulus : ScalarField::modulus; + validate_split_in_field(lo, hi, LO_BITS, field_modulus); } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp index 9291625f6f62..45992d7dff6d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_scalar.hpp @@ -38,7 +38,7 @@ template class cycle_scalar { using ScalarField = typename Curve::ScalarField; using BigScalarField = stdlib::bigfield; - static constexpr size_t NUM_BITS = ScalarField::modulus.get_msb() + 1; + static constexpr size_t NUM_BITS = ScalarField::modulus.get_msb() + 1; // equivalent for both bn254 and grumpkin static constexpr size_t LO_BITS = field_t::native::Params::MAX_BITS_PER_ENDOMORPHISM_SCALAR; static constexpr size_t HI_BITS = NUM_BITS - LO_BITS; @@ -92,9 +92,14 @@ template class cycle_scalar { { return _use_bn254_scalar_field_for_primality_test; } - void validate_scalar_is_in_field() const; + /** + * @brief Validates that the scalar (lo + hi * 2^LO_BITS) is less than the appropriate field modulus + * @details Checks against either bn254 scalar field or grumpkin scalar field based on internal flags + */ + void validate_scalar_is_in_field() const; explicit cycle_scalar(BigScalarField&); + /** * @brief Get the origin tag of the cycle_scalar (a merge of the lo and hi tags) * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index e8e2f893166f..091114dee82d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -10,6 +10,7 @@ #include "barretenberg/crypto/poseidon2/poseidon2.hpp" #include "barretenberg/stdlib/hash/poseidon2/poseidon2.hpp" #include "barretenberg/stdlib/primitives/field/field_conversion.hpp" +#include "barretenberg/stdlib/primitives/field/field_utils.hpp" #include "barretenberg/stdlib/primitives/group/cycle_group.hpp" #include "barretenberg/transcript/transcript.hpp" namespace bb::stdlib::recursion::honk { @@ -26,20 +27,17 @@ template struct StdlibTranscriptParams { } /** * @brief Split a challenge field element into two half-width challenges - * @details `lo` is 128 bits and `hi` is 126 bits. - * This should provide significantly more than our security parameter bound: 100 bits + * @details `lo` is 128 bits and `hi` is 126 bits which should provide significantly more than our security + * parameter bound: 100 bits. The decomposition is constrained to be unique. * * @param challenge * @return std::array */ static inline std::array split_challenge(const DataType& challenge) { - // Note: Current choice of bit splitting is somewhat arbitrary and based on historic use of cycle_scalar. - const size_t lo_bits = 128; - const size_t hi_bits = 126; - const auto [lo, hi] = challenge.split_at(lo_bits); - lo.create_range_constraint(lo_bits); - hi.create_range_constraint(hi_bits); + const size_t lo_bits = DataType::native::Params::MAX_BITS_PER_ENDOMORPHISM_SCALAR; + // Constuct a unique lo/hi decomposition of the challenge (hi_bits will be 254 - 128 = 126) + const auto [lo, hi] = split_unique(challenge, lo_bits); return std::array{ lo, hi }; } template static inline T convert_challenge(const DataType& challenge) From 5da71f18638d8e8330609676987e87f19b227c29 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 17 Sep 2025 20:37:33 +0000 Subject: [PATCH 11/17] fix wasm build --- .../src/barretenberg/stdlib/primitives/field/field_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp index 304298459b6b..f61de06fef34 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp @@ -18,7 +18,7 @@ void validate_split_in_field(const field_t& lo, { constexpr bool IS_ULTRA = Builder::CIRCUIT_TYPE == CircuitType::ULTRA; - const size_t hi_bits = field_modulus.get_msb() + 1 - lo_bits; + const size_t hi_bits = static_cast(field_modulus.get_msb()) + 1 - lo_bits; // Split the field modulus at the same position const uint256_t r_lo = field_modulus.slice(0, lo_bits); From e690fd2cb060088d017bf8d79be2cfc2da7fc366 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 18 Sep 2025 15:23:00 +0000 Subject: [PATCH 12/17] revert test changes for now --- .../primitives/group/cycle_group.test.cpp | 74 +++++++------------ 1 file changed, 25 insertions(+), 49 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 43323297bb30..460a7d519de2 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 @@ -20,8 +20,7 @@ using Group = typename Curve::Group; \ using bool_ct = stdlib::bool_t; \ using witness_ct = stdlib::witness_t; \ - using cycle_scalar_ct = cycle_group_ct::cycle_scalar; \ - using BigScalarField = typename cycle_scalar_ct::BigScalarField; + using cycle_scalar_ct = cycle_group_ct::cycle_scalar; using namespace bb; @@ -772,26 +771,22 @@ TYPED_TEST(CycleGroupTest, TestBatchMulGeneralMSM) // 1: add entry where point, scalar are witnesses expected += (element * scalar); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar1(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar1)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); // 2: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(cycle_group_ct(element)); - BigScalarField big_scalar2(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar2)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); // 3: add entry where point is witness, scalar is constant expected += (element * scalar); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar3(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar3)); + scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); // 4: add entry where point is constant, scalar is constant expected += (element * scalar); points.emplace_back(cycle_group_ct(element)); - BigScalarField big_scalar4(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar4)); + scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); } // Here and in the following cases assign different tags to points and scalars and get the union of them back @@ -818,12 +813,10 @@ TYPED_TEST(CycleGroupTest, TestBatchMulProducesInfinity) auto element = TestFixture::generators[0]; typename Group::Fr scalar = Group::Fr::random_element(&engine); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar1(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar1)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar2(&builder, -scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar2)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, -scalar)); const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -848,8 +841,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulMultiplyByZero) auto element = TestFixture::generators[0]; typename Group::Fr scalar = 0; points.emplace_back(cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); const auto expected_tag = assign_and_merge_tags(points, scalars); auto result = cycle_group_ct::batch_mul(points, scalars); @@ -877,16 +869,14 @@ TYPED_TEST(CycleGroupTest, TestBatchMulInputsAreInfinity) cycle_group_ct point = cycle_group_ct::from_witness(&builder, element); point.set_point_at_infinity(witness_ct(&builder, true)); points.emplace_back(point); - BigScalarField big_scalar1(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar1)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); } // is_infinity = constant { cycle_group_ct point = cycle_group_ct::from_witness(&builder, element); point.set_point_at_infinity(true); points.emplace_back(point); - BigScalarField big_scalar2(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar2)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -917,16 +907,14 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseInLookupTable) // 1: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(element); - BigScalarField big_scalar(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); scalars_native.emplace_back(uint256_t(scalar)); // 2: add entry where point is constant, scalar is constant element = plookup::fixed_base::table::rhs_generator_point(); expected += (element * scalar); points.emplace_back(element); - BigScalarField big_scalar_const(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); + scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); scalars_native.emplace_back(uint256_t(scalar)); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -958,16 +946,14 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) // 1: add entry where point is constant, scalar is witness expected += (element * scalar); points.emplace_back(element); - BigScalarField big_scalar(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); scalars_native.emplace_back(scalar); // 2: add entry where point is constant, scalar is constant element = plookup::fixed_base::table::rhs_generator_point(); expected += (element * scalar); points.emplace_back(element); - BigScalarField big_scalar_const(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); + scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); scalars_native.emplace_back(scalar); // 3: add entry where point is constant, scalar is witness @@ -975,8 +961,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) element = Group::one * Group::Fr::random_element(&engine); expected += (element * scalar); points.emplace_back(element); - BigScalarField big_scalar2(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar2)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); scalars_native.emplace_back(scalar); } const auto expected_tag = assign_and_merge_tags(points, scalars); @@ -1004,13 +989,11 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseZeroScalars) // 1: add entry where point is constant, scalar is witness points.emplace_back((element)); - BigScalarField big_scalar(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar)); + scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar)); // 2: add entry where point is constant, scalar is constant points.emplace_back((element)); - BigScalarField big_scalar_const(&builder, scalar); - scalars.emplace_back(cycle_scalar_ct(big_scalar_const)); + scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar)); } const auto expected_tag = assign_and_merge_tags(points, scalars); auto result = cycle_group_ct::batch_mul(points, scalars); @@ -1031,6 +1014,7 @@ TYPED_TEST(CycleGroupTest, TestMul) // case 1, general MSM with inputs that are combinations of constant and witnesses { cycle_group_ct point; + typename cycle_group_ct::cycle_scalar scalar; cycle_group_ct result; for (size_t i = 0; i < num_muls; ++i) { auto element = TestFixture::generators[i]; @@ -1039,8 +1023,7 @@ TYPED_TEST(CycleGroupTest, TestMul) // 1: add entry where point, scalar are witnesses point = (cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar1(&builder, native_scalar); - cycle_scalar_ct scalar = (cycle_scalar_ct(big_scalar1)); + scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar)); point.set_origin_tag(submitted_value_origin_tag); scalar.set_origin_tag(challenge_origin_tag); result = point * scalar; @@ -1049,21 +1032,16 @@ TYPED_TEST(CycleGroupTest, TestMul) // 2: add entry where point is constant, scalar is witness point = (cycle_group_ct(element)); - BigScalarField big_scalar2(&builder, native_scalar); - scalar = (cycle_scalar_ct(big_scalar2)); + scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar)); EXPECT_EQ((result).get_value(), (expected_result)); // 3: add entry where point is witness, scalar is constant point = (cycle_group_ct::from_witness(&builder, element)); - BigScalarField big_scalar3(&builder, native_scalar); - scalar = (cycle_scalar_ct(big_scalar3)); EXPECT_EQ((result).get_value(), (expected_result)); // 4: add entry where point is constant, scalar is constant point = (cycle_group_ct(element)); - BigScalarField big_scalar4(&builder, native_scalar); - scalar = (cycle_scalar_ct(big_scalar4)); EXPECT_EQ((result).get_value(), (expected_result)); } } @@ -1180,9 +1158,9 @@ TYPED_TEST(CycleGroupTest, MixedLengthScalarsIsNotSupported) std::vector scalars; // First scalar: 254 bits (default cycle_scalar::NUM_BITS) - uint256_t scalar1_value = uint256_t(123456789); - BigScalarField big_scalar1(&builder, typename Curve::ScalarField(scalar1_value)); - scalars.push_back(cycle_scalar_ct(big_scalar1)); + auto scalar1_value = FF::random_element(&engine); + auto scalar1 = FF_ct::from_witness(&builder, scalar1_value); + scalars.emplace_back(scalar1); EXPECT_EQ(scalars[0].num_bits(), cycle_scalar_ct::NUM_BITS); // Second scalar: 256 bits @@ -1216,10 +1194,8 @@ TYPED_TEST(CycleGroupTest, TestFixedBaseBatchMul) auto scalar1_val = Group::Fr::random_element(&engine); auto scalar2_val = Group::Fr::random_element(&engine); - BigScalarField big_scalar1(&builder, scalar1_val); - BigScalarField big_scalar2(&builder, scalar2_val); - scalars.push_back(cycle_scalar_ct(big_scalar1)); - scalars.push_back(cycle_scalar_ct(big_scalar2)); + scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar1_val)); + scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar2_val)); points.push_back(cycle_group_ct(lhs_generator)); // constant point points.push_back(cycle_group_ct(rhs_generator)); // constant point From 83be28bdb442b41accbb67e0ba6ee131e322e1e2 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 18 Sep 2025 17:32:42 +0000 Subject: [PATCH 13/17] add num gates pinning to cycle group tests --- .../circuit_checker/ultra_circuit_checker.cpp | 5 +- .../primitives/group/cycle_group.test.cpp | 91 +++++++++---------- 2 files changed, 46 insertions(+), 50 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp index dca6917b51d9..752a37d04356 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp @@ -22,8 +22,9 @@ UltraCircuitBuilder_ UltraCircuitChecker::prepare_cir { // Create a copy of the input circuit UltraCircuitBuilder_ builder{ builder_in }; - - builder.finalize_circuit(/*ensure_nonzero=*/true); // Test the ensure_nonzero gates as well + if (!builder.circuit_finalized) { // avoid warnings about finalizing an already finalized circuit + builder.finalize_circuit(/*ensure_nonzero=*/true); // Test the ensure_nonzero gates as well + } return builder; } 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 460a7d519de2..de7a3055e4ae 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 @@ -53,6 +53,18 @@ template class CycleGroupTest : public ::testing::Test { using CircuitTypes = ::testing::Types; TYPED_TEST_SUITE(CycleGroupTest, CircuitTypes); +// Utility function for gate count checking and circuit verification +template void check_circuit_and_gates(Builder& builder, uint32_t expected_gates) +{ + if (!builder.circuit_finalized) { + builder.finalize_circuit(/*ensure_nonzero=*/false); + } + uint32_t actual_gates = static_cast(builder.get_num_finalized_gates()); + EXPECT_EQ(actual_gates, expected_gates) + << "Gate count changed! Expected: " << expected_gates << ", Actual: " << actual_gates; + EXPECT_TRUE(CircuitChecker::check(builder)); +} + STANDARD_TESTING_TAGS /** * @brief Check basic tag interactions @@ -97,7 +109,7 @@ TYPED_TEST(CycleGroupTest, TestInfConstantWintnessRegression) cycle_group_ct a = cycle_group_ct::from_constant_witness(&builder, lhs); (void)a; EXPECT_FALSE(builder.failed()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 1); } /** @@ -113,7 +125,7 @@ TYPED_TEST(CycleGroupTest, TestInfWintnessRegression) cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); (void)a; EXPECT_FALSE(builder.failed()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 7); } /** @@ -152,7 +164,7 @@ TYPED_TEST(CycleGroupTest, TestOperatorNegRegression) cycle_group_ct c = a.unconditional_add(b); (void)c; EXPECT_FALSE(builder.failed()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 16); } /** @@ -175,7 +187,7 @@ TYPED_TEST(CycleGroupTest, TestConstantWitnessMixupRegression) auto w27 = w10 - w11; // and here (void)w26; (void)w27; - EXPECT_NO_THROW(CircuitChecker::check(builder)); // It won't be a throw anyway + check_circuit_and_gates(builder, 42); } /** @@ -191,7 +203,7 @@ TYPED_TEST(CycleGroupTest, TestConditionalAssignRegression) 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 + check_circuit_and_gates(builder, 2); } /** @@ -211,7 +223,7 @@ TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) 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 + check_circuit_and_gates(builder, 6); } /** @@ -227,7 +239,7 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveSucceed) cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); a.validate_is_on_curve(); EXPECT_FALSE(builder.failed()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 12); } /** @@ -246,7 +258,7 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveInfinitySucceed) cycle_group_ct a(x, y, /*_is_infinity=*/true); // marks this point as the point at infinity a.validate_is_on_curve(); EXPECT_FALSE(builder.failed()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 1); } /** @@ -367,7 +379,7 @@ TYPED_TEST(CycleGroupTest, TestStandardForm) EXPECT_EQ(standard_f_x, 0); EXPECT_EQ(standard_f_y, 0); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 16); } TYPED_TEST(CycleGroupTest, TestDbl) { @@ -393,8 +405,7 @@ TYPED_TEST(CycleGroupTest, TestDbl) EXPECT_EQ(result, expected); EXPECT_EQ(d.get_value(), expected); - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 16); // Ensure the tags stay the same after doubling EXPECT_EQ(c.get_origin_tag(), submitted_value_origin_tag); @@ -426,8 +437,7 @@ TYPED_TEST(CycleGroupTest, TestUnconditionalAdd) add(TestFixture::generators[0], TestFixture::generators[1], true, false); add(TestFixture::generators[0], TestFixture::generators[1], true, true); - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 35); } TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalAddSucceed) @@ -446,8 +456,7 @@ TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalAddSucceed) AffineElement result = c.get_value(); EXPECT_EQ(result, expected); - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 17); } TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalAddFail) @@ -464,9 +473,8 @@ TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalAddFail) a.checked_unconditional_add(b); EXPECT_TRUE(builder.failed()); - - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, false); + // No gate count check for failing test + EXPECT_FALSE(CircuitChecker::check(builder)); } TYPED_TEST(CycleGroupTest, TestAdd) @@ -561,8 +569,7 @@ TYPED_TEST(CycleGroupTest, TestAdd) EXPECT_EQ(c.get_origin_tag(), first_two_merged_tag); } - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 268); } TYPED_TEST(CycleGroupTest, TestUnconditionalSubtract) @@ -591,8 +598,7 @@ TYPED_TEST(CycleGroupTest, TestUnconditionalSubtract) subtract(TestFixture::generators[0], TestFixture::generators[1], true, false); subtract(TestFixture::generators[0], TestFixture::generators[1], true, true); - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 35); } TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalSubtractSucceed) @@ -611,8 +617,7 @@ TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalSubtractSucceed) AffineElement result = c.get_value(); EXPECT_EQ(result, expected); - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 17); } TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalSubtractFail) @@ -629,9 +634,8 @@ TYPED_TEST(CycleGroupTest, TestConstrainedUnconditionalSubtractFail) a.checked_unconditional_subtract(b); EXPECT_TRUE(builder.failed()); - - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, false); + // No gate count check for failing test + EXPECT_FALSE(CircuitChecker::check(builder)); } TYPED_TEST(CycleGroupTest, TestSubtract) @@ -729,8 +733,7 @@ TYPED_TEST(CycleGroupTest, TestSubtract) EXPECT_EQ(c.get_origin_tag(), first_two_merged_tag); } - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 274); } /** @@ -797,8 +800,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulGeneralMSM) // The tag should the union of all tags EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 4397); } TYPED_TEST(CycleGroupTest, TestBatchMulProducesInfinity) @@ -825,8 +827,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulProducesInfinity) EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 4023); } TYPED_TEST(CycleGroupTest, TestBatchMulMultiplyByZero) @@ -848,8 +849,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulMultiplyByZero) EXPECT_TRUE(result.is_point_at_infinity().get_value()); EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 3533); } TYPED_TEST(CycleGroupTest, TestBatchMulInputsAreInfinity) @@ -884,8 +884,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulInputsAreInfinity) EXPECT_TRUE(result.is_point_at_infinity().get_value()); EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 3557); } TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseInLookupTable) @@ -923,8 +922,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseInLookupTable) EXPECT_EQ(result.get_value(), crypto::pedersen_commitment::commit_native(scalars_native)); EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 2823); } TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) @@ -969,8 +967,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable) EXPECT_EQ(result.get_value(), AffineElement(expected)); EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 3399); } TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseZeroScalars) @@ -1000,8 +997,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseZeroScalars) EXPECT_EQ(result.is_point_at_infinity().get_value(), true); EXPECT_EQ(result.get_origin_tag(), expected_tag); - bool check_result = CircuitChecker::check(builder); - EXPECT_EQ(check_result, true); + check_circuit_and_gates(builder, 2838); } TYPED_TEST(CycleGroupTest, TestMul) @@ -1046,8 +1042,7 @@ TYPED_TEST(CycleGroupTest, TestMul) } } - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); + check_circuit_and_gates(builder, 6598); } TYPED_TEST(CycleGroupTest, TestOne) @@ -1088,7 +1083,7 @@ TYPED_TEST(CycleGroupTest, TestConversionFromBigfield) if (construct_witnesses) { EXPECT_FALSE(big_elt.is_constant()); EXPECT_FALSE(scalar_from_big_elt.is_constant()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 3499); } }; run_test(/*construct_witnesses=*/true); @@ -1130,7 +1125,7 @@ TYPED_TEST(CycleGroupTest, TestBatchMulIsConsistent) // TODO(https://github.com/AztecProtocol/barretenberg/issues/1020): Re-enable these. // EXPECT_FALSE(result1.is_constant()); // EXPECT_FALSE(result2.is_constant()); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 5289); } }; run_test(/*construct_witnesses=*/true); @@ -1206,6 +1201,6 @@ TYPED_TEST(CycleGroupTest, TestFixedBaseBatchMul) EXPECT_EQ(result.get_value(), expected); - EXPECT_TRUE(CircuitChecker::check(builder)); + check_circuit_and_gates(builder, 2909); } #pragma GCC diagnostic pop From 1de5c17967b3410d337851304b1662a482594da2 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 18 Sep 2025 17:58:30 +0000 Subject: [PATCH 14/17] use evaluate linear identity; no gate change --- .../stdlib/primitives/field/field_utils.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp index f61de06fef34..6bc99a3ecbc1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_utils.cpp @@ -16,8 +16,6 @@ void validate_split_in_field(const field_t& lo, const size_t lo_bits, const uint256_t& field_modulus) { - constexpr bool IS_ULTRA = Builder::CIRCUIT_TYPE == CircuitType::ULTRA; - const size_t hi_bits = static_cast(field_modulus.get_msb()) + 1 - lo_bits; // Split the field modulus at the same position @@ -33,11 +31,7 @@ void validate_split_in_field(const field_t& lo, if (!lo.is_constant()) { // We need to manually propagate the origin tag borrow.set_origin_tag(lo.get_origin_tag()); - if constexpr (IS_ULTRA) { - lo.get_context()->create_new_range_constraint(borrow.get_witness_index(), 1, "borrow"); - } else { - borrow.assert_equal(borrow * borrow); - } + lo.get_context()->create_new_range_constraint(borrow.get_witness_index(), 1, "borrow"); } // Hi range check = r_hi - hi - borrow @@ -62,18 +56,21 @@ std::pair, field_t> split_unique(const field_t(lo_val), field_t(hi_val) }; } // Create hi/lo witnesses - field_t lo = field_t::from_witness(field.get_context(), lo_val); - field_t hi = field_t::from_witness(field.get_context(), hi_val); + auto lo = field_t::from_witness(ctx, lo_val); + auto hi = field_t::from_witness(ctx, hi_val); - // Component 1: Reconstruction constraint + // Component 1: Reconstruction constraint lo + hi * 2^lo_bits - field == 0 const uint256_t shift = uint256_t(1) << lo_bits; - (lo + (hi * shift)).assert_equal(field); + auto zero = field_t::from_witness_index(ctx, ctx->zero_idx); + field_t::evaluate_linear_identity(lo, hi * shift, -field, zero); // Set the origin tag for the limbs lo.set_origin_tag(field.get_origin_tag()); From 12c79de1d84e2d4ed634bbdb45a8dfa2425c2dd0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 04:19:55 +0000 Subject: [PATCH 15/17] vk hash --- .../cpp/scripts/test_civc_standalone_vks_havent_changed.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh index 4cb206606d67..92e4a0490197 100755 --- a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh @@ -13,7 +13,7 @@ cd .. # - Generate a hash for versioning: sha256sum bb-civc-inputs.tar.gz # - Upload the compressed results: aws s3 cp bb-civc-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[hash(0:8)].tar.gz # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0 -pinned_short_hash="5aa55c9f" +pinned_short_hash="38836b38" pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-${pinned_short_hash}.tar.gz" function compress_and_upload { From 739ad7287cf2ecea64891c503a801df4fc9c3f11 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 16:32:15 +0000 Subject: [PATCH 16/17] vk hash --- .../cpp/scripts/test_civc_standalone_vks_havent_changed.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh index 92e4a0490197..3c3b1c3d52b7 100755 --- a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh @@ -13,7 +13,7 @@ cd .. # - Generate a hash for versioning: sha256sum bb-civc-inputs.tar.gz # - Upload the compressed results: aws s3 cp bb-civc-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[hash(0:8)].tar.gz # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0 -pinned_short_hash="38836b38" +pinned_short_hash="38da88c1" pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-${pinned_short_hash}.tar.gz" function compress_and_upload { From 99c1cd1b2c4a2bce9d6e835738807c6877b178fe Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 17:47:16 +0000 Subject: [PATCH 17/17] try again --- .../cpp/scripts/test_civc_standalone_vks_havent_changed.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh index 3c3b1c3d52b7..485f010abdfb 100755 --- a/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh @@ -13,7 +13,7 @@ cd .. # - Generate a hash for versioning: sha256sum bb-civc-inputs.tar.gz # - Upload the compressed results: aws s3 cp bb-civc-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[hash(0:8)].tar.gz # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0 -pinned_short_hash="38da88c1" +pinned_short_hash="04f38201" pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-${pinned_short_hash}.tar.gz" function compress_and_upload {