Skip to content

Commit

Permalink
refactor: optimize public call stack item hashing
Browse files Browse the repository at this point in the history
  • Loading branch information
LHerskind committed Jul 4, 2024
1 parent 58566d7 commit b790ce8
Show file tree
Hide file tree
Showing 19 changed files with 224 additions and 77 deletions.
1 change: 1 addition & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ 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 = 10;
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;
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,22 +15,6 @@ 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()`
Expand All @@ -44,6 +31,15 @@ impl PublicCallStackItem {
};
call_stack_item
}

fn get_compressed(self) -> PublicCallStackItemCompressed {
PublicCallStackItemCompressed {
contract_address: self.contract_address,
call_context: self.public_inputs.call_context,
function_data: self.function_data,
args_hash: self.public_inputs.args_hash
}
}
}

mod tests {
Expand All @@ -70,8 +66,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 = 0x00ae3637b279ce8be67fd283808915bce72cb2943cb9bdbcb19d35cf8ca8fe9e;
assert_eq(call_stack_item.get_compressed().hash(), test_data_call_stack_item_request_hash);
}

#[test]
Expand All @@ -88,7 +84,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 = 0x00ae3637b279ce8be67fd283808915bce72cb2943cb9bdbcb19d35cf8ca8fe9e;
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,75 @@
use crate::abis::{call_context::CallContext, function_data::FunctionData};
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;

struct PublicCallStackItemCompressed {
contract_address: AztecAddress,
call_context: CallContext,
function_data: FunctionData,
args_hash: Field,
}

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)
}
}

impl Hash for PublicCallStackItemCompressed {
fn hash(self) -> Field {
let item = self;
std::hash::pedersen_hash_with_separator([
item.contract_address.to_field(),
item.call_context.hash(),
item.function_data.hash(),
item.args_hash,
], 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,
}
}
}

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);

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(),
};
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 + 1;
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
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export const VALIDATION_REQUESTS_LENGTH = 1026;
export const PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
export const COMBINED_ACCUMULATED_DATA_LENGTH = 333;
export const COMBINED_CONSTANT_DATA_LENGTH = 40;
export const PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 10;
export const CALL_REQUEST_LENGTH = 7;
export const PRIVATE_ACCUMULATED_DATA_LENGTH = 1232;
export const PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2307;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PublicCallStackItem Computes a callstack item hash 1`] = `"0x23a1d22e7bf37df7d68e8fcbfb7e016c060194b7915e3771e2dcd72cea26e427"`;
exports[`PublicCallStackItem Computes a callstack item hash 1`] = `"0x00ae3637b279ce8be67fd283808915bce72cb2943cb9bdbcb19d35cf8ca8fe9e"`;

exports[`PublicCallStackItem Computes a callstack item request hash 1`] = `"0x022a2b82af83606ae5a8d4955ef6215e54025193356318aefbde3b5026952953"`;
exports[`PublicCallStackItem Computes a callstack item request hash 1`] = `"0x00ae3637b279ce8be67fd283808915bce72cb2943cb9bdbcb19d35cf8ca8fe9e"`;

exports[`PublicCallStackItem computes empty item hash 1`] = `Fr<0x211932b705c9a5dbbaf2433d2ee4b0a896ef9fb720a4efbe7c1e783747c36588>`;
exports[`PublicCallStackItem computes empty item hash 1`] = `Fr<0x00ccf05f0449b7f9f0e4b20585334b860596780a70e41b939f67731e6276fba1>`;

exports[`PublicCallStackItem computes hash 1`] = `Fr<0x2d9862fe4fb3db6fe24996cbc3064346aa4551ccd41246c1ca6b002b8b0201fd>`;
exports[`PublicCallStackItem computes hash 1`] = `Fr<0x153cad9cc1e6d97618fd6f920ebe80a1e1cfc8a1f336aab769397e66729b1b33>`;
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export class PublicCallRequest {
)
: CallerContext.empty();
return new CallRequest(
item.hash(),
item.getCompressed().hash(),
this.parentCallContext.storageContractAddress,
callerContext,
new Fr(this.callContext.sideEffectCounter),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ describe('PublicCallStackItem', () => {
it('computes hash', () => {
const seed = 9870243;
const item = makePublicCallStackItem(seed);
const hash = item.hash();
const hash = item.getCompressed().hash();
expect(hash).toMatchSnapshot();
});

it('computes empty item hash', () => {
const item = PublicCallStackItem.empty();
const hash = item.hash();
const hash = item.getCompressed().hash();
expect(hash).toMatchSnapshot();
});

Expand All @@ -36,7 +36,7 @@ describe('PublicCallStackItem', () => {
callStack.isExecutionRequest = true;
callStack.publicInputs.noteHashes[0] = new NoteHash(new Fr(1), 0);

const hash = callStack.hash();
const hash = callStack.getCompressed().hash();
expect(hash.toString()).toMatchSnapshot();

// Run with AZTEC_GENERATE_TEST_DATA=1 to update noir test data
Expand All @@ -54,7 +54,7 @@ describe('PublicCallStackItem', () => {
callStack.functionData = new FunctionData(new FunctionSelector(2), /*isPrivate=*/ false);
callStack.publicInputs.noteHashes[0] = new NoteHash(new Fr(1), 0);

const hash = callStack.hash();
const hash = callStack.getCompressed().hash();
expect(hash.toString()).toMatchSnapshot();

// Run with AZTEC_GENERATE_TEST_DATA=1 to update noir test data
Expand Down
Loading

0 comments on commit b790ce8

Please sign in to comment.