Skip to content

Commit

Permalink
feat: sayonara old hints (#10547)
Browse files Browse the repository at this point in the history
  • Loading branch information
@IlyasRidhuan
IlyasRidhuan authored Dec 10, 2024
1 parent 98cea58 commit dede16e
Show file tree
Hide file tree
Showing 18 changed files with 13 additions and 1,896 deletions.
1 change: 0 additions & 1 deletion barretenberg/cpp/src/barretenberg/bb/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ void avm_prove(const std::filesystem::path& public_inputs_path,
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.externalcall_hints size: ", avm_hints.externalcall_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());

Expand Down
14 changes: 3 additions & 11 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1958,10 +1958,7 @@ TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeSimple)
std::vector<FF> calldata = {};
std::vector<FF> returndata = {};

// Generate Hint for Sload operation
// side effect counter 0 = value 42
auto execution_hints = ExecutionHints().with_storage_value_hints({ { 0, 42 } });

ExecutionHints execution_hints;
auto trace = gen_trace(bytecode, calldata, public_inputs, returndata, execution_hints);

// CHECK SLOAD
Expand Down Expand Up @@ -2089,10 +2086,7 @@ TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes)
std::vector<FF> calldata = {};
std::vector<FF> returndata = {};

// Generate Hint for Sload operation
// side effect counter 0 = value 42
auto execution_hints = ExecutionHints().with_storage_value_hints({ { 0, 42 } });

ExecutionHints execution_hints;
auto trace = gen_trace(bytecode, calldata, public_inputs, returndata, execution_hints);

// CHECK SLOAD
Expand Down Expand Up @@ -2178,9 +2172,7 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
std::vector<FF> returndata = {};

// Generate Hint for hash exists operation
auto execution_hints = ExecutionHints()
.with_storage_value_hints({ { 0, 1 }, { 1, 1 }, { 2, 1 } })
.with_note_hash_exists_hints({ { 0, 1 }, { 1, 1 }, { 2, 1 } });
ExecutionHints execution_hints;

auto trace = gen_trace(bytecode, calldata, public_inputs, returndata, execution_hints);

Expand Down
93 changes: 5 additions & 88 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution_hints.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,6 @@ inline void read(uint8_t const*& it, AvmEnqueuedCallHint& hint)

struct ExecutionHints {
std::vector<AvmEnqueuedCallHint> enqueued_call_hints;
std::vector<std::pair<FF, FF>> storage_value_hints;
std::vector<std::pair<FF, FF>> note_hash_exists_hints;
std::vector<std::pair<FF, FF>> nullifier_exists_hints;
std::vector<std::pair<FF, FF>> l1_to_l2_message_exists_hints;
std::vector<ExternalCallHint> externalcall_hints;
std::map<FF, ContractInstanceHint> contract_instance_hints;
// We could make this address-indexed
std::vector<AvmContractBytecode> all_contract_bytecode;
Expand All @@ -251,32 +246,6 @@ struct ExecutionHints {

ExecutionHints() = default;

// Builder.
ExecutionHints& with_storage_value_hints(std::vector<std::pair<FF, FF>> storage_value_hints)
{
this->storage_value_hints = std::move(storage_value_hints);
return *this;
}
ExecutionHints& with_note_hash_exists_hints(std::vector<std::pair<FF, FF>> note_hash_exists_hints)
{
this->note_hash_exists_hints = std::move(note_hash_exists_hints);
return *this;
}
ExecutionHints& with_nullifier_exists_hints(std::vector<std::pair<FF, FF>> nullifier_exists_hints)
{
this->nullifier_exists_hints = std::move(nullifier_exists_hints);
return *this;
}
ExecutionHints& with_l1_to_l2_message_exists_hints(std::vector<std::pair<FF, FF>> l1_to_l2_message_exists_hints)
{
this->l1_to_l2_message_exists_hints = std::move(l1_to_l2_message_exists_hints);
return *this;
}
ExecutionHints& with_externalcall_hints(std::vector<ExternalCallHint> externalcall_hints)
{
this->externalcall_hints = std::move(externalcall_hints);
return *this;
}
ExecutionHints& with_contract_instance_hints(std::map<FF, ContractInstanceHint> contract_instance_hints)
{
this->contract_instance_hints = std::move(contract_instance_hints);
Expand All @@ -296,45 +265,13 @@ struct ExecutionHints {
}
}

// TODO: Cache.
// Side effect counter -> value
std::unordered_map<uint32_t, FF> get_side_effect_hints() const
{
std::unordered_map<uint32_t, FF> hints_map;
push_vec_into_map(hints_map, storage_value_hints);
push_vec_into_map(hints_map, nullifier_exists_hints);
return hints_map;
}

// Leaf index -> exists
std::unordered_map<uint32_t, FF> get_leaf_index_hints() const
{
std::unordered_map<uint32_t, FF> hints_map;
push_vec_into_map(hints_map, note_hash_exists_hints);
push_vec_into_map(hints_map, l1_to_l2_message_exists_hints);
return hints_map;
}

static ExecutionHints from(const std::vector<uint8_t>& data)
{
std::vector<std::pair<FF, FF>> storage_value_hints;
std::vector<std::pair<FF, FF>> note_hash_exists_hints;
std::vector<std::pair<FF, FF>> nullifier_exists_hints;
std::vector<std::pair<FF, FF>> l1_to_l2_message_exists_hints;

using serialize::read;
const auto* it = data.data();
std::vector<AvmEnqueuedCallHint> enqueued_call_hints;
read(it, enqueued_call_hints);

read(it, storage_value_hints);
read(it, note_hash_exists_hints);
read(it, nullifier_exists_hints);
read(it, l1_to_l2_message_exists_hints);

std::vector<ExternalCallHint> externalcall_hints;
read(it, externalcall_hints);

std::vector<ContractInstanceHint> contract_instance_hints_vec;
read(it, contract_instance_hints_vec);
std::map<FF, ContractInstanceHint> contract_instance_hints;
Expand Down Expand Up @@ -371,32 +308,17 @@ struct ExecutionHints {
" bytes out of " + std::to_string(data.size()) + " bytes");
}

return { std::move(enqueued_call_hints),
std::move(storage_value_hints),
std::move(note_hash_exists_hints),
std::move(nullifier_exists_hints),
std::move(l1_to_l2_message_exists_hints),
std::move(externalcall_hints),
std::move(contract_instance_hints),
std::move(all_contract_bytecode),
std::move(storage_read_hints),
std::move(storage_write_hints),
std::move(nullifier_read_hints),
std::move(nullifier_write_hints),
std::move(note_hash_read_hints),
std::move(note_hash_write_hints),
std::move(l1_to_l2_message_read_hints)
return { std::move(enqueued_call_hints), std::move(contract_instance_hints),
std::move(all_contract_bytecode), std::move(storage_read_hints),
std::move(storage_write_hints), std::move(nullifier_read_hints),
std::move(nullifier_write_hints), std::move(note_hash_read_hints),
std::move(note_hash_write_hints), std::move(l1_to_l2_message_read_hints)

};
}

private:
ExecutionHints(std::vector<AvmEnqueuedCallHint> enqueued_call_hints,
std::vector<std::pair<FF, FF>> storage_value_hints,
std::vector<std::pair<FF, FF>> note_hash_exists_hints,
std::vector<std::pair<FF, FF>> nullifier_exists_hints,
std::vector<std::pair<FF, FF>> l1_to_l2_message_exists_hints,
std::vector<ExternalCallHint> externalcall_hints,
std::map<FF, ContractInstanceHint> contract_instance_hints,
std::vector<AvmContractBytecode> all_contract_bytecode,
std::vector<PublicDataReadTreeHint> storage_read_hints,
Expand All @@ -408,11 +330,6 @@ struct ExecutionHints {
std::vector<AppendTreeHint> l1_to_l2_message_read_hints)

: enqueued_call_hints(std::move(enqueued_call_hints))
, storage_value_hints(std::move(storage_value_hints))
, note_hash_exists_hints(std::move(note_hash_exists_hints))
, nullifier_exists_hints(std::move(nullifier_exists_hints))
, l1_to_l2_message_exists_hints(std::move(l1_to_l2_message_exists_hints))
, externalcall_hints(std::move(externalcall_hints))
, contract_instance_hints(std::move(contract_instance_hints))
, all_contract_bytecode(std::move(all_contract_bytecode))
, storage_read_hints(std::move(storage_read_hints))
Expand Down
149 changes: 0 additions & 149 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2448,155 +2448,6 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t
.error = error };
}

/**
* @brief Create a kernel output opcode with set metadata output object
*
* Used for writing output opcode where one metadata value is written and comes from a hint
* {note_hash_exists, nullifier_exists, etc. } Where a boolean output if it exists must also be written
*
* @param indirect - Perform indirect memory resolution
* @param clk - The trace clk
* @param data_offset - The offset of the main value to output
* @param metadata_offset - The offset of the metadata (slot in the sload example)
* @return Row
*/
Row AvmTraceBuilder::create_kernel_output_opcode_with_set_metadata_output_from_hint(
uint32_t clk, uint32_t data_offset, [[maybe_unused]] uint32_t address_offset, uint32_t metadata_offset)
{
FF exists = execution_hints.get_side_effect_hints().at(side_effect_counter);

auto read_a = constrained_read_from_memory(
call_ptr, clk, data_offset, AvmMemoryTag::FF, AvmMemoryTag::U1, IntermRegister::IA);

auto write_b = constrained_write_to_memory(
call_ptr, clk, metadata_offset, exists, AvmMemoryTag::FF, AvmMemoryTag::U1, IntermRegister::IB);
bool tag_match = read_a.tag_match && write_b.tag_match;

return Row{
.main_clk = clk,
.main_ia = read_a.val,
.main_ib = write_b.val,
.main_ind_addr_a = FF(read_a.indirect_address),
.main_ind_addr_b = FF(write_b.indirect_address),
.main_internal_return_ptr = internal_return_ptr,
.main_mem_addr_a = FF(read_a.direct_address),
.main_mem_addr_b = FF(write_b.direct_address),
.main_pc = pc,
.main_r_in_tag = static_cast<uint32_t>(AvmMemoryTag::FF),
.main_rwa = 0,
.main_rwb = 1,
.main_sel_mem_op_a = 1,
.main_sel_mem_op_b = 1,
.main_sel_q_kernel_output_lookup = 1,
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(read_a.is_indirect)),
.main_sel_resolve_ind_addr_b = FF(static_cast<uint32_t>(write_b.is_indirect)),
.main_tag_err = static_cast<uint32_t>(!tag_match),
.main_w_in_tag = static_cast<uint32_t>(AvmMemoryTag::U1),
};
}

// Specifically for handling the L1TOL2MSGEXISTS and NOTEHASHEXISTS opcodes
Row AvmTraceBuilder::create_kernel_output_opcode_for_leaf_index(uint32_t clk,
uint32_t data_offset,
uint32_t leaf_index,
uint32_t metadata_offset)
{
// If doesnt exist, should not read_a, but instead get from public inputs
FF exists = execution_hints.get_leaf_index_hints().at(leaf_index);

auto read_a = constrained_read_from_memory(
call_ptr, clk, data_offset, AvmMemoryTag::FF, AvmMemoryTag::U1, IntermRegister::IA);

auto write_b = constrained_write_to_memory(
call_ptr, clk, metadata_offset, exists, AvmMemoryTag::FF, AvmMemoryTag::U1, IntermRegister::IB);
bool tag_match = read_a.tag_match && write_b.tag_match;

return Row{
.main_clk = clk,
.main_ia = read_a.val,
.main_ib = write_b.val,
.main_ind_addr_a = FF(read_a.indirect_address),
.main_ind_addr_b = FF(write_b.indirect_address),
.main_internal_return_ptr = internal_return_ptr,
.main_mem_addr_a = FF(read_a.direct_address),
.main_mem_addr_b = FF(write_b.direct_address),
.main_pc = pc,
.main_r_in_tag = static_cast<uint32_t>(AvmMemoryTag::FF),
.main_rwa = 0,
.main_rwb = 1,
.main_sel_mem_op_a = 1,
.main_sel_mem_op_b = 1,
.main_sel_q_kernel_output_lookup = 1,
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(read_a.is_indirect)),
.main_sel_resolve_ind_addr_b = FF(static_cast<uint32_t>(write_b.is_indirect)),
.main_tag_err = static_cast<uint32_t>(!tag_match),
.main_w_in_tag = static_cast<uint32_t>(AvmMemoryTag::U1),
};
}

/**
* @brief Create a kernel output opcode with set metadata output object
*
* Used for writing output opcode where one value is written and comes from a hint
* {sload}
*
* @param indirect - Perform indirect memory resolution
* @param clk - The trace clk
* @param data_offset - The offset of the main value to output
* @param metadata_offset - The offset of the metadata (slot in the sload example)
* @return Row
*/
RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect,
uint32_t clk,
uint32_t data_offset,
uint32_t metadata_offset)
{
FF value = execution_hints.get_side_effect_hints().at(side_effect_counter);
// TODO: throw error if incorrect

// We keep the first encountered error
AvmError error = AvmError::NO_ERROR;
auto [resolved_addrs, res_error] =
Addressing<2>::fromWire(indirect, call_ptr).resolve({ data_offset, metadata_offset }, mem_trace_builder);
auto [resolved_data, resolved_metadata] = resolved_addrs;
error = res_error;

auto write_a = constrained_write_to_memory(
call_ptr, clk, resolved_data, value, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA);
auto read_b = constrained_read_from_memory(
call_ptr, clk, resolved_metadata, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IB);
bool tag_match = write_a.tag_match && read_b.tag_match;

if (is_ok(error) && !tag_match) {
error = AvmError::CHECK_TAG_ERROR;
}

return RowWithError{ .row =
Row{
.main_clk = clk,
.main_ia = write_a.val,
.main_ib = read_b.val,
.main_ind_addr_a = FF(write_a.indirect_address),
.main_ind_addr_b = FF(read_b.indirect_address),
.main_internal_return_ptr = internal_return_ptr,
.main_mem_addr_a = FF(write_a.direct_address),
.main_mem_addr_b = FF(read_b.direct_address),
.main_op_err = FF(static_cast<uint32_t>(!is_ok(error))),
.main_pc = pc, // No PC increment here since we do it in the specific ops
.main_r_in_tag = static_cast<uint32_t>(AvmMemoryTag::FF),
.main_rwa = 1,
.main_rwb = 0,
.main_sel_mem_op_a = 1,
.main_sel_mem_op_b = 1,
.main_sel_q_kernel_output_lookup = 1,
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(write_a.is_indirect)),
.main_sel_resolve_ind_addr_b = FF(static_cast<uint32_t>(read_b.is_indirect)),
.main_tag_err = static_cast<uint32_t>(!tag_match),
.main_w_in_tag = static_cast<uint32_t>(AvmMemoryTag::FF),
},
.error = error };
}

/**************************************************************************************************
* WORLD STATE
**************************************************************************************************/
Expand Down
15 changes: 0 additions & 15 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,21 +326,6 @@ class AvmTraceBuilder {
uint32_t metadata_offset,
AvmMemoryTag metadata_r_tag);

Row create_kernel_output_opcode_with_set_metadata_output_from_hint(uint32_t clk,
uint32_t data_offset,
uint32_t address_offset,
uint32_t metadata_offset);

Row create_kernel_output_opcode_for_leaf_index(uint32_t clk,
uint32_t data_offset,
uint32_t leaf_index,
uint32_t metadata_offset);

RowWithError create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect,
uint32_t clk,
uint32_t data_offset,
uint32_t metadata_offset);

AvmError constrain_external_call(OpCode opcode,
uint16_t indirect,
uint32_t gas_offset,
Expand Down
Loading

1 comment on commit dede16e

@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: dede16e Previous: 985aef1 Ratio
wasmClientIVCBench/Full/6 91210.434041 ms/iter 84560.366332 ms/iter 1.08
wasmconstruct_proof_ultrahonk_power_of_2/20 16922.595208999996 ms/iter 15128.370046000002 ms/iter 1.12

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

CC: @ludamad @codygunton

Please sign in to comment.