fix: Correct circuit size estimation for UltraHonk (#6164)

Resolves AztecProtocol/aztec-packages#6161

Look into the issue for more context on the problem.

I moved `add_gates_to_ensure_all_polys_are_non_zero` to be part of
`finalize_circuit` for both the goblin and ultra builder. Then upon
creating an AcirFormat circuit we now call finalize_circuit. Previously
we were having to attach an additional buffer as a hack as we were using
`get_total_circuit_size` which correctly estimated a finalized circuit
size for plonk, but not for Honk. We will now have these additional
gates for UltraPlonk as well, but this is fine as they are not that many
gates (~12 gates) and we are removing plonk anyway.

acir_tests/run_acir_tests.sh

@@ -35,7 +35,7 @@ export BIN CRS_PATH VERBOSE BRANCH
 cd acir_tests
 
 # Convert them to array
-SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member witness_compression)
+SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member)
 
 function test() {
   cd $1

cpp/src/barretenberg/bb/main.cpp

@@ -164,10 +164,7 @@ 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);
 
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Add a buffer to the expected circuit size to
-    // account for the addition of "gates to ensure nonzero polynomials" (in Honk only).
-    const size_t additional_gates_buffer = 15; // conservatively large to be safe
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + additional_gates_buffer);
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
     init_bn254_crs(srs_size);
 
     // Construct Honk proof
@@ -601,8 +598,8 @@ void prove_honk(const std::string& bytecodePath, const std::string& witnessPath,
 
     auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, witness);
 
-    const size_t additional_gates_buffer = 15; // conservatively large to be safe
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + additional_gates_buffer);
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
+
     init_bn254_crs(srs_size);
 
     // Construct Honk proof
@@ -672,8 +669,7 @@ 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, {});
 
-    const size_t additional_gates_buffer = 15; // conservatively large to be safe
-    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size() + additional_gates_buffer);
+    size_t srs_size = builder.get_circuit_subgroup_size(builder.get_total_circuit_size());
     init_bn254_crs(srs_size);
 
     ProverInstance prover_inst(builder);

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

@@ -226,6 +226,8 @@ UltraCircuitBuilder create_circuit(const AcirFormat& constraint_system, size_t s
     bool has_valid_witness_assignments = !witness.empty();
     build_constraints(builder, constraint_system, has_valid_witness_assignments);
 
+    builder.finalize_circuit();
+
     return builder;
 };
 
@@ -252,6 +254,8 @@ 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);
 
+    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("0x470358e4d91c4c5296ef788b1165b2c439cd498f49c3f99386b002753ca3d0ee");
+    constexpr uint256_t CIRCUIT_HASH("0xc9d7e9c29d8555514b4ee1ed5cccc6da7e5a81585c6404f7ce379404f07414ec");
 
     const uint256_t circuit_hash = circuit.hash_circuit();
     // circuit is finalized now

cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp

@@ -12,22 +12,24 @@ namespace bb {
 
 template <typename FF> void GoblinUltraCircuitBuilder_<FF>::finalize_circuit()
 {
-    UltraCircuitBuilder_<UltraHonkArith<FF>>::finalize_circuit();
+    if (!this->circuit_finalized) {
+        add_goblin_gates_to_ensure_all_polys_are_non_zero();
+        UltraCircuitBuilder_<UltraHonkArith<FF>>::finalize_circuit();
+    }
 }
 
 /**
- * @brief Ensure all polynomials have at least one non-zero coefficient to avoid commiting to the zero-polynomial
+ * @brief Ensure all polynomials have at least one non-zero coefficient to avoid commiting to the zero-polynomial.
+ *        This only adds gates for the Goblin polynomials. Most polynomials are handled via the Ultra method,
+ *        which should be done by a separate call to the Ultra builder's non zero polynomial gates method.
  *
  * @param in Structure containing variables and witness selectors
  */
 // 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_gates_to_ensure_all_polys_are_non_zero()
+template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_goblin_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_gates_to_ensure_all_polys_are_non_zero();
+    void add_goblin_gates_to_ensure_all_polys_are_non_zero();
 
     size_t get_num_constant_gates() const override { return 0; }
 

cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp

@@ -40,6 +40,7 @@ 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();

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

@@ -44,8 +44,8 @@ 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
         if (is_structured) {
             for (auto& block : circuit.blocks.get()) {

cpp/src/barretenberg/ultra_honk/ultra_prover.cpp

@@ -77,6 +77,7 @@ template <IsUltraFlavor Flavor> HonkProof& UltraProver_<Flavor>::construct_proof
     OinkProver<Flavor> oink_prover(instance->proving_key, transcript);
     auto [proving_key, relation_params, alphas] = oink_prover.prove();
     instance->proving_key = std::move(proving_key);
+
     instance->relation_parameters = std::move(relation_params);
     instance->alphas = alphas;