Skip to content

Commit

Permalink
refactor: optimize public call stack item hashing (#7330)
Browse files Browse the repository at this point in the history
Fixes #7092 for public, very similar to #7285.

Introduces a `compressed` version of the `public_call_stack_item` that
is what we will be hashing instead of the full inputs with most values
populated by zeros.

Notable savings is the SchnorrAccount entrypoint which is reduces all
the way to 88K 👀
  • Loading branch information
LHerskind authored Jul 5, 2024
1 parent 586c0b0 commit 4a5093c
Show file tree
Hide file tree
Showing 19 changed files with 312 additions and 68 deletions.
7 changes: 4 additions & 3 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ library Constants {
uint256 internal constant SCOPED_ENCRYPTED_LOG_HASH_LENGTH = 5;
uint256 internal constant NOTE_LOG_HASH_LENGTH = 4;
uint256 internal constant NOTE_HASH_LENGTH = 2;
uint256 internal constant SCOPED_NOTE_HASH_LENGTH = 4;
uint256 internal constant SCOPED_NOTE_HASH_LENGTH = 3;
uint256 internal constant NULLIFIER_LENGTH = 3;
uint256 internal constant SCOPED_NULLIFIER_LENGTH = 4;
uint256 internal constant CALLER_CONTEXT_LENGTH = 3;
Expand All @@ -165,9 +165,10 @@ library Constants {
uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 333;
uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 40;
uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 16;
uint256 internal constant CALL_REQUEST_LENGTH = 7;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1232;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2307;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1168;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2243;
uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 983;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3258;
uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 383;
Expand Down
4 changes: 2 additions & 2 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl PrivateContext {
);

self.side_effect_counter = self.side_effect_counter + 1;
self.public_call_stack_hashes.push(item.hash());
self.public_call_stack_hashes.push(item.get_compressed().hash());
}

pub fn set_public_teardown_function<ARGS_COUNT>(
Expand Down Expand Up @@ -549,7 +549,7 @@ impl PrivateContext {
);

self.side_effect_counter = self.side_effect_counter + 1;
self.public_teardown_function_hash = item.hash();
self.public_teardown_function_hash = item.get_compressed().hash();
}

fn validate_call_stack_item_from_oracle(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ pub fn validate_call_against_request(public_call: PublicCallData, request: CallR
let call_stack_item = public_call.call_stack_item;

assert(
request.hash == call_stack_item.hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the call stack"
request.hash == call_stack_item.get_compressed().hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the call stack"
);

let call_context = call_stack_item.public_inputs.call_context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl PublicKernelAppLogicCircuitPrivateInputs {
// and we aren't updating the public end values, so we want this kernel circuit to solve.
// So just check that the call request is the same as the one we expected.
assert(
reverted_call_request.hash == self.public_call.call_stack_item.hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the call stack"
reverted_call_request.hash == self.public_call.call_stack_item.get_compressed().hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the call stack"
);
}

Expand Down Expand Up @@ -150,7 +150,7 @@ mod tests {
pub fn execute(&mut self) -> PublicKernelCircuitPublicInputs {
let public_call = self.public_call.finish();
// Adjust the call stack item hash for the current call in the previous iteration.
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
let is_delegate_call = public_call.call_stack_item.public_inputs.call_context.is_delegate_call;
self.previous_kernel.push_public_call_request(hash, is_delegate_call);
let previous_kernel = self.previous_kernel.to_public_kernel_data(true);
Expand Down Expand Up @@ -598,7 +598,7 @@ mod tests {
let mut builder = PublicKernelAppLogicCircuitPrivateInputsBuilder::new();
builder.public_call.public_inputs.revert_code = 1;
let public_call = builder.public_call.finish();
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();

builder.previous_kernel.push_public_call_request(hash, false);
builder.previous_kernel.push_public_call_request(hash, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,13 @@ mod tests {
pub fn stub_teardown_call(&mut self) {
let teardown_call = PublicCallDataBuilder::new();
let teardown_call = teardown_call.finish();
let teardown_call_hash = teardown_call.call_stack_item.hash();
let teardown_call_hash = teardown_call.call_stack_item.get_compressed().hash();
let teardown_is_delegate_call = teardown_call.call_stack_item.public_inputs.call_context.is_delegate_call;
self.previous_kernel.push_public_call_request(teardown_call_hash, teardown_is_delegate_call);
}

pub fn push_public_call(&mut self, public_call: PublicCallData) {
let public_call_hash = public_call.call_stack_item.hash();
let public_call_hash = public_call.call_stack_item.get_compressed().hash();
let setup_is_delegate_call = public_call.call_stack_item.public_inputs.call_context.is_delegate_call;
self.previous_kernel.push_public_call_request(public_call_hash, setup_is_delegate_call);
}
Expand Down Expand Up @@ -242,7 +242,7 @@ mod tests {
builder.stub_teardown_call();
let public_call = builder.public_call.finish();

let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
// Tweak the call stack item hash.
builder.previous_kernel.push_public_call_request(hash + 1, false);
let previous_kernel = builder.previous_kernel.to_public_kernel_data(false);
Expand Down Expand Up @@ -285,7 +285,7 @@ mod tests {

let public_call = builder.public_call.finish();

let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
// Caller context is empty for regular calls.
let is_delegate_call = false;
builder.previous_kernel.push_public_call_request(hash, is_delegate_call);
Expand Down Expand Up @@ -591,7 +591,7 @@ mod tests {
let mut builder = PublicKernelSetupCircuitPrivateInputsBuilder::new();
builder.public_call.public_inputs.revert_code = 0;
let public_call = builder.public_call.finish();
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();

builder.previous_kernel.push_public_call_request(hash, false);
builder.previous_kernel.push_public_call_request(hash, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl PublicKernelTeardownCircuitPrivateInputs {
let reverted_call_request = remaining_calls.pop();

assert(
reverted_call_request.hash == self.public_call.call_stack_item.hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the teardown call stack"
reverted_call_request.hash == self.public_call.call_stack_item.get_compressed().hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the teardown call stack"
);
}

Expand Down Expand Up @@ -198,7 +198,7 @@ mod tests {
pub fn execute(&mut self) -> PublicKernelCircuitPublicInputs {
let public_call = self.public_call.finish();
// Adjust the call stack item hash for the current call in the previous iteration.
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
let is_delegate_call = public_call.call_stack_item.public_inputs.call_context.is_delegate_call;
self.previous_kernel.push_public_teardown_call_request(hash, is_delegate_call);
let mut previous_kernel = self.previous_kernel.to_public_kernel_data(true);
Expand Down Expand Up @@ -254,7 +254,7 @@ mod tests {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new();
let public_call = builder.public_call.finish();

let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
// Tweak the call stack item hash.
builder.previous_kernel.push_public_teardown_call_request(hash + 1, false);
let previous_kernel = builder.previous_kernel.to_public_kernel_data(true);
Expand Down Expand Up @@ -295,7 +295,7 @@ mod tests {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new().is_delegate_call();
let public_call = builder.public_call.finish();

let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
// Caller context is empty for regular calls.
let is_delegate_call = false;
builder.previous_kernel.push_public_teardown_call_request(hash, is_delegate_call);
Expand Down Expand Up @@ -517,7 +517,7 @@ mod tests {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new();
builder.public_call.public_inputs.revert_code = 1;
let public_call = builder.public_call.finish();
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
builder.previous_kernel.push_public_teardown_call_request(hash, false);
// push again to check that the stack is cleared
builder.previous_kernel.push_public_teardown_call_request(hash, false);
Expand All @@ -535,7 +535,7 @@ mod tests {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new();
builder.previous_kernel.revert_code = 1;
let public_call = builder.public_call.finish();
let hash = public_call.call_stack_item.hash();
let hash = public_call.call_stack_item.get_compressed().hash();
builder.previous_kernel.push_public_teardown_call_request(hash, false);
// push again to check that we keep one item after popping the current call
builder.previous_kernel.push_public_teardown_call_request(hash, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mod call_request;
mod private_call_request;
mod private_call_stack_item;
mod public_call_stack_item;
mod public_call_stack_item_compressed;
mod call_context;
mod caller_context;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::abis::{function_data::FunctionData, public_circuit_public_inputs::PublicCircuitPublicInputs};
use crate::abis::{
function_data::FunctionData, public_circuit_public_inputs::PublicCircuitPublicInputs,
public_call_stack_item_compressed::PublicCallStackItemCompressed
};
use crate::address::AztecAddress;
use crate::constants::GENERATOR_INDEX__CALL_STACK_ITEM;
use crate::traits::Hash;
Expand All @@ -12,25 +15,9 @@ struct PublicCallStackItem {
is_execution_request: bool,
}

impl Hash for PublicCallStackItem {
fn hash(self) -> Field {
let item = if self.is_execution_request {
self.as_execution_request()
} else {
self
};

std::hash::pedersen_hash_with_separator([
item.contract_address.to_field(),
item.function_data.hash(),
item.public_inputs.hash(),
], GENERATOR_INDEX__CALL_STACK_ITEM)
}
}

impl PublicCallStackItem {
fn as_execution_request(self) -> Self {
// WARNING: if updating, see comment in public_call_stack_item.ts's `PublicCallStackItem.hash()`
// WARNING: if updating, see comment in public_call_stack_item.ts's `PublicCallStackItem.getCompressed()`
let public_inputs = self.public_inputs;
let mut request_public_inputs = PublicCircuitPublicInputs::empty();
request_public_inputs.call_context = public_inputs.call_context;
Expand All @@ -44,6 +31,25 @@ impl PublicCallStackItem {
};
call_stack_item
}

fn get_compressed(self) -> PublicCallStackItemCompressed {
let item = if self.is_execution_request {
self.as_execution_request()
} else {
self
};

PublicCallStackItemCompressed {
contract_address: item.contract_address,
call_context: item.public_inputs.call_context,
function_data: item.function_data,
args_hash: item.public_inputs.args_hash,
returns_hash: item.public_inputs.returns_hash,
revert_code: item.public_inputs.revert_code,
start_gas_left: item.public_inputs.start_gas_left,
end_gas_left: item.public_inputs.end_gas_left
}
}
}

mod tests {
Expand All @@ -70,8 +76,8 @@ mod tests {
let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: true, function_data };

// Value from public_call_stack_item.test.ts "Computes a callstack item request hash" test
let test_data_call_stack_item_request_hash = 0x022a2b82af83606ae5a8d4955ef6215e54025193356318aefbde3b5026952953;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_request_hash);
let test_data_call_stack_item_request_hash = 0x1331fdec4ec7bd6bb23447c47753659b68e3a285d812ab6eaf9258a902d16e8e;
assert_eq(call_stack_item.get_compressed().hash(), test_data_call_stack_item_request_hash);
}

#[test]
Expand All @@ -88,7 +94,7 @@ mod tests {
let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: false, function_data };

// Value from public_call_stack_item.test.ts "Computes a callstack item hash" test
let test_data_call_stack_item_hash = 0x23a1d22e7bf37df7d68e8fcbfb7e016c060194b7915e3771e2dcd72cea26e427;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_hash);
let test_data_call_stack_item_hash = 0x1331fdec4ec7bd6bb23447c47753659b68e3a285d812ab6eaf9258a902d16e8e;
assert_eq(call_stack_item.get_compressed().hash(), test_data_call_stack_item_hash);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
use crate::abis::{call_context::CallContext, function_data::FunctionData, gas::Gas};
use crate::address::AztecAddress;
use crate::constants::{GENERATOR_INDEX__CALL_STACK_ITEM, PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH};
use crate::traits::{Hash, Empty, Serialize, Deserialize};
use crate::utils::reader::Reader;

/**
* A compressed version of the PublicCallStackItem struct used to compute the "hash"
* of a PublicCallStackItem.
*
* Historically, we have been zeroing most values in the PublicCallStackItem struct
* to compute the hash involved when adding a PublicCallStackItem to the PublicCallStack.
*
* This struct is used to store the values that we did not zero out, and allow us to hash
* only these, thereby skipping a lot of computation and saving us a lot of constraints
*
* Essentially this struct exists such that we don't have a `hash` function in the
* PublicCallStackItem struct that practically throws away some values of the struct
* without clearly indicating that it does so.
*/
struct PublicCallStackItemCompressed {
contract_address: AztecAddress,
call_context: CallContext,
function_data: FunctionData,
args_hash: Field,
returns_hash: Field,
revert_code: u8,
start_gas_left: Gas,
end_gas_left: Gas,
}

impl Eq for PublicCallStackItemCompressed {
fn eq(self, other: PublicCallStackItemCompressed) -> bool {
(self.contract_address == other.contract_address)
& (self.call_context == other.call_context)
& (self.function_data == other.function_data)
& (self.args_hash == other.args_hash)
& (self.returns_hash == other.returns_hash)
& (self.revert_code == other.revert_code)
& (self.start_gas_left == other.start_gas_left)
& (self.end_gas_left == other.end_gas_left)
}
}

impl Hash for PublicCallStackItemCompressed {
fn hash(self) -> Field {
std::hash::pedersen_hash_with_separator(self.serialize(), GENERATOR_INDEX__CALL_STACK_ITEM)
}
}

impl Empty for PublicCallStackItemCompressed {
fn empty() -> Self {
PublicCallStackItemCompressed {
contract_address: AztecAddress::empty(),
call_context: CallContext::empty(),
function_data: FunctionData::empty(),
args_hash: 0,
returns_hash: 0,
revert_code: 0,
start_gas_left: Gas::empty(),
end_gas_left: Gas::empty(),
}
}
}

impl Serialize<PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH> for PublicCallStackItemCompressed {
fn serialize(self) -> [Field; PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH] {
let mut fields: BoundedVec<Field, PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH> = BoundedVec::new();

fields.push(self.contract_address.to_field());
fields.extend_from_array(self.call_context.serialize());
fields.extend_from_array(self.function_data.serialize());
fields.push(self.args_hash);
fields.push(self.returns_hash);
fields.push(self.revert_code as Field);
fields.extend_from_array(self.start_gas_left.serialize());
fields.extend_from_array(self.end_gas_left.serialize());

assert_eq(fields.len(), PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH);

fields.storage
}
}

impl Deserialize<PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH> for PublicCallStackItemCompressed {
fn deserialize(fields: [Field; PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH]) -> PublicCallStackItemCompressed {
let mut reader = Reader::new(fields);

let item = PublicCallStackItemCompressed {
contract_address: reader.read_struct(AztecAddress::deserialize),
call_context: reader.read_struct(CallContext::deserialize),
function_data: reader.read_struct(FunctionData::deserialize),
args_hash: reader.read(),
returns_hash: reader.read(),
revert_code: reader.read() as u8,
start_gas_left: reader.read_struct(Gas::deserialize),
end_gas_left: reader.read_struct(Gas::deserialize),
};
reader.finish();
item
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ global PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
global COMBINED_ACCUMULATED_DATA_LENGTH = MAX_NOTE_HASHES_PER_TX + MAX_NULLIFIERS_PER_TX + MAX_L2_TO_L1_MSGS_PER_TX + 6 + (MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX * PUBLIC_DATA_UPDATE_REQUEST_LENGTH) + GAS_LENGTH;
global COMBINED_CONSTANT_DATA_LENGTH = HEADER_LENGTH + TX_CONTEXT_LENGTH + GLOBAL_VARIABLES_LENGTH;

global PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = AZTEC_ADDRESS_LENGTH + CALL_CONTEXT_LENGTH + FUNCTION_DATA_LENGTH + 3 + 2 * GAS_LENGTH;
global CALL_REQUEST_LENGTH = 1 + AZTEC_ADDRESS_LENGTH + CALLER_CONTEXT_LENGTH + 2;
global PRIVATE_ACCUMULATED_DATA_LENGTH = (SCOPED_NOTE_HASH_LENGTH * MAX_NOTE_HASHES_PER_TX) + (SCOPED_NULLIFIER_LENGTH * MAX_NULLIFIERS_PER_TX) + (MAX_L2_TO_L1_MSGS_PER_TX * SCOPED_L2_TO_L1_MESSAGE_LENGTH) + (NOTE_LOG_HASH_LENGTH * MAX_NOTE_ENCRYPTED_LOGS_PER_TX) + (SCOPED_ENCRYPTED_LOG_HASH_LENGTH * MAX_ENCRYPTED_LOGS_PER_TX) + (SCOPED_LOG_HASH_LENGTH * MAX_UNENCRYPTED_LOGS_PER_TX) + (SCOPED_PRIVATE_CALL_REQUEST_LENGTH * MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX) + (CALL_REQUEST_LENGTH * MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX);
global PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 1 + VALIDATION_REQUESTS_LENGTH + PRIVATE_ACCUMULATED_DATA_LENGTH + COMBINED_CONSTANT_DATA_LENGTH + CALL_REQUEST_LENGTH + AZTEC_ADDRESS_LENGTH;
Expand Down
Loading

0 comments on commit 4a5093c

Please sign in to comment.