Skip to content

Commit

Permalink
fix: make gate counting functions less confusing and avoid estimations (
Browse files Browse the repository at this point in the history
#9046)

Removes unnecessary gate counting in Honk flows in main.cpp and instead
inits the SRS based on the actual finalized gate count taken from the
Prover.

Renames get_num_gates to get_estimated_num_finalized_gates, renames a
few other functions to add "estimated" to their names to reflect their
actual functionality and avoid misuse.

Replace estimating functions used in main.cpp and in c_bind.cpp with
functions that return the actual finalized gate count.

Fixes a bug in an earlier PR
#9042, which forgot
the ensure_nonzero = true argument to finalize_circuit(), and
undercounted the number of gates in the circuit, leading a smaller than
needed SRS.
  • Loading branch information
lucasxia01 authored Oct 9, 2024
1 parent 72e4867 commit 0bda0a4
Show file tree
Hide file tree
Showing 65 changed files with 348 additions and 300 deletions.
4 changes: 2 additions & 2 deletions barretenberg/acir_tests/flows/honk_sol.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export PROOF_AS_FIELDS="$(pwd)/proof_fields.json"
# Create a proof, write the solidity contract, write the proof as fields in order to extract the public inputs
$BIN prove_ultra_keccak_honk -o proof $FLAGS $BFLAG
$BIN write_vk_ultra_keccak_honk -o vk $FLAGS $BFLAG
$BIN proof_as_fields_honk -k vk -c $CRS_PATH -p $PROOF
$BIN contract_ultra_honk -k vk -c $CRS_PATH -o Verifier.sol
$BIN proof_as_fields_honk -k vk $FLAGS -p $PROOF
$BIN contract_ultra_honk -k vk $FLAGS -o Verifier.sol

# Export the paths to the environment variables for the js test runner
export VERIFIER_PATH="$(pwd)/Verifier.sol"
Expand Down
12 changes: 8 additions & 4 deletions barretenberg/acir_tests/flows/sol.sh
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#!/bin/sh
set -eu

VFLAG=${VERBOSE:+-v}
BFLAG="-b ./target/program.json"
FLAGS="-c $CRS_PATH $VFLAG"

export PROOF="$(pwd)/proof"
export PROOF_AS_FIELDS="$(pwd)/proof_fields.json"

# Create a proof, write the solidity contract, write the proof as fields in order to extract the public inputs
$BIN prove -o proof
$BIN write_vk -o vk
$BIN proof_as_fields -k vk -c $CRS_PATH -p $PROOF
$BIN contract -k vk -c $CRS_PATH -b ./target/program.json -o Key.sol
$BIN prove -o proof $FLAGS
$BIN write_vk -o vk $FLAGS
$BIN proof_as_fields -k vk $FLAGS -p $PROOF
$BIN contract -k vk $FLAGS $BFLAG -o Key.sol

# Export the paths to the environment variables for the js test runner
export KEY_PATH="$(pwd)/Key.sol"
Expand Down
57 changes: 25 additions & 32 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,15 @@ bool proveAndVerify(const std::string& bytecodePath, const std::string& witnessP
auto witness = get_witness(witnessPath);

acir_proofs::AcirComposer acir_composer{ 0, verbose_logging };
acir_composer.create_circuit(constraint_system, witness);

init_bn254_crs(acir_composer.get_dyadic_circuit_size());
acir_composer.create_finalized_circuit(constraint_system, witness);
init_bn254_crs(acir_composer.get_finalized_dyadic_circuit_size());

Timer pk_timer;
acir_composer.init_proving_key();
write_benchmark("pk_construction_time", pk_timer.milliseconds(), "acir_test", current_dir);

write_benchmark("gate_count", acir_composer.get_total_circuit_size(), "acir_test", current_dir);
write_benchmark("subgroup_size", acir_composer.get_dyadic_circuit_size(), "acir_test", current_dir);
write_benchmark("gate_count", acir_composer.get_finalized_total_circuit_size(), "acir_test", current_dir);
write_benchmark("subgroup_size", acir_composer.get_finalized_dyadic_circuit_size(), "acir_test", current_dir);

Timer proof_timer;
auto proof = acir_composer.create_proof();
Expand Down Expand Up @@ -199,12 +198,9 @@ 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, honk_recursion);

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
Prover prover{ builder };
init_bn254_crs(prover.proving_key->proving_key.circuit_size);
auto proof = prover.construct_proof();

// Verify Honk proof
Expand Down Expand Up @@ -670,8 +666,8 @@ void prove(const std::string& bytecodePath, const std::string& witnessPath, cons
auto witness = get_witness(witnessPath);

acir_proofs::AcirComposer acir_composer{ 0, verbose_logging };
acir_composer.create_circuit(constraint_system, witness);
init_bn254_crs(acir_composer.get_dyadic_circuit_size());
acir_composer.create_finalized_circuit(constraint_system, witness);
init_bn254_crs(acir_composer.get_finalized_dyadic_circuit_size());
acir_composer.init_proving_key();
auto proof = acir_composer.create_proof();

Expand All @@ -689,6 +685,8 @@ void prove(const std::string& bytecodePath, const std::string& witnessPath, cons
*
* Communication:
* - stdout: A JSON string of the number of ACIR opcodes and final backend circuit size.
* TODO(https://github.com/AztecProtocol/barretenberg/issues/1126): split this into separate Plonk and Honk functions as
* their gate count differs
*
* @param bytecodePath Path to the file containing the serialized circuit
*/
Expand All @@ -701,8 +699,9 @@ template <typename Builder = UltraCircuitBuilder> void gateCount(const std::stri
for (auto constraint_system : constraint_systems) {
auto builder = acir_format::create_circuit<Builder>(
constraint_system, 0, {}, honk_recursion, std::make_shared<bb::ECCOpQueue>(), true);
builder.finalize_circuit();
builder.finalize_circuit(/*ensure_nonzero=*/true);
size_t circuit_size = builder.num_gates;
vinfo("Calculated circuit size in gateCount: ", circuit_size);

// Build individual circuit report
std::string gates_per_opcode_str;
Expand Down Expand Up @@ -778,8 +777,9 @@ void write_vk(const std::string& bytecodePath, const std::string& outputPath)
{
auto constraint_system = get_constraint_system(bytecodePath, false);
acir_proofs::AcirComposer acir_composer{ 0, verbose_logging };
acir_composer.create_circuit(constraint_system);
init_bn254_crs(acir_composer.get_dyadic_circuit_size());
acir_composer.create_finalized_circuit(constraint_system);
acir_composer.finalize_circuit();
init_bn254_crs(acir_composer.get_finalized_dyadic_circuit_size());
acir_composer.init_proving_key();
auto vk = acir_composer.init_verification_key();
auto serialized_vk = to_buffer(*vk);
Expand All @@ -796,8 +796,9 @@ void write_pk(const std::string& bytecodePath, const std::string& outputPath)
{
auto constraint_system = get_constraint_system(bytecodePath, /*honk_recursion=*/false);
acir_proofs::AcirComposer acir_composer{ 0, verbose_logging };
acir_composer.create_circuit(constraint_system);
init_bn254_crs(acir_composer.get_dyadic_circuit_size());
acir_composer.create_finalized_circuit(constraint_system);
acir_composer.finalize_circuit();
init_bn254_crs(acir_composer.get_finalized_dyadic_circuit_size());
auto pk = acir_composer.init_proving_key();
auto serialized_pk = to_buffer(*pk);

Expand Down Expand Up @@ -1089,12 +1090,9 @@ UltraProver_<Flavor> compute_valid_prover(const std::string& bytecodePath, const
}

auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, witness, honk_recursion);

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);

return Prover{ builder };
auto prover = Prover{ builder };
init_bn254_crs(prover.proving_key->proving_key.circuit_size);
return std::move(prover);
}

/**
Expand Down Expand Up @@ -1219,12 +1217,9 @@ void write_recursion_inputs_honk(const std::string& bytecodePath,
auto witness = get_witness(witnessPath);
auto builder = acir_format::create_circuit<Builder>(constraints, 0, witness, honk_recursion);

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 and verification key
Prover prover{ builder };
init_bn254_crs(prover.proving_key->proving_key.circuit_size);
std::vector<FF> proof = prover.construct_proof();
VerificationKey verification_key(prover.proving_key->proving_key);

Expand Down Expand Up @@ -1306,8 +1301,9 @@ void prove_output_all(const std::string& bytecodePath, const std::string& witnes
auto witness = get_witness(witnessPath);

acir_proofs::AcirComposer acir_composer{ 0, verbose_logging };
acir_composer.create_circuit(constraint_system, witness);
init_bn254_crs(acir_composer.get_dyadic_circuit_size());
acir_composer.create_finalized_circuit(constraint_system, witness);
acir_composer.finalize_circuit();
init_bn254_crs(acir_composer.get_finalized_dyadic_circuit_size());
acir_composer.init_proving_key();
auto proof = acir_composer.create_proof();

Expand Down Expand Up @@ -1371,12 +1367,9 @@ void prove_honk_output_all(const std::string& bytecodePath,

auto builder = acir_format::create_circuit<Builder>(constraint_system, 0, witness, honk_recursion);

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
Prover prover{ builder };
init_bn254_crs(prover.proving_key->proving_key.circuit_size);
auto proof = prover.construct_proof();

// We have been given a directory, we will write the proof and verification key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ Builder generate_trace(size_t target_num_gates)
Fr x = Fr::random_element();
Fr y = Fr::random_element();

// Each loop adds 163 gates. Note: builder.get_num_gates() is very expensive here (bug?) and it's actually painful
// to use a `while` loop
// Each loop adds 163 gates. Note: builder.get_estimated_num_finalized_gates() is very expensive here (bug?) and
// it's actually painful to use a `while` loop
size_t num_iterations = target_num_gates / 163;
for (size_t _ = 0; _ < num_iterations; _++) {
op_queue->add_accumulate(a);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ TEST(UltraCircuitConstructor, CopyConstructor)

UltraCircuitBuilder duplicate_circuit_constructor{ circuit_constructor };

EXPECT_EQ(duplicate_circuit_constructor.get_num_gates(), circuit_constructor.get_num_gates());
EXPECT_EQ(duplicate_circuit_constructor.get_estimated_num_finalized_gates(),
circuit_constructor.get_estimated_num_finalized_gates());
EXPECT_TRUE(CircuitChecker::check(duplicate_circuit_constructor));
}

Expand Down Expand Up @@ -860,7 +861,8 @@ TEST(UltraCircuitConstructor, Ram)
// Test the builder copy constructor for a circuit with RAM gates
UltraCircuitBuilder duplicate_circuit_constructor{ circuit_constructor };

EXPECT_EQ(duplicate_circuit_constructor.get_num_gates(), circuit_constructor.get_num_gates());
EXPECT_EQ(duplicate_circuit_constructor.get_estimated_num_finalized_gates(),
circuit_constructor.get_estimated_num_finalized_gates());
EXPECT_TRUE(CircuitChecker::check(duplicate_circuit_constructor));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ template <typename Builder> bool UltraCircuitChecker::check(const Builder& build
{
// Create a copy of the input circuit and finalize it
Builder builder{ builder_in };
builder.finalize_circuit();
builder.finalize_circuit(/*ensure_nonzero=*/true); // Test the ensure_nonzero gates as well

// Construct a hash table for lookup table entries to efficiently determine if a lookup gate is valid
LookupHashTable lookup_hash_table;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ TEST(crypto_merkle_tree, test_check_membership)
bool_ct is_member_ =
check_membership(root, create_witness_hash_path(builder, db.get_hash_path(1)), field_ct(1), seven);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(is_member.get_value(), true);
Expand Down Expand Up @@ -71,7 +71,7 @@ TEST(crypto_merkle_tree, test_batch_update_membership)
field_ct start_idx = field_ct(witness_ct(&builder, fr(4)));
batch_update_membership(new_root, old_root, old_hash_path_1, values, start_idx);
batch_update_membership(new_root, old_root, old_hash_path_2, values, start_idx);
printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());
bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
}
Expand All @@ -87,7 +87,7 @@ TEST(crypto_merkle_tree, test_assert_check_membership)

assert_check_membership(root, create_witness_hash_path(builder, db.get_hash_path(0)), field_ct(0), zero);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
Expand All @@ -105,7 +105,7 @@ TEST(crypto_merkle_tree, test_assert_check_membership_fail)

assert_check_membership(root, create_witness_hash_path(builder, db.get_hash_path(0)), field_ct(1), zero);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, false);
Expand All @@ -132,7 +132,7 @@ TEST(crypto_merkle_tree, test_update_members)

update_membership(new_root, new_value, old_root, old_path, old_value, zero);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
Expand All @@ -156,7 +156,7 @@ TEST(crypto_merkle_tree, test_update_members)

update_membership(new_root, new_value, old_root, new_path, old_value, zero);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
Expand All @@ -179,7 +179,7 @@ TEST(crypto_merkle_tree, test_tree)

assert_check_tree(root, values);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());

bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
Expand Down Expand Up @@ -243,7 +243,7 @@ TEST(crypto_merkle_tree, test_update_memberships)

update_memberships(old_root_ct, new_roots_ct, new_values_ct, old_values_ct, old_hash_paths_ct, old_indices_ct);

printf("num gates = %zu\n", builder.get_num_gates());
printf("num gates = %zu\n", builder.get_estimated_num_finalized_gates());
bool result = CircuitChecker::check(builder);
EXPECT_EQ(result, true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ template <typename Builder> class GateCounter {
if (!collect_gates_per_opcode) {
return 0;
}
size_t new_gate_count = builder->get_num_gates();
size_t new_gate_count = builder->get_estimated_num_finalized_gates();
size_t diff = new_gate_count - prev_gate_count;
prev_gate_count = new_gate_count;
return diff;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class AcirIntegrationTest : public ::testing::Test {
Prover prover{ builder };
#ifdef LOG_SIZES
builder.blocks.summarize();
info("num gates = ", builder.get_num_gates());
info("total circuit size = ", builder.get_total_circuit_size());
info("num gates = ", builder.get_estimated_num_finalized_gates());
info("total circuit size = ", builder.get_estimated_total_circuit_size());
info("circuit size = ", prover.proving_key->proving_key.circuit_size);
info("log circuit size = ", prover.proving_key->proving_key.log_circuit_size);
#endif
Expand All @@ -83,8 +83,8 @@ class AcirIntegrationTest : public ::testing::Test {
auto prover = composer.create_prover(builder);
#ifdef LOG_SIZES
// builder.blocks.summarize();
// info("num gates = ", builder.get_num_gates());
// info("total circuit size = ", builder.get_total_circuit_size());
// info("num gates = ", builder.get_estimated_num_finalized_gates());
// info("total circuit size = ", builder.get_estimated_total_circuit_size());
#endif
auto proof = prover.construct_proof();
#ifdef LOG_SIZES
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ TEST_F(AcirAvmRecursionConstraint, TestBasicSingleAvmRecursionConstraint)
layer_1_circuits.push_back(create_inner_circuit(public_inputs_vec));
auto layer_2_circuit = create_outer_circuit(layer_1_circuits, public_inputs_vec);

info("circuit gates = ", layer_2_circuit.get_num_gates());
info("circuit gates = ", layer_2_circuit.get_estimated_num_finalized_gates());

auto proving_key = std::make_shared<DeciderProvingKey>(layer_2_circuit);
OuterProver prover(proving_key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ TEST_F(AcirHonkRecursionConstraint, TestBasicSingleHonkRecursionConstraint)

auto layer_2_circuit = create_outer_circuit(layer_1_circuits);

info("circuit gates = ", layer_2_circuit.get_num_gates());
info("circuit gates = ", layer_2_circuit.get_estimated_num_finalized_gates());

auto proving_key = std::make_shared<DeciderProvingKey>(layer_2_circuit);
Prover prover(proving_key);
Expand All @@ -215,7 +215,7 @@ TEST_F(AcirHonkRecursionConstraint, TestBasicDoubleHonkRecursionConstraints)

auto layer_2_circuit = create_outer_circuit(layer_1_circuits);

info("circuit gates = ", layer_2_circuit.get_num_gates());
info("circuit gates = ", layer_2_circuit.get_estimated_num_finalized_gates());

auto proving_key = std::make_shared<DeciderProvingKey>(layer_2_circuit);
Prover prover(proving_key);
Expand Down Expand Up @@ -273,7 +273,7 @@ TEST_F(AcirHonkRecursionConstraint, TestOneOuterRecursiveCircuit)

auto layer_3_circuit = create_outer_circuit(layer_2_circuits);
info("created second outer circuit");
info("number of gates in layer 3 = ", layer_3_circuit.get_num_gates());
info("number of gates in layer 3 = ", layer_3_circuit.get_estimated_num_finalized_gates());

auto proving_key = std::make_shared<DeciderProvingKey>(layer_3_circuit);
Prover prover(proving_key);
Expand Down Expand Up @@ -303,7 +303,7 @@ TEST_F(AcirHonkRecursionConstraint, TestFullRecursiveComposition)

auto layer_3_circuit = create_outer_circuit(layer_2_circuits);
info("created third outer circuit");
info("number of gates in layer 3 circuit = ", layer_3_circuit.get_num_gates());
info("number of gates in layer 3 circuit = ", layer_3_circuit.get_estimated_num_finalized_gates());

auto proving_key = std::make_shared<DeciderProvingKey>(layer_3_circuit);
Prover prover(proving_key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ TEST_F(AcirRecursionConstraint, TestBasicDoubleRecursionConstraints)

auto layer_2_circuit = create_outer_circuit(layer_1_circuits);

info("circuit gates = ", layer_2_circuit.get_num_gates());
info("circuit gates = ", layer_2_circuit.get_estimated_num_finalized_gates());

auto layer_2_composer = Composer();
auto prover = layer_2_composer.create_ultra_with_keccak_prover(layer_2_circuit);
Expand Down Expand Up @@ -357,7 +357,7 @@ TEST_F(AcirRecursionConstraint, TestOneOuterRecursiveCircuit)

auto layer_3_circuit = create_outer_circuit(layer_2_circuits);
info("created second outer circuit");
info("number of gates in layer 3 = ", layer_3_circuit.get_num_gates());
info("number of gates in layer 3 = ", layer_3_circuit.get_estimated_num_finalized_gates());

auto layer_3_composer = Composer();
auto prover = layer_3_composer.create_ultra_with_keccak_prover(layer_3_circuit);
Expand Down Expand Up @@ -386,7 +386,7 @@ TEST_F(AcirRecursionConstraint, TestFullRecursiveComposition)

auto layer_3_circuit = create_outer_circuit(layer_2_circuits);
info("created third outer circuit");
info("number of gates in layer 3 circuit = ", layer_3_circuit.get_num_gates());
info("number of gates in layer 3 circuit = ", layer_3_circuit.get_estimated_num_finalized_gates());

auto layer_3_composer = Composer();
auto prover = layer_3_composer.create_ultra_with_keccak_prover(layer_3_circuit);
Expand Down
Loading

0 comments on commit 0bda0a4

Please sign in to comment.