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-v7.tar.gz"
+pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-7f48a235.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

@@ -18,9 +18,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/client_ivc/client_ivc.cpp

@@ -404,7 +404,6 @@ HonkProof ClientIVC::decider_prove() const
     vinfo("prove decider...");
     fold_output.accumulator->proving_key.commitment_key = bn254_commitment_key;
     MegaDeciderProver decider_prover(fold_output.accumulator);
-    vinfo("finished decider proving.");
     return decider_prover.construct_proof();
 }
 

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();
     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,11 +145,32 @@ 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
-    for (size_t i = 0; i < num_public_inputs; ++i) {
-        proof.emplace_back(MAGIC_PUBLIC_INPUT);
+    // Get some values for a valid aggregation object and use them here to avoid divide by 0 or other issues.
+    std::array<fr, stdlib::recursion::PairingPoints<MegaCircuitBuilder>::PUBLIC_INPUTS_SIZE>
+        dummy_pairing_points_values = stdlib::recursion::PairingPoints<MegaCircuitBuilder>::construct_dummy();
+    size_t public_input_count = 0;
+    for (size_t i = 0; i < stdlib::recursion::PairingPoints<MegaCircuitBuilder>::PUBLIC_INPUTS_SIZE; i++) {
+        proof.emplace_back(dummy_pairing_points_values[i]);
+        public_input_count++;
     }
 
+    if (public_input_count < num_public_inputs) {
+        // Databus commitments if necessary
+        for (size_t i = 0; i < NUM_DATABUS_COMMITMENTS; ++i) {
+            // We represent commitments in the public inputs as biggroup elements.
+            using BigGroup = stdlib::element_default::element<MegaCircuitBuilder,
+                                                              stdlib::bigfield<MegaCircuitBuilder, bb::Bn254FqParams>,
+                                                              stdlib::field_t<MegaCircuitBuilder>,
+                                                              curve::BN254::Group>;
+            auto pub_input_comm_vals = BigGroup::construct_dummy();
+            for (const fr& comm_fr : pub_input_comm_vals) {
+                proof.emplace_back(comm_fr);
+                public_input_count++;
+            }
+        }
+    }
+    BB_ASSERT_EQ(public_input_count, num_public_inputs, "Mock oink proof has the wrong number of public inputs.");
+
     // Populate mock witness polynomial commitments
     auto mock_commitment = curve::BN254::AffineElement::one();
     std::vector<FF> mock_commitment_frs = field_conversion::convert_to_bn254_frs(mock_commitment);

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

@@ -138,10 +138,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/pairing_points.hpp

@@ -119,13 +119,36 @@ 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()
+    {
+        // 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>;
+        std::array<fr, PUBLIC_INPUTS_SIZE> dummy_pairing_points_values;
+        size_t idx = 0;
+        for (size_t i = 0; i < 2; i++) {
+            std::array<fr, BigGroup::PUBLIC_INPUTS_SIZE> element_vals = BigGroup::construct_dummy();
+            for (auto& val : element_vals) {
+                dummy_pairing_points_values[idx++] = val;
+            }
+        }
+
+        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)
+    // 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)
     {
         using BaseField = typename Curve::BaseField;
         // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): These are pairing points extracted from a
@@ -138,15 +161,7 @@ template <typename Builder_> struct PairingPoints {
         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);
+        PairingPoints<Builder> points_accumulator{ Group(x0, y0), Group(x1, y1) };
         points_accumulator.set_public();
     }
 };

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

@@ -0,0 +1,25 @@
+#include "barretenberg/stdlib/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);
+    EXPECT_TRUE(CircuitChecker::check(builder));
+}
+} // 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/stdlib/primitives/biggroup/biggroup.hpp

@@ -48,6 +48,21 @@ template <class Builder, class Fq, class Fr, class NativeGroup> class element {
     element(const element& other);
     element(element&& other) noexcept;
 
+    static std::array<fr, PUBLIC_INPUTS_SIZE> construct_dummy()
+    {
+        const typename NativeGroup::affine_element& native_val = NativeGroup::affine_element::one();
+        element val(native_val);
+        size_t idx = 0;
+        std::array<fr, PUBLIC_INPUTS_SIZE> limb_vals;
+        for (auto& limb : val.x.binary_basis_limbs) {
+            limb_vals[idx++] = limb.element.get_value();
+        }
+        for (auto& limb : val.y.binary_basis_limbs) {
+            limb_vals[idx++] = limb.element.get_value();
+        }
+        BB_ASSERT_EQ(idx, PUBLIC_INPUTS_SIZE);
+        return limb_vals;
+    }
     /**
      * @brief Set the witness indices for the x and y coordinates to public
      *

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/databus.hpp

@@ -13,8 +13,9 @@
 namespace bb {
 
 // We assume all kernels have space for two return data commitments on their public inputs
+constexpr uint32_t NUM_DATABUS_COMMITMENTS = 2;
 constexpr uint32_t PROPAGATED_DATABUS_COMMITMENT_SIZE = 8;
-constexpr uint32_t PROPAGATED_DATABUS_COMMITMENTS_SIZE = PROPAGATED_DATABUS_COMMITMENT_SIZE * 2; // Two databus comms
+constexpr uint32_t PROPAGATED_DATABUS_COMMITMENTS_SIZE = PROPAGATED_DATABUS_COMMITMENT_SIZE * NUM_DATABUS_COMMITMENTS;
 
 /**
  * @brief A DataBus column

barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp

@@ -115,7 +115,7 @@ template <IsUltraOrMegaHonk Flavor> HonkProof DeciderProver_<Flavor>::construct_
     // Fiat-Shamir: rho, y, x, z
     // Execute Shplemini PCS
     execute_pcs_rounds();
-
+    vinfo("finished decider proving.");
     return export_proof();
 }
 

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/decider_verifier.hpp

@@ -27,7 +27,14 @@ template <typename Flavor> class DeciderVerifier_ {
         bool libra_evals_verified;
         PairingPoints pairing_points;
 
-        bool check() { return sumcheck_verified && libra_evals_verified && pairing_points.check(); }
+        bool check()
+        {
+            bool pairing_check_verified = pairing_points.check();
+            vinfo("sumcheck_verified: ", sumcheck_verified);
+            vinfo("libra_evals_verified: ", libra_evals_verified);
+            vinfo("pairing_check_verified: ", pairing_check_verified);
+            return sumcheck_verified && libra_evals_verified && pairing_check_verified;
+        }
     };
 
   public:

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;
 }