chore: add goblin ops in add_gates_to_ensure_all_polys_are_non_zero

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

@@ -38,14 +38,20 @@ class ClientIVCTests : public ::testing::Test {
 
     /**
      * @brief Construct mock circuit with arithmetic gates and goblin ops
-     * @details Currently default sized to 2^16 to match kernel. (Note: op gates will bump size to next power of
-     2)
+     * @details Currently default sized to 2^16 to match kernel. (Note: dummy op gates added to avoid non-zero
+     * polynomials will bump size to next power of 2)
      *
      */
     static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 15)
     {
         Builder circuit{ ivc.goblin.op_queue };
         MockCircuits::construct_arithmetic_circuit(circuit, log2_num_gates);
+
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): We require goblin ops to be added to the
+        // function circuit because we cannot support zero commtiments. While the builder handles this at
+        // finalisation stage via the add_gates_to_ensure_all_polys_are_non_zero function for other UGH
+        // circuits (where we don't explicitly need to add goblin ops), in ClientIVC merge proving happens prior to
+        // folding where the absense of goblin ecc ops will result in zero commitments.
         MockCircuits::construct_goblin_ecc_op_circuit(circuit);
         return circuit;
     }

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

@@ -247,10 +247,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);
 
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/817): Add some arbitrary op gates to ensure the
-    // associated polynomials are non-zero and to give ECCVM and Translator some ECC ops to process.
-    MockCircuits::construct_goblin_ecc_op_circuit(builder);
-
     return builder;
 };
 

barretenberg/cpp/src/barretenberg/dsl/acir_proofs/goblin_acir_composer.cpp

@@ -17,7 +17,7 @@ void GoblinAcirComposer::create_circuit(acir_format::AcirFormat& constraint_syst
     acir_format::build_constraints(builder_, constraint_system, true);
 
     // TODO(https://github.com/AztecProtocol/barretenberg/issues/817): Add some arbitrary op gates to ensure the
-    // associated polynomials are non-zero and to give ECCVM and Translator some ECC ops to process.
+    //  to give ECCVM and Translator some ECC ops to process.
     MockCircuits::construct_goblin_ecc_op_circuit(builder_);
 }
 

barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp

@@ -45,7 +45,8 @@ TEST_F(GoblinRecursionTests, Vanilla)
     size_t NUM_CIRCUITS = 2;
     for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) {
 
-        // Construct and accumulate a mock function circuit
+        // Construct and accumulate a mock function circuit containing both arbitrary arithmetic gates and goblin
+        // ecc op gates to make it a meaningful test
         GoblinUltraCircuitBuilder function_circuit{ goblin.op_queue };
         MockCircuits::construct_arithmetic_circuit(function_circuit, /*target_log2_dyadic_size=*/8);
         MockCircuits::construct_goblin_ecc_op_circuit(function_circuit);

barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp

@@ -70,9 +70,11 @@ class GoblinMockCircuits {
         stdlib::generate_ecdsa_verification_test_circuit(builder, NUM_ITERATIONS); // min gates: ~41k
         stdlib::generate_merkle_membership_test_circuit(builder, NUM_ITERATIONS);  // min gates: ~29k
 
-        // Note: its not clear whether goblin ops will be supported for function circuits initially but currently
-        // UGH can only be used if some op gates are included so for now we'll assume each function circuit has
-        // some.
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): We require goblin ops to be added to the
+        // function circuit because we cannot support zero commtiments. While the builder handles this at
+        // ProverInstance creation stage via the add_gates_to_ensure_all_polys_are_non_zero function for other UGH
+        // circuits (where we don't explicitly need to add goblin ops), in ClientIVC merge proving happens prior to
+        // folding where the absense of goblin ecc ops will result in zero commitments.
         MockCircuits::construct_goblin_ecc_op_circuit(builder);
     }
 

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp

@@ -82,6 +82,10 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_gates_to_ensure_
     // dummy gate to be read into by previous poseidon internal gate via shifts
     this->create_dummy_gate(
         this->blocks.poseidon_internal, this->zero_idx, this->zero_idx, this->zero_idx, this->zero_idx);
+
+    // add dummy mul accum op and an equality op
+    this->queue_ecc_mul_accum(bb::g1::affine_element::one() * FF::random_element(), FF::random_element());
+    this->queue_ecc_eq();
 }
 
 /**

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mock_circuits.hpp

@@ -62,16 +62,15 @@ class MockCircuits {
     }
 
     /**
-     * @brief Populate a builder with some arbitrary goblinized ECC ops
+     * @brief Populate a builder with some arbitrary goblinized ECC ops, one of each type
      *
      * @param builder
      */
     static void construct_goblin_ecc_op_circuit(GoblinUltraCircuitBuilder& builder)
     {
-        // Add a mul accum op and an equality op
-        auto point = Point::one() * FF::random_element();
-        auto scalar = FF::random_element();
-        builder.queue_ecc_mul_accum(point, scalar);
+        // Add a mul accum op, an add accum op and an equality op
+        builder.queue_ecc_add_accum(Point::one() * FF::random_element());
+        builder.queue_ecc_mul_accum(Point::one() * FF::random_element(), FF::random_element());
         builder.queue_ecc_eq();
     }
 };