Skip to content

Commit

Permalink
chore(avm): re-enable bb-prover tests in CI, change some to check-cir…
Browse files Browse the repository at this point in the history
…cuit-only, enable multi-enqueued call tests (#11180)
  • Loading branch information
dbanks12 authored Jan 13, 2025
1 parent 9ab4cee commit 3092212
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 70 deletions.
41 changes: 29 additions & 12 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,27 @@ void print_avm_stats()
#endif
}

/**
* @brief Performs "check circuit" on the AVM circuit for the given public inputs and hints.
*
* @param public_inputs_path Path to the file containing the serialised avm public inputs
* @param hints_path Path to the file containing the serialised avm circuit hints
*/
void avm_check_circuit(const std::filesystem::path& public_inputs_path, const std::filesystem::path& hints_path)
{

const auto avm_public_inputs = AvmPublicInputs::from(read_file(public_inputs_path));
const auto avm_hints = bb::avm_trace::ExecutionHints::from(read_file(hints_path));
avm_hints.print_sizes();

vinfo("initializing crs with size: ", avm_trace::Execution::SRS_SIZE);
init_bn254_crs(avm_trace::Execution::SRS_SIZE);

avm_trace::Execution::check_circuit(avm_public_inputs, avm_hints);

print_avm_stats();
}

/**
* @brief Writes an avm proof and corresponding (incomplete) verification key to files.
*
Expand All @@ -700,18 +721,7 @@ void avm_prove(const std::filesystem::path& public_inputs_path,

const auto avm_public_inputs = AvmPublicInputs::from(read_file(public_inputs_path));
const auto avm_hints = bb::avm_trace::ExecutionHints::from(read_file(hints_path));

// Using [0] is fine now for the top-level call, but we might need to index by address in future
vinfo("bytecode size: ", avm_hints.all_contract_bytecode[0].bytecode.size());
vinfo("hints.storage_read_hints size: ", avm_hints.storage_read_hints.size());
vinfo("hints.storage_write_hints size: ", avm_hints.storage_write_hints.size());
vinfo("hints.nullifier_read_hints size: ", avm_hints.nullifier_read_hints.size());
vinfo("hints.nullifier_write_hints size: ", avm_hints.nullifier_write_hints.size());
vinfo("hints.note_hash_read_hints size: ", avm_hints.note_hash_read_hints.size());
vinfo("hints.note_hash_write_hints size: ", avm_hints.note_hash_write_hints.size());
vinfo("hints.l1_to_l2_message_read_hints size: ", avm_hints.l1_to_l2_message_read_hints.size());
vinfo("hints.contract_instance_hints size: ", avm_hints.contract_instance_hints.size());
vinfo("hints.contract_bytecode_hints size: ", avm_hints.all_contract_bytecode.size());
avm_hints.print_sizes();

vinfo("initializing crs with size: ", avm_trace::Execution::SRS_SIZE);
init_bn254_crs(avm_trace::Execution::SRS_SIZE);
Expand Down Expand Up @@ -1448,6 +1458,13 @@ int main(int argc, char* argv[])
std::filesystem::path public_inputs_path =
get_option(args, "--avm-public-inputs", "./target/avm_public_inputs.bin");
return avm2_verify(proof_path, public_inputs_path, vk_path) ? 0 : 1;
} else if (command == "avm_check_circuit") {
std::filesystem::path avm_public_inputs_path =
get_option(args, "--avm-public-inputs", "./target/avm_public_inputs.bin");
std::filesystem::path avm_hints_path = get_option(args, "--avm-hints", "./target/avm_hints.bin");
extern std::filesystem::path avm_dump_trace_path;
avm_dump_trace_path = get_option(args, "--avm-dump-trace", "");
avm_check_circuit(avm_public_inputs_path, avm_hints_path);
} else if (command == "avm_prove") {
std::filesystem::path avm_public_inputs_path =
get_option(args, "--avm-public-inputs", "./target/avm_public_inputs.bin");
Expand Down
38 changes: 36 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,40 @@ std::vector<FF> Execution::getDefaultPublicInputs()
return public_inputs_vec;
}

/**
* @brief Run the bytecode, generate the corresponding execution trace and check the circuit for
* execution of the supplied bytecode.
*
* @throws runtime_error exception when the bytecode is invalid.
*/
void Execution::check_circuit(AvmPublicInputs const& public_inputs, ExecutionHints const& execution_hints)
{
std::vector<FF> returndata;
std::vector<FF> calldata;
for (const auto& enqueued_call_hints : execution_hints.enqueued_call_hints) {
calldata.insert(calldata.end(), enqueued_call_hints.calldata.begin(), enqueued_call_hints.calldata.end());
}
std::vector<Row> trace = AVM_TRACK_TIME_V(
"prove/gen_trace", gen_trace(public_inputs, returndata, execution_hints, /*apply_e2e_assertions=*/true));
if (!avm_dump_trace_path.empty()) {
info("Dumping trace as CSV to: " + avm_dump_trace_path.string());
dump_trace_as_csv(trace, avm_dump_trace_path);
}
auto circuit_builder = bb::avm::AvmCircuitBuilder();
circuit_builder.set_trace(std::move(trace));
vinfo("Circuit subgroup size: 2^",
// this calculates the integer log2
std::bit_width(circuit_builder.get_circuit_subgroup_size()) - 1);

if (circuit_builder.get_circuit_subgroup_size() > SRS_SIZE) {
throw_or_abort("Circuit subgroup size (" + std::to_string(circuit_builder.get_circuit_subgroup_size()) +
") exceeds SRS_SIZE (" + std::to_string(SRS_SIZE) + ")");
}

vinfo("------- CHECKING CIRCUIT -------");
AVM_TRACK_TIME("prove/check_circuit", circuit_builder.check_circuit());
}

/**
* @brief Run the bytecode, generate the corresponding execution trace and prove the correctness
* of the execution of the supplied bytecode.
Expand Down Expand Up @@ -352,8 +386,8 @@ std::vector<Row> Execution::gen_trace(AvmPublicInputs const& public_inputs,
uint32_t start_side_effect_counter = 0;
// Temporary until we get proper nested call handling
std::vector<FF> calldata;
for (const auto& enqueued_call_hints : execution_hints.enqueued_call_hints) {
calldata.insert(calldata.end(), enqueued_call_hints.calldata.begin(), enqueued_call_hints.calldata.end());
for (const auto& enqueued_call_hint : execution_hints.enqueued_call_hints) {
calldata.insert(calldata.end(), enqueued_call_hint.calldata.begin(), enqueued_call_hint.calldata.end());
}
AvmTraceBuilder trace_builder =
Execution::trace_builder_constructor(public_inputs, execution_hints, start_side_effect_counter);
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class Execution {

static std::tuple<bb::avm::AvmFlavor::VerificationKey, bb::HonkProof> prove(
AvmPublicInputs const& public_inputs = AvmPublicInputs(), ExecutionHints const& execution_hints = {});
static void check_circuit(AvmPublicInputs const& public_inputs = AvmPublicInputs(),
ExecutionHints const& execution_hints = {});
static bool verify(bb::avm::AvmFlavor::VerificationKey vk, HonkProof const& proof);

private:
Expand Down
18 changes: 18 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution_hints.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,24 @@ struct ExecutionHints {
return *this;
}

void print_sizes() const
{
vinfo("hints.enqueued_call_hints size: ", enqueued_call_hints.size());
vinfo("hints.contract_instance_hints size: ", contract_instance_hints.size());
vinfo("hints.contract_bytecode_hints size: ", all_contract_bytecode.size());
if (all_contract_bytecode.size() > 0) {
// Using [0] is fine now for the top-level call, but we might need to index by address in future
vinfo("0th bytecode size: ", all_contract_bytecode[0].bytecode.size());
}
vinfo("hints.storage_read_hints size: ", storage_read_hints.size());
vinfo("hints.storage_write_hints size: ", storage_write_hints.size());
vinfo("hints.nullifier_read_hints size: ", nullifier_read_hints.size());
vinfo("hints.nullifier_write_hints size: ", nullifier_write_hints.size());
vinfo("hints.note_hash_read_hints size: ", note_hash_read_hints.size());
vinfo("hints.note_hash_write_hints size: ", note_hash_write_hints.size());
vinfo("hints.l1_to_l2_message_read_hints size: ", l1_to_l2_message_read_hints.size());
}

static void push_vec_into_map(std::unordered_map<uint32_t, FF>& into_map,
const std::vector<std::pair<FF, FF>>& from_pair_vec)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ contract AvmTest {
storage.single.read()
}

#[public]
fn read_assert_storage_single(a: Field) {
assert(a == storage.single.read(), "Storage value does not match input");
}

// should still be able to use ` -> pub *` for return type even though macro forces `pub`
#[public]
fn set_read_storage_single(a: Field) -> pub Field {
Expand Down
Loading

1 comment on commit 3092212

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 3092212 Previous: c4f4452 Ratio
nativeconstruct_proof_ultrahonk_power_of_2/20 4477.594582999984 ms/iter 4074.526970999983 ms/iter 1.10
Goblin::merge(t) 143002881 ns/iter 133608330 ns/iter 1.07

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.