fix: remove finalize from acir create circuit (#6585)

Largely undoing the changes made in
[this](AztecProtocol/aztec-packages#6164) PR
that added a call to `finalize_circuit` in the create_circuit methods in
acir_format. This change was originally made in order to facilitate
accurate final gate counts (previously handled via a hack). The finalize
model has been reverted and the hack has been replaced with a method
that computes the number of gates added by the `add_gates_to_ensure..`
methods in the builders. This change was necessary in order to allow
gates to be added to a circuit generated from acir, which is needed in
ClientIvc accumulation.

cpp/src/barretenberg/bb/main.cpp

@@ -164,7 +164,8 @@ bool proveAndVerifyHonkAcirFormat(acir_format::AcirFormat constraint_system, aci
     // Construct a bberg circuit from the acir representation
     auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, witness);
 
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
+    auto num_extra_gates = builder.get_num_gates_added_to_ensure_nonzero_polynomials();
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + num_extra_gates);
     init_bn254_crs(srs_size);
 
     // Construct Honk proof
@@ -591,8 +592,8 @@ void prove_honk(const std::string& bytecodePath, const std::string& witnessPath,
 
     auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, witness);
 
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
-
+    auto num_extra_gates = builder.get_num_gates_added_to_ensure_nonzero_polynomials();
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + num_extra_gates);
     init_bn254_crs(srs_size);
 
     // Construct Honk proof
@@ -662,7 +663,8 @@ template <IsUltraFlavor Flavor> void write_vk_honk(const std::string& bytecodePa
     auto constraint_system = get_constraint_system(bytecodePath);
     auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, {});
 
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
+    auto num_extra_gates = builder.get_num_gates_added_to_ensure_nonzero_polynomials();
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + num_extra_gates);
     init_bn254_crs(srs_size);
 
     ProverInstance prover_inst(builder);

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

@@ -349,8 +349,6 @@ UltraCircuitBuilder create_circuit(const AcirFormat& constraint_system,
     bool has_valid_witness_assignments = !witness.empty();
     build_constraints(builder, constraint_system, has_valid_witness_assignments, honk_recursion);
 
-    builder.finalize_circuit();
-
     return builder;
 };
 
@@ -378,8 +376,6 @@ GoblinUltraCircuitBuilder create_circuit(const AcirFormat& constraint_system,
     bool has_valid_witness_assignments = !witness.empty();
     acir_format::build_constraints(builder, constraint_system, has_valid_witness_assignments, honk_recursion);
 
-    builder.finalize_circuit();
-
     return builder;
 };
 

cpp/src/barretenberg/examples/join_split/join_split.test.cpp

@@ -701,7 +701,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change)
     // The below part detects any changes in the join-split circuit
     constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16;
 
-    constexpr uint256_t CIRCUIT_HASH("0xc9d7e9c29d8555514b4ee1ed5cccc6da7e5a81585c6404f7ce379404f07414ec");
+    constexpr uint256_t CIRCUIT_HASH("0x470358e4d91c4c5296ef788b1165b2c439cd498f49c3f99386b002753ca3d0ee");
 
     const uint256_t circuit_hash = circuit.hash_circuit();
     // circuit is finalized now

cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp

@@ -12,10 +12,8 @@ namespace bb {
 
 template <typename FF> void GoblinUltraCircuitBuilder_<FF>::finalize_circuit()
 {
-    if (!this->circuit_finalized) {
-        add_goblin_gates_to_ensure_all_polys_are_non_zero();
-        UltraCircuitBuilder_<UltraHonkArith<FF>>::finalize_circuit();
-    }
+    // All of the gates involved in finalization are part of the Ultra arithmetization
+    UltraCircuitBuilder_<UltraHonkArith<FF>>::finalize_circuit();
 }
 
 /**
@@ -28,8 +26,11 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::finalize_circuit()
 // TODO(#423): This function adds valid (but arbitrary) gates to ensure that the circuit which includes
 // them will not result in any zero-polynomials. It also ensures that the first coefficient of the wire
 // polynomials is zero, which is required for them to be shiftable.
-template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_goblin_gates_to_ensure_all_polys_are_non_zero()
+template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_gates_to_ensure_all_polys_are_non_zero()
 {
+    // Most polynomials are handled via the conventional Ultra method
+    UltraCircuitBuilder_<UltraHonkArith<FF>>::add_gates_to_ensure_all_polys_are_non_zero();
+
     // All that remains is to handle databus related and poseidon2 related polynomials. In what follows we populate the
     // calldata with some mock data then constuct a single calldata read gate
 

cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp

@@ -109,7 +109,7 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
     }
 
     void finalize_circuit();
-    void add_goblin_gates_to_ensure_all_polys_are_non_zero();
+    void add_gates_to_ensure_all_polys_are_non_zero();
 
     size_t get_num_constant_gates() const override { return 0; }
 
@@ -130,6 +130,22 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
         return num_ultra_gates + num_goblin_ecc_op_gates;
     }
 
+    /**
+     * @brief Dynamically compute the number of gates added by the "add_gates_to_ensure_all_polys_are_non_zero" method
+     * @note This does NOT add the gates to the present builder
+     *
+     */
+    size_t get_num_gates_added_to_ensure_nonzero_polynomials()
+    {
+        GoblinUltraCircuitBuilder_<FF> builder; // instantiate new builder
+
+        size_t num_gates_prior = builder.get_num_gates();
+        builder.add_gates_to_ensure_all_polys_are_non_zero();
+        size_t num_gates_post = builder.get_num_gates(); // accounts for finalization gates
+
+        return num_gates_post - num_gates_prior;
+    }
+
     /**x
      * @brief Print the number and composition of gates in the circuit
      *

cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp

@@ -40,12 +40,14 @@ template <typename Arithmetization> void UltraCircuitBuilder_<Arithmetization>::
      * our circuit is finalized, and we must not to execute these functions again.
      */
     if (!circuit_finalized) {
-        add_gates_to_ensure_all_polys_are_non_zero();
         process_non_native_field_multiplications();
         process_ROM_arrays();
         process_RAM_arrays();
         process_range_lists();
         circuit_finalized = true;
+    } else {
+        // Gates added after first call to finalize will not be processed since finalization is only performed once
+        info("WARNING: Redudant call to finalize_circuit(). Is this intentional?");
     }
 }
 

cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp

@@ -605,6 +605,22 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization_
         return count + romcount + ramcount + rangecount + nnfcount;
     }
 
+    /**
+     * @brief Dynamically compute the number of gates added by the "add_gates_to_ensure_all_polys_are_non_zero" method
+     * @note This does NOT add the gates to the present builder
+     *
+     */
+    size_t get_num_gates_added_to_ensure_nonzero_polynomials()
+    {
+        UltraCircuitBuilder_<Arithmetization> builder; // instantiate new builder
+
+        size_t num_gates_prior = builder.get_num_gates();
+        builder.add_gates_to_ensure_all_polys_are_non_zero();
+        size_t num_gates_post = builder.get_num_gates(); // accounts for finalization gates
+
+        return num_gates_post - num_gates_prior;
+    }
+
     /**
      * @brief Get combined size of all tables used in circuit
      *

cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp

@@ -44,6 +44,7 @@ template <class Flavor> class ProverInstance_ {
     ProverInstance_(Circuit& circuit, bool is_structured = false)
     {
         BB_OP_COUNT_TIME_NAME("ProverInstance(Circuit&)");
+        circuit.add_gates_to_ensure_all_polys_are_non_zero();
         circuit.finalize_circuit();
 
         // If using a structured trace, ensure that no block exceeds the fixed size