feat!: Removes normalize() calls on pairing points

barretenberg/cpp/scripts/test_civc_standalone_vks_havent_changed.sh

@@ -8,7 +8,7 @@ cd ..
 # IF A VK CHANGE IS EXPECTED - we need to redo this:
 # - Generate inputs: $root/yarn-project/end-to-end/bootstrap.sh generate_example_app_ivc_inputs
 # - Upload the compressed results: aws s3 cp bb-civc-inputs-[version].tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[version].tar.gz
-pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-v6.tar.gz"
+pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-v8.tar.gz"
 
 export inputs_tmp_dir=$(mktemp -d)
 trap 'rm -rf "$inputs_tmp_dir"' EXIT SIGINT

barretenberg/cpp/src/barretenberg/benchmark/ultra_bench/mock_circuits.hpp

@@ -20,9 +20,6 @@ namespace bb::mock_circuits {
  */
 template <typename Builder> void generate_basic_arithmetic_circuit(Builder& builder, size_t log2_num_gates)
 {
-    // Add default pairing points as its required, but this causes gates to be created...
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/984): Get rid of gates when creating default
-    // pairing points.
     stdlib::recursion::PairingPoints<Builder>::add_default_to_public_inputs(builder);
 
     stdlib::field_t a(stdlib::witness_t(&builder, fr::random_element()));

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_bigfield.test.cpp

@@ -41,7 +41,7 @@ using witness_ct = bn254::witness_ct;
  */
 void fix_bigfield_element(const fq_ct& element)
 {
-    for (int i = 0; i < 4; i++) {
+    for (size_t i = 0; i < 4; i++) {
         element.binary_basis_limbs[i].element.fix_witness();
     }
     element.prime_basis_limb.fix_witness();

barretenberg/cpp/src/barretenberg/commitment_schemes/pairing_points.hpp

@@ -78,6 +78,11 @@ class PairingPoints {
      */
     void aggregate(const PairingPoints& other)
     {
+        if (P0 == Point::infinity() || P1 == Point::infinity() || other.P0 == Point::infinity() ||
+            other.P1 == Point::infinity()) {
+            throw_or_abort("WARNING: Shouldn't be aggregating with Point at infinity! The pairing points are probably "
+                           "uninitialized.");
+        }
         Fr aggregation_separator = Fr::random_element();
         P0 = P0 + other.P0 * aggregation_separator;
         P1 = P1 + other.P1 * aggregation_separator;

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

@@ -94,16 +94,17 @@ void create_dummy_vkey_and_proof(typename Flavor::CircuitBuilder& builder,
         builder.assert_equal(builder.add_variable(fr::random_element()), proof_fields[offset].witness_index);
         offset++;
     }
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Using SMALL_DUMMY_VALUE might resolve this
-    // issue.
-    fr SMALL_DUMMY_VALUE(2); // arbtirary small value that shouldn't cause builder problems.
-    // The aggregation object
+
+    // Get some values for a valid aggregation object and use them here to avoid divide by 0 or other issues.
+    std::array<fr, PairingPoints<Builder>::PUBLIC_INPUTS_SIZE> dummy_pairing_points_values =
+        PairingPoints<Builder>::construct_dummy_pairing_points();
     for (size_t i = 0; i < PairingPoints<Builder>::PUBLIC_INPUTS_SIZE; i++) {
-        builder.assert_equal(builder.add_variable(SMALL_DUMMY_VALUE), proof_fields[offset].witness_index);
+        builder.assert_equal(builder.add_variable(dummy_pairing_points_values[i]), proof_fields[offset].witness_index);
         offset++;
     }
 
     // IPA claim
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1392): Don't use random elements here.
     if constexpr (HasIPAAccumulator<Flavor>) {
         for (size_t i = 0; i < bb::IPA_CLAIM_SIZE; i++) {
             builder.assert_equal(builder.add_variable(fr::random_element()), proof_fields[offset].witness_index);

barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp

@@ -145,9 +145,9 @@ std::vector<ClientIVC::FF> create_mock_oink_proof(const size_t num_public_inputs
     std::vector<FF> proof;
 
     // Populate mock public inputs
-    FF MAGIC_PUBLIC_INPUT = 2; // arbitrary small non-zero value to avoid errors
+    FF SMALL_DUMMY_VALUE = 2; // arbitrary small non-zero value to avoid errors
     for (size_t i = 0; i < num_public_inputs; ++i) {
-        proof.emplace_back(MAGIC_PUBLIC_INPUT);
+        proof.emplace_back(SMALL_DUMMY_VALUE);
     }
 
     // Populate mock witness polynomial commitments

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

@@ -133,10 +133,6 @@ UltraRecursiveVerifier_<Flavor>::Output UltraRecursiveVerifier_<Flavor>::verify_
                                                sumcheck_output.claimed_libra_evaluation);
 
     auto pairing_points = PCS::reduce_verify_batch_opening_claim(opening_claim, transcript);
-
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Investigate if normalize() calls are needed.
-    pairing_points[0] = pairing_points[0].normalize();
-    pairing_points[1] = pairing_points[1].normalize();
     output.points_accumulator.aggregate(pairing_points);
 
     // Extract the IPA claim from the public inputs

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

@@ -249,7 +249,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 = 871733;
+            uint32_t NUM_GATES_EXPECTED = 871531;
             BB_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 "

barretenberg/cpp/src/barretenberg/stdlib/plonk_recursion/pairing_points.hpp

@@ -151,35 +151,67 @@ template <typename Builder_> struct PairingPoints {
         Group P1 = Group::reconstruct_from_public(P1_limbs);
         return { P0, P1 };
     }
+
+    static std::array<fr, PUBLIC_INPUTS_SIZE> construct_dummy_pairing_points()
+    {
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): These are pairing points extracted
+        // from a valid proof. This is a workaround because we can't represent the point at infinity in biggroup yet.
+        fq x0("0x031e97a575e9d05a107acb64952ecab75c020998797da7842ab5d6d1986846cf");
+        fq y0("0x178cbf4206471d722669117f9758a4c410db10a01750aebb5666547acf8bd5a4");
+        fq x1("0x0f94656a2ca489889939f81e9c74027fd51009034b3357f0e91b8a11e7842c38");
+        fq y1("0x1b52c2020d7464a0c80c0da527a08193fe27776f50224bd6fb128b46c1ddb67f");
+
+        // We just biggroup here instead of Group (which is either biggroup or biggroup_goblin) because this is the most
+        // efficient way of setting the default pairing points. If we use biggroup_goblin elements, we have to convert
+        // them back to biggroup elements anyway to add them to the public inputs...
+        using BigGroup = element_default::
+            element<Builder, bigfield<Builder, bb::Bn254FqParams>, field_t<Builder>, curve::BN254::Group>;
+        BigGroup P0(x0, y0);
+        BigGroup P1(x1, y1);
+        std::array<fr, PUBLIC_INPUTS_SIZE> dummy_pairing_points_values;
+        size_t idx = 0;
+        std::array<bigfield<Builder, bb::Bn254FqParams>, 4> elements = { P0.x, P0.y, P1.x, P1.y };
+        for (auto& element : elements) {
+            for (auto& limb : element.binary_basis_limbs) {
+                dummy_pairing_points_values[idx++] = limb.element.get_value();
+            }
+        }
+        return dummy_pairing_points_values;
+    }
+
     /**
-     * @brief Constructs an arbitrary but valid aggregation state from a valid set of pairing inputs.
+     * @brief Adds default public inputs to the builder.
+     * @details This should cost exactly 20 gates because there's 4 bigfield elements and each have 5 total
+     * witnesses including the prime limb.
      *
      * @param builder
-     * @return PairingPoints<Builder>
      */
-    static PairingPoints<Builder> construct_default(typename Curve::Builder& builder)
-    {
-        using BaseField = typename Curve::BaseField;
-        // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): These are pairing points extracted from a
-        // valid proof. This is a workaround because we can't represent the point at infinity in biggroup yet.
-        uint256_t x0_val("0x031e97a575e9d05a107acb64952ecab75c020998797da7842ab5d6d1986846cf");
-        uint256_t y0_val("0x178cbf4206471d722669117f9758a4c410db10a01750aebb5666547acf8bd5a4");
-        uint256_t x1_val("0x0f94656a2ca489889939f81e9c74027fd51009034b3357f0e91b8a11e7842c38");
-        uint256_t y1_val("0x1b52c2020d7464a0c80c0da527a08193fe27776f50224bd6fb128b46c1ddb67f");
-        BaseField x0 = BaseField::from_witness(&builder, x0_val);
-        BaseField y0 = BaseField::from_witness(&builder, y0_val);
-        BaseField x1 = BaseField::from_witness(&builder, x1_val);
-        BaseField y1 = BaseField::from_witness(&builder, y1_val);
-        // PairingPoints<Builder> points_accumulator{ Group(x0, y0), Group(x1, y1) };
-        return { Group(x0, y0), Group(x1, y1) };
-    }
-
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/984): Check how many gates this costs and if they're
-    // necessary.
     static void add_default_to_public_inputs(Builder& builder)
     {
-        PairingPoints<Builder> points_accumulator = construct_default(builder);
-        points_accumulator.set_public();
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): These are pairing points extracted
+        // from a valid proof. This is a workaround because we can't represent the point at infinity in biggroup yet.
+        fq x0("0x031e97a575e9d05a107acb64952ecab75c020998797da7842ab5d6d1986846cf");
+        fq y0("0x178cbf4206471d722669117f9758a4c410db10a01750aebb5666547acf8bd5a4");
+        fq x1("0x0f94656a2ca489889939f81e9c74027fd51009034b3357f0e91b8a11e7842c38");
+        fq y1("0x1b52c2020d7464a0c80c0da527a08193fe27776f50224bd6fb128b46c1ddb67f");
+
+        // We just biggroup here instead of Group (which is either biggroup or biggroup_goblin) because this is the most
+        // efficient way of setting the default pairing points. If we use biggroup_goblin elements, we have to convert
+        // them back to biggroup elements anyway to add them to the public inputs...
+        using BigGroup = element_default::
+            element<Builder, bigfield<Builder, bb::Bn254FqParams>, field_t<Builder>, curve::BN254::Group>;
+        BigGroup P0(x0, y0);
+        BigGroup P1(x1, y1);
+        P0.convert_constant_to_fixed_witness(&builder);
+        P1.convert_constant_to_fixed_witness(&builder);
+        if (builder.pairing_inputs_public_input_key.is_set()) {
+            throw_or_abort("Error: trying to set PairingPoints as public inputs when it already contains one.");
+        }
+        uint32_t start_idx = P0.set_public();
+        P1.set_public();
+
+        builder.pairing_inputs_public_input_key.start_idx = start_idx;
+        info("Num gates after set_public: ", builder.num_gates);
     }
 };
 

barretenberg/cpp/src/barretenberg/stdlib/plonk_recursion/pairing_points.test.cpp

@@ -0,0 +1,24 @@
+#include "barretenberg/stdlib/plonk_recursion/pairing_points.hpp"
+#include "barretenberg/srs/global_crs.hpp"
+#include <gtest/gtest.h>
+
+namespace bb::stdlib::recursion {
+
+template <typename Builder> class PairingPointsTests : public testing::Test {
+  public:
+    static void SetUpTestSuite() { bb::srs::init_file_crs_factory(bb::srs::bb_crs_path()); }
+};
+
+using Builders = testing::Types<UltraCircuitBuilder, MegaCircuitBuilder>;
+TYPED_TEST_SUITE(PairingPointsTests, Builders);
+
+TYPED_TEST(PairingPointsTests, ConstructDefault)
+{
+    TypeParam builder;
+    info("Num gates: ", builder.num_gates);
+    PairingPoints<TypeParam>::add_default_to_public_inputs(builder);
+    info("Num gates after add_default_to_public_inputs: ", builder.num_gates);
+    builder.finalize_circuit(/*ensure_nonzero=*/true);
+    info("Num gates: ", builder.num_gates);
+}
+} // namespace bb::stdlib::recursion

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

@@ -62,7 +62,7 @@ template <typename Builder, typename T> class bigfield {
     static constexpr size_t NUM_LIMBS = 4;
 
     Builder* context;
-    mutable Limb binary_basis_limbs[NUM_LIMBS];
+    mutable std::array<Limb, NUM_LIMBS> binary_basis_limbs;
     mutable field_t<Builder> prime_basis_limb;
 
     bigfield(const field_t<Builder>& low_bits,

barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.hpp

@@ -173,8 +173,8 @@ template <IsUltraOrMegaHonk Flavor> class DeciderProvingKey_ {
 
             // Set the pairing point accumulator indices. This should exist for all flavors.
             ASSERT(circuit.pairing_inputs_public_input_key.is_set() &&
-                   "Honk circuit must output a pairing point accumulator. If this is a test, you might need to add a "
-                   "default one through a method in PairingPoints.");
+                   "Honk circuit must output a pairing point accumulator. If this is a test, you might need to add a \
+                   default one through a method in PairingPoints.");
             proving_key.pairing_inputs_public_input_key = circuit.pairing_inputs_public_input_key;
 
             if constexpr (HasIPAAccumulator<Flavor>) { // Set the IPA claim indices

barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp

@@ -75,6 +75,14 @@ template <typename Flavor> bool UltraVerifier_<Flavor>::verify_proof(const HonkP
 
     DeciderVerifier decider_verifier{ verification_key, transcript };
     auto decider_output = decider_verifier.verify();
+    if (!decider_output.sumcheck_verified) {
+        info("Sumcheck failed!");
+        return false;
+    }
+    if (!decider_output.libra_evals_verified) {
+        info("Libra evals failed!");
+        return false;
+    }
 
     // Extract nested pairing points from the proof
     // TODO(https://github.com/AztecProtocol/barretenberg/issues/1094): Handle pairing points in keccak flavors.

barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp

@@ -155,10 +155,6 @@ AvmRecursiveVerifier_<Flavor>::PairingPoints AvmRecursiveVerifier_<Flavor>::veri
         padding_indicator_array, claim_batcher, output.challenge, Commitment::one(&builder), transcript);
 
     auto pairing_points = PCS::reduce_verify_batch_opening_claim(opening_claim, transcript);
-
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1352): Investigate if normalize() calls are needed.
-    pairing_points[0] = pairing_points[0].normalize();
-    pairing_points[1] = pairing_points[1].normalize();
     return pairing_points;
 }