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: make gate counting functions less confusing and avoid estimations #9046

Merged
merged 17 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

core change: avoiding a needless builder computation just to get an incorrect srs 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

core change: fix thats needed to properly add in the ensure nonzero gates

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
Loading