Skip to content

Commit

Permalink
fix: change limit counters
Browse files Browse the repository at this point in the history
  • Loading branch information
@IlyasRidhuan
IlyasRidhuan committed Dec 17, 2024
1 parent 092ed98 commit c114f31
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 18 deletions.
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ enum class AvmError : uint32_t {
RADIX_OUT_OF_BOUNDS,
DUPLICATE_NULLIFIER,
SIDE_EFFECT_LIMIT_REACHED,
STATIC_CALL_ALTERATION
};

} // namespace bb::avm_trace
4 changes: 4 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ FF AvmMerkleTreeTraceBuilder::perform_storage_write([[maybe_unused]] uint32_t cl
// Update the low leaf
tree_snapshots.public_data_tree.root =
unconstrained_update_leaf_index(low_preimage_hash, static_cast<uint64_t>(low_index), low_path);
// Update the set of writes to unique slots
public_data_unique_writes.insert(slot);
return tree_snapshots.public_data_tree.root;
}
// Check the low leaf conditions (i.e. the slot is sandwiched by the low nullifier, or the new slot is a max
Expand All @@ -172,6 +174,8 @@ FF AvmMerkleTreeTraceBuilder::perform_storage_write([[maybe_unused]] uint32_t cl
// Insert the new leaf into the tree
tree_snapshots.public_data_tree.root = unconstrained_update_leaf_index(leaf_preimage_hash, index, insertion_path);
tree_snapshots.public_data_tree.size++;
// Update the set of writes to unique slots
public_data_unique_writes.insert(slot);
return tree_snapshots.public_data_tree.root;
}

Expand Down
9 changes: 8 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "barretenberg/vm/avm/trace/gadgets/poseidon2.hpp"
#include "barretenberg/vm/avm/trace/public_inputs.hpp"

#include <unordered_set>
#include <vector>
namespace bb::avm_trace {

Expand Down Expand Up @@ -42,7 +43,12 @@ class AvmMerkleTreeTraceBuilder {
FF compute_public_tree_leaf_slot(uint32_t clk, FF contract_address, FF leaf_index);

TreeSnapshots& get_tree_snapshots() { return tree_snapshots; }
void set_tree_snapshots(TreeSnapshots& tree_snapshots) { this->tree_snapshots = tree_snapshots; }
void restore_tree_state(TreeSnapshots& tree_snapshots, std::unordered_set<FF>& public_data_unique_writes)
{
this->tree_snapshots = tree_snapshots;
this->public_data_unique_writes = public_data_unique_writes;
}
std::unordered_set<FF> get_public_data_unique_writes() { return public_data_unique_writes; }

// Public Data Tree
bool perform_storage_read(uint32_t clk,
Expand Down Expand Up @@ -119,6 +125,7 @@ class AvmMerkleTreeTraceBuilder {
std::vector<MerkleEntry> merkle_check_trace;
TreeSnapshots non_revertible_tree_snapshots;
TreeSnapshots tree_snapshots;
std::unordered_set<FF> public_data_unique_writes;
MerkleEntry compute_root_from_path(uint32_t clk,
const FF& leaf_value,
const uint64_t leaf_index,
Expand Down
63 changes: 46 additions & 17 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ uint32_t finalize_rng_chks_for_testing(std::vector<Row>& main_trace,
auto old_size = main_trace.size();
for (auto const& clk : custom_clk) {
if (clk >= old_size) {
main_trace.push_back(Row{ .main_clk = FF(clk) });
Row row{};
row.main_clk = clk;
main_trace.push_back(row);
}
}

Expand Down Expand Up @@ -219,6 +221,19 @@ uint32_t AvmTraceBuilder::get_inserted_note_hashes_count()
public_inputs.start_tree_snapshots.note_hash_tree.size;
}

uint32_t AvmTraceBuilder::get_inserted_nullifiers_count()
{
return merkle_tree_trace_builder.get_tree_snapshots().nullifier_tree.size -
public_inputs.start_tree_snapshots.nullifier_tree.size;
}

// Keeping track of the public data writes isn't as simple as comparing sizes
// because of updates to leaves that were already in the tree and state squashing
uint32_t AvmTraceBuilder::get_public_data_writes_count()
{
return static_cast<uint32_t>(merkle_tree_trace_builder.get_public_data_unique_writes().size());
}

void AvmTraceBuilder::insert_private_state(const std::vector<FF>& siloed_nullifiers,
const std::vector<FF>& unique_note_hashes)
{
Expand Down Expand Up @@ -297,6 +312,7 @@ void AvmTraceBuilder::pay_fee()
throw std::runtime_error("Not enough balance for fee payer to pay for transaction");
}

// TS equivalent:
// writeStorage(FEE_JUICE_ADDRESS, balance_slot, updated_balance);
PublicDataWriteTreeHint write_hint = execution_hints.storage_write_hints.at(storage_write_counter++);
ASSERT(write_hint.new_leaf_preimage.value == updated_balance);
Expand Down Expand Up @@ -2623,15 +2639,17 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3

AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t slot_offset)
{
// We keep the first encountered error
AvmError error = AvmError::NO_ERROR;
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;
uint32_t unique_public_data_slot_writes = get_public_data_writes_count();
AvmError error = current_ext_call_ctx.is_static_call ? AvmError::STATIC_CALL_ALTERATION
: unique_public_data_slot_writes >= MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX
? AvmError::SIDE_EFFECT_LIMIT_REACHED
: AvmError::NO_ERROR;

if (storage_write_counter >= MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX) {
if (!is_ok(error)) {
// NOTE: the circuit constraint for this limit should only be applied
// for the storage writes performed by this opcode. An exception should before
// made for the fee juice storage write made after teardown.
error = AvmError::SIDE_EFFECT_LIMIT_REACHED;
auto row = Row{
.main_clk = clk,
.main_internal_return_ptr = internal_return_ptr,
Expand Down Expand Up @@ -2805,8 +2823,11 @@ AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash
{
auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
uint32_t inserted_note_hashes_count = get_inserted_note_hashes_count();
if (inserted_note_hashes_count >= MAX_NOTE_HASHES_PER_TX) {
AvmError error = AvmError::SIDE_EFFECT_LIMIT_REACHED;
AvmError error = current_ext_call_ctx.is_static_call ? AvmError::STATIC_CALL_ALTERATION
: inserted_note_hashes_count >= MAX_NOTE_HASHES_PER_TX ? AvmError::SIDE_EFFECT_LIMIT_REACHED
: AvmError::NO_ERROR;

if (!is_ok(error)) {
auto row = Row{
.main_clk = clk,
.main_internal_return_ptr = internal_return_ptr,
Expand All @@ -2820,8 +2841,12 @@ AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash
return error;
}

auto [row, error] = create_kernel_output_opcode(indirect, clk, note_hash_offset);
auto [row, output_error] = create_kernel_output_opcode(indirect, clk, note_hash_offset);
row.main_sel_op_emit_note_hash = FF(1);
if (is_ok(error)) {
error = output_error;
}

row.main_op_err = FF(static_cast<uint32_t>(!is_ok(error)));

AppendTreeHint note_hash_write_hint = execution_hints.note_hash_write_hints.at(note_hash_write_counter++);
Expand Down Expand Up @@ -2952,12 +2977,14 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect,

AvmError AvmTraceBuilder::op_emit_nullifier(uint8_t indirect, uint32_t nullifier_offset)
{
// We keep the first encountered error
AvmError error = AvmError::NO_ERROR;
auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;

if (nullifier_write_counter >= MAX_NULLIFIERS_PER_TX) {
error = AvmError::SIDE_EFFECT_LIMIT_REACHED;
uint32_t inserted_nullifier_count = get_inserted_nullifiers_count();
AvmError error = current_ext_call_ctx.is_static_call ? AvmError::STATIC_CALL_ALTERATION
: inserted_nullifier_count >= MAX_NULLIFIERS_PER_TX ? AvmError::SIDE_EFFECT_LIMIT_REACHED
: AvmError::NO_ERROR;

if (!is_ok(error)) {
auto row = Row{
.main_clk = clk,
.main_internal_return_ptr = internal_return_ptr,
Expand Down Expand Up @@ -3531,6 +3558,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode,
current_ext_call_ctx.last_pc = pc;
current_ext_call_ctx.success_offset = resolved_success_offset;
current_ext_call_ctx.tree_snapshot = merkle_tree_trace_builder.get_tree_snapshots();
current_ext_call_ctx.public_data_unique_writes = merkle_tree_trace_builder.get_public_data_unique_writes();
external_call_ctx_stack.emplace(current_ext_call_ctx);

// Ext Ctx setup
Expand All @@ -3541,6 +3569,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode,

current_ext_call_ctx = ExtCallCtx{ .context_id = static_cast<uint8_t>(clk),
.parent_id = current_ext_call_ctx.context_id,
.is_static_call = opcode == OpCode::STATICCALL,
.contract_address = read_addr.val,
.calldata = calldata,
.nested_returndata = {},
Expand All @@ -3549,7 +3578,8 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode,
.l2_gas = static_cast<uint32_t>(read_gas_l2.val),
.da_gas = static_cast<uint32_t>(read_gas_da.val),
.internal_return_ptr_stack = {},
.tree_snapshot = {} };
.tree_snapshot = {},
.public_data_unique_writes = {} };
set_pc(0);

return error;
Expand Down Expand Up @@ -3662,8 +3692,6 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset
// Update the call_ptr before we write the success flag
set_call_ptr(static_cast<uint8_t>(current_ext_call_ctx.context_id));
write_to_memory(current_ext_call_ctx.success_offset, FF::one(), AvmMemoryTag::U1);
// If we successfully returned, we update the current parent's tree state
current_ext_call_ctx.tree_snapshot = merkle_tree_trace_builder.get_tree_snapshots();
}
}

Expand Down Expand Up @@ -3760,7 +3788,8 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset
set_call_ptr(static_cast<uint8_t>(current_ext_call_ctx.context_id));
write_to_memory(current_ext_call_ctx.success_offset, FF::zero(), AvmMemoryTag::U1);
// Reset the tree state to parent's snapshot
merkle_tree_trace_builder.set_tree_snapshots(current_ext_call_ctx.tree_snapshot);
merkle_tree_trace_builder.restore_tree_state(current_ext_call_ctx.tree_snapshot,
current_ext_call_ctx.public_data_unique_writes);
}
}

Expand All @@ -3775,7 +3804,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset
.main_internal_return_ptr = FF(internal_return_ptr),
.main_op_err = FF(static_cast<uint32_t>(!is_ok(error))),
.main_pc = pc,
.main_sel_op_external_return = 1,
.main_sel_op_external_revert = 1,
});

pc = is_top_level ? UINT32_MAX : current_ext_call_ctx.last_pc;
Expand Down
4 changes: 4 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ class AvmTraceBuilder {
struct ExtCallCtx {
uint32_t context_id; // This is the unique id of the ctx, we'll use the clk
uint32_t parent_id;
bool is_static_call = false;
FF contract_address{};
std::vector<FF> calldata;
std::vector<FF> nested_returndata;
Expand All @@ -272,6 +273,7 @@ class AvmTraceBuilder {
uint32_t da_gas;
std::stack<uint32_t> internal_return_ptr_stack;
TreeSnapshots tree_snapshot; // This is the tree state at the time of the call
std::unordered_set<FF> public_data_unique_writes;
};

ExtCallCtx current_ext_call_ctx{};
Expand Down Expand Up @@ -363,6 +365,8 @@ class AvmTraceBuilder {
IntermRegister reg,
AvmMemTraceBuilder::MemOpOwner mem_op_owner = AvmMemTraceBuilder::MAIN);
uint32_t get_inserted_note_hashes_count();
uint32_t get_inserted_nullifiers_count();
uint32_t get_public_data_writes_count();
FF get_tx_hash() const { return public_inputs.previous_non_revertible_accumulated_data.nullifiers[0]; }

// TODO: remove these once everything is constrained.
Expand Down

0 comments on commit c114f31

Please sign in to comment.