feat(execution): add the syscall resources to vm_resources (#1412)

crates/blockifier/src/execution/deprecated_entry_point_execution.rs

@@ -241,7 +241,12 @@ pub fn finalize_execution(
         .get_execution_resources(&vm)
         .map_err(VirtualMachineError::RunnerError)?
         .filter_unused_builtins();
+    // TODO(Ori, 14/2/2024): Rename `vm_resources`.
     syscall_handler.resources.vm_resources += &vm_resources_without_inner_calls;
+    let versioned_constants = syscall_handler.context.versioned_constants();
+    // Take into account the syscall resources of the current call.
+    syscall_handler.resources.vm_resources += &versioned_constants
+        .get_additional_os_syscall_resources(&syscall_handler.syscall_counter)?;
 
     let full_call_vm_resources = &syscall_handler.resources.vm_resources - &previous_vm_resources;
     Ok(CallInfo {

crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs

@@ -134,8 +134,11 @@ fn test_nested_library_call() {
         calldata: calldata![stark_felt!(key), stark_felt!(value)],
         ..nested_storage_entry_point
     };
-    let storage_entry_point_vm_resources =
-        VmExecutionResources { n_steps: 42, ..Default::default() };
+    let storage_entry_point_vm_resources = VmExecutionResources {
+        n_steps: 218,
+        n_memory_holes: 0,
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 2)]),
+    };
     let nested_storage_call_info = CallInfo {
         call: nested_storage_entry_point,
         execution: CallExecution::from_retdata(retdata![stark_felt!(value + 1)]),
@@ -145,9 +148,9 @@ fn test_nested_library_call() {
         ..Default::default()
     };
     let mut library_call_vm_resources = VmExecutionResources {
-        n_steps: 39,
-        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 1)]),
-        ..Default::default()
+        n_steps: 790,
+        n_memory_holes: 4,
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 21)]),
     };
     library_call_vm_resources += &storage_entry_point_vm_resources;
     let library_call_info = CallInfo {
@@ -167,7 +170,11 @@ fn test_nested_library_call() {
     };
 
     // Nested library call cost: library_call(inner) + library_call(library_call(inner)).
-    let mut main_call_vm_resources = VmExecutionResources { n_steps: 45, ..Default::default() };
+    let mut main_call_vm_resources = VmExecutionResources {
+        n_steps: 796,
+        n_memory_holes: 4,
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 20)]),
+    };
     main_call_vm_resources += &(&library_call_vm_resources * 2);
     let expected_call_info = CallInfo {
         call: main_entry_point.clone(),
@@ -218,7 +225,11 @@ fn test_call_contract() {
             ..trivial_external_entry_point
         },
         execution: expected_execution.clone(),
-        vm_resources: VmExecutionResources { n_steps: 42, ..Default::default() },
+        vm_resources: VmExecutionResources {
+            n_steps: 218,
+            n_memory_holes: 0,
+            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 2)]),
+        },
         storage_read_values: vec![StarkFelt::ZERO, stark_felt!(value)],
         accessed_storage_keys: HashSet::from([StorageKey(patricia_key!(key))]),
         ..Default::default()
@@ -235,9 +246,9 @@ fn test_call_contract() {
         },
         execution: expected_execution,
         vm_resources: VmExecutionResources {
-            n_steps: 81,
-            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 1)]),
-            ..Default::default()
+            n_steps: 1017,
+            n_memory_holes: 4,
+            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 23)]),
         },
         ..Default::default()
     };

crates/blockifier/src/execution/entry_point_execution.rs

@@ -335,7 +335,12 @@ pub fn finalize_execution(
         .get_execution_resources(&vm)
         .map_err(VirtualMachineError::RunnerError)?
         .filter_unused_builtins();
+    // TODO(Ori, 14/2/2024): Rename `vm_resources`.
     syscall_handler.resources.vm_resources += &vm_resources_without_inner_calls;
+    let versioned_constants = syscall_handler.context.versioned_constants();
+    // Take into account the syscall resources of the current call.
+    syscall_handler.resources.vm_resources += &versioned_constants
+        .get_additional_os_syscall_resources(&syscall_handler.syscall_counter)?;
 
     let full_call_vm_resources = &syscall_handler.resources.vm_resources - &previous_vm_resources;
     Ok(CallInfo {

crates/blockifier/src/execution/syscalls/syscalls_test.rs

@@ -542,9 +542,9 @@ fn test_nested_library_call() {
         ..nested_storage_entry_point
     };
     let storage_entry_point_vm_resources = VmExecutionResources {
-        n_steps: 143,
+        n_steps: 319,
         n_memory_holes: 1,
-        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 5)]),
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 7)]),
     };
     let nested_storage_call_info = CallInfo {
         call: nested_storage_entry_point,
@@ -559,9 +559,9 @@ fn test_nested_library_call() {
         ..Default::default()
     };
     let library_call_vm_resources = VmExecutionResources {
-        n_steps: 411,
-        n_memory_holes: 2,
-        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 13)]),
+        n_steps: 1338,
+        n_memory_holes: 6,
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 35)]),
     };
     let library_call_info = CallInfo {
         call: library_entry_point,
@@ -588,9 +588,9 @@ fn test_nested_library_call() {
     };
 
     let main_call_vm_resources = VmExecutionResources {
-        n_steps: 765,
-        n_memory_holes: 4,
-        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 23)]),
+        n_steps: 3370,
+        n_memory_holes: 16,
+        builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 87)]),
     };
     let expected_call_info = CallInfo {
         call: main_entry_point.clone(),

crates/blockifier/src/transaction/transaction_utils.rs

@@ -29,10 +29,8 @@ pub fn calculate_tx_resources(
     let l1_blob_gas_usage = usize_from_u128(gas_vector.blob_gas)
         .expect("This conversion should not fail as the value is a converted usize.");
     // Add additional Cairo resources needed for the OS to run the transaction.
-    let vm_and_syscall_usage = &execution_resources.vm_resources
-        + &versioned_constants.get_additional_os_syscall_resources(&execution_resources.syscall_counter)?;
-    let total_vm_usage =
-        &vm_and_syscall_usage + &versioned_constants.get_additional_os_tx_resources(tx_type, calldata_length)?;
+    let total_vm_usage = &execution_resources.vm_resources
+        + &versioned_constants.get_additional_os_tx_resources(tx_type, calldata_length)?;
     let mut total_vm_usage = total_vm_usage.filter_unused_builtins();
     // The segment arena" builtin is not part of SHARP (not in any proof layout).
     // Each instance requires approximately 10 steps in the OS.

crates/blockifier/src/transaction/transactions_test.rs

@@ -309,9 +309,9 @@ fn default_invoke_tx_args(
         range_check: 102,
         n_steps: 4496,
         vm_resources: VmExecutionResources {
-            n_steps:  62,
-            n_memory_holes:  0,
-            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 1)]),
+            n_steps:  822,
+            n_memory_holes:  4,
+            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 21)]),
         },
         validate_gas_consumed: 0,
         execute_gas_consumed: 0,
@@ -323,9 +323,9 @@ fn default_invoke_tx_args(
         range_check: 115,
         n_steps: 4949,
         vm_resources: VmExecutionResources {
-            n_steps: 284,
-            n_memory_holes: 1,
-            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 7)]),
+            n_steps: 1106,
+            n_memory_holes: 5,
+            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 28)]),
         },
         validate_gas_consumed: 14360, // The gas consumption results from parsing the input
             // arguments.
@@ -1701,9 +1701,9 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) {
             ..Default::default()
         },
         vm_resources: VmExecutionResources {
-            n_steps: 143,
+            n_steps: 232,
             n_memory_holes: 1,
-            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 5)]),
+            builtin_instance_counter: HashMap::from([(RANGE_CHECK_BUILTIN_NAME.to_string(), 6)]),
         },
         accessed_storage_keys: HashSet::from_iter(vec![accessed_storage_key]),
         ..Default::default()

crates/blockifier/src/versioned_constants.rs

@@ -15,6 +15,7 @@ use thiserror::Error;
 
 use crate::execution::deprecated_syscalls::hint_processor::SyscallCounter;
 use crate::execution::deprecated_syscalls::DeprecatedSyscallSelector;
+use crate::execution::errors::PostExecutionError;
 use crate::transaction::errors::TransactionExecutionError;
 use crate::transaction::transaction_types::TransactionType;
 
@@ -97,10 +98,6 @@ impl VersionedConstants {
         self.os_resources.resources_for_tx_type(tx_type, calldata_length)
     }
 
-    // Calculates the additional resources needed for the OS to run the given transaction;
-    // i.e., the resources of the Starknet OS function `execute_transactions_inner`.
-    // Also adds the resources needed for the fee transfer execution, performed in the end·
-    // of every transaction.
     pub fn get_additional_os_tx_resources(
         &self,
         tx_type: TransactionType,
@@ -109,12 +106,10 @@ impl VersionedConstants {
         self.os_resources.get_additional_os_tx_resources(tx_type, calldata_length)
     }
 
-    /// Calculates the additional resources needed for the OS to run the given syscalls;
-    /// i.e., the resources of the Starknet OS function `execute_syscalls`.
     pub fn get_additional_os_syscall_resources(
         &self,
         syscall_counter: &SyscallCounter,
-    ) -> Result<VmExecutionResources, TransactionExecutionError> {
+    ) -> Result<VmExecutionResources, PostExecutionError> {
         self.os_resources.get_additional_os_syscall_resources(syscall_counter)
     }
 
@@ -161,11 +156,11 @@ pub struct OsResources {
 }
 
 impl OsResources {
-    // Calculates the additional resources needed for the OS to run the given transaction;
-    // i.e., the resources of the Starknet OS function `execute_transactions_inner`.
-    // Also adds the resources needed for the fee transfer execution, performed in the end·
-    // of every transaction.
-    pub fn get_additional_os_tx_resources(
+    /// Calculates the additional resources needed for the OS to run the given transaction;
+    /// i.e., the resources of the Starknet OS function `execute_transactions_inner`.
+    /// Also adds the resources needed for the fee transfer execution, performed in the end·
+    /// of every transaction.
+    fn get_additional_os_tx_resources(
         &self,
         tx_type: TransactionType,
         calldata_length: usize,
@@ -175,10 +170,10 @@ impl OsResources {
 
     /// Calculates the additional resources needed for the OS to run the given syscalls;
     /// i.e., the resources of the Starknet OS function `execute_syscalls`.
-    pub fn get_additional_os_syscall_resources(
+    fn get_additional_os_syscall_resources(
         &self,
         syscall_counter: &SyscallCounter,
-    ) -> Result<VmExecutionResources, TransactionExecutionError> {
+    ) -> Result<VmExecutionResources, PostExecutionError> {
         let mut os_additional_vm_resources = VmExecutionResources::default();
         for (syscall_selector, count) in syscall_counter {
             let syscall_resources =