Skip to content

Commit

Permalink
chore: Delete isInternal and isConstructor fields from FunctionData
Browse files Browse the repository at this point in the history
  • Loading branch information
spalladino committed Mar 15, 2024
1 parent fed70a1 commit 1b75e93
Show file tree
Hide file tree
Showing 41 changed files with 120 additions and 397 deletions.
8 changes: 4 additions & 4 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ library Constants {
uint256 internal constant CONTRACT_STORAGE_READ_LENGTH = 2;
uint256 internal constant CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH = 2;
uint256 internal constant ETH_ADDRESS_LENGTH = 1;
uint256 internal constant FUNCTION_DATA_LENGTH = 4;
uint256 internal constant FUNCTION_DATA_LENGTH = 2;
uint256 internal constant FUNCTION_LEAF_PREIMAGE_LENGTH = 5;
uint256 internal constant GLOBAL_VARIABLES_LENGTH = 6;
uint256 internal constant HEADER_LENGTH = 23;
Expand All @@ -114,13 +114,13 @@ library Constants {
uint256 internal constant NULLIFIER_KEY_VALIDATION_REQUEST_LENGTH = 4;
uint256 internal constant NULLIFIER_KEY_VALIDATION_REQUEST_CONTEXT_LENGTH = 5;
uint256 internal constant PARTIAL_STATE_REFERENCE_LENGTH = 6;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 215;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 213;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 210;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 202;
uint256 internal constant STATE_REFERENCE_LENGTH = 8;
uint256 internal constant TX_CONTEXT_DATA_LENGTH = 4;
uint256 internal constant TX_REQUEST_LENGTH = 10;
uint256 internal constant ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH = 13;
uint256 internal constant TX_REQUEST_LENGTH = 8;
uint256 internal constant ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH = 11;
uint256 internal constant GET_NOTES_ORACLE_RETURN_LENGTH = 674;
uint256 internal constant NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP = 2048;
uint256 internal constant NULLIFIERS_NUM_BYTES_PER_BASE_ROLLUP = 2048;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use dep::std;
use dep::types::{
abis::{
call_request::CallRequest, accumulated_data::CombinedAccumulatedData, function_data::FunctionData,
call_request::CallRequest, accumulated_data::CombinedAccumulatedData,
kernel_circuit_public_inputs::PrivateKernelCircuitPublicInputsBuilder,
membership_witness::NoteHashReadRequestMembershipWitness,
private_circuit_public_inputs::PrivateCircuitPublicInputs,
Expand Down Expand Up @@ -353,13 +353,6 @@ pub fn validate_call_against_request(private_call: PrivateCallData, request: Cal

let call_context = call_stack_item.public_inputs.call_context;

// Ensures that if the function is internal, only the contract itself can call it.
if call_stack_item.function_data.is_internal {
assert(
call_context.msg_sender.eq(call_context.storage_contract_address), "call is internal but msg_sender is not self"
);
}

if call_context.is_delegate_call {
let caller_context = request.caller_context;
assert(!caller_context.is_empty(), "caller context cannot be empty for delegate calls");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ impl PrivateKernelInitCircuitPrivateInputs {

let function_data = call_stack_item.function_data;
assert(function_data.is_private, "Private kernel circuit can only execute a private function");
assert(function_data.is_internal == false, "Cannot call an internal function directly");

let call_context = call_stack_item.public_inputs.call_context;
assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall");
Expand Down Expand Up @@ -277,15 +276,6 @@ mod tests {
builder.failed();
}

#[test(should_fail_with="Cannot call an internal function directly")]
fn private_function_is_internal_fails() {
let mut builder = PrivateKernelInitInputsBuilder::new();

builder.private_call.function_data.is_internal = true;

builder.failed();
}

#[test(should_fail_with="Users cannot make a static call")]
fn private_function_static_call_fails() {
let mut builder = PrivateKernelInitInputsBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,6 @@ mod tests {
*self
}

pub fn is_internal_call(&mut self) -> Self {
let _ = self.private_call.is_internal_call();
self.previous_kernel.contract_address = self.private_call.contract_address;
*self
}

pub fn execute(&mut self) -> PrivateKernelInnerCircuitPublicInputs {
let private_call = self.private_call.finish();
// Update the previous_kernel's private_call_stack with the current call_stack_item.
Expand Down Expand Up @@ -269,24 +263,6 @@ mod tests {
builder.failed();
}

#[test]
fn internal_call_succeeds() {
let mut builder = PrivateKernelInnerInputsBuilder::new().is_internal_call();
builder.succeeded();
}

#[test(should_fail_with = "call is internal but msg_sender is not self")]
fn private_function_incorrect_is_internal() {
let mut builder = PrivateKernelInnerInputsBuilder::new().is_internal_call();

// Tweak the (storage) contract_address to be different to msg_sender.
let msg_sender = builder.private_call.public_inputs.call_context.msg_sender.to_field();
builder.private_call.contract_address = AztecAddress::from_field(msg_sender + 1);
builder.private_call.public_inputs.call_context.storage_contract_address = AztecAddress::from_field(msg_sender + 1);

builder.failed();
}

#[test]
fn call_requests_succeeds() {
let mut builder = PrivateKernelInnerInputsBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,12 +501,6 @@ pub fn validate_call_against_request(public_call: PublicCallData, request: CallR

let call_context = call_stack_item.public_inputs.call_context;

if (call_stack_item.function_data.is_internal) {
assert(
call_stack_item.contract_address.eq(call_context.msg_sender), "msg_sender must be self for internal calls"
);
}

if call_context.is_delegate_call {
let caller_context = request.caller_context;
assert(!caller_context.is_empty(), "caller context cannot be empty for delegate calls");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ mod tests {
*self
}

pub fn is_internal_call(&mut self) -> Self {
let _ = self.public_call.is_internal_call();
self.previous_kernel.contract_address = self.public_call.contract_address;
*self
}

pub fn get_current_public_data_reads(self) -> [PublicDataRead; MAX_PUBLIC_DATA_READS_PER_CALL] {
let read_requests = self.public_call.public_inputs.contract_storage_reads.storage;
compute_public_data_reads(self.public_call.contract_address, read_requests)
Expand Down Expand Up @@ -209,24 +203,6 @@ mod tests {
builder.failed();
}

#[test]
fn internal_call_succeeds() {
let mut builder = PublicKernelSetupCircuitPrivateInputsBuilder::new().is_internal_call();
builder.succeeded();
}

#[test(should_fail_with="msg_sender must be self for internal calls")]
fn invalid_is_internal() {
let mut builder = PublicKernelSetupCircuitPrivateInputsBuilder::new().is_internal_call();

// Tweak the (storage) contract_address to be different to msg_sender.
let msg_sender = builder.public_call.public_inputs.call_context.msg_sender.to_field();
builder.public_call.contract_address = AztecAddress::from_field(msg_sender + 1);
builder.public_call.public_inputs.call_context.storage_contract_address = AztecAddress::from_field(msg_sender + 1);

builder.failed();
}

#[test(should_fail_with="Contract address cannot be zero")]
fn contract_address_must_be_valid() {
let mut builder = PublicKernelSetupCircuitPrivateInputsBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ mod tests {
*self
}

pub fn is_internal_call(&mut self) -> Self {
let _ = self.public_call.is_internal_call();
self.previous_kernel.contract_address = self.public_call.contract_address;
*self
}

pub fn get_current_public_data_reads(self) -> [PublicDataRead; MAX_PUBLIC_DATA_READS_PER_CALL] {
let read_requests = self.public_call.public_inputs.contract_storage_reads.storage;
compute_public_data_reads(self.public_call.contract_address, read_requests)
Expand Down Expand Up @@ -154,24 +148,6 @@ mod tests {
builder.failed();
}

#[test]
fn internal_call_succeeds() {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new().is_internal_call();
builder.succeeded();
}

#[test(should_fail_with="msg_sender must be self for internal calls")]
fn invalid_is_internal() {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new().is_internal_call();

// Tweak the (storage) contract_address to be different to msg_sender.
let msg_sender = builder.public_call.public_inputs.call_context.msg_sender.to_field();
builder.public_call.contract_address = AztecAddress::from_field(msg_sender + 1);
builder.public_call.public_inputs.call_context.storage_contract_address = AztecAddress::from_field(msg_sender + 1);

builder.failed();
}

#[test(should_fail_with="Contract address cannot be zero")]
fn contract_address_must_be_valid() {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,14 @@ use crate::{
};

struct FunctionData {
// First four bytes of the abi encoding
// of a function.
selector : FunctionSelector,
is_internal : bool,
is_private : bool,
is_constructor : bool,
}

impl Eq for FunctionData {
fn eq(self, other: Self) -> bool {
self.selector.eq(other.selector) &
self.is_internal == other.is_internal &
self.is_private == other.is_private &
self.is_constructor == other.is_constructor
self.is_private == other.is_private
}
}

Expand All @@ -29,9 +23,7 @@ impl Serialize<FUNCTION_DATA_LENGTH> for FunctionData {
fn serialize(self) -> [Field; FUNCTION_DATA_LENGTH] {
[
self.selector.to_field(),
self.is_internal as Field,
self.is_private as Field,
self.is_constructor as Field,
]
}
}
Expand All @@ -40,9 +32,7 @@ impl Deserialize<FUNCTION_DATA_LENGTH> for FunctionData {
fn deserialize(serialized: [Field; FUNCTION_DATA_LENGTH]) -> Self {
Self {
selector: FunctionSelector::from_field(serialized[0]),
is_internal: serialized[1] as bool,
is_private: serialized[2] as bool,
is_constructor: serialized[3] as bool,
is_private: serialized[1] as bool,
}
}
}
Expand All @@ -67,5 +57,6 @@ fn empty_hash() {
let hash = data.hash();

// Value from function_data.test.ts "computes empty item hash" test
assert_eq(hash, 0x200569267c0f73ac89aaa414239398db9445dd4ad3a8cf37015cd55b8d4c5e8d);
let test_data_empty_hash = 0x27b1d0839a5b23baf12a8d195b18ac288fcf401afb2f70b8a4b529ede5fa9fed;
assert_eq(hash, test_data_empty_hash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ fn empty_hash() {
let hash = item.hash();

// Value from private_call_stack_item.test.ts "computes empty item hash" test
let test_data_empty_hash = 0x19196a5f02621a64ce289fb09fada7fd650a6874cb63e7d10c0d9a9bf5a366f5;
let test_data_empty_hash = 0x134b57e317f1554b9c4f547e617338fcc8ff04c6d96a278f1752b26a462c5d25;
assert_eq(hash, test_data_empty_hash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ mod tests {
#[test]
fn compute_call_stack_item_request_hash() {
let contract_address = AztecAddress::from_field(1);
let function_data = FunctionData { selector: FunctionSelector::from_u32(2), is_internal: false, is_private: false, is_constructor: false };
let function_data = FunctionData { selector: FunctionSelector::from_u32(2), is_private: false };

let mut public_inputs: PublicCircuitPublicInputs = dep::std::unsafe::zeroed();
public_inputs.new_note_hashes[0] = SideEffect{
Expand All @@ -69,14 +69,14 @@ 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 = 0x00edd2f10c0cdf776ee2fff3c799bae6df5771f5013a2d5d7154601dffdcf869;
let test_data_call_stack_item_request_hash = 0x02e15f4157b5e2cb0a7ec3dfec18c6812ef16e1da319b364e5a11e337dfca414;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_request_hash);
}

#[test]
fn compute_call_stack_item_hash() {
let contract_address = AztecAddress::from_field(1);
let function_data = FunctionData { selector: FunctionSelector::from_u32(2), is_internal: false, is_private: false, is_constructor: false };
let function_data = FunctionData { selector: FunctionSelector::from_u32(2), is_private: false };

let mut public_inputs: PublicCircuitPublicInputs = dep::std::unsafe::zeroed();
public_inputs.new_note_hashes[0] = SideEffect{
Expand All @@ -87,7 +87,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 = 0x1cec0b51f9394405a626c3b77081c96f1bdcb8bacf96960ae4749068f9b4da0d;
let test_data_call_stack_item_hash = 0x0f22ddeca80a2c6f455165f1d2d1950c5e1b772bdc312742d1de089b424f0f5f;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_hash);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ global CONTRACT_INSTANCE_LENGTH: u64 = 6;
global CONTRACT_STORAGE_READ_LENGTH: u64 = 2;
global CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH: u64 = 2;
global ETH_ADDRESS_LENGTH = 1;
global FUNCTION_DATA_LENGTH: u64 = 4;
global FUNCTION_DATA_LENGTH: u64 = 2;
global FUNCTION_LEAF_PREIMAGE_LENGTH: u64 = 5;
global GLOBAL_VARIABLES_LENGTH: u64 = 6;
global HEADER_LENGTH: u64 = 23; // 2 for last_archive, 7 for content commitment, 8 for state reference, 6 for global vars
Expand All @@ -163,7 +163,7 @@ global L2_TO_L1_MESSAGE_LENGTH: u64 = 2;
global NULLIFIER_KEY_VALIDATION_REQUEST_LENGTH = 4;
global NULLIFIER_KEY_VALIDATION_REQUEST_CONTEXT_LENGTH = 5;
global PARTIAL_STATE_REFERENCE_LENGTH: u64 = 6;
global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = 215;
global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = 213;
// Change this ONLY if you have changed the PrivateCircuitPublicInputs structure.
// In other words, if the structure/size of the public inputs of a function call changes then we should change this
// constant as well PRIVATE_CALL_STACK_ITEM_LENGTH
Expand All @@ -172,9 +172,9 @@ global PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = 210;
global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = 202;
global STATE_REFERENCE_LENGTH: u64 = 8; // 2 for snap + 8 for partial
global TX_CONTEXT_DATA_LENGTH: u64 = 4;
global TX_REQUEST_LENGTH: u64 = 10;
global TX_REQUEST_LENGTH: u64 = 8; // 2 + TX_CONTEXT_DATA_LENGTH + FUNCTION_DATA_LENGTH

global ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH: Field = 13; // 2 + FUNCTION_DATA_LENGTH + CALL_CONTEXT_LENGTH
global ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH: Field = 11; // 2 + FUNCTION_DATA_LENGTH + CALL_CONTEXT_LENGTH
global GET_NOTES_ORACLE_RETURN_LENGTH: u64 = 674;
global NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP: Field = 2048;
global NULLIFIERS_NUM_BYTES_PER_BASE_ROLLUP: Field = 2048;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ struct ContractFunction {
global default_private_function = ContractFunction {
data: FunctionData {
selector: FunctionSelector { inner: 1010101 },
is_internal: false,
is_private: true,
is_constructor: false,
},
vk_hash: 0,
acir_hash: 1111,
Expand All @@ -32,33 +30,10 @@ global default_private_function = ContractFunction {
},
};

global default_internal_private_function = ContractFunction {
data: FunctionData {
selector: FunctionSelector { inner: 2020202 },
is_internal: true,
is_private: true,
is_constructor: false,
},
vk_hash: 2222,
acir_hash: 2222,
membership_witness: FunctionLeafMembershipWitness {
leaf_index: 1,
sibling_path: [
0x1b208d72e8313fab273c9508bb07b497f2b433360250b3db2c9e20e238e2cff0,
0x21dbfd1d029bf447152fcf89e355c334610d1632436ba170f738107266a71550,
0x0bcd1f91cf7bdd471d0a30c58c4706f3fdab3807a954b8f5b5e3bfec87d001bb,
0x06e62084ee7b602fe9abc15632dda3269f56fb0c6e12519a2eb2ec897091919d,
0x03c9e2e67178ac638746f068907e6677b4cc7a9592ef234ab6ab518f17efffa0,
],
},
};

global default_public_function = ContractFunction {
data: FunctionData {
selector: FunctionSelector { inner: 3030303 },
is_internal: false,
is_private: false,
is_constructor: false,
},
vk_hash: 0,
acir_hash: 3333,
Expand All @@ -74,23 +49,3 @@ global default_public_function = ContractFunction {
},
};

global default_internal_public_function = ContractFunction {
data: FunctionData {
selector: FunctionSelector { inner: 4040404 },
is_internal: true,
is_private: false,
is_constructor: false,
},
vk_hash: 0,
acir_hash: 4444,
membership_witness: FunctionLeafMembershipWitness {
leaf_index: 3,
sibling_path: [
0x1fc20a5f4a9bf052ae4fee30281fd09908a25063c749bc35939502ffaeaee8c2,
0x1bed44f12632c0a6343cd886bd3e548bb5e8d2fd35fe9bc81f28defd4ed885b0,
0x0837a67313f4dbbd8d6971c0672f961f0a3b9e218c1395d327915209292acbbf,
0x2e0ef36ddc5db29acb6ef904999046f835ce7c78a40c3f7a0edb03b2f917a765,
0x1a9fdb505152f9c2baaffe4a30ee80775b58ebf8c2dde76435835b085c6f70ca,
],
},
};
Loading

0 comments on commit 1b75e93

Please sign in to comment.