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 { 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 98c71036e0ad..7907f68d444a 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, input.public_inputs.size()); 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 aff7a8733554..d37ebfaed865 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp @@ -139,7 +139,7 @@ class ECCVMRecursiveTests : public ::testing::Test { } // Check that the size of the recursive verifier is consistent with historical expectation - uint32_t NUM_GATES_EXPECTED = 213923; + uint32_t NUM_GATES_EXPECTED = 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/goblin_verifier/goblin_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/goblin_recursive_verifier.test.cpp index ae5489ecf4f8..59da953be31f 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 * @@ -208,13 +247,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; @@ -232,18 +265,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; @@ -296,9 +321,6 @@ TEST_F(GoblinRecursiveVerifierTests, TranslatorMergeConsistencyFailure) { { - using Commitment = TranslatorFlavor::Commitment; - using FF = TranslatorFlavor::FF; - using BF = TranslatorFlavor::BF; Builder builder; @@ -310,27 +332,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 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 4ef57b0ee2e6..04c63168a588 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 @@ -293,7 +293,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 = 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/biggroup/biggroup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp index fcb4fcac9a33..3fd097c98207 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 a69d2f3f8ea6..489db27f4ab7 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 f8f35b818883..c698b5855d48 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..344e24e5fdbb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.cpp @@ -5,46 +5,43 @@ // ===================== #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. + * @brief Converts an in-circuit `fr`element to an `fq`, i.e. `field_t` --> `bigfield`. * - * TODO(https://github.com/AztecProtocol/barretenberg/issues/850): audit this function more carefully + * @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& 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 + 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 + 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); - // 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"); + BB_ASSERT_EQ(static_cast(low_val) + (static_cast(hi_val) << NUM_BITS_IN_TWO_LIMBS), + value, + "field_conversion: limb decomposition"); + // 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); - std::vector> fr_vec{ low, hi }; - return convert_from_bn254_frs>(builder, 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 c4778d410c38..9750520585fa 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field_conversion.hpp @@ -21,11 +21,40 @@ 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 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 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 + */ +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 (fr::accumulate(std::vector>(fr_vec.begin(), fr_vec.end())).is_zero()); + } else { + // Efficiently compute ((x^2 + 5 y^2) == 0) + const fr x_sqr = fr_vec[0].sqr(); + const fr y = fr_vec[1]; + const fr five_y = y * bb::fr(5); + return (y.madd(five_y, x_sqr).is_zero()); + } +} 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(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) { if constexpr (std::is_same_v>) { @@ -38,15 +67,14 @@ 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) { - fr shift(static_cast(1) << NUM_LIMB_BITS); + 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); result[1] = input.binary_basis_limbs[2].element + (input.binary_basis_limbs[3].element * shift); @@ -65,13 +93,10 @@ 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>) { - using BaseField = bn254_element::BaseField; - return 2 * calc_num_bn254_frs(); - } else if constexpr (IsAnyOf>) { - return 2 * calc_num_bn254_frs>(); + } else if constexpr (IsAnyOf, grumpkin_element>) { + return 2 * calc_num_bn254_frs(); } else { // Array or Univariate return calc_num_bn254_frs() * (std::tuple_size::value); @@ -79,69 +104,78 @@ 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 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" + * 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), 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. + * 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(Builder& builder, std::span> fr_vec) +template T convert_from_bn254_frs(std::span> fr_vec) { - if constexpr (IsAnyOf>) { - BB_ASSERT_EQ(fr_vec.size(), 1U); + using field_ct = fr; + using bigfield_ct = fq; + + 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 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>) { - BB_ASSERT_EQ(fr_vec.size(), 2U); - goblin_field 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(); - 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)); - - // 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; - } else if constexpr (IsAnyOf>) { - using BaseField = fr; - 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)); - grumpkin_element result(x, y, x.is_zero() && y.is_zero()); - return result; + } else if constexpr (IsAnyOf>) { + // 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 + using Basefield = T::BaseField; + + constexpr size_t base_field_frs = expected_size / 2; + + 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 + // `on_curve_check` in `ECCVMTranscriptRelationImpl`. + out.validate_on_curve(); + return out; } else { - // Array or Univariate + // Case 6: Array or Univariate T val; - constexpr size_t FieldScalarSize = calc_num_bn254_frs(); - BB_ASSERT_EQ(fr_vec.size(), FieldScalarSize * std::tuple_size::value); + 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( - builder, fr_vec.subspan(FieldScalarSize * i, FieldScalarSize)); + x = convert_from_bn254_frs(fr_vec.subspan(scalar_frs * i, scalar_frs)); ++i; } return val; @@ -149,42 +183,76 @@ template T convert_from_bn254_frs(Builder& builde } /** - * @brief Conversion from transcript values to frs - * @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>) { + 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>) { - // 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); - std::vector> fr_vec(fr_vec_x.begin(), fr_vec_x.end()); + } else if constexpr (IsAnyOf, grumpkin_element>) { + 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()); 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()); @@ -194,19 +262,20 @@ 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 -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(); + 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(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..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 @@ -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 @@ -9,23 +10,30 @@ 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: - template void check_conversion(Builder& builder, T x) + // Serialize and deserialize + 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(builder, frs); - EXPECT_EQ(x.get_value(), y.get_value()); + auto out = bb::stdlib::field_conversion::convert_from_bn254_frs(frs); + bool expected = std::is_same_v ? !point_at_infinity : true; + + EXPECT_EQ(in.get_value() == out.get_value(), expected); + + auto ctx = in.get_context(); + + EXPECT_EQ(CircuitChecker::check(*ctx), valid_circuit); } - 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()); @@ -35,176 +43,269 @@ 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(builder, x1); + bb::fr field_element_val( + std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits + fr field_element(&builder, field_element_val); + this->check_conversion(field_element); - bb::fr x2_val(bb::fr::modulus_minus_two); // modulus - 2 - fr x2(&builder, x2_val); - this->check_conversion(builder, x2); + field_element_val = bb::fr::modulus_minus_two; // modulus - 2 + field_element = fr(&builder, field_element_val); + this->check_conversion(field_element); - bb::fr x3_val(1); - fr x3(&builder, x3_val); - this->check_conversion(builder, x3); + 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 grumpkin::fr values - grumpkin::fr x1_val(std::string("9a807b615c4d3e2fa0b1c2d3e4f56789fedcba9876543210abcdef0123456789")); // 256 bits - fq x1(&builder, x1_val); - this->check_conversion(builder, x1); + // Constructing bigfield objects with bb::fq values + 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; - Builder builder; + { // Serialize and deserialize the bn254 generator + Builder builder; + + 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 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(); - // 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); + 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 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(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 x2_val(1, grumpkin::fr::modulus_minus_two); - bn254_element x2 = bn254_element::from_witness(&builder, x2_val); - this->check_conversion(builder, x2); + curve::BN254::AffineElement group_element_val(1, 4); + bn254_element group_element; + if constexpr (IsAnyOf) { + EXPECT_THROW_OR_ABORT(group_element = bn254_element::from_witness(&builder, group_element_val), + ""); + } else { + group_element = bn254_element::from_witness(&builder, group_element_val); + this->check_conversion(group_element); + } + } } /** * @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 group_element = + grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::one()); + 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 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 group_element = + grumpkin_element::from_witness(&builder, curve::Grumpkin::AffineElement::infinity()); + this->check_conversion(group_element); + } +} + +TYPED_TEST(stdlib_field_conversion, DeserializePointAtInfinity) { using Builder = TypeParam; Builder builder; + const fr zero(fr::from_witness_index(&builder, builder.zero_idx)); + + { + std::vector> zeros(4, zero); + + bn254_element point_at_infinity = + field_conversion::convert_from_bn254_frs>(zeros); - // 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); + 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); - 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); + EXPECT_TRUE(point_at_infinity.is_point_at_infinity().get_value()); + EXPECT_TRUE(CircuitChecker::check(builder)); + } } /** * @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(builder, 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(builder, 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{ - fq( + std::array, 4> array_of_fqs_4{ + 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(builder, 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(builder, 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{ - { 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"))) } + Univariate, 4> univariate{ + { 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(builder, 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; @@ -215,4 +316,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/primitives/group/cycle_group.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp index 7b9e4b811ca8..2407478d55f1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.cpp @@ -161,7 +161,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; } @@ -226,7 +226,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 3e40c855158e..abe98660f904 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; @@ -84,7 +85,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 de7a3055e4ae..ffcfc313f4ea 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 @@ -227,7 +227,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) @@ -237,14 +237,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()); check_circuit_and_gates(builder, 12); } /** * @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) @@ -256,14 +256,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()); check_circuit_and_gates(builder, 1); } /** * @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) @@ -275,14 +275,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) @@ -294,7 +294,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)); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp index 091114dee82d..8600e7998f4e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/transcript/transcript.hpp @@ -54,9 +54,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/goblin_avm_recursive_verifier.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/goblin_avm_recursive_verifier.hpp index e1bfd1952442..17f5c8cdb241 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 895bfe715db4..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,14 +77,14 @@ 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(); 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; } }