From ed84fe3bcc29c69b1e9d9caafd2c2c2134a67dce Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 2 May 2024 22:22:31 +0100 Subject: [PATCH] fix: Correct circuit size estimation for UltraHonk (#6164) Resolves https://github.com/AztecProtocol/aztec-packages/issues/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. --- barretenberg/acir_tests/run_acir_tests.sh | 2 +- barretenberg/cpp/src/barretenberg/bb/main.cpp | 12 ++++-------- .../barretenberg/dsl/acir_format/acir_format.cpp | 4 ++++ .../examples/join_split/join_split.test.cpp | 2 +- .../goblin_ultra_circuit_builder.cpp | 14 ++++++++------ .../goblin_ultra_circuit_builder.hpp | 2 +- .../ultra_circuit_builder.cpp | 1 + .../sumcheck/instance/prover_instance.hpp | 2 +- .../src/barretenberg/ultra_honk/ultra_prover.cpp | 1 + 9 files changed, 22 insertions(+), 18 deletions(-) diff --git a/barretenberg/acir_tests/run_acir_tests.sh b/barretenberg/acir_tests/run_acir_tests.sh index e75f8fb1a6b..88189a43438 100755 --- a/barretenberg/acir_tests/run_acir_tests.sh +++ b/barretenberg/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 diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 8ff5c5ba338..9db639ea0a5 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/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(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(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 void write_vk_honk(const std::string& bytecodePa auto constraint_system = get_constraint_system(bytecodePath); auto builder = acir_format::create_circuit(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); 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 73699047852..96ddd4337e5 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/barretenberg/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; }; 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 773c69d8941..8d8c239eaec 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("0x470358e4d91c4c5296ef788b1165b2c439cd498f49c3f99386b002753ca3d0ee"); + constexpr uint256_t CIRCUIT_HASH("0xc9d7e9c29d8555514b4ee1ed5cccc6da7e5a81585c6404f7ce379404f07414ec"); 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 84258f30cb6..58542f3e151 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,22 +12,24 @@ namespace bb { template void GoblinUltraCircuitBuilder_::finalize_circuit() { - UltraCircuitBuilder_>::finalize_circuit(); + if (!this->circuit_finalized) { + add_goblin_gates_to_ensure_all_polys_are_non_zero(); + UltraCircuitBuilder_>::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 void GoblinUltraCircuitBuilder_::add_gates_to_ensure_all_polys_are_non_zero() +template void GoblinUltraCircuitBuilder_::add_goblin_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 bcd6ee219c7..33f5373cc14 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_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; } 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..b23e41e4b6c 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,6 +40,7 @@ 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 48b5c8e4ff3..dfed89eabc8 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.hpp @@ -44,8 +44,8 @@ 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 if (is_structured) { for (auto& block : circuit.blocks.get()) { diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_prover.cpp index 92166a00142..7030f194cb4 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_prover.cpp @@ -77,6 +77,7 @@ template HonkProof& UltraProver_::construct_proof OinkProver 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;