refactor: add PublicCallStackItem and remove side_effect_counter from call_context

l1-contracts/slither_output.md

@@ -258,9 +258,9 @@ solc-0.8.23 is not recommended for deployment
 Impact: Informational
 Confidence: Medium
  - [ ] ID-27
-Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L130) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L123)
+Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L132) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L125)
 
-src/core/libraries/ConstantsGen.sol#L130
+src/core/libraries/ConstantsGen.sol#L132
 
 
  - [ ] ID-28

l1-contracts/src/core/libraries/ConstantsGen.sol

@@ -97,7 +97,7 @@ library Constants {
   uint256 internal constant MAX_NOTES_PER_PAGE = 10;
   uint256 internal constant VIEW_NOTE_ORACLE_RETURN_LENGTH = 212;
   uint256 internal constant AZTEC_ADDRESS_LENGTH = 1;
-  uint256 internal constant CALL_CONTEXT_LENGTH = 7;
+  uint256 internal constant CALL_CONTEXT_LENGTH = 6;
   uint256 internal constant CONTENT_COMMITMENT_LENGTH = 7;
   uint256 internal constant CONTRACT_INSTANCE_LENGTH = 6;
   uint256 internal constant CONTRACT_STORAGE_READ_LENGTH = 2;
@@ -112,9 +112,11 @@ 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 = 213;
-  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 210;
-  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 202;
+  uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 212;
+  uint256 internal constant PUBLIC_CALL_STACK_ITEM_LENGTH = 205;
+  uint256 internal constant PUBLIC_FUNCTION_CALL_RESULT_LENGTH = 16;
+  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 209;
+  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 201;
   uint256 internal constant STATE_REFERENCE_LENGTH = 8;
   uint256 internal constant TX_CONTEXT_DATA_LENGTH = 4;
   uint256 internal constant TX_REQUEST_LENGTH = 8;

noir-projects/aztec-nr/aztec/src/context/private_context.nr

@@ -23,8 +23,9 @@ use dep::protocol_types::{
     MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
     MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
     MAX_PUBLIC_DATA_READS_PER_CALL, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL,
-    MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
-    MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL, NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
+    MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL,
+    MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL,
+    NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
 },
     contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
     grumpkin_private_key::GrumpkinPrivateKey, hash::hash_args, header::Header,
@@ -332,7 +333,6 @@ impl PrivateContext {
             is_delegate_call
         );
 
-        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);
         self.side_effect_counter = item.public_inputs.end_side_effect_counter + 1;
 
@@ -441,14 +441,19 @@ impl PrivateContext {
 
         let mut reader = Reader::new(fields);
 
-        // Note: Not using PublicCirclePublicInputs::deserialize here, because everything below args_hash is 0 and
-        // there is no more data in fields because there is only ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_SIZE fields!
+        // Note: _Not_ using PublicCirclePublicInputs::deserialize here
+        // the data returned by the oracle call is compressed and can't be used with ::deserialize
+        // read the relevant values manually and fill the rest with 0s
         let item = PublicCallStackItem {
             contract_address: AztecAddress::from_field(reader.read()),
             function_data: reader.read_struct(FunctionData::deserialize),
             public_inputs: PublicCircuitPublicInputs {
                 call_context: reader.read_struct(CallContext::deserialize),
                 args_hash: reader.read(),
+                // the counter that enqueued the call
+                start_side_effect_counter: reader.read() as u32,
+                // everything below is 0 (see comment above)
+                end_side_effect_counter: 0,
                 return_values: [0; RETURN_VALUES_LENGTH],
                 nullifier_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_READ_REQUESTS_PER_CALL],
                 nullifier_non_existent_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL],
@@ -458,8 +463,6 @@ impl PrivateContext {
                 new_note_hashes: [SideEffect::empty(); MAX_NEW_NOTE_HASHES_PER_CALL],
                 new_nullifiers: [SideEffectLinkedToNoteHash::empty(); MAX_NEW_NULLIFIERS_PER_CALL],
                 new_l2_to_l1_msgs: [L2ToL1Message::empty(); MAX_NEW_L2_TO_L1_MSGS_PER_CALL],
-                start_side_effect_counter: 0,
-                end_side_effect_counter: 0,
                 unencrypted_logs_hash: [0; NUM_FIELDS_PER_SHA256],
                 unencrypted_log_preimages_length: 0,
                 historical_header: Header::empty(),
@@ -473,7 +476,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);
         // We increment the sideffect counter by one, to account for the call itself being a side effect.
         self.side_effect_counter = self.side_effect_counter + 1;
 
@@ -483,20 +486,6 @@ impl PrivateContext {
         assert(item.public_inputs.call_context.is_delegate_call == is_delegate_call);
         assert(item.public_inputs.call_context.is_static_call == is_static_call);
 
-        if (is_delegate_call) {
-            // For delegate calls, we also constrain the execution context address for the nested call to be equal to our address.
-            assert(
-                item.public_inputs.call_context.storage_contract_address.eq(self.inputs.call_context.storage_contract_address)
-            );
-            assert(item.public_inputs.call_context.msg_sender.eq(self.inputs.call_context.msg_sender));
-        } else {
-            // For non-delegate calls, we also constrain the execution context address for the nested call to be equal to the address we called.
-            assert(item.public_inputs.call_context.storage_contract_address.eq(contract_address));
-            assert(
-                item.public_inputs.call_context.msg_sender.eq(self.inputs.call_context.storage_contract_address)
-            );
-        }
-
         self.public_call_stack_hashes.push(item.hash());
     }
 }

noir-projects/aztec-nr/aztec/src/context/public_context.nr

@@ -4,8 +4,8 @@ use crate::{
 };
 use dep::protocol_types::{
     abis::{
-    global_variables::GlobalVariables, function_selector::FunctionSelector,
-    private_circuit_public_inputs::PrivateCircuitPublicInputs,
+    call_context::CallContext, function_data::FunctionData, global_variables::GlobalVariables,
+    function_selector::FunctionSelector, private_circuit_public_inputs::PrivateCircuitPublicInputs,
     public_call_stack_item::PublicCallStackItem,
     public_circuit_public_inputs::PublicCircuitPublicInputs, read_request::ReadRequest,
     side_effect::{SideEffect, SideEffectLinkedToNoteHash}
@@ -279,17 +279,60 @@ impl PublicContext {
         is_static_call: bool,
         is_delegate_call: bool
     ) -> [Field; RETURN_VALUES_LENGTH] {
-        let side_effect_counter = self.side_effect_counter;
-        // TODO get next value from output of `call_public_function_internal`
-        self.side_effect_counter += 1;
-
-        call_public_function_internal(
+        let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
+        let fields = call_public_function_internal(
             contract_address,
             function_selector,
             args_hash,
-            side_effect_counter,
+            self.side_effect_counter,
             is_static_call,
             is_delegate_call
-        )
+        );
+
+        // TODO send back a fully formed PublicCallStackItem
+        let mut reader = Reader::new(fields);
+        let item = PublicCallStackItem {
+            contract_address: AztecAddress::from_field(reader.read()),
+            function_data: reader.read_struct(FunctionData::deserialize),
+            public_inputs: PublicCircuitPublicInputs {
+                call_context: reader.read_struct(CallContext::deserialize),
+                args_hash: reader.read(),
+                start_side_effect_counter: reader.read() as u32,
+                end_side_effect_counter: reader.read() as u32,
+                return_values: reader.read_array([0; RETURN_VALUES_LENGTH]),
+                // everything below is 0
+                nullifier_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_READ_REQUESTS_PER_CALL],
+                nullifier_non_existent_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL],
+                contract_storage_update_requests: [StorageUpdateRequest::empty(); MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL],
+                contract_storage_reads: [StorageRead::empty(); MAX_PUBLIC_DATA_READS_PER_CALL],
+                public_call_stack_hashes: [0; MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL],
+                new_note_hashes: [SideEffect::empty(); MAX_NEW_NOTE_HASHES_PER_CALL],
+                new_nullifiers: [SideEffectLinkedToNoteHash::empty(); MAX_NEW_NULLIFIERS_PER_CALL],
+                new_l2_to_l1_msgs: [L2ToL1Message::empty(); MAX_NEW_L2_TO_L1_MSGS_PER_CALL],
+                unencrypted_logs_hash: [0; NUM_FIELDS_PER_SHA256],
+                unencrypted_log_preimages_length: 0,
+                historical_header: Header::empty(),
+                prover_address: AztecAddress::zero(),
+                reverted: false
+            },
+            is_execution_request: false
+        };
+        reader.finish();
+
+        assert_eq(item.public_inputs.start_side_effect_counter, self.side_effect_counter);
+        self.side_effect_counter = item.public_inputs.end_side_effect_counter + 1;
+
+        assert(contract_address.eq(item.contract_address));
+        assert(function_selector.eq(item.function_data.selector));
+
+        assert(args_hash == item.public_inputs.args_hash);
+
+        // Assert that the call context of the enqueued call generated by the oracle matches our request.
+        assert(item.public_inputs.call_context.is_delegate_call == is_delegate_call);
+        assert(item.public_inputs.call_context.is_static_call == is_static_call);
+
+        // TODO track public call stack item hashes in the context
+
+        item.public_inputs.return_values
     }
 }

noir-projects/aztec-nr/aztec/src/oracle/public_call.nr

@@ -1,4 +1,7 @@
-use dep::protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress, constants::RETURN_VALUES_LENGTH};
+use dep::protocol_types::{
+    abis::function_selector::FunctionSelector, address::AztecAddress,
+    abis::public_call_stack_item::PublicCallStackItem, constants::PUBLIC_FUNCTION_CALL_RESULT_LENGTH
+};
 
 #[oracle(callPublicFunction)]
 fn call_public_function_oracle(
@@ -8,7 +11,7 @@ fn call_public_function_oracle(
     _side_effect_counter: u32,
     _is_static_call: bool,
     _is_delegate_call: bool
-) -> [Field; RETURN_VALUES_LENGTH] {}
+) -> [Field; PUBLIC_FUNCTION_CALL_RESULT_LENGTH] {}
 
 unconstrained pub fn call_public_function_internal(
     contract_address: AztecAddress,
@@ -17,13 +20,16 @@ unconstrained pub fn call_public_function_internal(
     side_effect_counter: u32,
     is_static_call: bool,
     is_delegate_call: bool
-) -> [Field; RETURN_VALUES_LENGTH] {
-    call_public_function_oracle(
+) -> [Field; PUBLIC_FUNCTION_CALL_RESULT_LENGTH] {
+    let fields = call_public_function_oracle(
         contract_address,
         function_selector,
         args_hash,
         side_effect_counter,
         is_static_call,
         is_delegate_call
-    )
+    );
+
+    // TODO: this should be a PublicCallStackItem, but it's big and blows up the bytecode size
+    fields
 }

noir-projects/noir-protocol-circuits/crates/types/src/abis/call_context.nr

@@ -14,8 +14,6 @@ struct CallContext {
 
     is_delegate_call : bool,
     is_static_call : bool,
-
-    side_effect_counter : u32,
 }
 // docs:end:call-context
 
@@ -28,7 +26,6 @@ impl CallContext {
         assert(self.function_selector.to_field() == 0);
         assert(self.is_delegate_call == false);
         assert(self.is_static_call == false);
-        assert(self.side_effect_counter == 0);
     }
 }
 
@@ -41,7 +38,6 @@ impl Eq for CallContext {
             & call_context.function_selector.eq(self.function_selector)
             & (call_context.is_delegate_call == self.is_delegate_call)
             & (call_context.is_static_call == self.is_static_call)
-            & (call_context.side_effect_counter == self.side_effect_counter)
     }
 }
 
@@ -60,7 +56,6 @@ impl Serialize<CALL_CONTEXT_LENGTH> for CallContext {
             self.function_selector.to_field(),
             self.is_delegate_call as Field,
             self.is_static_call as Field,
-            self.side_effect_counter as Field,
         ]
     }
 }
@@ -74,7 +69,6 @@ impl Deserialize<CALL_CONTEXT_LENGTH> for CallContext {
             function_selector: FunctionSelector::from_field(serialized[3]),
             is_delegate_call: serialized[4] as bool,
             is_static_call: serialized[5] as bool,
-            side_effect_counter: serialized[6] as u32,
         }
     }
 }

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 = 0x134b57e317f1554b9c4f547e617338fcc8ff04c6d96a278f1752b26a462c5d25;
+    let test_data_empty_hash = 0x25dcfe64d56a1575de78bc4474e0093cba37f28a664b81121c38fc89bdf0a8d5;
     assert_eq(hash, test_data_empty_hash);
 }

noir-projects/noir-protocol-circuits/crates/types/src/abis/private_circuit_public_inputs.nr

@@ -177,6 +177,6 @@ fn empty_hash() {
     let hash = inputs.hash();
 
     // Value from private_circuit_public_inputs.test.ts "computes empty item hash" test
-    let test_data_empty_hash = 0x2b5ba01a6b73b68b4f44196e2dea49afd4076333e2dee8eddc9186e080f18201;
+    let test_data_empty_hash = 0x2745ec62624afeb19b86af3d440db1f8c3432e1d17a074c75cb8f44999fd3fae;
     assert_eq(hash, test_data_empty_hash);
 }

noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr

@@ -1,12 +1,13 @@
 use crate::abis::{function_data::FunctionData, public_circuit_public_inputs::PublicCircuitPublicInputs};
 use crate::address::AztecAddress;
-use crate::constants::GENERATOR_INDEX__CALL_STACK_ITEM;
-use crate::traits::Hash;
+use crate::constants::{GENERATOR_INDEX__CALL_STACK_ITEM, PUBLIC_CALL_STACK_ITEM_LENGTH};
+use crate::traits::{Hash, Serialize, Deserialize};
+use crate::utils::reader::Reader;
 
 struct PublicCallStackItem {
     contract_address: AztecAddress,
-    public_inputs: PublicCircuitPublicInputs,
     function_data: FunctionData,
+    public_inputs: PublicCircuitPublicInputs,
     // True if this call stack item represents a request to execute a function rather than a
     // fulfilled execution. Used when enqueuing calls from private to public functions.
     is_execution_request: bool,
@@ -45,6 +46,44 @@ impl PublicCallStackItem {
     }
 }
 
+impl Serialize<PUBLIC_CALL_STACK_ITEM_LENGTH> for PublicCallStackItem {
+    fn serialize(self) -> [Field; PUBLIC_CALL_STACK_ITEM_LENGTH] {
+        let mut fields: BoundedVec<Field, PUBLIC_CALL_STACK_ITEM_LENGTH> = BoundedVec::new();
+
+        fields.push(self.contract_address.to_field());
+        fields.extend_from_array(self.function_data.serialize());
+        fields.extend_from_array(self.public_inputs.serialize());
+        fields.push(self.is_execution_request as Field);
+
+        assert_eq(fields.len(), PUBLIC_CALL_STACK_ITEM_LENGTH);
+
+        fields.storage
+    }
+}
+
+impl Deserialize<PUBLIC_CALL_STACK_ITEM_LENGTH> for PublicCallStackItem {
+    fn deserialize(serialized: [Field; PUBLIC_CALL_STACK_ITEM_LENGTH]) -> Self {
+        let mut reader = Reader::new(serialized);
+        let item = Self {
+            contract_address: reader.read_struct(AztecAddress::deserialize),
+            function_data: reader.read_struct(FunctionData::deserialize),
+            public_inputs: reader.read_struct(PublicCircuitPublicInputs::deserialize),
+            is_execution_request: reader.read() as bool,
+        };
+
+        item
+    }
+}
+
+impl Eq for PublicCallStackItem {
+    fn eq(self, other: Self) -> bool {
+        self.contract_address.eq(other.contract_address) &
+        self.function_data.eq(other.function_data) &
+        self.public_inputs.eq(other.public_inputs) &
+        self.is_execution_request.eq(other.is_execution_request)
+    }
+}
+
 mod tests {
     use crate::{
         abis::{
@@ -55,6 +94,23 @@ mod tests {
         address::AztecAddress, constants::GENERATOR_INDEX__CALL_STACK_ITEM, traits::Hash
     };
 
+    #[test]
+    fn serialize_smoke() {
+        let item: PublicCallStackItem = dep::std::unsafe::zeroed();
+        let serialized = item.serialize();
+        let deserialized = PublicCallStackItem::deserialize(serialized);
+        assert(item.eq(deserialized));
+    }
+
+    #[test]
+    fn serialize_exec_request_smoke() {
+        let mut item: PublicCallStackItem = dep::std::unsafe::zeroed();
+        item.is_execution_request = true;
+        let serialized = item.serialize();
+        let deserialized = PublicCallStackItem::deserialize(serialized);
+        assert(item.eq(deserialized));
+    }
+
     #[test]
     fn compute_call_stack_item_request_hash() {
         let contract_address = AztecAddress::from_field(1);
@@ -69,7 +125,7 @@ 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 = 0x02e15f4157b5e2cb0a7ec3dfec18c6812ef16e1da319b364e5a11e337dfca414;
+        let test_data_call_stack_item_request_hash = 0x0a408fba24eff0b40c9a7d4cb2ff1a8f651fbd7eeaeedc3488b6d2fcfede33e0;
         assert_eq(call_stack_item.hash(), test_data_call_stack_item_request_hash);
     }
 
@@ -87,7 +143,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 = 0x0f22ddeca80a2c6f455165f1d2d1950c5e1b772bdc312742d1de089b424f0f5f;
+        let test_data_call_stack_item_hash = 0x2cd2eb79283cc7c9b22ccfafd6a200f153e8114e3f7d6adf527bfa76db59d26b;
         assert_eq(call_stack_item.hash(), test_data_call_stack_item_hash);
     }
 }