feat(blockifier): update gas and vm resources computations

crates/blockifier/src/execution/call_info.rs

@@ -113,6 +113,10 @@ impl ChargedResources {
     pub fn from_execution_resources(resources: ExecutionResources) -> Self {
         Self { vm_resources: resources, ..Default::default() }
     }
+
+    pub fn from_gas(gas_for_fee: GasAmount) -> Self {
+        Self { gas_for_fee, ..Default::default() }
+    }
 }
 
 impl Add<&ChargedResources> for &ChargedResources {

crates/blockifier/src/execution/entry_point_execution.rs

@@ -7,7 +7,7 @@ use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
 use cairo_vm::vm::errors::memory_errors::MemoryError;
 use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
 use cairo_vm::vm::runners::builtin_runner::BuiltinRunner;
-use cairo_vm::vm::runners::cairo_runner::{CairoArg, CairoRunner};
+use cairo_vm::vm::runners::cairo_runner::{CairoArg, CairoRunner, ExecutionResources};
 use cairo_vm::vm::security::verify_secure_runner;
 use num_traits::{ToPrimitive, Zero};
 use starknet_api::execution_resources::GasAmount;
@@ -360,32 +360,27 @@ fn maybe_fill_holes(
     Ok(())
 }
 
-/// Calculates the total gas for fee in the current call + subtree.
-#[allow(dead_code)]
-fn to_gas_for_fee(
+/// Calculates the gas for fee in the current call.
+pub fn to_gas_for_fee(
     tracked_resource: &TrackedResource,
     gas_consumed: u64,
     inner_calls: &[CallInfo],
 ) -> GasAmount {
     // The Sierra gas consumed in this specific call is `gas_consumed`
     // (= total gas of self + subtree), minus the sum of all inner calls Sierra gas consumed.
     // To compute the total Sierra gas to charge (of self + subtree), if the tracked resource is
-    // Sierra gas, we add this amount to the total gas to charge for in the subtree:
+    // Sierra gas, we later add this amount to the total gas to charge for in the subtree:
     // gas_for_fee = gas_consumed - subtree_gas_consumed + subtree_gas_to_fee.
     GasAmount(match tracked_resource {
         // If the tracked resource is CairoSteps, then all tracked resources of all calls in
         // the subtree are also CairoSteps. Thus, the total gas to charge in this subtree is zero.
         TrackedResource::CairoSteps => 0,
         TrackedResource::SierraGas => gas_consumed
-            .checked_sub(
-                inner_calls
-                    .iter()
-                    .map(|call| call.execution.gas_consumed - call.charged_resources.gas_for_fee.0)
-                    .sum::<u64>(),
-            )
+            .checked_sub(inner_calls.iter().map(|call| call.execution.gas_consumed).sum::<u64>())
             .expect("gas_for_fee unexpectedly underflowed."),
     })
 }
+
 pub fn finalize_execution(
     mut runner: CairoRunner,
     mut syscall_handler: SyscallHintProcessor<'_>,
@@ -410,29 +405,38 @@ pub fn finalize_execution(
 
     let call_result = get_call_result(&runner, &syscall_handler)?;
 
-    // Take into account the resources of the current call, without inner calls.
-    // Has to happen after marking holes in segments as accessed.
-    let mut vm_resources_without_inner_calls = runner
-        .get_execution_resources()
-        .map_err(VirtualMachineError::RunnerError)?
-        .filter_unused_builtins();
-    let versioned_constants = syscall_handler.context.versioned_constants();
-    if versioned_constants.segment_arena_cells {
-        vm_resources_without_inner_calls
-            .builtin_instance_counter
-            .get_mut(&BuiltinName::segment_arena)
-            .map_or_else(|| {}, |val| *val *= SEGMENT_ARENA_BUILTIN_SIZE);
-    }
-    // Take into account the syscall resources of the current call.
-    vm_resources_without_inner_calls +=
-        &versioned_constants.get_additional_os_syscall_resources(&syscall_handler.syscall_counter);
+    let vm_resources_without_inner_calls = match tracked_resource {
+        TrackedResource::CairoSteps => {
+            // Take into account the resources of the current call, without inner calls.
+            // Has to happen after marking holes in segments as accessed.
+            let mut vm_resources_without_inner_calls = runner
+                .get_execution_resources()
+                .map_err(VirtualMachineError::RunnerError)?
+                .filter_unused_builtins();
+            let versioned_constants = syscall_handler.context.versioned_constants();
+            if versioned_constants.segment_arena_cells {
+                vm_resources_without_inner_calls
+                    .builtin_instance_counter
+                    .get_mut(&BuiltinName::segment_arena)
+                    .map_or_else(|| {}, |val| *val *= SEGMENT_ARENA_BUILTIN_SIZE);
+            }
+            // Take into account the syscall resources of the current call.
+            vm_resources_without_inner_calls += &versioned_constants
+                .get_additional_os_syscall_resources(&syscall_handler.syscall_counter);
+            vm_resources_without_inner_calls
+        }
+        TrackedResource::SierraGas => ExecutionResources::default(),
+    };
 
     syscall_handler.finalize();
 
     let charged_resources_without_inner_calls = ChargedResources {
         vm_resources: vm_resources_without_inner_calls,
-        // TODO(tzahi): Replace with a computed value.
-        gas_for_fee: GasAmount(0),
+        gas_for_fee: to_gas_for_fee(
+            &tracked_resource,
+            call_result.gas_consumed,
+            &syscall_handler.inner_calls,
+        ),
     };
     let charged_resources = &charged_resources_without_inner_calls
         + &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

crates/blockifier/src/execution/entry_point_execution_test.rs

@@ -1,15 +1,32 @@
+use std::sync::Arc;
+
+use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
+use rstest::rstest;
+use starknet_api::abi::abi_utils::selector_from_name;
 use starknet_api::execution_resources::GasAmount;
+use starknet_api::transaction::fields::Calldata;
 
+use crate::context::ChainInfo;
 use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources};
 use crate::execution::contract_class::TrackedResource;
+use crate::execution::entry_point::CallEntryPoint;
 use crate::execution::entry_point_execution::to_gas_for_fee;
+use crate::test_utils::contracts::FeatureContract;
+use crate::test_utils::initial_test_state::test_state;
+use crate::test_utils::syscall::build_recurse_calldata;
+use crate::test_utils::{
+    trivial_external_entry_point_new,
+    CairoVersion,
+    CompilerBasedVersion,
+    BALANCE,
+};
 
 #[test]
 /// Verifies that every call from the inner most to the outer has the expected gas_for_fee for the
 /// following topology (marked as TrackedResource(gas_consumed)):
 //       Gas(8) -> Gas(3) -> VM(2) -> VM(1)
 //            \ -> VM(4)
-// Expected values are 2 -> 1 -> 0 -> 0.
+// Expected values are 1 -> 1 -> 0 -> 0.
 //                      \-> 0.
 fn test_gas_for_fee() {
     // First branch - 3 nested calls.
@@ -51,5 +68,83 @@ fn test_gas_for_fee() {
     });
 
     // Outer call.
-    assert_eq!(to_gas_for_fee(&TrackedResource::SierraGas, 8, &inner_calls).0, 2);
+    assert_eq!(to_gas_for_fee(&TrackedResource::SierraGas, 8, &inner_calls).0, 1);
+}
+
+/// Asserts that the charged resources of a call is consistent with the inner calls in its subtree.
+fn assert_charged_resource_as_expected_rec(call_info: &CallInfo) {
+    let inner_calls = &call_info.inner_calls;
+    let mut children_vm_resources = ExecutionResources::default();
+    let mut children_gas = GasAmount(0);
+    for child_call_info in inner_calls.iter() {
+        let ChargedResources { gas_for_fee, vm_resources } = &child_call_info.charged_resources;
+        children_vm_resources += vm_resources;
+        children_gas += *gas_for_fee;
+    }
+
+    let ChargedResources { gas_for_fee, vm_resources } = &call_info.charged_resources;
+
+    match call_info.tracked_resource {
+        TrackedResource::SierraGas => {
+            assert_eq!(vm_resources, &children_vm_resources);
+            assert!(gas_for_fee > &children_gas)
+        }
+        TrackedResource::CairoSteps => {
+            assert_eq!(gas_for_fee, &children_gas);
+            assert!(vm_resources.n_steps > children_vm_resources.n_steps)
+        }
+    }
+
+    for child_call_info in inner_calls.iter() {
+        assert_charged_resource_as_expected_rec(child_call_info);
+    }
+}
+
+#[rstest]
+fn test_charged_resources_computation(
+    #[values(
+        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0),
+        CompilerBasedVersion::OldCairo1
+    )]
+    third_contract_version: CompilerBasedVersion,
+    #[values(
+        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0),
+        CompilerBasedVersion::OldCairo1
+    )]
+    fourth_contract_version: CompilerBasedVersion,
+    #[values(
+        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0),
+        CompilerBasedVersion::OldCairo1
+    )]
+    second_branch_contract_version: CompilerBasedVersion,
+) {
+    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
+    let chain_info = &ChainInfo::create_for_testing();
+    let contracts = CompilerBasedVersion::iter().map(|version| version.get_test_contract());
+    let mut state = test_state(
+        chain_info,
+        BALANCE,
+        &contracts.map(|contract| (contract, 1)).collect::<Vec<_>>(),
+    );
+    let call_versions = [
+        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1),
+        CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1),
+        third_contract_version,
+        fourth_contract_version,
+    ];
+
+    let first_calldata = build_recurse_calldata(&call_versions);
+    let second_calldata = build_recurse_calldata(&[second_branch_contract_version]);
+    let outer_calldata = Calldata(Arc::new(
+        (*first_calldata.0).iter().copied().chain((*second_calldata.0).iter().copied()).collect(),
+    ));
+    let call_contract_selector = selector_from_name("test_call_two_contracts");
+    let entry_point_call = CallEntryPoint {
+        entry_point_selector: call_contract_selector,
+        calldata: outer_calldata,
+        ..trivial_external_entry_point_new(test_contract)
+    };
+    let call_info = entry_point_call.execute_directly(&mut state).unwrap();
+
+    assert_charged_resource_as_expected_rec(&call_info);
 }

crates/blockifier/src/execution/entry_point_test.rs

@@ -513,8 +513,8 @@ fn test_storage_related_members() {
 }
 
 #[test]
-fn test_cairo1_entry_point_segment_arena() {
-    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
+fn test_old_cairo1_entry_point_segment_arena() {
+    let test_contract = FeatureContract::CairoStepsTestContract;
     let chain_info = &ChainInfo::create_for_testing();
     let mut state = test_state(chain_info, BALANCE, &[(test_contract, 1)]);
     let calldata = calldata![];

crates/blockifier/src/execution/native/entry_point_execution.rs

@@ -1,7 +1,6 @@
 use cairo_native::execution_result::ContractExecutionResult;
 use cairo_native::utils::BuiltinCosts;
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
-use starknet_api::execution_resources::GasAmount;
 
 use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata};
 use crate::execution::contract_class::TrackedResource;
@@ -10,6 +9,7 @@ use crate::execution::entry_point::{
     EntryPointExecutionContext,
     EntryPointExecutionResult,
 };
+use crate::execution::entry_point_execution::to_gas_for_fee;
 use crate::execution::errors::{EntryPointExecutionError, PostExecutionError};
 use crate::execution::native::contract_class::NativeContractClassV1;
 use crate::execution::native::syscall_handler::NativeSyscallHandler;
@@ -87,8 +87,11 @@ fn create_callinfo(
 
     let charged_resources_without_inner_calls = ChargedResources {
         vm_resources: ExecutionResources::default(),
-        // TODO(tzahi): Replace with a computed value.
-        gas_for_fee: GasAmount(0),
+        gas_for_fee: to_gas_for_fee(
+            &TrackedResource::SierraGas,
+            gas_consumed,
+            &syscall_handler.inner_calls,
+        ),
     };
     let charged_resources = &charged_resources_without_inner_calls
         + &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs

@@ -1,7 +1,5 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 
-use cairo_vm::types::builtin_name::BuiltinName;
-use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
 use pretty_assertions::assert_eq;
 use starknet_api::abi::abi_utils::selector_from_name;
 use starknet_api::execution_resources::GasAmount;
@@ -16,16 +14,10 @@ use crate::execution::syscalls::syscall_tests::constants::{
     REQUIRED_GAS_LIBRARY_CALL_TEST,
     REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
 };
-use crate::execution::syscalls::SyscallSelector;
 use crate::retdata;
 use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::initial_test_state::test_state;
-use crate::test_utils::{
-    get_syscall_resources,
-    trivial_external_entry_point_new,
-    CairoVersion,
-    BALANCE,
-};
+use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, BALANCE};
 use crate::versioned_constants::VersionedConstants;
 
 #[cfg_attr(
@@ -188,18 +180,7 @@ fn test_nested_library_call(test_contract: FeatureContract) {
         ..nested_storage_entry_point
     };
 
-    let first_storage_entry_point_resources = if_native(&test_contract)(
-        ChargedResources { vm_resources: ExecutionResources::default(), gas_for_fee: GasAmount(0) },
-        ChargedResources::from_execution_resources(ExecutionResources {
-            n_steps: 244,
-            n_memory_holes: 0,
-            builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 7)]),
-        }),
-    );
-    let storage_entry_point_resources = if_native(&test_contract)(
-        ChargedResources { vm_resources: ExecutionResources::default(), gas_for_fee: GasAmount(0) },
-        first_storage_entry_point_resources.clone(),
-    );
+    let storage_entry_point_gas = GasAmount(if_native(&test_contract)(26990, 16990));
 
     // The default VersionedConstants is used in the execute_directly call bellow.
     let tracked_resource = test_contract.get_runnable_class().tracked_resource(
@@ -214,33 +195,23 @@ fn test_nested_library_call(test_contract: FeatureContract) {
             gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
             ..CallExecution::default()
         },
-        charged_resources: first_storage_entry_point_resources,
+        charged_resources: ChargedResources::from_gas(storage_entry_point_gas),
         tracked_resource,
         storage_read_values: vec![felt!(value + 1)],
         accessed_storage_keys: HashSet::from([storage_key!(key + 1)]),
         ..Default::default()
     };
 
-    let library_call_resources = if_native(&test_contract)(
-        ChargedResources { vm_resources: ExecutionResources::default(), gas_for_fee: GasAmount(0) },
-        ChargedResources::from_execution_resources(
-            &get_syscall_resources(SyscallSelector::LibraryCall)
-                + &ExecutionResources {
-                    n_steps: 377,
-                    n_memory_holes: 0,
-                    builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 15)]),
-                },
-        ),
-    );
-
     let library_call_info = CallInfo {
         call: library_entry_point,
         execution: CallExecution {
             retdata: retdata![felt!(value + 1)],
             gas_consumed: REQUIRED_GAS_LIBRARY_CALL_TEST,
             ..CallExecution::default()
         },
-        charged_resources: library_call_resources,
+        charged_resources: ChargedResources::from_gas(GasAmount(if_native(&test_contract)(
+            187970, 167970,
+        ))),
         inner_calls: vec![nested_storage_call_info],
         tracked_resource,
         ..Default::default()
@@ -253,33 +224,23 @@ fn test_nested_library_call(test_contract: FeatureContract) {
             gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST,
             ..CallExecution::default()
         },
-        charged_resources: storage_entry_point_resources,
+        charged_resources: ChargedResources::from_gas(storage_entry_point_gas),
         storage_read_values: vec![felt!(value)],
         accessed_storage_keys: HashSet::from([storage_key!(key)]),
         tracked_resource,
         ..Default::default()
     };
 
-    let main_call_resources = if_native(&test_contract)(
-        ChargedResources { vm_resources: ExecutionResources::default(), gas_for_fee: GasAmount(0) },
-        ChargedResources::from_execution_resources(
-            &(&get_syscall_resources(SyscallSelector::LibraryCall) * 3)
-                + &ExecutionResources {
-                    n_steps: 727,
-                    n_memory_holes: 2,
-                    builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 27)]),
-                },
-        ),
-    );
-
     let expected_call_info = CallInfo {
         call: main_entry_point.clone(),
         execution: CallExecution {
             retdata: retdata![felt!(value)],
             gas_consumed: 475110,
             ..CallExecution::default()
         },
-        charged_resources: main_call_resources,
+        charged_resources: ChargedResources::from_gas(GasAmount(if_native(&test_contract)(
+            515110, 475110,
+        ))),
         inner_calls: vec![library_call_info, storage_call_info],
         tracked_resource,
         ..Default::default()