chore: field_conversion clean-up/int audit (#16898)

### 🧾 Audit Context

`stdlib::field_conversion` provides the serialization/deserialization
methods for `stdlib::Transcript`.

### 🛠️ Changes Made

- Simplified the logic by making it more uniform and getting rid of
redundant arguments
- Enabled `on_curve` checks for Ultra deserialization of group elements
(BN254 and Grumpkin)
- Removed redundant range constraints in `fr --> bigfield` conversion
and optimized the constraint ensuring that `fr = lo + hi * shift`. Note
that limbs were constrained to be (136, 118) bits, although such range
constraints are enforced by `bigfield(low, hi)` constructor.
- Optimized `point_at_infinity` check for Grumpkin points.
- Expanded docs and made them Doxygen-friendly
- Added tests and [an
issue](AztecProtocol/barretenberg#1541) that
will be resolved in a follow-up
- The vks and circuit sizes have changed due to enabled checks and the
small optimizations described above

### 📌 Notes for Reviewers

Agreed with @suyash67 that existing point-at-infinity issue
AztecProtocol/barretenberg#1527 should be
handled by ensuring the consistent representation of points at infinity
in all groups. Added a test that will fail once the issue is resolved.

---------

Co-authored-by: ledwards2225 <[email protected]>

Author: iakovenkos

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 {

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<VerificationKey>(builder, key_fields);
+    auto mega_vk = std::make_shared<VerificationKey>(key_fields);
     auto mega_vk_and_hash = std::make_shared<RecursiveVKAndHash>(mega_vk, vk_hash);
     ClientIVCRecursiveVerifier::StdlibProof stdlib_proof(proof_fields, input.public_inputs.size());
 

barretenberg/cpp/src/barretenberg/dsl/acir_format/honk_recursion_constraint.cpp

@@ -141,7 +141,7 @@ HonkRecursionConstraintOutput<typename Flavor::CircuitBuilder> create_honk_recur
     }
 
     // Recursively verify the proof
-    auto vkey = std::make_shared<RecursiveVerificationKey>(builder, key_fields);
+    auto vkey = std::make_shared<RecursiveVerificationKey>(key_fields);
     auto vk_and_hash = std::make_shared<RecursiveVKAndHash>(vkey, vk_hash);
     RecursiveVerifier verifier(&builder, vk_and_hash);
     UltraRecursiveVerifierOutput<Builder> verifier_output = verifier.template verify_proof<IO>(proof_fields);

barretenberg/cpp/src/barretenberg/flavor/mega_recursive_flavor.hpp

@@ -140,18 +140,18 @@ template <typename BuilderType> class MegaRecursiveFlavor_ {
          * @param builder
          * @param elements
          */
-        VerificationKey(CircuitBuilder& builder, std::span<FF> elements)
+        VerificationKey(std::span<FF> elements)
         {
             using namespace bb::stdlib::field_conversion;
 
             size_t num_frs_read = 0;
 
-            this->log_circuit_size = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->num_public_inputs = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->pub_inputs_offset = deserialize_from_frs<FF>(builder, elements, num_frs_read);
+            this->log_circuit_size = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->num_public_inputs = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->pub_inputs_offset = deserialize_from_frs<FF>(elements, num_frs_read);
 
             for (Commitment& commitment : this->get_all()) {
-                commitment = deserialize_from_frs<Commitment>(builder, elements, num_frs_read);
+                commitment = deserialize_from_frs<Commitment>(elements, num_frs_read);
             }
 
             if (num_frs_read != elements.size()) {
@@ -174,7 +174,7 @@ template <typename BuilderType> 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);
         }
 
         /**

barretenberg/cpp/src/barretenberg/flavor/ultra_recursive_flavor.hpp

@@ -140,18 +140,18 @@ template <typename BuilderType> class UltraRecursiveFlavor_ {
          * @param builder
          * @param elements
          */
-        VerificationKey(CircuitBuilder& builder, std::span<FF> elements)
+        VerificationKey(std::span<FF> elements)
         {
             using namespace bb::stdlib::field_conversion;
 
             size_t num_frs_read = 0;
 
-            this->log_circuit_size = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->num_public_inputs = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->pub_inputs_offset = deserialize_from_frs<FF>(builder, elements, num_frs_read);
+            this->log_circuit_size = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->num_public_inputs = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->pub_inputs_offset = deserialize_from_frs<FF>(elements, num_frs_read);
 
             for (Commitment& commitment : this->get_all()) {
-                commitment = deserialize_from_frs<Commitment>(builder, elements, num_frs_read);
+                commitment = deserialize_from_frs<Commitment>(elements, num_frs_read);
             }
         }
 
@@ -170,7 +170,7 @@ template <typename BuilderType> 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);
         }
     };
 

barretenberg/cpp/src/barretenberg/flavor/ultra_rollup_recursive_flavor.hpp

@@ -86,18 +86,18 @@ template <typename BuilderType> class UltraRollupRecursiveFlavor_ : public Ultra
          * @param builder
          * @param elements
          */
-        VerificationKey(CircuitBuilder& builder, std::span<FF> elements)
+        VerificationKey(std::span<FF> elements)
         {
             using namespace bb::stdlib::field_conversion;
 
             size_t num_frs_read = 0;
 
-            this->log_circuit_size = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->num_public_inputs = deserialize_from_frs<FF>(builder, elements, num_frs_read);
-            this->pub_inputs_offset = deserialize_from_frs<FF>(builder, elements, num_frs_read);
+            this->log_circuit_size = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->num_public_inputs = deserialize_from_frs<FF>(elements, num_frs_read);
+            this->pub_inputs_offset = deserialize_from_frs<FF>(elements, num_frs_read);
 
             for (Commitment& commitment : this->get_all()) {
-                commitment = deserialize_from_frs<Commitment>(builder, elements, num_frs_read);
+                commitment = deserialize_from_frs<Commitment>(elements, num_frs_read);
             }
         }
 
@@ -116,7 +116,7 @@ template <typename BuilderType> 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);
         }
     };
 

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<uint32_t>(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.";

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<Commitment>(); // 4
+    static constexpr size_t eval_frs = bb::field_conversion::calc_num_bn254_frs<FF>();         // 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<BF>();
+
     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<Commitment>(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<std::ptrdiff_t>(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<Commitment>();
-            // 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<BF>();
-            // 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<Commitment>(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<std::ptrdiff_t>(offset));
-        };
-
         tamper_with_op_commitment(proof.translator_proof);
         // Construct and check the Goblin Recursive Verifier circuit
 

barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp

@@ -293,7 +293,7 @@ template <typename RecursiveFlavor> class RecursiveVerifierTest : public testing
         }
         // Check the size of the recursive verifier
         if constexpr (std::same_as<RecursiveFlavor, MegaZKRecursiveFlavor_<UltraCircuitBuilder>>) {
-            uint32_t NUM_GATES_EXPECTED = 796978;
+            uint32_t NUM_GATES_EXPECTED = 801953;
             ASSERT_EQ(static_cast<uint32_t>(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.";

barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp

@@ -45,6 +45,7 @@ template <class Builder_, class Fq, class Fr, class NativeGroup> 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;