Fix get_execution_info syscall (#1081)

* Mark read-only segments

* Move call

* Remove debug prints

* Remove fn

* Add test case

* Add comment

* Restore newlines

* Add function comment

* Fix test values

* Undo changes to makefile

* Undo changes to makefile

* clippy

Author: fmoletta

rpc_state_reader/tests/sir_tests.rs

@@ -272,6 +272,11 @@ fn test_get_gas_price() {
         186551, // real block     186552
         RpcChain::MainNet
     )]
+#[test_case(
+    "0x176a92e8df0128d47f24eebc17174363457a956fa233cc6a7f8561bfbd5023a",
+    317092, // real block 317093
+    RpcChain::MainNet
+)]
 fn starknet_in_rust_test_case_tx(hash: &str, block_number: u64, chain: RpcChain) {
     let (tx_info, trace, receipt) = execute_tx(hash, chain, BlockNumber(block_number));
 

src/syscalls/business_logic_syscall_handler.rs

@@ -133,6 +133,7 @@ pub struct BusinessLogicSyscallHandler<'a, S: StateReader> {
     pub(crate) support_reverted: bool,
     pub(crate) entry_point_selector: Felt252,
     pub(crate) selector_to_syscall: &'a HashMap<Felt252, &'static str>,
+    pub(crate) execution_info_ptr: Option<Relocatable>,
 }
 
 // TODO: execution entry point may no be a parameter field, but there is no way to generate a default for now
@@ -171,6 +172,7 @@ impl<'a, S: StateReader> BusinessLogicSyscallHandler<'a, S> {
             support_reverted,
             entry_point_selector,
             selector_to_syscall: &SELECTOR_TO_SYSCALL,
+            execution_info_ptr: None,
         }
     }
     pub fn default_with_state(state: &'a mut CachedState<S>) -> Self {
@@ -227,6 +229,7 @@ impl<'a, S: StateReader> BusinessLogicSyscallHandler<'a, S> {
             support_reverted: false,
             entry_point_selector,
             selector_to_syscall: &SELECTOR_TO_SYSCALL,
+            execution_info_ptr: None,
         }
     }
 
@@ -533,10 +536,10 @@ impl<'a, S: StateReader> BusinessLogicSyscallHandler<'a, S> {
     /// them as accessed.
     pub(crate) fn validate_read_only_segments(
         &self,
-        runner: &mut VirtualMachine,
+        vm: &mut VirtualMachine,
     ) -> Result<(), TransactionError> {
         for (segment_ptr, segment_size) in self.read_only_segments.clone() {
-            let used_size = runner
+            let used_size = vm
                 .get_segment_used_size(segment_ptr.segment_index as usize)
                 .ok_or(TransactionError::InvalidSegmentSize)?;
 
@@ -548,7 +551,7 @@ impl<'a, S: StateReader> BusinessLogicSyscallHandler<'a, S> {
             if seg_size != used_size.into() {
                 return Err(TransactionError::OutOfBound);
             }
-            runner.mark_address_range_as_accessed(segment_ptr, used_size)?;
+            vm.mark_address_range_as_accessed(segment_ptr, used_size)?;
         }
         Ok(())
     }
@@ -625,63 +628,69 @@ impl<'a, S: StateReader> BusinessLogicSyscallHandler<'a, S> {
         })
     }
 
+    // Returns the pointer to the segment with the execution info if it was already written.
+    // If it wasn't, it writes the execution info into memory and returns its start address.
+    fn get_or_allocate_execution_info(
+        &mut self,
+        vm: &mut VirtualMachine,
+    ) -> Result<Relocatable, SyscallHandlerError> {
+        if let Some(ptr) = self.execution_info_ptr {
+            return Ok(ptr);
+        }
+
+        // Allocate block_info
+        let block_info = &self.block_context.block_info;
+        let block_info_data = vec![
+            MaybeRelocatable::from(Felt252::from(block_info.block_number)),
+            MaybeRelocatable::from(Felt252::from(block_info.block_timestamp)),
+            MaybeRelocatable::from(&block_info.sequencer_address.0),
+        ];
+        let block_info_ptr = self.allocate_segment(vm, block_info_data)?;
+
+        // Allocate signature
+        let signature: Vec<MaybeRelocatable> = self
+            .tx_execution_context
+            .signature
+            .iter()
+            .map(MaybeRelocatable::from)
+            .collect();
+        let signature_start_ptr = self.allocate_segment(vm, signature)?;
+        let signature_end_ptr = (signature_start_ptr + self.tx_execution_context.signature.len())?;
+
+        // Allocate tx info
+        let tx_info = &self.tx_execution_context;
+        let tx_info_data = vec![
+            MaybeRelocatable::from(&tx_info.version),
+            MaybeRelocatable::from(&tx_info.account_contract_address.0),
+            MaybeRelocatable::from(Felt252::from(tx_info.max_fee)),
+            signature_start_ptr.into(),
+            signature_end_ptr.into(),
+            MaybeRelocatable::from(&tx_info.transaction_hash),
+            MaybeRelocatable::from(&self.block_context.starknet_os_config.chain_id),
+            MaybeRelocatable::from(&tx_info.nonce),
+        ];
+        let tx_info_ptr = self.allocate_segment(vm, tx_info_data)?;
+
+        // Allocate execution_info
+        let execution_info = vec![
+            block_info_ptr.into(),
+            tx_info_ptr.into(),
+            MaybeRelocatable::from(&self.caller_address.0),
+            MaybeRelocatable::from(&self.contract_address.0),
+            MaybeRelocatable::from(&self.entry_point_selector),
+        ];
+        let execution_info_ptr = self.allocate_segment(vm, execution_info)?;
+
+        self.execution_info_ptr = Some(execution_info_ptr);
+        Ok(execution_info_ptr)
+    }
+
     fn get_execution_info(
-        &self,
+        &mut self,
         vm: &mut VirtualMachine,
         remaining_gas: u128,
     ) -> Result<SyscallResponse, SyscallHandlerError> {
-        let tx_info = &self.tx_execution_context;
-        let block_info = &self.block_context.block_info;
-
-        let mut res_segment = vm.add_memory_segment();
-
-        let signature_start = res_segment;
-        for s in tx_info.signature.iter() {
-            vm.insert_value(res_segment, s)?;
-            res_segment = (res_segment + 1)?;
-        }
-        let signature_end = res_segment;
-
-        let tx_info_ptr = res_segment;
-        vm.insert_value::<Felt252>(res_segment, tx_info.version.clone())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value(res_segment, tx_info.account_contract_address.0.clone())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, tx_info.max_fee.into())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value(res_segment, signature_start)?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value(res_segment, signature_end)?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value(res_segment, tx_info.transaction_hash.clone())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(
-            res_segment,
-            self.block_context.starknet_os_config.chain_id.clone(),
-        )?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, tx_info.nonce.clone())?;
-        res_segment = (res_segment + 1)?;
-
-        let block_info_ptr = res_segment;
-        vm.insert_value::<Felt252>(res_segment, block_info.block_number.into())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, block_info.block_timestamp.into())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, block_info.sequencer_address.0.clone())?;
-        res_segment = (res_segment + 1)?;
-
-        let exec_info_ptr = res_segment;
-        vm.insert_value(res_segment, block_info_ptr)?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value(res_segment, tx_info_ptr)?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, self.caller_address.0.clone())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, self.contract_address.0.clone())?;
-        res_segment = (res_segment + 1)?;
-        vm.insert_value::<Felt252>(res_segment, self.entry_point_selector.clone())?;
-
+        let exec_info_ptr = self.get_or_allocate_execution_info(vm)?;
         Ok(SyscallResponse {
             gas: remaining_gas,
             body: Some(ResponseBody::GetExecutionInfo { exec_info_ptr }),

tests/deploy_account.rs

@@ -177,11 +177,11 @@ fn internal_deploy_account_cairo1() {
     let n_steps;
     #[cfg(not(feature = "cairo_1_tests"))]
     {
-        n_steps = 3936;
+        n_steps = 3921;
     }
     #[cfg(feature = "cairo_1_tests")]
     {
-        n_steps = 3952;
+        n_steps = 3937;
     }
 
     assert_eq!(
@@ -217,7 +217,7 @@ fn internal_deploy_account_cairo1() {
                     n_steps: 144,
                     #[cfg(feature="cairo_1_tests")]
                     n_steps: 155,
-                    n_memory_holes: 17,
+                    n_memory_holes: 2,
                     builtin_instance_counter:
                     [
                     ("range_check_builtin", 2),