diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp index a07295684751..60d52d73ba8d 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp +++ b/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; } diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 2cc00db97c71..75db931222ce 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/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; }; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/goblin_acir_composer.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/goblin_acir_composer.cpp index 5d754a168a7c..4ef7c02c3457 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/goblin_acir_composer.cpp +++ b/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_); } diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp index 49b3f08028f8..623c1118b4ab 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp +++ b/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); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index 4f4fbdee852a..cb87096063ee 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/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); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp index 0267a5be4fa4..f6a4c2901a1f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp @@ -82,6 +82,10 @@ template void GoblinUltraCircuitBuilder_::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(); } /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mock_circuits.hpp index 8b27ab0afb21..cb523ce7186a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mock_circuits.hpp +++ b/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(); } };