chore: Delete isInternal and isConstructor fields from FunctionData (#…

…5232) Deletes the isInternal field from FunctionData, since it is now managed as an app macro instead of a kernel circuit check, and the isConstructor field, since constructors are also managed in app land.
AztecProtocol · Mar 15, 2024 · dea3f87 · dea3f87
1 parent 25ce33b
commit dea3f87
Show file tree

Hide file tree

Showing 45 changed files with 135 additions and 412 deletions.
diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol
@@ -103,7 +103,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;
@@ -112,13 +112,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;

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr
@@ -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,
@@ -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");

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr
@@ -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");
@@ -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();

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr
@@ -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.
@@ -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();

diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr
@@ -461,12 +461,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");

diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr
@@ -127,12 +127,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)
@@ -211,24 +205,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();

diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr
@@ -108,12 +108,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)
@@ -156,24 +150,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();

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/function_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/function_data.nr
@@ -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
     }
 }
 
@@ -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,
         ]
     }
 }
@@ -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,
         }
     }
 }
@@ -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);
 }
diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_call_stack_item.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_call_stack_item.nr
@@ -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);
 }
diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr
@@ -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{
@@ -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{
@@ -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);
     }
 }
diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr
@@ -152,7 +152,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
@@ -161,7 +161,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
@@ -170,9 +170,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;

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixtures/contract_functions.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixtures/contract_functions.nr
@@ -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,
@@ -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,
@@ -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,
-        ],
-    },
-};