From c11599e7a1f0021ea67e3841e5adf8e2e24c01cc Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 21 May 2024 22:36:43 +0000 Subject: [PATCH 1/5] remove finalize from acir create circuit --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 10 ++++++---- .../barretenberg/dsl/acir_format/acir_format.cpp | 4 ++-- .../goblin_ultra_circuit_builder.hpp | 16 ++++++++++++++++ .../ultra_circuit_builder.hpp | 15 +++++++++++++++ .../ultra_honk/goblin_ultra_composer.test.cpp | 2 ++ .../ultra_honk/ultra_composer.test.cpp | 3 +++ 6 files changed, 44 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index f8308b5e7c0..1424e3dd582 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/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(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(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 void write_vk_honk(const std::string& bytecodePa auto constraint_system = get_constraint_system(bytecodePath); auto builder = acir_format::create_circuit(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); 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 3dc7fc15e6c..06269a8fe17 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -349,7 +349,7 @@ 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(); + // builder.finalize_circuit(); return builder; }; @@ -378,7 +378,7 @@ 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(); + // builder.finalize_circuit(); return builder; }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index 33f5373cc14..8d6c91c513a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -130,6 +130,22 @@ template 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 + * + */ + size_t get_num_gates_added_to_ensure_nonzero_polynomials() + { + GoblinUltraCircuitBuilder_ builder; // instantiate new builder + + size_t num_gates_prior = builder.get_num_gates(); + builder.add_goblin_gates_to_ensure_all_polys_are_non_zero(); + 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 * diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index feecbf0938d..2d21b763cfc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -605,6 +605,21 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase 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 * diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp index d33607a6bdb..2be24b4f0f8 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp @@ -33,6 +33,8 @@ class GoblinUltraHonkComposerTests : public ::testing::Test { */ bool construct_and_verify_honk_proof(auto& builder) { + auto num_added = builder.get_num_gates_added_to_ensure_nonzero_polynomials(); + info("num added = ", num_added); auto instance = std::make_shared>(builder); GoblinUltraProver prover(instance); auto verification_key = std::make_shared(instance->proving_key); diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp index 330a2ab8c14..703231fc97b 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp @@ -30,6 +30,9 @@ std::vector add_variables(auto& circuit_builder, std::vector v void prove_and_verify(auto& circuit_builder, bool expected_result) { + auto num_added = circuit_builder.get_num_gates_added_to_ensure_nonzero_polynomials(); + info("num added = ", num_added); + auto instance = std::make_shared(circuit_builder); UltraProver prover(instance); auto verification_key = std::make_shared(instance->proving_key); From 4e547e7d3251475a7100342f87f9f8012afbd4d9 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 22 May 2024 13:52:02 +0000 Subject: [PATCH 2/5] revert to the old finalize model --- .../examples/join_split/join_split.test.cpp | 2 +- .../goblin_ultra_circuit_builder.cpp | 11 ++++++----- .../goblin_ultra_circuit_builder.hpp | 3 +-- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 1 - .../sumcheck/instance/prover_instance.hpp | 1 + 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp b/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp index 8d8c239eaec..773c69d8941 100644 --- a/barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp +++ b/barretenberg/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 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 dd12ae51109..5373d837a44 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 @@ -12,10 +12,8 @@ namespace bb { template void GoblinUltraCircuitBuilder_::finalize_circuit() { - if (!this->circuit_finalized) { - add_goblin_gates_to_ensure_all_polys_are_non_zero(); - UltraCircuitBuilder_>::finalize_circuit(); - } + // All of the gates involved in finalization are part of the Ultra arithmetization + UltraCircuitBuilder_>::finalize_circuit(); } /** @@ -28,8 +26,11 @@ template void GoblinUltraCircuitBuilder_::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 void GoblinUltraCircuitBuilder_::add_goblin_gates_to_ensure_all_polys_are_non_zero() +template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_all_polys_are_non_zero() { + // Most polynomials are handled via the conventional Ultra method + UltraCircuitBuilder_>::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 diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index 8d6c91c513a..c81f61e9468 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -109,7 +109,7 @@ template 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; } @@ -139,7 +139,6 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui GoblinUltraCircuitBuilder_ builder; // instantiate new builder size_t num_gates_prior = builder.get_num_gates(); - builder.add_goblin_gates_to_ensure_all_polys_are_non_zero(); builder.add_gates_to_ensure_all_polys_are_non_zero(); size_t num_gates_post = builder.get_num_gates(); // accounts for finalization gates diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index b23e41e4b6c..258a0dfed15 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -40,7 +40,6 @@ template void UltraCircuitBuilder_:: * 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(); diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp index 13bb18d65f8..1e8a38db941 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp @@ -44,6 +44,7 @@ template 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 From 67a0bafdc491455bcd582be49c0319205f8e5606 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 22 May 2024 15:11:14 +0000 Subject: [PATCH 3/5] add duplicate finalize warning --- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index 258a0dfed15..25322635586 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -45,6 +45,9 @@ template void UltraCircuitBuilder_:: 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?"); } } From ed181b48e3ce05cddb5c9537a1edd8cb7ce8c1b0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 22 May 2024 15:35:21 +0000 Subject: [PATCH 4/5] cleanup --- .../cpp/src/barretenberg/dsl/acir_format/acir_format.cpp | 4 ---- .../barretenberg/ultra_honk/goblin_ultra_composer.test.cpp | 2 -- .../cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp | 3 --- 3 files changed, 9 deletions(-) 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 06269a8fe17..b61834c593e 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/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; }; diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp index 2be24b4f0f8..d33607a6bdb 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp @@ -33,8 +33,6 @@ class GoblinUltraHonkComposerTests : public ::testing::Test { */ bool construct_and_verify_honk_proof(auto& builder) { - auto num_added = builder.get_num_gates_added_to_ensure_nonzero_polynomials(); - info("num added = ", num_added); auto instance = std::make_shared>(builder); GoblinUltraProver prover(instance); auto verification_key = std::make_shared(instance->proving_key); diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp index 703231fc97b..330a2ab8c14 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_composer.test.cpp @@ -30,9 +30,6 @@ std::vector add_variables(auto& circuit_builder, std::vector v void prove_and_verify(auto& circuit_builder, bool expected_result) { - auto num_added = circuit_builder.get_num_gates_added_to_ensure_nonzero_polynomials(); - info("num added = ", num_added); - auto instance = std::make_shared(circuit_builder); UltraProver prover(instance); auto verification_key = std::make_shared(instance->proving_key); From 246ffaf6228f92c7bfd5b712737d9784d27f611c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 22 May 2024 16:03:51 +0000 Subject: [PATCH 5/5] comment --- .../stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp | 1 + .../stdlib_circuit_builders/ultra_circuit_builder.hpp | 1 + 2 files changed, 2 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp index c81f61e9468..b4482e90c7a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.hpp @@ -132,6 +132,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui /** * @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() diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index 2d21b763cfc..49e7598dc1f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -607,6 +607,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase