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

refactor: counters #7342

Merged
merged 10 commits into from
Jul 8, 2024
22 changes: 11 additions & 11 deletions barretenberg/cpp/pil/avm/constants_gen.pil
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ namespace constants(256);
pol ADDRESS_SELECTOR = 1;
pol STORAGE_ADDRESS_SELECTOR = 1;
pol FUNCTION_SELECTOR_SELECTOR = 2;
pol START_GLOBAL_VARIABLES = 29;
pol CHAIN_ID_SELECTOR = 29;
pol VERSION_SELECTOR = 30;
pol BLOCK_NUMBER_SELECTOR = 31;
pol TIMESTAMP_SELECTOR = 32;
pol COINBASE_SELECTOR = 33;
pol FEE_PER_DA_GAS_SELECTOR = 35;
pol FEE_PER_L2_GAS_SELECTOR = 36;
pol END_GLOBAL_VARIABLES = 37;
pol START_SIDE_EFFECT_COUNTER = 37;
pol TRANSACTION_FEE_SELECTOR = 40;
pol START_GLOBAL_VARIABLES = 28;
pol CHAIN_ID_SELECTOR = 28;
pol VERSION_SELECTOR = 29;
pol BLOCK_NUMBER_SELECTOR = 30;
pol TIMESTAMP_SELECTOR = 31;
pol COINBASE_SELECTOR = 32;
pol FEE_PER_DA_GAS_SELECTOR = 34;
pol FEE_PER_L2_GAS_SELECTOR = 35;
pol END_GLOBAL_VARIABLES = 36;
pol START_SIDE_EFFECT_COUNTER = 36;
pol TRANSACTION_FEE_SELECTOR = 39;
pol START_NOTE_HASH_EXISTS_WRITE_OFFSET = 0;
pol START_NULLIFIER_EXISTS_OFFSET = 16;
pol START_NULLIFIER_NON_EXISTS_OFFSET = 32;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1227,56 +1227,56 @@ template <typename FF_> class mainImpl {
// Contribution 118
{
Avm_DECLARE_VIEWS(118);
auto tmp = (main_sel_op_transaction_fee * (kernel_kernel_in_offset - FF(40)));
auto tmp = (main_sel_op_transaction_fee * (kernel_kernel_in_offset - FF(39)));
tmp *= scaling_factor;
std::get<118>(evals) += tmp;
}
// Contribution 119
{
Avm_DECLARE_VIEWS(119);
auto tmp = (main_sel_op_chain_id * (kernel_kernel_in_offset - FF(29)));
auto tmp = (main_sel_op_chain_id * (kernel_kernel_in_offset - FF(28)));
tmp *= scaling_factor;
std::get<119>(evals) += tmp;
}
// Contribution 120
{
Avm_DECLARE_VIEWS(120);
auto tmp = (main_sel_op_version * (kernel_kernel_in_offset - FF(30)));
auto tmp = (main_sel_op_version * (kernel_kernel_in_offset - FF(29)));
tmp *= scaling_factor;
std::get<120>(evals) += tmp;
}
// Contribution 121
{
Avm_DECLARE_VIEWS(121);
auto tmp = (main_sel_op_block_number * (kernel_kernel_in_offset - FF(31)));
auto tmp = (main_sel_op_block_number * (kernel_kernel_in_offset - FF(30)));
tmp *= scaling_factor;
std::get<121>(evals) += tmp;
}
// Contribution 122
{
Avm_DECLARE_VIEWS(122);
auto tmp = (main_sel_op_timestamp * (kernel_kernel_in_offset - FF(32)));
auto tmp = (main_sel_op_timestamp * (kernel_kernel_in_offset - FF(31)));
tmp *= scaling_factor;
std::get<122>(evals) += tmp;
}
// Contribution 123
{
Avm_DECLARE_VIEWS(123);
auto tmp = (main_sel_op_coinbase * (kernel_kernel_in_offset - FF(33)));
auto tmp = (main_sel_op_coinbase * (kernel_kernel_in_offset - FF(32)));
tmp *= scaling_factor;
std::get<123>(evals) += tmp;
}
// Contribution 124
{
Avm_DECLARE_VIEWS(124);
auto tmp = (main_sel_op_fee_per_da_gas * (kernel_kernel_in_offset - FF(35)));
auto tmp = (main_sel_op_fee_per_da_gas * (kernel_kernel_in_offset - FF(34)));
tmp *= scaling_factor;
std::get<124>(evals) += tmp;
}
// Contribution 125
{
Avm_DECLARE_VIEWS(125);
auto tmp = (main_sel_op_fee_per_l2_gas * (kernel_kernel_in_offset - FF(36)));
auto tmp = (main_sel_op_fee_per_l2_gas * (kernel_kernel_in_offset - FF(35)));
tmp *= scaling_factor;
std::get<125>(evals) += tmp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#define AZTEC_ADDRESS_LENGTH 1
#define GAS_FEES_LENGTH 2
#define GAS_LENGTH 2
#define CALL_CONTEXT_LENGTH 6
#define CALL_CONTEXT_LENGTH 5
#define CONTENT_COMMITMENT_LENGTH 4
#define CONTRACT_STORAGE_READ_LENGTH 3
#define CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH 3
Expand All @@ -30,23 +30,23 @@
#define STATE_REFERENCE_LENGTH 8
#define TOTAL_FEES_LENGTH 1
#define HEADER_LENGTH 23
#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 482
#define PUBLIC_CONTEXT_INPUTS_LENGTH 41
#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 481
#define PUBLIC_CONTEXT_INPUTS_LENGTH 40
#define SENDER_SELECTOR 0
#define ADDRESS_SELECTOR 1
#define STORAGE_ADDRESS_SELECTOR 1
#define FUNCTION_SELECTOR_SELECTOR 2
#define START_GLOBAL_VARIABLES 29
#define CHAIN_ID_SELECTOR 29
#define VERSION_SELECTOR 30
#define BLOCK_NUMBER_SELECTOR 31
#define TIMESTAMP_SELECTOR 32
#define COINBASE_SELECTOR 33
#define FEE_PER_DA_GAS_SELECTOR 35
#define FEE_PER_L2_GAS_SELECTOR 36
#define END_GLOBAL_VARIABLES 37
#define START_SIDE_EFFECT_COUNTER 37
#define TRANSACTION_FEE_SELECTOR 40
#define START_GLOBAL_VARIABLES 28
#define CHAIN_ID_SELECTOR 28
#define VERSION_SELECTOR 29
#define BLOCK_NUMBER_SELECTOR 30
#define TIMESTAMP_SELECTOR 31
#define COINBASE_SELECTOR 32
#define FEE_PER_DA_GAS_SELECTOR 34
#define FEE_PER_L2_GAS_SELECTOR 35
#define END_GLOBAL_VARIABLES 36
#define START_SIDE_EFFECT_COUNTER 36
#define TRANSACTION_FEE_SELECTOR 39
#define START_NOTE_HASH_EXISTS_WRITE_OFFSET 0
#define START_NULLIFIER_EXISTS_OFFSET 16
#define START_NULLIFIER_NON_EXISTS_OFFSET 32
Expand Down
44 changes: 22 additions & 22 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ library Constants {
uint256 internal constant GAS_FEES_LENGTH = 2;
uint256 internal constant GAS_LENGTH = 2;
uint256 internal constant GAS_SETTINGS_LENGTH = 7;
uint256 internal constant CALL_CONTEXT_LENGTH = 6;
uint256 internal constant CALL_CONTEXT_LENGTH = 5;
uint256 internal constant CONTENT_COMMITMENT_LENGTH = 4;
uint256 internal constant CONTRACT_INSTANCE_LENGTH = 5;
uint256 internal constant CONTRACT_STORAGE_READ_LENGTH = 3;
Expand Down Expand Up @@ -164,29 +164,29 @@ library Constants {
uint256 internal constant NULLIFIER_LENGTH = 3;
uint256 internal constant SCOPED_NULLIFIER_LENGTH = 4;
uint256 internal constant CALLER_CONTEXT_LENGTH = 3;
uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 16;
uint256 internal constant SCOPED_PRIVATE_CALL_REQUEST_LENGTH = 17;
uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 15;
uint256 internal constant SCOPED_PRIVATE_CALL_REQUEST_LENGTH = 16;
uint256 internal constant ROLLUP_VALIDATION_REQUESTS_LENGTH = 2;
uint256 internal constant STATE_REFERENCE_LENGTH = 8;
uint256 internal constant TX_CONTEXT_LENGTH = 9;
uint256 internal constant TX_REQUEST_LENGTH = 13;
uint256 internal constant TOTAL_FEES_LENGTH = 1;
uint256 internal constant HEADER_LENGTH = 23;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 433;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 482;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 436;
uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH = 41;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 428;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 481;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 431;
uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH = 40;
uint256 internal constant AGGREGATION_OBJECT_LENGTH = 16;
uint256 internal constant SCOPED_READ_REQUEST_LEN = 3;
uint256 internal constant PUBLIC_DATA_READ_LENGTH = 2;
uint256 internal constant VALIDATION_REQUESTS_LENGTH = 1026;
uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 333;
uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 41;
uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 16;
uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 15;
uint256 internal constant CALL_REQUEST_LENGTH = 7;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1168;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2244;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1160;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2236;
uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 983;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3259;
uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 384;
Expand All @@ -211,18 +211,18 @@ library Constants {
uint256 internal constant ADDRESS_SELECTOR = 1;
uint256 internal constant STORAGE_ADDRESS_SELECTOR = 1;
uint256 internal constant FUNCTION_SELECTOR_SELECTOR = 2;
uint256 internal constant START_GLOBAL_VARIABLES = 29;
uint256 internal constant CHAIN_ID_SELECTOR = 29;
uint256 internal constant VERSION_SELECTOR = 30;
uint256 internal constant BLOCK_NUMBER_SELECTOR = 31;
uint256 internal constant TIMESTAMP_SELECTOR = 32;
uint256 internal constant COINBASE_SELECTOR = 33;
uint256 internal constant UNUSED_FEE_RECIPIENT_SELECTOR = 34;
uint256 internal constant FEE_PER_DA_GAS_SELECTOR = 35;
uint256 internal constant FEE_PER_L2_GAS_SELECTOR = 36;
uint256 internal constant END_GLOBAL_VARIABLES = 37;
uint256 internal constant START_SIDE_EFFECT_COUNTER = 37;
uint256 internal constant TRANSACTION_FEE_SELECTOR = 40;
uint256 internal constant START_GLOBAL_VARIABLES = 28;
uint256 internal constant CHAIN_ID_SELECTOR = 28;
uint256 internal constant VERSION_SELECTOR = 29;
uint256 internal constant BLOCK_NUMBER_SELECTOR = 30;
uint256 internal constant TIMESTAMP_SELECTOR = 31;
uint256 internal constant COINBASE_SELECTOR = 32;
uint256 internal constant UNUSED_FEE_RECIPIENT_SELECTOR = 33;
uint256 internal constant FEE_PER_DA_GAS_SELECTOR = 34;
uint256 internal constant FEE_PER_L2_GAS_SELECTOR = 35;
uint256 internal constant END_GLOBAL_VARIABLES = 36;
uint256 internal constant START_SIDE_EFFECT_COUNTER = 36;
uint256 internal constant TRANSACTION_FEE_SELECTOR = 39;
uint256 internal constant START_NOTE_HASH_EXISTS_WRITE_OFFSET = 0;
uint256 internal constant START_NULLIFIER_EXISTS_OFFSET = 16;
uint256 internal constant START_NULLIFIER_NON_EXISTS_OFFSET = 32;
Expand Down
3 changes: 1 addition & 2 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ impl PrivateContext {
is_delegate_call
);

assert_eq(item.public_inputs.call_context.side_effect_counter, start_side_effect_counter);
assert_eq(item.public_inputs.start_side_effect_counter, start_side_effect_counter);
let end_side_effect_counter = item.public_inputs.end_side_effect_counter;
self.side_effect_counter = end_side_effect_counter + 1;
Expand Down Expand Up @@ -564,7 +563,7 @@ impl PrivateContext {
assert(contract_address.eq(item.contract_address));
assert(function_selector.eq(item.function_data.selector));

assert_eq(item.public_inputs.call_context.side_effect_counter, self.side_effect_counter);
assert_eq(item.public_inputs.start_side_effect_counter, self.side_effect_counter);

assert(args_hash == item.public_inputs.args_hash);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub fn parse_public_call_stack_item_from_oracle(fields: [Field; ENQUEUE_PUBLIC_F
function_data: FunctionData { selector: FunctionSelector::from_field(reader.read()), is_private: false },
public_inputs: PublicCircuitPublicInputs {
call_context: reader.read_struct(CallContext::deserialize),
start_side_effect_counter: reader.read_u32(),
args_hash: reader.read(),
returns_hash: 0,
note_hash_read_requests: [ReadRequest::empty(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
Expand All @@ -97,7 +98,6 @@ pub fn parse_public_call_stack_item_from_oracle(fields: [Field; ENQUEUE_PUBLIC_F
note_hashes: [NoteHash::empty(); MAX_NOTE_HASHES_PER_CALL],
nullifiers: [Nullifier::empty(); MAX_NULLIFIERS_PER_CALL],
l2_to_l1_msgs: [L2ToL1Message::empty(); MAX_L2_TO_L1_MSGS_PER_CALL],
start_side_effect_counter: 0,
end_side_effect_counter: 0,
unencrypted_logs_hashes: [LogHash::empty(); MAX_UNENCRYPTED_LOGS_PER_CALL],
historical_header: Header::empty(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,6 @@ impl PrivateCallDataValidator {
let call_context = public_inputs.call_context;
assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall");
assert(call_context.is_static_call == false, "Users cannot make a static call");

let min_revertible_side_effect_counter = public_inputs.min_revertible_side_effect_counter;
let first_revertible_index = find_first_revertible_private_call_request_index(public_inputs);
// No need to check that the min_revertible_side_effect_counter falls in the counter range of the private call.
// It is valid as long as it does not fall in the middle of any nested call.
validate_split_ranges(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it down to validate_private_call_requests, which is run by both init and inner. Because now we can set min_revertible_side_effect_counter from any private function, not just the entrypoint. So we need to perform this check when the counter is set.

min_revertible_side_effect_counter,
first_revertible_index,
public_inputs.private_call_requests,
self.array_lengths.private_call_requests
);
}

// Confirm that the TxRequest (user's intent) matches the private call being executed.
Expand Down Expand Up @@ -263,18 +252,34 @@ impl PrivateCallDataValidator {
}

fn validate_private_call_requests(self) {
let call_requests = self.data.call_stack_item.public_inputs.private_call_requests;
let public_inputs = self.data.call_stack_item.public_inputs;
let call_requests = public_inputs.private_call_requests;
let num_requests = self.array_lengths.private_call_requests;
let mut should_check = true;
for i in 0..call_requests.len() {
should_check &= i != num_requests;
if should_check {
validate_caller_context(
call_requests[i].caller_context,
self.data.call_stack_item.public_inputs.call_context
);
validate_caller_context(call_requests[i].caller_context, public_inputs.call_context);
}
}

// Check that the min_revertible_side_effect_counter does not fall in the middle of any nested calls.
// If it is possible, one can create a custom account contract, set the min_revertible_side_effect_counter to a
// value that falls in a transfer function, make the tx revert which then preserves the note hashes and discards
// the nullifiers.
//
// We don't have to use the aggregated min_revertible_side_effect_counter in the output for the below check.
// If the current call's min_revertible_side_effect_counter is 0, it means the counter might be set by another
// function. This check for that function guarantees that the counter won't fall into one of its nested calls.
// In this case, we can simply use 0 and make the following check pass.
let min_revertible_side_effect_counter = public_inputs.min_revertible_side_effect_counter;
let first_revertible_index = find_first_revertible_private_call_request_index(public_inputs);
validate_split_ranges(
min_revertible_side_effect_counter,
first_revertible_index,
public_inputs.private_call_requests,
self.array_lengths.private_call_requests
);
}

fn validate_public_call_requests(self) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ pub fn split_to_public(
data: PrivateAccumulatedDataBuilder,
min_revertible_side_effect_counter: u32
) -> (PublicAccumulatedDataBuilder, PublicAccumulatedDataBuilder) {
assert(min_revertible_side_effect_counter != 0, "min_revertible_side_effect_counter must not be 0");

let mut non_revertible_builder = PublicAccumulatedDataBuilder::empty();
let mut revertible_builder = PublicAccumulatedDataBuilder::empty();

Expand All @@ -26,10 +28,7 @@ pub fn split_to_public(
}

let nullifiers = data.nullifiers;
// First nullifier is always non-revertible.
let first_nullifier = nullifiers.get_unchecked(0).expose_to_public();
non_revertible_builder.nullifiers.push(first_nullifier);
for i in 1..nullifiers.max_len() {
for i in 0..nullifiers.max_len() {
if i < nullifiers.len() {
let nullifier = nullifiers.get_unchecked(i);
let public_nullifier = nullifier.expose_to_public();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ impl TailToPublicOutputValidator {

fn validate_propagated_sorted_siloed_values(self, hints: TailToPublicOutputHints) {
let split_counter = self.previous_kernel.min_revertible_side_effect_counter;
assert(split_counter != 0, "min_revertible_side_effect_counter must not be 0");

let prev_data = self.previous_kernel.end;
let output_non_revertible = self.output.end_non_revertible;
let output_revertible = self.output.end;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ mod tests {
let mut previous_kernel = FixtureBuilder::new().in_vk_tree(PRIVATE_KERNEL_INNER_INDEX);
previous_kernel.tx_context.gas_settings.gas_limits = Gas::new(1_000_000, 1_000_000);
previous_kernel.set_first_nullifier();
previous_kernel.end_setup();
previous_kernel.push_public_call_request(1, false);

PrivateKernelTailToPublicInputsBuilder { previous_kernel }
Expand Down Expand Up @@ -170,6 +171,13 @@ mod tests {
builder.failed();
}

#[test(should_fail_with="min_revertible_side_effect_counter must not be 0")]
fn zero_min_revertible_side_effect_counter_fails() {
let mut builder = PrivateKernelTailToPublicInputsBuilder::new();
builder.previous_kernel.min_revertible_side_effect_counter = 0;
builder.failed();
}

#[test(should_fail_with="Must have public calls when exporting public kernel data from the tail circuit")]
fn no_public_calls_should_fail() {
let mut builder = PrivateKernelTailToPublicInputsBuilder::new();
Expand Down
Loading
Loading