From 9206325479ff241d6f2ad074d5261886e1a3ee7f Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Mon, 8 Sep 2025 18:10:16 +0000 Subject: [PATCH 01/22] initial clean-up --- .../civc_recursion_constraints.cpp | 2 +- .../acir_format/honk_recursion_constraint.cpp | 2 +- .../flavor/mega_recursive_flavor.hpp | 12 ++-- .../flavor/ultra_recursive_flavor.hpp | 12 ++-- .../flavor/ultra_rollup_recursive_flavor.hpp | 12 ++-- .../eccvm_recursive_verifier.test.cpp | 2 + .../stdlib/primitives/biggroup/biggroup.hpp | 1 + .../primitives/biggroup/biggroup_goblin.hpp | 5 ++ .../primitives/biggroup/biggroup_impl.hpp | 7 +++ .../primitives/field/field_conversion.cpp | 2 +- .../primitives/field/field_conversion.hpp | 58 +++++++++---------- .../field/field_conversion.test.cpp | 36 ++++++------ .../stdlib/transcript/transcript.hpp | 4 +- .../recursion/recursive_flavor.hpp | 2 +- 14 files changed, 85 insertions(+), 72 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/civc_recursion_constraints.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/civc_recursion_constraints.cpp index 85254379cdfb..76bebb9ee773 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/civc_recursion_constraints.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/civc_recursion_constraints.cpp @@ -111,7 +111,7 @@ create_civc_recursion_constraints(Builder& builder, } // Recursively verify CIVC proof - auto mega_vk = std::make_shared(builder, key_fields); + auto mega_vk = std::make_shared(key_fields); auto mega_vk_and_hash = std::make_shared(mega_vk, vk_hash); ClientIVCRecursiveVerifier::StdlibProof stdlib_proof(proof_fields); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp index 597493ca30c1..1a8b22d9d4ee 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp @@ -141,7 +141,7 @@ HonkRecursionConstraintOutput create_honk_recur } // Recursively verify the proof - auto vkey = std::make_shared(builder, key_fields); + auto vkey = std::make_shared(key_fields); auto vk_and_hash = std::make_shared(vkey, vk_hash); RecursiveVerifier verifier(&builder, vk_and_hash); UltraRecursiveVerifierOutput verifier_output = verifier.template verify_proof(proof_fields); diff --git a/barretenberg/cpp/src/barretenberg/flavor/mega_recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/flavor/mega_recursive_flavor.hpp index 6825642ea30e..d91b7bb5c3af 100644 --- a/barretenberg/cpp/src/barretenberg/flavor/mega_recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/flavor/mega_recursive_flavor.hpp @@ -140,18 +140,18 @@ template class MegaRecursiveFlavor_ { * @param builder * @param elements */ - VerificationKey(CircuitBuilder& builder, std::span elements) + VerificationKey(std::span elements) { using namespace bb::stdlib::field_conversion; size_t num_frs_read = 0; - this->log_circuit_size = deserialize_from_frs(builder, elements, num_frs_read); - this->num_public_inputs = deserialize_from_frs(builder, elements, num_frs_read); - this->pub_inputs_offset = deserialize_from_frs(builder, elements, num_frs_read); + this->log_circuit_size = deserialize_from_frs(elements, num_frs_read); + this->num_public_inputs = deserialize_from_frs(elements, num_frs_read); + this->pub_inputs_offset = deserialize_from_frs(elements, num_frs_read); for (Commitment& commitment : this->get_all()) { - commitment = deserialize_from_frs(builder, elements, num_frs_read); + commitment = deserialize_from_frs(elements, num_frs_read); } if (num_frs_read != elements.size()) { @@ -174,7 +174,7 @@ template class MegaRecursiveFlavor_ { for (const auto& idx : witness_indices) { vk_fields.emplace_back(FF::from_witness_index(&builder, idx)); } - return VerificationKey(builder, vk_fields); + return VerificationKey(vk_fields); } /** diff --git a/barretenberg/cpp/src/barretenberg/flavor/ultra_recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/flavor/ultra_recursive_flavor.hpp index 41db37d00d1d..7bce1ab1429f 100644 --- a/barretenberg/cpp/src/barretenberg/flavor/ultra_recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/flavor/ultra_recursive_flavor.hpp @@ -140,18 +140,18 @@ template class UltraRecursiveFlavor_ { * @param builder * @param elements */ - VerificationKey(CircuitBuilder& builder, std::span elements) + VerificationKey(std::span elements) { using namespace bb::stdlib::field_conversion; size_t num_frs_read = 0; - this->log_circuit_size = deserialize_from_frs(builder, elements, num_frs_read); - this->num_public_inputs = deserialize_from_frs(builder, elements, num_frs_read); - this->pub_inputs_offset = deserialize_from_frs(builder, elements, num_frs_read); + this->log_circuit_size = deserialize_from_frs(elements, num_frs_read); + this->num_public_inputs = deserialize_from_frs(elements, num_frs_read); + this->pub_inputs_offset = deserialize_from_frs(elements, num_frs_read); for (Commitment& commitment : this->get_all()) { - commitment = deserialize_from_frs(builder, elements, num_frs_read); + commitment = deserialize_from_frs(elements, num_frs_read); } } @@ -170,7 +170,7 @@ template class UltraRecursiveFlavor_ { for (const auto& idx : witness_indices) { vk_fields.emplace_back(FF::from_witness_index(&builder, idx)); } - return VerificationKey(builder, vk_fields); + return VerificationKey(vk_fields); } }; diff --git a/barretenberg/cpp/src/barretenberg/flavor/ultra_rollup_recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/flavor/ultra_rollup_recursive_flavor.hpp index ae438fb4871f..1d9181f9b356 100644 --- a/barretenberg/cpp/src/barretenberg/flavor/ultra_rollup_recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/flavor/ultra_rollup_recursive_flavor.hpp @@ -86,18 +86,18 @@ template class UltraRollupRecursiveFlavor_ : public Ultra * @param builder * @param elements */ - VerificationKey(CircuitBuilder& builder, std::span elements) + VerificationKey(std::span elements) { using namespace bb::stdlib::field_conversion; size_t num_frs_read = 0; - this->log_circuit_size = deserialize_from_frs(builder, elements, num_frs_read); - this->num_public_inputs = deserialize_from_frs(builder, elements, num_frs_read); - this->pub_inputs_offset = deserialize_from_frs(builder, elements, num_frs_read); + this->log_circuit_size = deserialize_from_frs(elements, num_frs_read); + this->num_public_inputs = deserialize_from_frs(elements, num_frs_read); + this->pub_inputs_offset = deserialize_from_frs(elements, num_frs_read); for (Commitment& commitment : this->get_all()) { - commitment = deserialize_from_frs(builder, elements, num_frs_read); + commitment = deserialize_from_frs(elements, num_frs_read); } } @@ -116,7 +116,7 @@ template class UltraRollupRecursiveFlavor_ : public Ultra for (const auto& idx : witness_indices) { vk_fields.emplace_back(FF::from_witness_index(&builder, idx)); } - return VerificationKey(builder, vk_fields); + return VerificationKey(vk_fields); } }; 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 ad59e61ea5c9..ba7d1ec16b67 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 @@ -95,6 +95,8 @@ class ECCVMRecursiveTests : public ::testing::Test { auto [opening_claim, ipa_transcript] = verifier.verify_proof(proof); stdlib::recursion::PairingPoints::add_default_to_public_inputs(outer_circuit); + const size_t fixed_size = 218783; + info("Fixed size ", fixed_size); info("Recursive Verifier: num gates = ", outer_circuit.get_estimated_num_finalized_gates()); // Check for a failure flag in the recursive verifier circuit diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index 678b2dc56723..5b68c049fe9b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp @@ -45,6 +45,7 @@ template class element { element(); element(const typename NativeGroup::affine_element& input); element(const Fq& x, const Fq& y); + element(const Fq& x, const Fq& y, const bool_ct& is_infinity); element(const element& other); element(element&& other) noexcept; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index c4c717e9b4f8..be730399ff55 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -56,6 +56,11 @@ template class goblin_el , y(y) , _is_infinity(false) {} + goblin_element(const Fq& x, const Fq& y, const bool_ct is_infinity) + : x(x) + , y(y) + , _is_infinity(is_infinity) + {} goblin_element(const goblin_element& other) = default; goblin_element(goblin_element&& other) noexcept = default; goblin_element& operator=(const goblin_element& other) = default; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp index da38938ea8a7..4c35b7ffeb8b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp @@ -36,6 +36,13 @@ element::element(const Fq& x_in, const Fq& y_in) , _is_infinity(x.get_context() ? x.get_context() : y.get_context(), false) {} +template +element::element(const Fq& x_in, const Fq& y_in, const bool_ct& is_infinity) + : x(x_in) + , y(y_in) + , _is_infinity(is_infinity) +{} + template element::element(const element& other) : x(other.x) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index aff68d9169e0..1c32893b5e79 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -44,7 +44,7 @@ template fq convert_to_grumpkin_fr(Builder& builder, builder.assert_equal(f.witness_index, sum.witness_index, "assert_equal"); std::vector> fr_vec{ low, hi }; - return convert_from_bn254_frs>(builder, fr_vec); + return convert_from_bn254_frs>(fr_vec); } template fq convert_to_grumpkin_fr(UltraCircuitBuilder& builder, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index c4778d410c38..8a714696abc4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -91,46 +91,45 @@ template constexpr size_t calc_num_bn254_frs() * @todo https://github.com/AztecProtocol/barretenberg/issues/1065 optimize validate_on_curve and check points * reconstructed from the transcript */ -template T convert_from_bn254_frs(Builder& builder, std::span> fr_vec) +template T convert_from_bn254_frs(std::span> fr_vec) { - if constexpr (IsAnyOf>) { + using field_ct = fr; + using bigfield_ct = fq; + // Can be bigfield or goblin_field + using basefield_ct = bn254_element::BaseField; + + ASSERT(validate_context(fr_vec)); + + if constexpr (IsAnyOf) { + // Case 1: input type matches the output type BB_ASSERT_EQ(fr_vec.size(), 1U); return fr_vec[0]; - } else if constexpr (IsAnyOf>) { - BB_ASSERT_EQ(fr_vec.size(), 2U); - fq result(fr_vec[0], fr_vec[1]); - return result; - } else if constexpr (IsAnyOf>) { + } else if constexpr (IsAnyOf>) { + // Cases 2 and 3: a field_ct element needs to be represented in bigfield/goblin_field BB_ASSERT_EQ(fr_vec.size(), 2U); - goblin_field result(fr_vec[0], fr_vec[1]); + T result(fr_vec[0], fr_vec[1]); return result; } else if constexpr (IsAnyOf>) { - using BaseField = bn254_element::BaseField; - constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); + // Case 4: Convert a vector of field_ct to a BN254 point + + constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); BB_ASSERT_EQ(fr_vec.size(), 2 * BASE_FIELD_SCALAR_SIZE); - bn254_element result; - result.x = convert_from_bn254_frs(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); - result.y = convert_from_bn254_frs( - builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + basefield_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); + basefield_ct y = convert_from_bn254_frs( + fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); // We have a convention that the group element is at infinity if both x/y coordinates are 0. // We also know that all bn254 field elements are 136-bit scalars. // Therefore we can do a cheap "iszero" check by checking the vector sum is 0 - fr sum; - for (size_t i = 0; i < BASE_FIELD_SCALAR_SIZE; i += 1) { - sum = sum.add_two(fr_vec[2 * i], fr_vec[2 * i + 1]); - } - result.set_point_at_infinity(sum.is_zero()); - return result; + fr sum = field_ct::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())); + return bn254_element(x, y, sum.is_zero()); } else if constexpr (IsAnyOf>) { - using BaseField = fr; - constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); + constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); BB_ASSERT_EQ(fr_vec.size(), 2 * BASE_FIELD_SCALAR_SIZE); - fr x = - convert_from_bn254_frs>(builder, fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); - fr y = convert_from_bn254_frs>( - builder, fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + field_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); + field_ct y = + convert_from_bn254_frs(fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); grumpkin_element result(x, y, x.is_zero() && y.is_zero()); return result; } else { @@ -141,7 +140,7 @@ template T convert_from_bn254_frs(Builder& builde size_t i = 0; for (auto& x : val) { x = convert_from_bn254_frs( - builder, fr_vec.subspan(FieldScalarSize * i, FieldScalarSize)); + fr_vec.subspan(FieldScalarSize * i, FieldScalarSize)); ++i; } return val; @@ -164,6 +163,7 @@ template std::vector> convert_to_bn25 std::vector> fr_vec{ val }; return fr_vec; } else if constexpr (IsAnyOf>) { + // Bigfield return convert_grumpkin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf>) { return convert_goblin_fr_to_bn254_frs(val); @@ -202,11 +202,11 @@ template std::vector> convert_to_bn25 * @param num_frs_read Index at which to read into buffer */ template -TargetType deserialize_from_frs(Builder& builder, std::span> elements, size_t& num_frs_read) +TargetType deserialize_from_frs(std::span> elements, size_t& num_frs_read) { size_t num_frs = calc_num_bn254_frs(); BB_ASSERT_GTE(elements.size(), num_frs_read + num_frs); - TargetType result = convert_from_bn254_frs(builder, elements.subspan(num_frs_read, num_frs)); + TargetType result = convert_from_bn254_frs(elements.subspan(num_frs_read, num_frs)); num_frs_read += num_frs; return result; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index ca448a8ece37..a2b0bd0f6eca 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -11,21 +11,21 @@ template using grumpkin_element = cycle_group; template class StdlibFieldConversionTests : public ::testing::Test { public: - template void check_conversion(Builder& builder, T x) + template void check_conversion(T x) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); EXPECT_EQ(len, frs.size()); - auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(builder, frs); + auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); EXPECT_EQ(x.get_value(), y.get_value()); } - template void check_conversion_iterable(Builder& builder, T x) + template void check_conversion_iterable(T x) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); EXPECT_EQ(len, frs.size()); - auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(builder, frs); + auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); EXPECT_EQ(x.size(), y.size()); for (auto [val1, val2] : zip_view(x, y)) { EXPECT_EQ(val1.get_value(), val2.get_value()); @@ -46,15 +46,15 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionFr) Builder builder; bb::fr x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits fr x1(&builder, x1_val); - this->check_conversion(builder, x1); + this->check_conversion(x1); bb::fr x2_val(bb::fr::modulus_minus_two); // modulus - 2 fr x2(&builder, x2_val); - this->check_conversion(builder, x2); + this->check_conversion(x2); bb::fr x3_val(1); fr x3(&builder, x3_val); - this->check_conversion(builder, x3); + this->check_conversion(x3); } /** @@ -68,7 +68,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinFr) // Constructing bigfield objects with grumpkin::fr values grumpkin::fr x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits fq x1(&builder, x1_val); - this->check_conversion(builder, x1); + this->check_conversion(x1); } /** @@ -83,11 +83,11 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) // Constructing element objects with curve::BN254::AffineElement values curve::BN254::AffineElement x1_val(1, 2); bn254_element x1 = bn254_element::from_witness(&builder, x1_val); - this->check_conversion(builder, x1); + this->check_conversion(x1); curve::BN254::AffineElement x2_val(1, grumpkin::fr::modulus_minus_two); bn254_element x2 = bn254_element::from_witness(&builder, x2_val); - this->check_conversion(builder, x2); + this->check_conversion(x2); } /** @@ -102,11 +102,11 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) // Constructing element objects with curve::Grumpkin::AffineElement values curve::Grumpkin::AffineElement x1_val(12, 100); grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); - this->check_conversion(builder, x1); + this->check_conversion(x1); curve::Grumpkin::AffineElement x2_val(1, grumpkin::fr::modulus_minus_two); grumpkin_element x2 = grumpkin_element::from_witness(&builder, x2_val); - this->check_conversion(builder, x2); + this->check_conversion(x2); } /** @@ -121,7 +121,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayBn254Fr) std::array, 4> x1{ fr(&builder, 1), fr(&builder, 2), fr(&builder, 3), fr(&builder, 4) }; - this->check_conversion_iterable(builder, x1); + this->check_conversion_iterable(x1); std::array, 7> x2{ fr(&builder, bb::fr::modulus_minus_two), fr(&builder, bb::fr::modulus_minus_two - 123), @@ -130,7 +130,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayBn254Fr) fr(&builder, 367032), fr(&builder, 12985028), fr(&builder, bb::fr::modulus_minus_two - 125015028) }; - this->check_conversion_iterable(builder, x2); + this->check_conversion_iterable(x2); } /** @@ -156,7 +156,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayGrumpkinFr) &builder, static_cast(std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), }; - this->check_conversion_iterable(builder, x1); + this->check_conversion_iterable(x1); } /** @@ -171,7 +171,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateBn254Fr) Univariate, 4> x{ { fr(&builder, 1), fr(&builder, 2), fr(&builder, 3), fr(&builder, 4) } }; - this->check_conversion_iterable(builder, x); + this->check_conversion_iterable(x); } /** @@ -197,7 +197,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateGrumpkinFr) static_cast( std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))) } }; - this->check_conversion_iterable(builder, x); + this->check_conversion_iterable(x); } /** @@ -215,4 +215,4 @@ TYPED_TEST(StdlibFieldConversionTests, ConvertChallengeGrumpkinFr) auto expected = uint256_t(chal.get_value()); EXPECT_EQ(uint256_t(result.get_value()), expected); } -} // namespace bb::stdlib::field_conversion_tests \ No newline at end of file +} // namespace bb::stdlib::field_conversion_tests diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index dc5562072a18..12432918b67d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -58,9 +58,7 @@ template struct StdlibTranscriptParams { template static inline T deserialize(std::span frs) { ASSERT(!frs.empty()); - ASSERT(frs[0].get_context() != nullptr); - Builder* builder = frs[0].get_context(); - return bb::stdlib::field_conversion::convert_from_bn254_frs(*builder, frs); + return bb::stdlib::field_conversion::convert_from_bn254_frs(frs); } template static inline std::vector serialize(const T& element) diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp index 895bfe715db4..dc1c9d5c47c0 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp @@ -84,7 +84,7 @@ class AvmRecursiveFlavor { for (Commitment& comm : this->get_all()) { comm = stdlib::field_conversion::convert_from_bn254_frs( - builder, elements.subspan(num_frs_read, num_frs_Comm)); + elements.subspan(num_frs_read, num_frs_Comm)); num_frs_read += num_frs_Comm; } } From 7d1760af24c34ebc6bb1afd61c4c05e464525281 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 09:56:50 +0000 Subject: [PATCH 02/22] move asserts --- .../stdlib/primitives/field/field_conversion.hpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 8a714696abc4..49bf6912adcb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -98,22 +98,22 @@ template T convert_from_bn254_frs(std::span::BaseField; + constexpr size_t expected_size = calc_num_bn254_frs(); + BB_ASSERT_EQ(fr_vec.size(), expected_size); + ASSERT(validate_context(fr_vec)); if constexpr (IsAnyOf) { // Case 1: input type matches the output type - BB_ASSERT_EQ(fr_vec.size(), 1U); return fr_vec[0]; } else if constexpr (IsAnyOf>) { // Cases 2 and 3: a field_ct element needs to be represented in bigfield/goblin_field - BB_ASSERT_EQ(fr_vec.size(), 2U); T result(fr_vec[0], fr_vec[1]); return result; } else if constexpr (IsAnyOf>) { // Case 4: Convert a vector of field_ct to a BN254 point constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); - BB_ASSERT_EQ(fr_vec.size(), 2 * BASE_FIELD_SCALAR_SIZE); basefield_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); basefield_ct y = convert_from_bn254_frs( @@ -126,7 +126,6 @@ template T convert_from_bn254_frs(std::span(x, y, sum.is_zero()); } else if constexpr (IsAnyOf>) { constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); - BB_ASSERT_EQ(fr_vec.size(), 2 * BASE_FIELD_SCALAR_SIZE); field_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); field_ct y = convert_from_bn254_frs(fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); From abe1b42d5e2aa417b141e7228525e9d6704c80ef Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 11:34:57 +0000 Subject: [PATCH 03/22] simplify further --- .../primitives/field/field_conversion.hpp | 85 ++++++++++--------- .../stdlib/primitives/group/cycle_group.hpp | 1 + 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 49bf6912adcb..1f61cb59e4a9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -24,6 +24,29 @@ template using grumpkin_element = cycle_group; static constexpr uint64_t NUM_LIMB_BITS = NUM_LIMB_BITS_IN_FIELD_SIMULATION; static constexpr uint64_t TOTAL_BITS = 254; +/** + * @brief Under the assumption that (x, y) is a point on the curve (bn254 or Grumpkin), check whether it + * corresponds to (0, 0), which is the point at infinity in our conventions. + * + * BN254: In the case of a bn254 point, the bigfield limbs (x_lo, x_hi, y_lo, y_hi) are range constrained, and their sum + * is a non-negative integer not exceeding 2^138, i.e. it does not overflow the fq modulus, hence all limbs must be 0. + * + * Grumpkin: We are using the fact that (x^2 + y^2 = 0) has no non-trivial solutions on Grumpkin, as Grumpkin modulus is + * == 3 mod 4. + * + * @return + */ +template bool_t check_point_at_infinity(std::span> fr_vec) +{ + if constexpr (IsAnyOf>) { + return (field_t::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); + } else { + field_t x_sqr = fr_vec[0].sqr(); + field_t y = fr_vec[1]; + return (y.madd(y, x_sqr).is_zero()); + } +} + template fq convert_to_grumpkin_fr(Builder& builder, const fr& f); template inline T convert_challenge(Builder& builder, const fr& challenge) @@ -46,7 +69,7 @@ inline std::vector> convert_goblin_fr_to_bn254_frs(const goblin_fiel template inline std::vector> convert_grumpkin_fr_to_bn254_frs(const fq& input) { - fr shift(static_cast(1) << NUM_LIMB_BITS); + static constexpr bb::fr shift(static_cast(1) << NUM_LIMB_BITS); std::vector> result(2); result[0] = input.binary_basis_limbs[0].element + (input.binary_basis_limbs[1].element * shift); result[1] = input.binary_basis_limbs[2].element + (input.binary_basis_limbs[3].element * shift); @@ -95,8 +118,6 @@ template T convert_from_bn254_frs(std::span; using bigfield_ct = fq; - // Can be bigfield or goblin_field - using basefield_ct = bn254_element::BaseField; constexpr size_t expected_size = calc_num_bn254_frs(); BB_ASSERT_EQ(fr_vec.size(), expected_size); @@ -108,39 +129,25 @@ template T convert_from_bn254_frs(std::span>) { // Cases 2 and 3: a field_ct element needs to be represented in bigfield/goblin_field - T result(fr_vec[0], fr_vec[1]); - return result; - } else if constexpr (IsAnyOf>) { - // Case 4: Convert a vector of field_ct to a BN254 point + return T(fr_vec[0], fr_vec[1]); + } else if constexpr (IsAnyOf, grumpkin_element>) { + // Case 4 and 5: Convert a vector of frs to a group element + using basefield_ct = typename T::BaseField; - constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); + constexpr size_t base_field_frs = expected_size / 2; - basefield_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); - basefield_ct y = convert_from_bn254_frs( - fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); + basefield_ct x = convert_from_bn254_frs(fr_vec.subspan(0, base_field_frs)); + basefield_ct y = convert_from_bn254_frs(fr_vec.subspan(base_field_frs, base_field_frs)); - // We have a convention that the group element is at infinity if both x/y coordinates are 0. - // We also know that all bn254 field elements are 136-bit scalars. - // Therefore we can do a cheap "iszero" check by checking the vector sum is 0 - fr sum = field_ct::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())); - return bn254_element(x, y, sum.is_zero()); - } else if constexpr (IsAnyOf>) { - constexpr size_t BASE_FIELD_SCALAR_SIZE = calc_num_bn254_frs(); - field_ct x = convert_from_bn254_frs(fr_vec.subspan(0, BASE_FIELD_SCALAR_SIZE)); - field_ct y = - convert_from_bn254_frs(fr_vec.subspan(BASE_FIELD_SCALAR_SIZE, BASE_FIELD_SCALAR_SIZE)); - grumpkin_element result(x, y, x.is_zero() && y.is_zero()); - return result; + return T(x, y, check_point_at_infinity(fr_vec)); } else { // Array or Univariate + using field_type = typename T::value_type; T val; - constexpr size_t FieldScalarSize = calc_num_bn254_frs(); - BB_ASSERT_EQ(fr_vec.size(), FieldScalarSize * std::tuple_size::value); - size_t i = 0; - for (auto& x : val) { - x = convert_from_bn254_frs( - fr_vec.subspan(FieldScalarSize * i, FieldScalarSize)); - ++i; + const size_t target_size = val.size(); + constexpr size_t scalar_frs = expected_size / target_size; + for (size_t i = 0; i < target_size; i++) { + val[i] = convert_from_bn254_frs(fr_vec.subspan(scalar_frs * i, scalar_frs)); } return val; } @@ -166,18 +173,12 @@ template std::vector> convert_to_bn25 return convert_grumpkin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf>) { return convert_goblin_fr_to_bn254_frs(val); - } else if constexpr (IsAnyOf>) { + } else if constexpr (IsAnyOf, grumpkin_element>) { // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Consider handling point at infinity. - using BaseField = bn254_element::BaseField; - auto fr_vec_x = convert_to_bn254_frs(val.x); - auto fr_vec_y = convert_to_bn254_frs(val.y); - std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); - fr_vec.insert(fr_vec.end(), fr_vec_y.begin(), fr_vec_y.end()); - return fr_vec; - } else if constexpr (IsAnyOf>) { - using BaseField = fr; - auto fr_vec_x = convert_to_bn254_frs(val.x); - auto fr_vec_y = convert_to_bn254_frs(val.y); + using BaseField = T::BaseField; + + BaseField fr_vec_x = convert_to_bn254_frs(val.x); + BaseField fr_vec_y = convert_to_bn254_frs(val.y); std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); fr_vec.insert(fr_vec.end(), fr_vec_y.begin(), fr_vec_y.end()); return fr_vec; @@ -203,7 +204,7 @@ template std::vector> convert_to_bn25 template TargetType deserialize_from_frs(std::span> elements, size_t& num_frs_read) { - size_t num_frs = calc_num_bn254_frs(); + constexpr size_t num_frs = calc_num_bn254_frs(); BB_ASSERT_GTE(elements.size(), num_frs_read + num_frs); TargetType result = convert_from_bn254_frs(elements.subspan(num_frs_read, num_frs)); num_frs_read += num_frs; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index 53d360c52920..8ebcaf4e1137 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -36,6 +36,7 @@ concept IsUltraArithmetic = (Builder::CIRCUIT_TYPE == CircuitType::ULTRA); template class cycle_group { public: using field_t = stdlib::field_t; + using BaseField = field_t; using bool_t = stdlib::bool_t; using witness_t = stdlib::witness_t; using FF = typename Builder::FF; From 0c8f5e6690cf440b3ef88fc18af561e5f855b49f Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 13:16:13 +0000 Subject: [PATCH 04/22] convert to grumpkin more efficiently --- .../primitives/field/field_conversion.cpp | 29 ++++++++++++------- .../primitives/field/field_conversion.hpp | 22 +++++++------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index 1c32893b5e79..80f95a6ecdaa 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -21,28 +21,35 @@ namespace bb::stdlib::field_conversion { * TODO(https://github.com/AztecProtocol/barretenberg/issues/850): audit this function more carefully * @tparam Builder */ -template fq convert_to_grumpkin_fr(Builder& builder, const fr& f) +template fq convert_to_grumpkin_fr(Builder& builder, const fr& fr_element) { - constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 - constexpr uint64_t UPPER_TWO_LIMB_BITS = TOTAL_BITS - NUM_BITS_IN_TWO_LIMBS; // 118 + static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; + static constexpr uint64_t NUM_LAST_LIMB_BITS = fq::NUM_LAST_LIMB_BITS; // 118 + + constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 + constexpr uint256_t shift = (uint256_t(1) << NUM_BITS_IN_TWO_LIMBS); // split f into low_bits_in and high_bits_in constexpr uint256_t LIMB_MASK = shift - 1; // mask for upper 128 bits - const uint256_t value = f.get_value(); + const uint256_t value = fr_element.get_value(); const uint256_t low_val = static_cast(value & LIMB_MASK); const uint256_t hi_val = static_cast(value >> NUM_BITS_IN_TWO_LIMBS); fr low{ witness_t(&builder, low_val) }; fr hi{ witness_t(&builder, hi_val) }; - // range constrain low to 136 bits and hi to 118 bits - builder.create_range_constraint(low.witness_index, NUM_BITS_IN_TWO_LIMBS, "create_range_constraint"); - builder.create_range_constraint(hi.witness_index, UPPER_TWO_LIMB_BITS, "create_range_constraint"); - BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), value); + // range constrain low to 136 bits and hi to 118 bits + builder.create_range_constraint( + low.get_normalized_witness_index(), NUM_BITS_IN_TWO_LIMBS, "field_conversion: create_range_constraint"); + builder.create_range_constraint( + hi.get_normalized_witness_index(), NUM_LAST_LIMB_BITS, "field_conversion: create_range_constraint"); + + BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), + value, + "field_conversion: limb decomposition"); // checks this decomposition low + hi * 2^64 = value with an assert_equal - auto sum = low + hi * shift; - builder.assert_equal(f.witness_index, sum.witness_index, "assert_equal"); - + const fr zero = fr::from_witness_index(&builder, 0); + fr::evaluate_linear_identity(hi * shift, low, -fr_element, zero); std::vector> fr_vec{ low, hi }; return convert_from_bn254_frs>(fr_vec); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 1f61cb59e4a9..da14f027281c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -20,10 +20,6 @@ template using fr = field_t; template using fq = bigfield; template using bn254_element = element, fr, curve::BN254::Group>; template using grumpkin_element = cycle_group; - -static constexpr uint64_t NUM_LIMB_BITS = NUM_LIMB_BITS_IN_FIELD_SIMULATION; -static constexpr uint64_t TOTAL_BITS = 254; - /** * @brief Under the assumption that (x, y) is a point on the curve (bn254 or Grumpkin), check whether it * corresponds to (0, 0), which is the point at infinity in our conventions. @@ -69,6 +65,8 @@ inline std::vector> convert_goblin_fr_to_bn254_frs(const goblin_fiel template inline std::vector> convert_grumpkin_fr_to_bn254_frs(const fq& input) { + static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; + static constexpr bb::fr shift(static_cast(1) << NUM_LIMB_BITS); std::vector> result(2); result[0] = input.binary_basis_limbs[0].element + (input.binary_basis_limbs[1].element * shift); @@ -142,12 +140,14 @@ template T convert_from_bn254_frs(std::span(fr_vec)); } else { // Array or Univariate - using field_type = typename T::value_type; T val; - const size_t target_size = val.size(); - constexpr size_t scalar_frs = expected_size / target_size; - for (size_t i = 0; i < target_size; i++) { - val[i] = convert_from_bn254_frs(fr_vec.subspan(scalar_frs * i, scalar_frs)); + using element_type = typename T::value_type; + const size_t scalar_frs = calc_num_bn254_frs(); + + size_t i = 0; + for (auto& x : val) { + x = convert_from_bn254_frs(fr_vec.subspan(scalar_frs * i, scalar_frs)); + ++i; } return val; } @@ -177,8 +177,8 @@ template std::vector> convert_to_bn25 // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Consider handling point at infinity. using BaseField = T::BaseField; - BaseField fr_vec_x = convert_to_bn254_frs(val.x); - BaseField fr_vec_y = convert_to_bn254_frs(val.y); + std::vector> fr_vec_x = convert_to_bn254_frs(val.x); + std::vector> fr_vec_y = convert_to_bn254_frs(val.y); std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); fr_vec.insert(fr_vec.end(), fr_vec_y.begin(), fr_vec_y.end()); return fr_vec; From d2e6bc88fc958951f5247fd9b664821006c0ec3d Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 14:26:57 +0000 Subject: [PATCH 05/22] enable `on_curve` validation for points received in the transcript --- .../stdlib/primitives/field/field_conversion.hpp | 4 +++- .../stdlib/primitives/group/cycle_group.cpp | 4 ++-- .../stdlib/primitives/group/cycle_group.hpp | 2 +- .../stdlib/primitives/group/cycle_group.test.cpp | 16 ++++++++-------- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index da14f027281c..a10d77601062 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -137,7 +137,9 @@ template T convert_from_bn254_frs(std::span(fr_vec.subspan(0, base_field_frs)); basefield_ct y = convert_from_bn254_frs(fr_vec.subspan(base_field_frs, base_field_frs)); - return T(x, y, check_point_at_infinity(fr_vec)); + T out(x, y, check_point_at_infinity(fr_vec)); + out.validate_on_curve(); + return out; } else { // Array or Univariate T val; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 39fd0a830b65..7107fb3a1736 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -160,7 +160,7 @@ cycle_group cycle_group::from_witness(Builder* _context, const result._is_infinity = bool_t(witness_t(_context, _in.is_point_at_infinity())); result._is_constant = false; result._is_standard = true; - result.validate_is_on_curve(); + result.validate_on_curve(); result.set_free_witness_tag(); return result; } @@ -225,7 +225,7 @@ template typename cycle_group::AffineElement cycle_g * * @tparam Builder */ -template void cycle_group::validate_is_on_curve() const +template void cycle_group::validate_on_curve() const { // This class is for short Weierstrass curves only! static_assert(Group::curve_a == 0); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp index 8ebcaf4e1137..6168332654f1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.hpp @@ -90,7 +90,7 @@ template class cycle_group { void standardize(); bool is_standard() const { return this->_is_standard; }; cycle_group get_standard_form(); - void validate_is_on_curve() const; + void validate_on_curve() const; cycle_group dbl(const std::optional hint = std::nullopt) const requires IsUltraArithmetic; cycle_group unconditional_add(const cycle_group& other, 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 9999e14162f8..a8b7abe40654 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 @@ -215,7 +215,7 @@ TYPED_TEST(CycleGroupTest, TestConditionalAssignSuperMixupRegression) } /** - * @brief Checks that a point on the curve passes the validate_is_on_curve check + * @brief Checks that a point on the curve passes the validate_on_curve check * */ TYPED_TEST(CycleGroupTest, TestValidateOnCurveSucceed) @@ -225,14 +225,14 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveSucceed) auto lhs = TestFixture::generators[0]; cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); - a.validate_is_on_curve(); + a.validate_on_curve(); EXPECT_FALSE(builder.failed()); EXPECT_TRUE(CircuitChecker::check(builder)); } /** * @brief Checks that a point that is not on the curve but marked as the point at infinity passes the - * validate_is_on_curve check + * validate_on_curve check * @details Should pass since marking it with _is_infinity=true makes whatever other point data invalid. */ TYPED_TEST(CycleGroupTest, TestValidateOnCurveInfinitySucceed) @@ -244,14 +244,14 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveInfinitySucceed) auto y = stdlib::field_t::from_witness(&builder, 1); cycle_group_ct a(x, y, /*_is_infinity=*/true); // marks this point as the point at infinity - a.validate_is_on_curve(); + a.validate_on_curve(); EXPECT_FALSE(builder.failed()); EXPECT_TRUE(CircuitChecker::check(builder)); } /** * @brief Checks that a point that is not on the curve but *not* marked as the point at infinity fails the - * validate_is_on_curve check + * validate_on_curve check * @details (1, 1) is not on the either the Grumpkin curve or the BN254 curve. */ TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail) @@ -263,14 +263,14 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail) auto y = stdlib::field_t::from_witness(&builder, 1); cycle_group_ct a(x, y, /*_is_infinity=*/false); - a.validate_is_on_curve(); + a.validate_on_curve(); EXPECT_TRUE(builder.failed()); EXPECT_FALSE(CircuitChecker::check(builder)); } /** * @brief Checks that a point that is not on the curve but *not* marked as the point at infinity fails the - * validate_is_on_curve check + * validate_on_curve check * @details (1, 1) is not on the either the Grumpkin curve or the BN254 curve. */ TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail2) @@ -282,7 +282,7 @@ TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail2) auto y = stdlib::field_t::from_witness(&builder, 1); cycle_group_ct a(x, y, /*_is_infinity=*/bool_ct(witness_ct(&builder, false))); - a.validate_is_on_curve(); + a.validate_on_curve(); EXPECT_TRUE(builder.failed()); EXPECT_FALSE(CircuitChecker::check(builder)); } From acdaa5130349924cf74e6751da8c23984aeae05e Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 15:34:54 +0000 Subject: [PATCH 06/22] docs + question --- .../primitives/field/field_conversion.hpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index a10d77601062..393d44d9b8a1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -21,10 +21,9 @@ template using fq = bigfield; template using bn254_element = element, fr, curve::BN254::Group>; template using grumpkin_element = cycle_group; /** - * @brief Under the assumption that (x, y) is a point on the curve (bn254 or Grumpkin), check whether it - * corresponds to (0, 0), which is the point at infinity in our conventions. + * @brief Check whether a point corresponds to (0, 0), the conventional representation of the point infinity. * - * BN254: In the case of a bn254 point, the bigfield limbs (x_lo, x_hi, y_lo, y_hi) are range constrained, and their sum + * bn254: In the case of a bn254 point, the bigfield limbs (x_lo, x_hi, y_lo, y_hi) are range constrained, and their sum * is a non-negative integer not exceeding 2^138, i.e. it does not overflow the fq modulus, hence all limbs must be 0. * * Grumpkin: We are using the fact that (x^2 + y^2 = 0) has no non-trivial solutions on Grumpkin, as Grumpkin modulus is @@ -35,8 +34,10 @@ template using grumpkin_element = cycle_group; template bool_t check_point_at_infinity(std::span> fr_vec) { if constexpr (IsAnyOf>) { + // Sum the limbs and check whether the sum is 0 return (field_t::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); } else { + // Efficiently compute ((x^2 + y^2) == 0) field_t x_sqr = fr_vec[0].sqr(); field_t y = fr_vec[1]; return (y.madd(y, x_sqr).is_zero()); @@ -57,10 +58,8 @@ template inline T convert_challenge(Builder& buil template inline std::vector> convert_goblin_fr_to_bn254_frs(const goblin_field& input) { - std::vector> result(2); - result[0] = input.limbs[0]; - result[1] = input.limbs[1]; - return result; + + return { input.limbs[0], input.limbs[1] }; } template inline std::vector> convert_grumpkin_fr_to_bn254_frs(const fq& input) @@ -86,13 +85,11 @@ template constexpr size_t calc_num_bn254_frs() { if constexpr (IsAnyOf>) { return Bn254FrParams::NUM_BN254_SCALARS; - } else if constexpr (IsAnyOf> || IsAnyOf>) { + } else if constexpr (IsAnyOf, goblin_field>) { return Bn254FqParams::NUM_BN254_SCALARS; - } else if constexpr (IsAnyOf>) { + } else if constexpr (IsAnyOf, grumpkin_element>) { using BaseField = bn254_element::BaseField; return 2 * calc_num_bn254_frs(); - } else if constexpr (IsAnyOf>) { - return 2 * calc_num_bn254_frs>(); } else { // Array or Univariate return calc_num_bn254_frs() * (std::tuple_size::value); @@ -126,7 +123,8 @@ template T convert_from_bn254_frs(std::span>) { - // Cases 2 and 3: a field_ct element needs to be represented in bigfield/goblin_field + // Q: need to range constrain? Or are they range constrained before? + // Cases 2 and 3: a bigfield/goblin_field element is reconstructed from low and high limbs. return T(fr_vec[0], fr_vec[1]); } else if constexpr (IsAnyOf, grumpkin_element>) { // Case 4 and 5: Convert a vector of frs to a group element @@ -138,6 +136,8 @@ template T convert_from_bn254_frs(std::span(fr_vec.subspan(base_field_frs, base_field_frs)); T out(x, y, check_point_at_infinity(fr_vec)); + // Note that in the case of bn254 with Mega arithmetization, the check is delegated to ECCVM, see + // `on_curve_check` in `ECCVMTranscriptRelationImpl`. out.validate_on_curve(); return out; } else { From a63559d840c0ced0d6df1d5aa27cb003c381b721 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 9 Sep 2025 16:52:36 +0000 Subject: [PATCH 07/22] add missing range constraints --- .../ultra_recursive_verifier.test.cpp | 2 +- .../primitives/field/field_conversion.cpp | 13 ++++------ .../primitives/field/field_conversion.hpp | 24 +++++++++++++++---- 3 files changed, 25 insertions(+), 14 deletions(-) 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..396e14c2c748 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 = 803143; 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_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index 80f95a6ecdaa..c7511091680a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -17,14 +17,13 @@ namespace bb::stdlib::field_conversion { * pieces, one that is the 136 lower bits and one that is the 118 higher bits. Then, we can split these two pieces into * their bigfield limbs through convert_from_bn254_frs, which is actually just a bigfield constructor that takes in two * two-limb frs. - * + * Ensure that it's only used in ECCVMRecursive * TODO(https://github.com/AztecProtocol/barretenberg/issues/850): audit this function more carefully * @tparam Builder */ template fq convert_to_grumpkin_fr(Builder& builder, const fr& fr_element) { static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; - static constexpr uint64_t NUM_LAST_LIMB_BITS = fq::NUM_LAST_LIMB_BITS; // 118 constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 @@ -38,11 +37,7 @@ template fq convert_to_grumpkin_fr(Builder& builder, fr low{ witness_t(&builder, low_val) }; fr hi{ witness_t(&builder, hi_val) }; - // range constrain low to 136 bits and hi to 118 bits - builder.create_range_constraint( - low.get_normalized_witness_index(), NUM_BITS_IN_TWO_LIMBS, "field_conversion: create_range_constraint"); - builder.create_range_constraint( - hi.get_normalized_witness_index(), NUM_LAST_LIMB_BITS, "field_conversion: create_range_constraint"); + constrain_bigfield_limbs(low, hi); BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), value, @@ -50,8 +45,8 @@ template fq convert_to_grumpkin_fr(Builder& builder, // checks this decomposition low + hi * 2^64 = value with an assert_equal const fr zero = fr::from_witness_index(&builder, 0); fr::evaluate_linear_identity(hi * shift, low, -fr_element, zero); - std::vector> fr_vec{ low, hi }; - return convert_from_bn254_frs>(fr_vec); + + return fq(low, hi); } template fq convert_to_grumpkin_fr(UltraCircuitBuilder& builder, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 393d44d9b8a1..314578fa4ffe 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -20,6 +20,17 @@ template using fr = field_t; template using fq = bigfield; template using bn254_element = element, fr, curve::BN254::Group>; template using grumpkin_element = cycle_group; + +template static void constrain_bigfield_limbs(const fr& lo, const fr& hi) +{ + static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; + static constexpr uint64_t NUM_LAST_LIMB_BITS = NUM_LIMB_BITS + fq::NUM_LAST_LIMB_BITS; // 118 + static constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 + + // range constrain low to 136 bits and hi to 118 bits + lo.create_range_constraint(NUM_BITS_IN_TWO_LIMBS, "field_conversion: create_range_constraint"); + hi.create_range_constraint(NUM_LAST_LIMB_BITS, "field_conversion: create_range_constraint"); +} /** * @brief Check whether a point corresponds to (0, 0), the conventional representation of the point infinity. * @@ -35,11 +46,11 @@ template bool_t check_point_at_infinity( { if constexpr (IsAnyOf>) { // Sum the limbs and check whether the sum is 0 - return (field_t::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); + return (fr::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); } else { // Efficiently compute ((x^2 + y^2) == 0) - field_t x_sqr = fr_vec[0].sqr(); - field_t y = fr_vec[1]; + const fr x_sqr = fr_vec[0].sqr(); + const fr y = fr_vec[1]; return (y.madd(y, x_sqr).is_zero()); } } @@ -123,8 +134,13 @@ template T convert_from_bn254_frs(std::span>) { - // Q: need to range constrain? Or are they range constrained before? + // Q: need to range constrain when Mega? Must be handled in Translator. // Cases 2 and 3: a bigfield/goblin_field element is reconstructed from low and high limbs. + + if constexpr (std::is_same_v) { + constrain_bigfield_limbs(fr_vec[0], fr_vec[1]); + } + return T(fr_vec[0], fr_vec[1]); } else if constexpr (IsAnyOf, grumpkin_element>) { // Case 4 and 5: Convert a vector of frs to a group element From af8117bbe537aaa8e828cc1fb514c4c81ec60eac Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Wed, 10 Sep 2025 11:40:54 +0000 Subject: [PATCH 08/22] expand convert_from_bn254_frs docs --- .../primitives/field/field_conversion.hpp | 48 +++++++++++++------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 314578fa4ffe..4135ef9f7605 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -108,17 +108,39 @@ template constexpr size_t calc_num_bn254_frs() } /** - * @brief Conversions from vector of fr elements to transcript types. - * @details We want to support the following types: fr, fq, - * bn254_element, grumpkin_element, std::array, for - * FF = fr or fq, and N is arbitrary - * @tparam Builder - * @tparam T - * @param builder - * @param fr_vec - * @return T - * @todo https://github.com/AztecProtocol/barretenberg/issues/1065 optimize validate_on_curve and check points - * reconstructed from the transcript + * @brief Core stdlib Transcript deserialization method. + * @details Deserializes a vector of in-circuit `fr`s, i.e. `field_t` elements, into + * - `field_t` — no conversion needed + * + * - \ref bb::stdlib::bigfield< Builder, T > "bigfield" — 2 input `field_t`s must be range-constrained as low and high + * limbs of a `bigfield` element, then the corresponding `bigfield` constructor is applied. Specific to + * \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - \ref bb::stdlib::goblin_field< Builder > "goblin field element" — in contrast to `bigfield`, range constraints are + * performed in `Translator` (see \ref TranslatorDeltaRangeConstraintRelationImpl "Translator Range Constraint" + * relation). Feed the limbs to the `bigfield` constructor and set the `point_at_infinity` flag derived by the + * `check_point_at_infinity` method. Specific to \ref MegaCircuitBuilder_ "MegaCircuitBuilder". + * + * - \ref bb::stdlib::element_goblin::goblin_element< Builder_, Fq, Fr, NativeGroup > "bn254 goblin point" — input + * vector of size 4 is transformed into a pair of `goblin_field` elements, which are fed into the relevant constructor + * with the `point_at_infinity` flag derived by the `check_point_at_infinity` method. Note that `validate_on_curve` is a + * vacuous method in this case, as these checks are performed in ECCVM + * (see \ref bb::ECCVMTranscriptRelationImpl< FF_ > "ECCVM Transcript" relation). + * Specific to \ref MegaCircuitBuilder_ "MegaCircuitBuilder". + * + * - \ref bb::stdlib::element_default::element< Builder_, Fq, Fr, NativeGroup > "bn254 point" — reconstruct a pair of + * `bigfield` elements (x, y), applying required range constraints, and check that the resulting point lies on the + * curve. Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - \ref cycle_group "Grumpkin point" — since the grumpkin base field is `fr`, the reconstruction is trivial. We check + * in-circuit whether the resulting point lies on the curve and whether it is a point at infinity. + * Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - `Univariate` or a `std::array` of elements of the above types. + * + * @tparam Builder `UltraCircuitBuilder` or `MegaCircuitBuilder` + * @tparam T Target object type + * @param fr_vec Input `field_t` elements */ template T convert_from_bn254_frs(std::span> fr_vec) { @@ -134,9 +156,7 @@ template T convert_from_bn254_frs(std::span>) { - // Q: need to range constrain when Mega? Must be handled in Translator. // Cases 2 and 3: a bigfield/goblin_field element is reconstructed from low and high limbs. - if constexpr (std::is_same_v) { constrain_bigfield_limbs(fr_vec[0], fr_vec[1]); } @@ -157,7 +177,7 @@ template T convert_from_bn254_frs(std::span(); From 5bfb6f675513dac72ff1458f7a4f64a22c39a427 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Wed, 10 Sep 2025 15:55:00 +0000 Subject: [PATCH 09/22] `convert_to_grumpkin_fr` docs --- .../primitives/field/field_conversion.cpp | 18 +++++++----------- .../primitives/field/field_conversion.hpp | 1 + 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index c7511091680a..ad429bd993c0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -5,20 +5,16 @@ // ===================== #include "barretenberg/stdlib/primitives/field/field_conversion.hpp" -#include "barretenberg/common/assert.hpp" - namespace bb::stdlib::field_conversion { /** - * @brief Converts a challenge to a fq - * @details We sometimes need challenges that are a bb::fq element, so we need to convert the bb::fr challenge to a - * bb::fq type. We do this by in a similar fashion to the convert_from_bn254_frs function that converts to a - * fq. In fact, we do call that function that the end, but we first have to split the fr into two - * pieces, one that is the 136 lower bits and one that is the 118 higher bits. Then, we can split these two pieces into - * their bigfield limbs through convert_from_bn254_frs, which is actually just a bigfield constructor that takes in two - * two-limb frs. - * Ensure that it's only used in ECCVMRecursive - * TODO(https://github.com/AztecProtocol/barretenberg/issues/850): audit this function more carefully + * @brief Converts an in-circuit `fr`element to an `fq`, i.e. `field_t` --> `bigfield`. + * + * @details Our circuit builders are `fr`-native, which results in challenges being `field_t` elements. However, + * ECCVMRecursiveVerifier and IPA Recursive Verification need challenges that are `bigfield` elements. We do this in + * a similar fashion to the `convert_from_bn254_frs` function that converts to a `bigfield`. We split the `field_t` + * into two pieces, one that is the 136 lower bits and one that is the 118 higher bits, assert the correctness of the + * decomposition, and invoke the `bigfield` constructor. * @tparam Builder */ template fq convert_to_grumpkin_fr(Builder& builder, const fr& fr_element) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 4135ef9f7605..1d5138efc62a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -31,6 +31,7 @@ template static void constrain_bigfield_limbs(const fr Date: Mon, 15 Sep 2025 13:30:36 +0000 Subject: [PATCH 10/22] fix and add tests --- .../primitives/field/field_conversion.cpp | 1 + .../primitives/field/field_conversion.hpp | 3 +- .../field/field_conversion.test.cpp | 136 +++++++++++++----- 3 files changed, 99 insertions(+), 41 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index ad429bd993c0..be997705d4f6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -19,6 +19,7 @@ namespace bb::stdlib::field_conversion { */ template fq convert_to_grumpkin_fr(Builder& builder, const fr& fr_element) { + ASSERT(!fr_element.is_constant()); static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 1d5138efc62a..b9c12ee9a825 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -100,8 +100,7 @@ template constexpr size_t calc_num_bn254_frs() } else if constexpr (IsAnyOf, goblin_field>) { return Bn254FqParams::NUM_BN254_SCALARS; } else if constexpr (IsAnyOf, grumpkin_element>) { - using BaseField = bn254_element::BaseField; - return 2 * calc_num_bn254_frs(); + return 2 * calc_num_bn254_frs(); } else { // Array or Univariate return calc_num_bn254_frs() * (std::tuple_size::value); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index a2b0bd0f6eca..f7c1ff444278 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -1,4 +1,5 @@ #include "barretenberg/stdlib/primitives/field/field_conversion.hpp" +#include "barretenberg/circuit_checker/circuit_checker.hpp" #include "barretenberg/common/zip_view.hpp" #include @@ -11,13 +12,17 @@ template using grumpkin_element = cycle_group; template class StdlibFieldConversionTests : public ::testing::Test { public: - template void check_conversion(T x) + template void check_conversion(T x, bool valid_circuit = true) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); EXPECT_EQ(len, frs.size()); auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); EXPECT_EQ(x.get_value(), y.get_value()); + + auto ctx = x.get_context(); + + EXPECT_EQ(CircuitChecker::check(*ctx), valid_circuit); } template void check_conversion_iterable(T x) @@ -65,10 +70,9 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinFr) using Builder = TypeParam; Builder builder; - // Constructing bigfield objects with grumpkin::fr values - grumpkin::fr x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits - fq x1(&builder, x1_val); - this->check_conversion(x1); + // Constructing bigfield objects with bb::fq values + bb::fq x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits + this->check_conversion(fq::from_witness(&builder, x1_val)); } /** @@ -79,15 +83,33 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) { using Builder = TypeParam; Builder builder; + { // Serialize and deserialize the bn254 generator + bn254_element x1 = bn254_element::from_witness(&builder, curve::BN254::AffineElement::one()); + this->check_conversion(x1); + } + { // Serialize and deserialize a valid bn254 point + curve::BN254::AffineElement x2_val(1, bb::fq::modulus_minus_two); + bn254_element x2 = bn254_element::from_witness(&builder, x2_val); + this->check_conversion(x2); + } - // Constructing element objects with curve::BN254::AffineElement values - curve::BN254::AffineElement x1_val(1, 2); - bn254_element x1 = bn254_element::from_witness(&builder, x1_val); - this->check_conversion(x1); + // TODO(Sergei): Enable when point at infinity is consistent + // { // Serialize and deserialize the point at infinity + // bn254_element x2 = + // bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); + // this->check_conversion(x2); + // } - curve::BN254::AffineElement x2_val(1, grumpkin::fr::modulus_minus_two); - bn254_element x2 = bn254_element::from_witness(&builder, x2_val); - this->check_conversion(x2); + { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve + curve::BN254::AffineElement x1_val(1, 4); + bn254_element x1; + if constexpr (IsAnyOf) { + EXPECT_THROW_OR_ABORT(x1 = bn254_element::from_witness(&builder, x1_val), ""); + } else { + x1 = bn254_element::from_witness(&builder, x1_val); + this->check_conversion(x1); + } + } } /** @@ -99,14 +121,50 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) using Builder = TypeParam; Builder builder; - // Constructing element objects with curve::Grumpkin::AffineElement values - curve::Grumpkin::AffineElement x1_val(12, 100); - grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); - this->check_conversion(x1); + { // Serialize and deserialize the Grumpkin generator + grumpkin_element x1 = + grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::one()); + this->check_conversion(x1); + } - curve::Grumpkin::AffineElement x2_val(1, grumpkin::fr::modulus_minus_two); - grumpkin_element x2 = grumpkin_element::from_witness(&builder, x2_val); - this->check_conversion(x2); + { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve + curve::Grumpkin::AffineElement x1_val(12, 100); + grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); + this->check_conversion(x1, false); + } + + // TODO(Sergei): Enable when point at infinity is consistent + // { // Serialize and deserialize the point at infinity + // grumpkin_element x2 = + // grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); + // this->check_conversion(x2); + // } +} + +TYPED_TEST(StdlibFieldConversionTests, DeserializePointAtInfinity) +{ + using Builder = TypeParam; + Builder builder; + const fr zero(fr::from_witness_index(&builder, 0)); + + { + std::vector> zeros(4, zero); + + bn254_element point_at_infinity = + field_conversion::convert_from_bn254_frs>(zeros); + + EXPECT_TRUE(point_at_infinity.is_point_at_infinity().get_value()); + EXPECT_TRUE(CircuitChecker::check(builder)); + } + { + std::vector> zeros(2, zero); + + grumpkin_element point_at_infinity = + field_conversion::convert_from_bn254_frs>(zeros); + + EXPECT_TRUE(point_at_infinity.is_point_at_infinity().get_value()); + EXPECT_TRUE(CircuitChecker::check(builder)); + } } /** @@ -143,18 +201,18 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayGrumpkinFr) // Constructing std::array objects with fq values std::array, 4> x1{ - fq( + fq::from_witness( &builder, - static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), - fq( + static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), + fq::from_witness( &builder, - static_cast(std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))), - fq( + static_cast(std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))), + fq::from_witness( &builder, - static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), - fq( + static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), + fq::from_witness( &builder, - static_cast(std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), + static_cast(std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), }; this->check_conversion_iterable(x1); } @@ -184,18 +242,18 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateGrumpkinFr) // Constructing std::array objects with fq values Univariate, 4> x{ - { fq(&builder, - static_cast( - std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), - fq(&builder, - static_cast( - std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))), - fq(&builder, - static_cast( - std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), - fq(&builder, - static_cast( - std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))) } + { fq::from_witness( + &builder, + static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), + fq::from_witness( + &builder, + static_cast(std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))), + fq::from_witness( + &builder, + static_cast(std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), + fq::from_witness( + &builder, + static_cast(std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))) } }; this->check_conversion_iterable(x); } From f21bb970d27e253a577cc76b51038fd502b6deb5 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Mon, 15 Sep 2025 13:37:15 +0000 Subject: [PATCH 11/22] fix avm build --- .../constraining/recursion/goblin_avm_recursive_verifier.hpp | 2 +- .../vm2/constraining/recursion/recursive_flavor.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp index 50f5af2d0d67..0021a8b7ef80 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp @@ -223,7 +223,7 @@ class AvmGoblinRecursiveVerifier { const FF mega_hash = stdlib::poseidon2::hash(mega_hash_buffer); // Construct a Mega-arithmetized AVM2 recursive verifier circuit - auto stdlib_key = std::make_shared(mega_builder, std::span(key_fields)); + auto stdlib_key = std::make_shared(std::span(key_fields)); AvmRecursiveVerifier recursive_verifier{ mega_builder, stdlib_key }; MegaPairingPoints points_accumulator = recursive_verifier.verify_proof(mega_stdlib_proof, mega_public_inputs); diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp index dc1c9d5c47c0..f9468be118a0 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp @@ -77,7 +77,7 @@ class AvmRecursiveFlavor { * @param builder * @param elements */ - VerificationKey(CircuitBuilder& builder, std::span elements) + VerificationKey(std::span elements) { size_t num_frs_read = 0; size_t num_frs_Comm = stdlib::field_conversion::calc_num_bn254_frs(); From 3e792a5e6c838ff2170ab20bbb4aab9ddfd81f9e Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Mon, 15 Sep 2025 17:04:36 +0000 Subject: [PATCH 12/22] TestBasicDoubleHonkRecursionConstraints debugging --- .../graph_description_ultra_recursive_verifier.test.cpp | 2 +- .../barretenberg/circuit_checker/ultra_circuit_checker.cpp | 3 +++ .../dsl/acir_format/honk_recursion_constraint.test.cpp | 7 +++++++ .../stdlib/primitives/field/field_conversion.hpp | 7 +++---- 4 files changed, 14 insertions(+), 5 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..3e6e41693e31 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(), 3); 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/circuit_checker/ultra_circuit_checker.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp index dca6917b51d9..5819f4134a7a 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp @@ -132,6 +132,9 @@ bool UltraCircuitChecker::check_block(Builder& builder, result = result && check_relation(values, params); if (!result) { + for (auto val : values.get_all()) { + info(val); + } return report_fail("Failed Arithmetic relation at row idx = ", idx); } result = result && check_relation(values, params); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp index 88e004b68983..67249cc926fe 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp @@ -125,6 +125,9 @@ template class AcirHonkRecursionConstraint : public : ProgramMetadata metadata{ .recursive = true, .honk_recursion = honk_recursion }; AcirProgram program{ constraint_system, witness }; auto builder = create_circuit(program, metadata); + { + info("circuit checker inner ", CircuitChecker::check(builder)); + }; return builder; } @@ -201,6 +204,10 @@ template class AcirHonkRecursionConstraint : public : } AcirProgram program{ constraint_system, witness }; BuilderType outer_circuit = create_circuit(program, metadata); + { + info("outer ", CircuitChecker::check(outer_circuit)); + info(outer_circuit.err()); + } return outer_circuit; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index b9c12ee9a825..f36e35193614 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -38,8 +38,7 @@ template static void constrain_bigfield_limbs(const fr bool_t check_point_at_infinity( // Sum the limbs and check whether the sum is 0 return (fr::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); } else { - // Efficiently compute ((x^2 + y^2) == 0) + // Efficiently compute ((x^2 + 5 y^2) == 0) const fr x_sqr = fr_vec[0].sqr(); const fr y = fr_vec[1]; - return (y.madd(y, x_sqr).is_zero()); + return (y.madd(y, x_sqr * bb::fr(5)).is_zero()); } } From 74fcd3d6e2441b469445c98faee3b13c567e4529 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Mon, 15 Sep 2025 18:06:18 +0000 Subject: [PATCH 13/22] fix acir test --- .../src/barretenberg/circuit_checker/ultra_circuit_checker.cpp | 3 --- .../dsl/acir_format/honk_recursion_constraint.test.cpp | 3 --- .../barretenberg/stdlib/primitives/field/field_conversion.cpp | 2 +- 3 files changed, 1 insertion(+), 7 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 5819f4134a7a..dca6917b51d9 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_checker.cpp @@ -132,9 +132,6 @@ bool UltraCircuitChecker::check_block(Builder& builder, result = result && check_relation(values, params); if (!result) { - for (auto val : values.get_all()) { - info(val); - } return report_fail("Failed Arithmetic relation at row idx = ", idx); } result = result && check_relation(values, params); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp index 67249cc926fe..1ce100a8f83c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp @@ -125,9 +125,6 @@ template class AcirHonkRecursionConstraint : public : ProgramMetadata metadata{ .recursive = true, .honk_recursion = honk_recursion }; AcirProgram program{ constraint_system, witness }; auto builder = create_circuit(program, metadata); - { - info("circuit checker inner ", CircuitChecker::check(builder)); - }; return builder; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index be997705d4f6..3551db0a32ff 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -40,7 +40,7 @@ template fq convert_to_grumpkin_fr(Builder& builder, value, "field_conversion: limb decomposition"); // checks this decomposition low + hi * 2^64 = value with an assert_equal - const fr zero = fr::from_witness_index(&builder, 0); + const fr zero = fr::from_witness_index(&builder, builder.zero_idx); fr::evaluate_linear_identity(hi * shift, low, -fr_element, zero); return fq(low, hi); From 3d4efb4b64461987f52f1221fe00744e0bda1565 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 16 Sep 2025 09:23:16 +0000 Subject: [PATCH 14/22] fix tests --- .../eccvm_recursive_verifier.test.cpp | 2 +- .../goblin_recursive_verifier.test.cpp | 85 ++++++++++--------- 2 files changed, 44 insertions(+), 43 deletions(-) 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 7441ca25fa73..33f4f1dc87ab 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 @@ -141,7 +141,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 = 216432; 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/goblin_verifier/goblin_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/goblin_recursive_verifier.test.cpp index 80c180bd8d4d..79157638d844 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/goblin_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/goblin_recursive_verifier.test.cpp @@ -25,16 +25,55 @@ class GoblinRecursiveVerifierTests : public testing::Test { using RecursiveCommitment = GoblinRecursiveVerifier::MergeVerifier::Commitment; using MergeCommitments = MergeVerifier::InputCommitments; using RecursiveMergeCommitments = GoblinRecursiveVerifier::MergeVerifier::InputCommitments; - + using FF = TranslatorFlavor::FF; + using BF = TranslatorFlavor::BF; static void SetUpTestSuite() { bb::srs::init_file_crs_factory(bb::srs::bb_crs_path()); } + // Compute the size of a Translator commitment (in bb::fr's) + static constexpr size_t comm_frs = bb::field_conversion::calc_num_bn254_frs(); // 4 + static constexpr size_t eval_frs = bb::field_conversion::calc_num_bn254_frs(); // 1 + + // The `op` wire commitment is currently the second element of the proof, following the + // `accumulated_result` which is a BN254 BaseField element. + static constexpr size_t offset = bb::field_conversion::calc_num_bn254_frs(); + struct ProverOutput { GoblinProof proof; Goblin::VerificationKey verifier_input; MergeCommitments merge_commitments; RecursiveMergeCommitments recursive_merge_commitments; }; + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1298): + // Better recursion testing - create more flexible proof tampering tests. + // Modify the `op` commitment which a part of the Merge protocol. + static void tamper_with_op_commitment(HonkProof& translator_proof) + { + + // Extract `op` fields and convert them to a Commitment object + auto element_frs = std::span{ translator_proof }.subspan(offset, comm_frs); + auto op_commitment = NativeTranscriptParams::template deserialize(element_frs); + // Modify the commitment + op_commitment = op_commitment * FF(2); + // Serialize the tampered commitment into the proof (overwriting the valid one). + auto op_commitment_reserialized = bb::NativeTranscriptParams::serialize(op_commitment); + std::copy(op_commitment_reserialized.begin(), + op_commitment_reserialized.end(), + translator_proof.begin() + static_cast(offset)); + }; + + // Translator proof ends with [..., Libra:quotient_eval, Shplonk:Q, KZG:W]. We invalidate the proof by multiplying + // the eval by 2 (it leads to a Libra consistency check failure). + static void tamper_with_libra_eval(HonkProof& translator_proof) + { + // Proof tail size + static constexpr size_t tail_size = 2 * comm_frs + eval_frs; // 2*4 + 1 = 9 + + // Index of the target field (one fr) from the beginning + const size_t idx = translator_proof.size() - tail_size; + // Tamper: multiply by 2 (or tweak however you like) + translator_proof[idx] = translator_proof[idx] + translator_proof[idx]; + }; /** * @brief Create a goblin proof and the VM verification keys needed by the goblin recursive verifier * @@ -207,13 +246,7 @@ TEST_F(GoblinRecursiveVerifierTests, TranslatorFailure) // Tamper with the Translator proof preamble { GoblinProof tampered_proof = proof; - for (auto& val : tampered_proof.translator_proof) { - if (val > 0) { // tamper by finding the first non-zero value and incrementing it by 1 - val += 1; - break; - } - } - + tamper_with_op_commitment(tampered_proof.translator_proof); Builder builder; RecursiveMergeCommitments recursive_merge_commitments; @@ -231,18 +264,10 @@ TEST_F(GoblinRecursiveVerifierTests, TranslatorFailure) verifier.verify(tampered_proof, recursive_merge_commitments, MergeSettings::APPEND); EXPECT_FALSE(CircuitChecker::check(builder)); } - // Tamper with the Translator proof non-preamble values + // Tamper with the Translator proof non - preamble values { auto tampered_proof = proof; - int seek = 10; - for (auto& val : tampered_proof.translator_proof) { - if (val > 0) { // tamper by finding the tenth non-zero value and incrementing it by 1 - if (--seek == 0) { - val += 1; - break; - } - } - } + tamper_with_libra_eval(tampered_proof.translator_proof); Builder builder; @@ -295,9 +320,6 @@ TEST_F(GoblinRecursiveVerifierTests, TranslatorMergeConsistencyFailure) { { - using Commitment = TranslatorFlavor::Commitment; - using FF = TranslatorFlavor::FF; - using BF = TranslatorFlavor::BF; Builder builder; @@ -309,27 +331,6 @@ TEST_F(GoblinRecursiveVerifierTests, TranslatorMergeConsistencyFailure) // Check natively that the proof is correct. EXPECT_TRUE(Goblin::verify(proof, merge_commitments, verifier_transcript, MergeSettings::APPEND)); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1298): - // Better recursion testing - create more flexible proof tampering tests. - // Modify the `op` commitment which a part of the Merge protocol. - auto tamper_with_op_commitment = [](HonkProof& translator_proof) { - // Compute the size of a Translator commitment (in bb::fr's) - static constexpr size_t num_frs_comm = bb::field_conversion::calc_num_bn254_frs(); - // The `op` wire commitment is currently the second element of the proof, following the - // `accumulated_result` which is a BN254 BaseField element. - static constexpr size_t offset = bb::field_conversion::calc_num_bn254_frs(); - // Extract `op` fields and convert them to a Commitment object - auto element_frs = std::span{ translator_proof }.subspan(offset, num_frs_comm); - auto op_commitment = NativeTranscriptParams::template deserialize(element_frs); - // Modify the commitment - op_commitment = op_commitment * FF(2); - // Serialize the tampered commitment into the proof (overwriting the valid one). - auto op_commitment_reserialized = bb::NativeTranscriptParams::serialize(op_commitment); - std::copy(op_commitment_reserialized.begin(), - op_commitment_reserialized.end(), - translator_proof.begin() + static_cast(offset)); - }; - tamper_with_op_commitment(proof.translator_proof); // Construct and check the Goblin Recursive Verifier circuit From 883d484863f01da87d6d4bd4178cb1191cf5e935 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 16 Sep 2025 10:56:05 +0000 Subject: [PATCH 15/22] remove redundant range constraints --- ...cription_ultra_recursive_verifier.test.cpp | 2 +- .../honk_recursion_constraint.test.cpp | 4 --- .../eccvm_recursive_verifier.test.cpp | 4 +-- .../ultra_recursive_verifier.test.cpp | 2 +- .../primitives/field/field_conversion.cpp | 2 -- .../primitives/field/field_conversion.hpp | 34 +++++++------------ 6 files changed, 15 insertions(+), 33 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 3e6e41693e31..3af551b8483d 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(), 3); + EXPECT_EQ(connected_components.size(), 4); 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/dsl/acir_format/honk_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp index 1ce100a8f83c..88e004b68983 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.test.cpp @@ -201,10 +201,6 @@ template class AcirHonkRecursionConstraint : public : } AcirProgram program{ constraint_system, witness }; BuilderType outer_circuit = create_circuit(program, metadata); - { - info("outer ", CircuitChecker::check(outer_circuit)); - info(outer_circuit.err()); - } return outer_circuit; } 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 33f4f1dc87ab..d51bfdf2ad74 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 @@ -95,8 +95,6 @@ class ECCVMRecursiveTests : public ::testing::Test { auto [opening_claim, ipa_transcript] = verifier.verify_proof(proof); stdlib::recursion::PairingPoints::add_default_to_public_inputs(outer_circuit); - const size_t fixed_size = 218783; - info("Fixed size ", fixed_size); info("Recursive Verifier: num gates = ", outer_circuit.get_estimated_num_finalized_gates()); // Check for a failure flag in the recursive verifier circuit @@ -141,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 = 216432; + uint32_t NUM_GATES_EXPECTED = 213775; 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 396e14c2c748..e71adb72114d 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 = 803143; + uint32_t NUM_GATES_EXPECTED = 801953; 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_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index 3551db0a32ff..cf0779f6f94a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -34,8 +34,6 @@ template fq convert_to_grumpkin_fr(Builder& builder, fr low{ witness_t(&builder, low_val) }; fr hi{ witness_t(&builder, hi_val) }; - constrain_bigfield_limbs(low, hi); - BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), value, "field_conversion: limb decomposition"); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index f36e35193614..92c19854ea9e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -21,24 +21,14 @@ template using fq = bigfield; template using bn254_element = element, fr, curve::BN254::Group>; template using grumpkin_element = cycle_group; -template static void constrain_bigfield_limbs(const fr& lo, const fr& hi) -{ - static constexpr uint64_t NUM_LIMB_BITS = fq::NUM_LIMB_BITS; - static constexpr uint64_t NUM_LAST_LIMB_BITS = NUM_LIMB_BITS + fq::NUM_LAST_LIMB_BITS; // 118 - static constexpr uint64_t NUM_BITS_IN_TWO_LIMBS = 2 * NUM_LIMB_BITS; // 136 - - // range constrain low to 136 bits and hi to 118 bits - lo.create_range_constraint(NUM_BITS_IN_TWO_LIMBS, "field_conversion: create_range_constraint"); - hi.create_range_constraint(NUM_LAST_LIMB_BITS, "field_conversion: create_range_constraint"); -} - /** * @brief Check whether a point corresponds to (0, 0), the conventional representation of the point infinity. * * bn254: In the case of a bn254 point, the bigfield limbs (x_lo, x_hi, y_lo, y_hi) are range constrained, and their sum * is a non-negative integer not exceeding 2^138, i.e. it does not overflow the fq modulus, hence all limbs must be 0. * - * Grumpkin: We are using the fact that (x^2 + 5 * y^2 = 0) has no non-trivial solutions in Grumpkin base field + * Grumpkin: We are using the observation that (x^2 + 5 * y^2 = 0) has no non-trivial solutions in fr, since fr modulus + * p == 2 mod 5, i.e. 5 is not a square mod p. * * @return */ @@ -57,6 +47,12 @@ template bool_t check_point_at_infinity( template fq convert_to_grumpkin_fr(Builder& builder, const fr& f); +/** + * @brief A stdlib Transcript method needed to convert an `fr` challenge to a `bigfield` one. Splits an `fr` into limbs + * that are fed into the bigfield constructor. + * TODO(): Remove redundant constraints caused by splitting and conversion. + * + */ template inline T convert_challenge(Builder& builder, const fr& challenge) { if constexpr (std::is_same_v>) { @@ -69,7 +65,6 @@ template inline T convert_challenge(Builder& buil template inline std::vector> convert_goblin_fr_to_bn254_frs(const goblin_field& input) { - return { input.limbs[0], input.limbs[1] }; } @@ -111,9 +106,8 @@ template constexpr size_t calc_num_bn254_frs() * @details Deserializes a vector of in-circuit `fr`s, i.e. `field_t` elements, into * - `field_t` — no conversion needed * - * - \ref bb::stdlib::bigfield< Builder, T > "bigfield" — 2 input `field_t`s must be range-constrained as low and high - * limbs of a `bigfield` element, then the corresponding `bigfield` constructor is applied. Specific to - * \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * - \ref bb::stdlib::bigfield< Builder, T > "bigfield" — 2 input `field_t`s are fed into `bigfield` constructor that + * ensures that they are properly constrained. Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". * * - \ref bb::stdlib::goblin_field< Builder > "goblin field element" — in contrast to `bigfield`, range constraints are * performed in `Translator` (see \ref TranslatorDeltaRangeConstraintRelationImpl "Translator Range Constraint" @@ -128,8 +122,8 @@ template constexpr size_t calc_num_bn254_frs() * Specific to \ref MegaCircuitBuilder_ "MegaCircuitBuilder". * * - \ref bb::stdlib::element_default::element< Builder_, Fq, Fr, NativeGroup > "bn254 point" — reconstruct a pair of - * `bigfield` elements (x, y), applying required range constraints, and check that the resulting point lies on the - * curve. Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * `bigfield` elements (x, y), check whether the resulting point is a point at infinity and ensure it lies on the curve. + * Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". * * - \ref cycle_group "Grumpkin point" — since the grumpkin base field is `fr`, the reconstruction is trivial. We check * in-circuit whether the resulting point lies on the curve and whether it is a point at infinity. @@ -156,10 +150,6 @@ template T convert_from_bn254_frs(std::span>) { // Cases 2 and 3: a bigfield/goblin_field element is reconstructed from low and high limbs. - if constexpr (std::is_same_v) { - constrain_bigfield_limbs(fr_vec[0], fr_vec[1]); - } - return T(fr_vec[0], fr_vec[1]); } else if constexpr (IsAnyOf, grumpkin_element>) { // Case 4 and 5: Convert a vector of frs to a group element From 3c3712830257b76d5e9cdfecce8545a50a607dcf Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 16 Sep 2025 12:25:13 +0000 Subject: [PATCH 16/22] clean-up --- .../primitives/field/field_conversion.hpp | 92 ++++++++++++++----- .../field/field_conversion.test.cpp | 27 +++--- 2 files changed, 82 insertions(+), 37 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 92c19854ea9e..2b1b5e933142 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -50,7 +50,8 @@ template fq convert_to_grumpkin_fr(Builder& builder, /** * @brief A stdlib Transcript method needed to convert an `fr` challenge to a `bigfield` one. Splits an `fr` into limbs * that are fed into the bigfield constructor. - * TODO(): Remove redundant constraints caused by splitting and conversion. + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1541): Remove redundant constraints caused by splitting and + * conversion to bigfield. * */ template inline T convert_challenge(Builder& builder, const fr& challenge) @@ -153,12 +154,12 @@ template T convert_from_bn254_frs(std::span, grumpkin_element>) { // Case 4 and 5: Convert a vector of frs to a group element - using basefield_ct = typename T::BaseField; + using Basefield = T::BaseField; constexpr size_t base_field_frs = expected_size / 2; - basefield_ct x = convert_from_bn254_frs(fr_vec.subspan(0, base_field_frs)); - basefield_ct y = convert_from_bn254_frs(fr_vec.subspan(base_field_frs, base_field_frs)); + Basefield x = convert_from_bn254_frs(fr_vec.subspan(0, base_field_frs)); + Basefield y = convert_from_bn254_frs(fr_vec.subspan(base_field_frs, base_field_frs)); T out(x, y, check_point_at_infinity(fr_vec)); // Note that in the case of bn254 with Mega arithmetization, the check is delegated to ECCVM, see @@ -181,37 +182,77 @@ template T convert_from_bn254_frs(std::spans - * @details We want to support the following types: bool, size_t, uint32_t, uint64_t, fr, fq, - * bn254_element, grumpkin_element, std::array, for FF = fr/fq, and N is arbitrary. - * @tparam Builder - * @tparam T - * @param val - * @return std::vector> + * @brief Core stdlib Transcript serialization method. + * @details Serializes an object into a flat vector of in-circuit `fr`, i.e. \ref bb::stdlib::field_t "field_t" + * elements. This is the inverse of \ref convert_from_bn254_frs (up to the + * conventional point-at-infinity representation; see TODO below). + * + * Serializes the following types: + * + * - \ref bb::stdlib::field_t "field_t" — no conversion needed; output a single `fr`. + * + * - \ref bb::stdlib::bigfield "bigfield" (\ref bb::stdlib::bigfield< Builder, T >) — output 2 `fr` limbs obtained from + * the bigfield’s binary-basis limbs recombined according to `NUM_LIMB_BITS`. Specific to + * \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - \ref bb::stdlib::goblin_field "goblin field element" (\ref bb::stdlib::goblin_field< Builder >) — emit 2 `fr` limbs + * by exposing the goblin field’s internal limbs (low, high) as-is. Range constraints are enforced in Translator + * (see \ref TranslatorDeltaRangeConstraintRelationImpl "Translator Range Constraint" relation). Specific to + * \ref MegaCircuitBuilder_ "MegaCircuitBuilder". + * + * - \ref bb::stdlib::element_goblin::goblin_element "bn254 goblin point" + * (\ref bb::stdlib::element_goblin::goblin_element< Builder_, Fq, Fr, NativeGroup >) — serialize the pair of + * coordinates `(x, y)` by concatenating the encodings of each coordinate in the base field (goblin/bigfield form). + * The point-at-infinity flag is not emitted here; it is re-derived during deserialization via + * \ref check_point_at_infinity. Specific to \ref MegaCircuitBuilder_ "MegaCircuitBuilder". + * + * - \ref bb::stdlib::element_default::element "bn254 point" + * (\ref bb::stdlib::element_default::element< Builder_, Fq, Fr, NativeGroup >) — serialize `(x, y)` by concatenating + * the encodings of the two \ref bb::stdlib::bigfield "bigfield" coordinates. Specific to + * \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - \ref cycle_group "Grumpkin point" — serialize `(x, y)` in the base field `fr` by concatenating their encodings. + * The point-at-infinity flag is not emitted; it is re-derived during deserialization via + * \ref check_point_at_infinity. Specific to \ref UltraCircuitBuilder_ "UltraCircuitBuilder". + * + * - `bb::Univariate` or `std::array` of any of the above — serialize element-wise and concatenate. + * + * Round-trip note: + * - For supported types, `convert_to_bn254_frs(val)` followed by `convert_from_bn254_frs(...)` reconstructs an + * equivalent in-circuit object, assuming the same arithmetization and that range/ECC checks are enforced where + * applicable during reconstruction (see \ref bb::ECCVMTranscriptRelationImpl< FF_ > "ECCVM Transcript" relation). + * + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): make the point-at-infinity representation fully + * uniform across (de)serialization paths. + * + * @tparam Builder `UltraCircuitBuilder` or `MegaCircuitBuilder` + * @tparam T Target object type + * @param val Value to serialize + * @return Flat vector of `fr` elements */ template std::vector> convert_to_bn254_frs(const T& val) { - if constexpr (IsAnyOf>) { - std::vector> fr_vec{ val }; - return fr_vec; - } else if constexpr (IsAnyOf>) { - // Bigfield + using field_ct = fr; + using bigfield_ct = fq; + + if constexpr (IsAnyOf) { + ; + return std::vector{ val }; + } else if constexpr (IsAnyOf) { return convert_grumpkin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf>) { return convert_goblin_fr_to_bn254_frs(val); } else if constexpr (IsAnyOf, grumpkin_element>) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Consider handling point at infinity. using BaseField = T::BaseField; - std::vector> fr_vec_x = convert_to_bn254_frs(val.x); - std::vector> fr_vec_y = convert_to_bn254_frs(val.y); - std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); + std::vector fr_vec_x = convert_to_bn254_frs(val.x); + std::vector fr_vec_y = convert_to_bn254_frs(val.y); + std::vector fr_vec(fr_vec_x.begin(), fr_vec_x.end()); fr_vec.insert(fr_vec.end(), fr_vec_y.begin(), fr_vec_y.end()); return fr_vec; } else { // Array or Univariate - std::vector> fr_vec; + std::vector fr_vec; for (auto& x : val) { auto tmp_vec = convert_to_bn254_frs(x); fr_vec.insert(fr_vec.end(), tmp_vec.begin(), tmp_vec.end()); @@ -221,11 +262,12 @@ template std::vector> convert_to_bn25 } /** - * @brief Deserialize an object of specified type from a buffer of field elements; update provided read count in place + * @brief A stdlib VerificationKey-specific method. + * + * @details Deserialize an object of specified type from a buffer of field elements; update provided read count in place * * @tparam TargetType Type to reconstruct from buffer of field elements - * @param builder - * @param elements Buffer of field elements + * @param elements Buffer of `field_t` elements * @param num_frs_read Index at which to read into buffer */ template diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index f7c1ff444278..03eeaffad6c9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -12,6 +12,7 @@ template using grumpkin_element = cycle_group; template class StdlibFieldConversionTests : public ::testing::Test { public: + // Serialize and deserialize template void check_conversion(T x, bool valid_circuit = true) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); @@ -93,12 +94,13 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) this->check_conversion(x2); } - // TODO(Sergei): Enable when point at infinity is consistent - // { // Serialize and deserialize the point at infinity - // bn254_element x2 = - // bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); - // this->check_conversion(x2); - // } + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Flip `valid_circuit` flag when point at infinity + // is consistently represented + { // Serialize and deserialize the point at infinity + bn254_element x2 = + bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); + this->check_conversion(x2, false); + } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve curve::BN254::AffineElement x1_val(1, 4); @@ -133,12 +135,13 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) this->check_conversion(x1, false); } - // TODO(Sergei): Enable when point at infinity is consistent - // { // Serialize and deserialize the point at infinity - // grumpkin_element x2 = - // grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); - // this->check_conversion(x2); - // } + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Flip `valid_circuit` flag when point at infinity + // is consistently represented + { // Serialize and deserialize the point at infinity + grumpkin_element x2 = + grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); + this->check_conversion(x2, false); + } } TYPED_TEST(StdlibFieldConversionTests, DeserializePointAtInfinity) From 95c68fd058a6ba96683ec892adac776fff77057a Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 16 Sep 2025 14:13:09 +0000 Subject: [PATCH 17/22] fix test --- .../stdlib/primitives/field/field_conversion.test.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index 03eeaffad6c9..b1769acc8f92 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -13,13 +13,13 @@ template using grumpkin_element = cycle_group; template class StdlibFieldConversionTests : public ::testing::Test { public: // Serialize and deserialize - template void check_conversion(T x, bool valid_circuit = true) + template void check_conversion(T x, bool valid_circuit = true, bool point_at_infinity = false) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); EXPECT_EQ(len, frs.size()); auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); - EXPECT_EQ(x.get_value(), y.get_value()); + EXPECT_EQ(x.get_value() == y.get_value(), !point_at_infinity); auto ctx = x.get_context(); @@ -99,7 +99,8 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) { // Serialize and deserialize the point at infinity bn254_element x2 = bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); - this->check_conversion(x2, false); + // The circuit is valid, because the point at infinity is set to `one`. + this->check_conversion(x2, /* valid circuit */ true, /* point at infinity */ true); } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve @@ -132,7 +133,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve curve::Grumpkin::AffineElement x1_val(12, 100); grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); - this->check_conversion(x1, false); + this->check_conversion(x1, /* valid circuit */ false); } // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Flip `valid_circuit` flag when point at infinity @@ -140,7 +141,7 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) { // Serialize and deserialize the point at infinity grumpkin_element x2 = grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); - this->check_conversion(x2, false); + this->check_conversion(x2, /* valid circuit */ false, /* point at infinity */ true); } } From 6e9b48bef1ade90999d5af07f763ddf38133d380 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Tue, 16 Sep 2025 15:28:00 +0000 Subject: [PATCH 18/22] fix silly bugs in field conversion tests --- .../field/field_conversion.test.cpp | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index b1769acc8f92..5d96f149773e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -19,7 +19,9 @@ template class StdlibFieldConversionTests : public ::testing: auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); EXPECT_EQ(len, frs.size()); auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); - EXPECT_EQ(x.get_value() == y.get_value(), !point_at_infinity); + bool expected = std::is_same_v ? !point_at_infinity : true; + + EXPECT_EQ(x.get_value() == y.get_value(), expected); auto ctx = x.get_context(); @@ -83,20 +85,25 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinFr) TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) { using Builder = TypeParam; - Builder builder; { // Serialize and deserialize the bn254 generator + Builder builder; + bn254_element x1 = bn254_element::from_witness(&builder, curve::BN254::AffineElement::one()); this->check_conversion(x1); } { // Serialize and deserialize a valid bn254 point + Builder builder; + curve::BN254::AffineElement x2_val(1, bb::fq::modulus_minus_two); bn254_element x2 = bn254_element::from_witness(&builder, x2_val); this->check_conversion(x2); } - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Flip `valid_circuit` flag when point at infinity - // is consistently represented + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Remove `point_at_infinity` flag when point at + // infinity is consistently represented. { // Serialize and deserialize the point at infinity + Builder builder; + bn254_element x2 = bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); // The circuit is valid, because the point at infinity is set to `one`. @@ -104,6 +111,8 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve + Builder builder; + curve::BN254::AffineElement x1_val(1, 4); bn254_element x1; if constexpr (IsAnyOf) { @@ -122,26 +131,28 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) { using Builder = TypeParam; - Builder builder; { // Serialize and deserialize the Grumpkin generator + Builder builder; grumpkin_element x1 = grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::one()); this->check_conversion(x1); } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve + Builder builder; + curve::Grumpkin::AffineElement x1_val(12, 100); grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); this->check_conversion(x1, /* valid circuit */ false); } - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Flip `valid_circuit` flag when point at infinity - // is consistently represented { // Serialize and deserialize the point at infinity + Builder builder; + grumpkin_element x2 = grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); - this->check_conversion(x2, /* valid circuit */ false, /* point at infinity */ true); + this->check_conversion(x2); } } From 0ed5343b03a609d719b42dbe4726747e26d72487 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Wed, 17 Sep 2025 08:37:50 +0000 Subject: [PATCH 19/22] address review comments --- .../primitives/field/field_conversion.cpp | 2 +- .../primitives/field/field_conversion.hpp | 4 +- .../field/field_conversion.test.cpp | 150 +++++++++++------- 3 files changed, 92 insertions(+), 64 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp index cf0779f6f94a..344e24e5fdbb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -37,7 +37,7 @@ template fq convert_to_grumpkin_fr(Builder& builder, BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), value, "field_conversion: limb decomposition"); - // checks this decomposition low + hi * 2^64 = value with an assert_equal + // check the decomposition low + hi * 2^136 = value in circuit const fr zero = fr::from_witness_index(&builder, builder.zero_idx); fr::evaluate_linear_identity(hi * shift, low, -fr_element, zero); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp index 2b1b5e933142..9750520585fa 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -41,7 +41,8 @@ template bool_t check_point_at_infinity( // Efficiently compute ((x^2 + 5 y^2) == 0) const fr x_sqr = fr_vec[0].sqr(); const fr y = fr_vec[1]; - return (y.madd(y, x_sqr * bb::fr(5)).is_zero()); + const fr five_y = y * bb::fr(5); + return (y.madd(five_y, x_sqr).is_zero()); } } @@ -236,7 +237,6 @@ template std::vector> convert_to_bn25 using bigfield_ct = fq; if constexpr (IsAnyOf) { - ; return std::vector{ val }; } else if constexpr (IsAnyOf) { return convert_grumpkin_fr_to_bn254_frs(val); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index 5d96f149773e..cad7e72509de 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -10,7 +10,7 @@ template using fq = bigfield; template using bn254_element = element, fr, curve::BN254::Group>; template using grumpkin_element = cycle_group; -template class StdlibFieldConversionTests : public ::testing::Test { +template class stdlib_field_conversion : public ::testing::Test { public: // Serialize and deserialize template void check_conversion(T x, bool valid_circuit = true, bool point_at_infinity = false) @@ -43,83 +43,99 @@ template class StdlibFieldConversionTests : public ::testing: using BuilderTypes = testing::Types; -TYPED_TEST_SUITE(StdlibFieldConversionTests, BuilderTypes); +TYPED_TEST_SUITE(stdlib_field_conversion, BuilderTypes); /** * @brief Field conversion test for fr */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionFr) +TYPED_TEST(stdlib_field_conversion, FieldConversionFr) { using Builder = TypeParam; Builder builder; - bb::fr x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits - fr x1(&builder, x1_val); - this->check_conversion(x1); - - bb::fr x2_val(bb::fr::modulus_minus_two); // modulus - 2 - fr x2(&builder, x2_val); - this->check_conversion(x2); - - bb::fr x3_val(1); - fr x3(&builder, x3_val); - this->check_conversion(x3); + bb::fr field_element_val( + std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits + fr field_element(&builder, field_element_val); + this->check_conversion(field_element); + + field_element_val = bb::fr::modulus_minus_two; // modulus - 2 + field_element = fr(&builder, field_element_val); + this->check_conversion(field_element); + + field_element_val = bb::fr(1); + field_element = fr(&builder, field_element_val); + this->check_conversion(field_element); } /** * @brief Field conversion test for fq */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinFr) +TYPED_TEST(stdlib_field_conversion, FieldConversionGrumpkinFr) { using Builder = TypeParam; Builder builder; // Constructing bigfield objects with bb::fq values - bb::fq x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits - this->check_conversion(fq::from_witness(&builder, x1_val)); + bb::fq field_element_val( + std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits + this->check_conversion(fq::from_witness(&builder, field_element_val)); } /** * @brief Field conversion test for bn254_element * */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) +TYPED_TEST(stdlib_field_conversion, FieldConversionBN254AffineElement) { using Builder = TypeParam; { // Serialize and deserialize the bn254 generator Builder builder; - bn254_element x1 = bn254_element::from_witness(&builder, curve::BN254::AffineElement::one()); - this->check_conversion(x1); + bn254_element group_element = + bn254_element::from_witness(&builder, curve::BN254::AffineElement::one()); + this->check_conversion(group_element); } { // Serialize and deserialize a valid bn254 point Builder builder; - curve::BN254::AffineElement x2_val(1, bb::fq::modulus_minus_two); - bn254_element x2 = bn254_element::from_witness(&builder, x2_val); - this->check_conversion(x2); + curve::BN254::AffineElement group_element_val(1, bb::fq::modulus_minus_two); + bn254_element group_element = bn254_element::from_witness(&builder, group_element_val); + this->check_conversion(group_element); } + { // Serialize and deserialize random Grumpkin points + Builder builder; + const size_t num_points = 50; + const curve::BN254::AffineElement native_generator = curve::BN254::AffineElement::one(); + + for (size_t i = 0; i < num_points; i++) { + bb::fr random_scalar = bb::fr::random_element(); + bn254_element group_element = + bn254_element::from_witness(&builder, native_generator * random_scalar); + this->check_conversion(group_element); + } + } // TODO(https://github.com/AztecProtocol/barretenberg/issues/1527): Remove `point_at_infinity` flag when point at // infinity is consistently represented. { // Serialize and deserialize the point at infinity Builder builder; - bn254_element x2 = + bn254_element group_element = bn254_element::from_witness(&builder, curve::BN254::AffineElement::infinity()); // The circuit is valid, because the point at infinity is set to `one`. - this->check_conversion(x2, /* valid circuit */ true, /* point at infinity */ true); + this->check_conversion(group_element, /* valid circuit */ true, /* point at infinity */ true); } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve Builder builder; - curve::BN254::AffineElement x1_val(1, 4); - bn254_element x1; + curve::BN254::AffineElement group_element_val(1, 4); + bn254_element group_element; if constexpr (IsAnyOf) { - EXPECT_THROW_OR_ABORT(x1 = bn254_element::from_witness(&builder, x1_val), ""); + EXPECT_THROW_OR_ABORT(group_element = bn254_element::from_witness(&builder, group_element_val), + ""); } else { - x1 = bn254_element::from_witness(&builder, x1_val); - this->check_conversion(x1); + group_element = bn254_element::from_witness(&builder, group_element_val); + this->check_conversion(group_element); } } } @@ -128,35 +144,47 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionBN254AffineElement) * @brief Field conversion test for grumpkin_element * */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionGrumpkinAffineElement) +TYPED_TEST(stdlib_field_conversion, FieldConversionGrumpkinAffineElement) { using Builder = TypeParam; { // Serialize and deserialize the Grumpkin generator Builder builder; - grumpkin_element x1 = + grumpkin_element group_element = grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::one()); - this->check_conversion(x1); + this->check_conversion(group_element); + } + { // Serialize and deserialize random Grumpkin points + Builder builder; + const size_t num_points = 50; + const curve::Grumpkin::AffineElement native_generator = curve::Grumpkin::AffineElement::one(); + + for (size_t i = 0; i < num_points; i++) { + bb::fq random_scalar = bb::fq::random_element(); + grumpkin_element group_element = + grumpkin_element::from_witness(&builder, native_generator * random_scalar); + this->check_conversion(group_element); + } } { // Serialize and deserialize "coordinates" that do not correspond to any point on the curve Builder builder; - curve::Grumpkin::AffineElement x1_val(12, 100); - grumpkin_element x1 = grumpkin_element::from_witness(&builder, x1_val); - this->check_conversion(x1, /* valid circuit */ false); + curve::Grumpkin::AffineElement group_element_val(12, 100); + grumpkin_element group_element = grumpkin_element::from_witness(&builder, group_element_val); + this->check_conversion(group_element, /* valid circuit */ false); } { // Serialize and deserialize the point at infinity Builder builder; - grumpkin_element x2 = + grumpkin_element group_element = grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); - this->check_conversion(x2); + this->check_conversion(group_element); } } -TYPED_TEST(StdlibFieldConversionTests, DeserializePointAtInfinity) +TYPED_TEST(stdlib_field_conversion, DeserializePointAtInfinity) { using Builder = TypeParam; Builder builder; @@ -185,37 +213,37 @@ TYPED_TEST(StdlibFieldConversionTests, DeserializePointAtInfinity) /** * @brief Field conversion test for std::array, N> */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayBn254Fr) +TYPED_TEST(stdlib_field_conversion, FieldConversionArrayBn254Fr) { using Builder = TypeParam; Builder builder; // Constructing std::array objects with fr values - std::array, 4> x1{ + std::array, 4> array_of_frs_4{ fr(&builder, 1), fr(&builder, 2), fr(&builder, 3), fr(&builder, 4) }; - this->check_conversion_iterable(x1); - - std::array, 7> x2{ fr(&builder, bb::fr::modulus_minus_two), - fr(&builder, bb::fr::modulus_minus_two - 123), - fr(&builder, 215215125), - fr(&builder, 102701750), - fr(&builder, 367032), - fr(&builder, 12985028), - fr(&builder, bb::fr::modulus_minus_two - 125015028) }; - this->check_conversion_iterable(x2); + this->check_conversion_iterable(array_of_frs_4); + + std::array, 7> array_of_frs_7{ fr(&builder, bb::fr::modulus_minus_two), + fr(&builder, bb::fr::modulus_minus_two - 123), + fr(&builder, 215215125), + fr(&builder, 102701750), + fr(&builder, 367032), + fr(&builder, 12985028), + fr(&builder, bb::fr::modulus_minus_two - 125015028) }; + this->check_conversion_iterable(array_of_frs_7); } /** * @brief Field conversion test for std::array, N> */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayGrumpkinFr) +TYPED_TEST(stdlib_field_conversion, FieldConversionArrayGrumpkinFr) { using Builder = TypeParam; Builder builder; // Constructing std::array objects with fq values - std::array, 4> x1{ + std::array, 4> array_of_fqs_4{ fq::from_witness( &builder, static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), @@ -229,34 +257,34 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionArrayGrumpkinFr) &builder, static_cast(std::string("018555a8eb50cf07f64b019ebaf3af3c925c93e631f3ecd455db07bbb52bbdd3"))), }; - this->check_conversion_iterable(x1); + this->check_conversion_iterable(array_of_fqs_4); } /** * @brief Field conversion test for Univariate, N> */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateBn254Fr) +TYPED_TEST(stdlib_field_conversion, FieldConversionUnivariateBn254Fr) { using Builder = TypeParam; Builder builder; // Constructing Univariate objects with fr values - Univariate, 4> x{ + Univariate, 4> univariate{ { fr(&builder, 1), fr(&builder, 2), fr(&builder, 3), fr(&builder, 4) } }; - this->check_conversion_iterable(x); + this->check_conversion_iterable(univariate); } /** * @brief Field conversion test for Univariate, N> */ -TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateGrumpkinFr) +TYPED_TEST(stdlib_field_conversion, FieldConversionUnivariateGrumpkinFr) { using Builder = TypeParam; Builder builder; // Constructing std::array objects with fq values - Univariate, 4> x{ + Univariate, 4> univariate{ { fq::from_witness( &builder, static_cast(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789"))), @@ -270,14 +298,14 @@ TYPED_TEST(StdlibFieldConversionTests, FieldConversionUnivariateGrumpkinFr) &builder, static_cast(std::string("2bf1eaf87f7d27e8dc4056e9af975985bccc89077a21891d6c7b6ccce0631f95"))) } }; - this->check_conversion_iterable(x); + this->check_conversion_iterable(univariate); } /** * @brief Convert challenge test for fq * */ -TYPED_TEST(StdlibFieldConversionTests, ConvertChallengeGrumpkinFr) +TYPED_TEST(stdlib_field_conversion, ConvertChallengeGrumpkinFr) { using Builder = TypeParam; Builder builder; From 27c18bb2918648e4d6a81de55dbefefd2213608e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 18:47:18 +0000 Subject: [PATCH 20/22] field conversion test cleanup --- .../primitives/field/field_conversion.test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index cad7e72509de..10846d2724d1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -13,17 +13,17 @@ template using grumpkin_element = cycle_group; template class stdlib_field_conversion : public ::testing::Test { public: // Serialize and deserialize - template void check_conversion(T x, bool valid_circuit = true, bool point_at_infinity = false) + template void check_conversion(T in, bool valid_circuit = true, bool point_at_infinity = false) { size_t len = bb::stdlib::field_conversion::calc_num_bn254_frs(); - auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(x); + auto frs = bb::stdlib::field_conversion::convert_to_bn254_frs(in); EXPECT_EQ(len, frs.size()); - auto y = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); + auto out = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); bool expected = std::is_same_v ? !point_at_infinity : true; - EXPECT_EQ(x.get_value() == y.get_value(), expected); + EXPECT_EQ(in.get_value() == out.get_value(), expected); - auto ctx = x.get_context(); + auto ctx = in.get_context(); EXPECT_EQ(CircuitChecker::check(*ctx), valid_circuit); } @@ -188,7 +188,7 @@ TYPED_TEST(stdlib_field_conversion, DeserializePointAtInfinity) { using Builder = TypeParam; Builder builder; - const fr zero(fr::from_witness_index(&builder, 0)); + const fr zero(fr::from_witness_index(&builder, builder->zero_idx)); { std::vector> zeros(4, zero); From a84c115625985069124467d2401caa5dd77835ea Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 18:52:46 +0000 Subject: [PATCH 21/22] 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 485f010abdfb..5fd9eb1b205b 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="04f38201" +pinned_short_hash="e70f08be" 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 599e0fc4657b29dc100686ac0e036661a79dad4f Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 19 Sep 2025 19:15:18 +0000 Subject: [PATCH 22/22] fix build --- .../stdlib/primitives/field/field_conversion.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp index 10846d2724d1..0bdd7073c24e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.test.cpp @@ -188,7 +188,7 @@ TYPED_TEST(stdlib_field_conversion, DeserializePointAtInfinity) { using Builder = TypeParam; Builder builder; - const fr zero(fr::from_witness_index(&builder, builder->zero_idx)); + const fr zero(fr::from_witness_index(&builder, builder.zero_idx)); { std::vector> zeros(4, zero);