Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct circuit size estimation for UltraHonk #6164

Merged
merged 12 commits into from
May 2, 2024
2 changes: 1 addition & 1 deletion barretenberg/acir_tests/run_acir_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this not just be builder.num_gates now? also add a comment explaining why we don't have to add any buffers/do any computations here anymore

init_bn254_crs(srs_size);

// Construct Honk proof
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading