refactor: call requests (#7483)

- Stop revealing start counters of public call requests from private. - Export PublicCallRequest structs instead of hashes from public inputs. - Gate counts have increased because of the larger data (Will try to reduce them in the next pr). But now the app circuit can save 5k for each public function call. - Remove caller context. Instead, use call context in the call request to check addresses and function selector. - Add missing checks: - Nested function selector. - Teardown call request. - Empty public data update requests.
AztecProtocol · Jul 25, 2024 · ffedf39 · ffedf39
1 parent 344ca6f
commit ffedf39
Show file tree

Hide file tree

Showing 132 changed files with 1,980 additions and 3,162 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/aztec_constants.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/aztec_constants.hpp
@@ -27,10 +27,11 @@
 #define LOG_HASH_LENGTH 3
 #define NOTE_HASH_LENGTH 2
 #define NULLIFIER_LENGTH 3
+#define PUBLIC_CALL_REQUEST_LENGTH 14
 #define STATE_REFERENCE_LENGTH 8
 #define TOTAL_FEES_LENGTH 1
 #define HEADER_LENGTH 23
-#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 481
+#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 689
 #define PUBLIC_CONTEXT_INPUTS_LENGTH 40
 #define SENDER_SELECTOR 0
 #define ADDRESS_SELECTOR 1

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp
@@ -60,7 +60,7 @@ inline const uint32_t PCPI_PUBLIC_CALLSTACK_OFFSET =
     PCPI_PUBLIC_DATA_READ_OFFSET + (MAX_PUBLIC_DATA_READS_PER_CALL * CONTRACT_STORAGE_READ_LENGTH);
 
 inline const uint32_t PCPI_NEW_NOTE_HASHES_OFFSET =
-    PCPI_PUBLIC_CALLSTACK_OFFSET + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL;
+    PCPI_PUBLIC_CALLSTACK_OFFSET + (MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL * PUBLIC_CALL_REQUEST_LENGTH);
 
 inline const uint32_t PCPI_NEW_NULLIFIERS_OFFSET =
     PCPI_NEW_NOTE_HASHES_OFFSET + (MAX_NOTE_HASHES_PER_CALL * NOTE_HASH_LENGTH);

diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol
@@ -165,18 +165,18 @@ library Constants {
   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;
-  uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 15;
-  uint256 internal constant SCOPED_PRIVATE_CALL_REQUEST_LENGTH = 16;
+  uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 13;
+  uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 10;
+  uint256 internal constant PUBLIC_CALL_REQUEST_LENGTH = 14;
   uint256 internal constant ROLLUP_VALIDATION_REQUESTS_LENGTH = 2;
   uint256 internal constant STATE_REFERENCE_LENGTH = 8;
   uint256 internal constant TX_CONTEXT_LENGTH = 9;
   uint256 internal constant TX_REQUEST_LENGTH = 13;
   uint256 internal constant TOTAL_FEES_LENGTH = 1;
   uint256 internal constant HEADER_LENGTH = 23;
-  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 444;
-  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 481;
-  uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 447;
+  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 645;
+  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 689;
+  uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 648;
   uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH = 40;
   uint256 internal constant AGGREGATION_OBJECT_LENGTH = 16;
   uint256 internal constant SCOPED_READ_REQUEST_LEN = 3;
@@ -185,16 +185,13 @@ library Constants {
   uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
   uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 364;
   uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 41;
-  uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 15;
-  uint256 internal constant CALL_REQUEST_LENGTH = 7;
-  uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1160;
-  uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2300;
-  uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 991;
-  uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3339;
+  uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1336;
+  uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2483;
+  uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 1215;
+  uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 4011;
   uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 415;
   uint256 internal constant CONSTANT_ROLLUP_DATA_LENGTH = 11;
   uint256 internal constant BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH = 28;
-  uint256 internal constant ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH = 9;
   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/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr
@@ -6,20 +6,18 @@ use crate::{
     key_validation_request::get_key_validation_request, arguments, returns::pack_returns,
     call_private_function::call_private_function_internal, header::get_header_at,
     logs::{emit_encrypted_note_log, emit_encrypted_event_log},
-    enqueue_public_function_call::{
-    enqueue_public_function_call_internal, set_public_teardown_function_call_internal,
-    parse_public_call_stack_item_from_oracle
-}
+    enqueue_public_function_call::{enqueue_public_function_call_internal, set_public_teardown_function_call_internal}
 }
 };
 use dep::protocol_types::{
     abis::{
-    caller_context::CallerContext, function_selector::FunctionSelector,
+    call_context::CallContext, function_selector::FunctionSelector, gas::Gas,
     max_block_number::MaxBlockNumber,
     validation_requests::{KeyValidationRequest, KeyValidationRequestAndGenerator},
     private_call_request::PrivateCallRequest, private_circuit_public_inputs::PrivateCircuitPublicInputs,
-    public_call_stack_item::PublicCallStackItem, read_request::ReadRequest, note_hash::NoteHash,
-    nullifier::Nullifier, log_hash::{LogHash, NoteLogHash, EncryptedLogHash}
+    public_call_request::PublicCallRequest, public_call_stack_item::PublicCallStackItem,
+    public_call_stack_item_compressed::PublicCallStackItemCompressed, read_request::ReadRequest,
+    note_hash::NoteHash, nullifier::Nullifier, log_hash::{LogHash, NoteLogHash, EncryptedLogHash}
 },
     address::{AztecAddress, EthAddress},
     constants::{
@@ -54,8 +52,8 @@ struct PrivateContext {
     nullifiers: BoundedVec<Nullifier, MAX_NULLIFIERS_PER_CALL>,
 
     private_call_requests : BoundedVec<PrivateCallRequest, MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL>,
-    public_call_stack_hashes : BoundedVec<Field, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL>,
-    public_teardown_function_hash: Field,
+    public_call_requests : BoundedVec<PublicCallRequest, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL>,
+    public_teardown_call_request: PublicCallRequest,
     l2_to_l1_msgs : BoundedVec<L2ToL1Message, MAX_L2_TO_L1_MSGS_PER_CALL>,
     // docs:end:private-context
 
@@ -89,8 +87,8 @@ impl PrivateContext {
             nullifiers: BoundedVec::new(),
             historical_header: inputs.historical_header,
             private_call_requests: BoundedVec::new(),
-            public_call_stack_hashes: BoundedVec::new(),
-            public_teardown_function_hash: 0,
+            public_call_requests: BoundedVec::new(),
+            public_teardown_call_request: PublicCallRequest::empty(),
             l2_to_l1_msgs: BoundedVec::new(),
             note_encrypted_logs_hashes: BoundedVec::new(),
             encrypted_logs_hashes: BoundedVec::new(),
@@ -164,8 +162,8 @@ impl PrivateContext {
             note_hashes: self.note_hashes.storage,
             nullifiers: self.nullifiers.storage,
             private_call_requests: self.private_call_requests.storage,
-            public_call_stack_hashes: self.public_call_stack_hashes.storage,
-            public_teardown_function_hash: self.public_teardown_function_hash,
+            public_call_requests: self.public_call_requests.storage,
+            public_teardown_call_request: self.public_teardown_call_request,
             l2_to_l1_msgs: self.l2_to_l1_msgs.storage,
             start_side_effect_counter: self.inputs.start_side_effect_counter,
             end_side_effect_counter: self.side_effect_counter,
@@ -353,7 +351,7 @@ impl PrivateContext {
     ) -> PackedReturns {
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
         let start_side_effect_counter = self.side_effect_counter;
-        let item = call_private_function_internal(
+        let (end_side_effect_counter, returns_hash) = call_private_function_internal(
             contract_address,
             function_selector,
             args_hash,
@@ -362,8 +360,6 @@ impl PrivateContext {
             is_delegate_call
         );
 
-        assert_eq(item.public_inputs.start_side_effect_counter, start_side_effect_counter);
-        let end_side_effect_counter = item.public_inputs.end_side_effect_counter;
         self.side_effect_counter = end_side_effect_counter + 1;
 
         // TODO (fees) figure out why this crashes the prover and enable it
@@ -379,49 +375,25 @@ impl PrivateContext {
         //     self.min_revertible_side_effect_counter = item.public_inputs.min_revertible_side_effect_counter;
         // }
 
-        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 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);
-
-        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)
-            );
-        }
+        let call_context = self.generate_call_context(
+            contract_address,
+            function_selector,
+            is_static_call,
+            is_delegate_call
+        );
 
-        let mut caller_context = CallerContext::empty();
-        caller_context.is_static_call = self.inputs.call_context.is_static_call;
-        if is_delegate_call {
-            caller_context.msg_sender = self.inputs.call_context.msg_sender;
-            caller_context.storage_contract_address = self.inputs.call_context.storage_contract_address;
-        }
         self.private_call_requests.push(
             PrivateCallRequest {
-            target: 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,
-            caller_context,
+            contract_address,
+            call_context,
+            args_hash,
+            returns_hash,
             start_side_effect_counter,
             end_side_effect_counter
         }
         );
 
-        PackedReturns::new(item.public_inputs.returns_hash)
+        PackedReturns::new(returns_hash)
     }
 
     pub fn call_public_function<let ARGS_COUNT: u32>(
@@ -489,28 +461,37 @@ impl PrivateContext {
         is_static_call: bool,
         is_delegate_call: bool
     ) {
+        let counter = self.next_counter();
+
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
-        let fields = enqueue_public_function_call_internal(
+        enqueue_public_function_call_internal(
             contract_address,
             function_selector,
             args_hash,
-            self.side_effect_counter,
+            counter,
             is_static_call,
             is_delegate_call
         );
 
-        let item = parse_public_call_stack_item_from_oracle(fields);
-        self.validate_call_stack_item_from_oracle(
-            item,
+        let call_context = self.generate_call_context(
             contract_address,
             function_selector,
-            args_hash,
             is_static_call,
             is_delegate_call
         );
 
-        self.side_effect_counter = self.side_effect_counter + 1;
-        self.public_call_stack_hashes.push(item.get_compressed().hash());
+        let item = PublicCallStackItemCompressed {
+            contract_address,
+            call_context,
+            args_hash,
+            returns_hash: 0,
+            revert_code: 0,
+            start_gas_left: Gas::empty(),
+            end_gas_left: Gas::empty()
+        };
+
+        let call_request = PublicCallRequest { item, counter };
+        self.public_call_requests.push(call_request);
     }
 
     pub fn set_public_teardown_function<let ARGS_COUNT: u32>(
@@ -532,63 +513,59 @@ impl PrivateContext {
         is_static_call: bool,
         is_delegate_call: bool
     ) {
+        let counter = self.next_counter();
+
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
-        let fields = set_public_teardown_function_call_internal(
+        set_public_teardown_function_call_internal(
             contract_address,
             function_selector,
             args_hash,
-            self.side_effect_counter,
+            counter,
             is_static_call,
             is_delegate_call
         );
 
-        let item = parse_public_call_stack_item_from_oracle(fields);
-        self.validate_call_stack_item_from_oracle(
-            item,
+        let call_context = self.generate_call_context(
             contract_address,
             function_selector,
-            args_hash,
             is_static_call,
             is_delegate_call
         );
 
-        self.side_effect_counter = self.side_effect_counter + 1;
-        self.public_teardown_function_hash = item.get_compressed().hash();
+        let item = PublicCallStackItemCompressed {
+            contract_address,
+            call_context,
+            args_hash,
+            returns_hash: 0,
+            revert_code: 0,
+            start_gas_left: Gas::empty(),
+            end_gas_left: Gas::empty()
+        };
+
+        self.public_teardown_call_request = PublicCallRequest {
+            item,
+            counter,
+        };
     }
 
-    fn validate_call_stack_item_from_oracle(
+    fn generate_call_context(
         self,
-        item: PublicCallStackItem,
         contract_address: AztecAddress,
         function_selector: FunctionSelector,
-        args_hash: Field,
         is_static_call: bool,
         is_delegate_call: bool
-    ) {
-        assert(contract_address.eq(item.contract_address));
-        assert(function_selector.eq(item.function_data.selector));
-
-        assert_eq(item.public_inputs.start_side_effect_counter, self.side_effect_counter);
-
-        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);
-
-        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));
+    ) -> CallContext {
+        let msg_sender = if is_delegate_call {
+            self.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.this_address()
+        };
+        let storage_contract_address = if is_delegate_call {
+            self.this_address()
+        } else {
+            contract_address
+        };
+        CallContext { msg_sender, storage_contract_address, function_selector, is_static_call, is_delegate_call }
     }
 
     fn next_counter(&mut self) -> u32 {
@@ -614,8 +591,8 @@ impl Empty for PrivateContext {
             note_hashes: BoundedVec::new(),
             nullifiers: BoundedVec::new(),
             private_call_requests: BoundedVec::new(),
-            public_call_stack_hashes: BoundedVec::new(),
-            public_teardown_function_hash: 0,
+            public_call_requests: BoundedVec::new(),
+            public_teardown_call_request: PublicCallRequest::empty(),
             l2_to_l1_msgs: BoundedVec::new(),
             historical_header: Header::empty(),
             note_encrypted_logs_hashes: BoundedVec::new(),

diff --git a/noir-projects/aztec-nr/aztec/src/oracle/call_private_function.nr b/noir-projects/aztec-nr/aztec/src/oracle/call_private_function.nr
@@ -1,7 +1,4 @@
-use dep::protocol_types::{
-    abis::{function_selector::FunctionSelector, private_call_stack_item::PrivateCallStackItem},
-    address::AztecAddress, constants::PRIVATE_CALL_STACK_ITEM_LENGTH
-};
+use dep::protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress, utils::reader::Reader};
 
 #[oracle(callPrivateFunction)]
 unconstrained fn call_private_function_oracle(
@@ -11,7 +8,7 @@ unconstrained fn call_private_function_oracle(
     _start_side_effect_counter: u32,
     _is_static_call: bool,
     _is_delegate_call: bool
-) -> [Field; PRIVATE_CALL_STACK_ITEM_LENGTH] {}
+) -> [Field; 2] {}
 
 unconstrained pub fn call_private_function_internal(
     contract_address: AztecAddress,
@@ -20,7 +17,7 @@ unconstrained pub fn call_private_function_internal(
     start_side_effect_counter: u32,
     is_static_call: bool,
     is_delegate_call: bool
-) -> PrivateCallStackItem {
+) -> (u32, Field) {
     let fields = call_private_function_oracle(
         contract_address,
         function_selector,
@@ -30,5 +27,9 @@ unconstrained pub fn call_private_function_internal(
         is_delegate_call
     );
 
-    PrivateCallStackItem::deserialize(fields)
+    let mut reader = Reader::new(fields);
+    let end_side_effect_counter = reader.read_u32();
+    let returns_hash = reader.read();
+
+    (end_side_effect_counter, returns_hash)
 }