diff --git a/crates/blockifier/resources/versioned_constants_0_13_0.json b/crates/blockifier/resources/versioned_constants_0_13_0.json index b5a7203236..1407a6a7fa 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_0.json +++ b/crates/blockifier/resources/versioned_constants_0_13_0.json @@ -567,7 +567,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/resources/versioned_constants_0_13_1.json b/crates/blockifier/resources/versioned_constants_0_13_1.json index cb05b58cb7..56c7cc5203 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_1.json +++ b/crates/blockifier/resources/versioned_constants_0_13_1.json @@ -603,7 +603,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/resources/versioned_constants_0_13_1_1.json b/crates/blockifier/resources/versioned_constants_0_13_1_1.json index 4f9f5885cf..ede36b46e0 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_1_1.json +++ b/crates/blockifier/resources/versioned_constants_0_13_1_1.json @@ -603,7 +603,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/resources/versioned_constants_0_13_2.json b/crates/blockifier/resources/versioned_constants_0_13_2.json index 571cce2ee0..1366040075 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_2.json +++ b/crates/blockifier/resources/versioned_constants_0_13_2.json @@ -610,7 +610,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/resources/versioned_constants_0_13_2_1.json b/crates/blockifier/resources/versioned_constants_0_13_2_1.json index ea79b39e53..aecdba873a 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_2_1.json +++ b/crates/blockifier/resources/versioned_constants_0_13_2_1.json @@ -610,7 +610,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/resources/versioned_constants_0_13_3.json b/crates/blockifier/resources/versioned_constants_0_13_3.json index ea79b39e53..aecdba873a 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_3.json +++ b/crates/blockifier/resources/versioned_constants_0_13_3.json @@ -610,7 +610,7 @@ }, "validate_max_n_steps": 1000000, "validate_max_sierra_gas": 10000000000, - "min_compiler_version_for_sierra_gas": "2.8.0", + "min_compiler_version_for_sierra_gas": "100.0.0", "vm_resource_fee_cost": { "builtins": { "add_mod_builtin": [ diff --git a/crates/blockifier/src/execution/call_info.rs b/crates/blockifier/src/execution/call_info.rs index e078376b48..948699544d 100644 --- a/crates/blockifier/src/execution/call_info.rs +++ b/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 { diff --git a/crates/blockifier/src/execution/entry_point_execution.rs b/crates/blockifier/src/execution/entry_point_execution.rs index cdb9c8c7dc..b9a6531eeb 100644 --- a/crates/blockifier/src/execution/entry_point_execution.rs +++ b/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,20 @@ 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 consumed in the current call. +pub fn gas_consumed_without_inner_calls( 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: - // 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::(), - ) - .expect("gas_for_fee unexpectedly underflowed."), + .checked_sub(inner_calls.iter().map(|call| call.execution.gas_consumed).sum::()) + .expect("gas_consumed unexpectedly underflowed."), }) } + pub fn finalize_execution( mut runner: CairoRunner, mut syscall_handler: SyscallHintProcessor<'_>, @@ -410,30 +398,40 @@ pub fn finalize_execution( let call_result = get_call_result(&runner, &syscall_handler, &tracked_resource)?; - // 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.base.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.base.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: gas_consumed_without_inner_calls( + &tracked_resource, + call_result.gas_consumed, + &syscall_handler.base.inner_calls, + ), }; + let charged_resources = &charged_resources_without_inner_calls + &CallInfo::summarize_charged_resources(syscall_handler.base.inner_calls.iter()); diff --git a/crates/blockifier/src/execution/entry_point_execution_test.rs b/crates/blockifier/src/execution/entry_point_execution_test.rs index 958bf8d837..2fe0d29542 100644 --- a/crates/blockifier/src/execution/entry_point_execution_test.rs +++ b/crates/blockifier/src/execution/entry_point_execution_test.rs @@ -1,15 +1,33 @@ +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_execution::to_gas_for_fee; +use crate::execution::entry_point::CallEntryPoint; +use crate::execution::entry_point_execution::gas_consumed_without_inner_calls; +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, + RunnableCairo1, + 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. @@ -20,7 +38,7 @@ fn test_gas_for_fee() { (TrackedResource::SierraGas, 3, 1), ] { assert_eq!( - to_gas_for_fee(&tracked_resource, gas_consumed, &inner_calls).0, + gas_consumed_without_inner_calls(&tracked_resource, gas_consumed, &inner_calls).0, expected_gas_for_fee ); inner_calls = vec![CallInfo { @@ -38,7 +56,10 @@ fn test_gas_for_fee() { // Second branch - 1 call. let (tracked_resource, gas_consumed, expected_gas_for_fee) = (TrackedResource::CairoSteps, 4, 0); - assert_eq!(to_gas_for_fee(&tracked_resource, gas_consumed, &[]).0, expected_gas_for_fee); + assert_eq!( + gas_consumed_without_inner_calls(&tracked_resource, gas_consumed, &[]).0, + expected_gas_for_fee + ); inner_calls.push(CallInfo { execution: CallExecution { gas_consumed, ..Default::default() }, @@ -51,5 +72,83 @@ fn test_gas_for_fee() { }); // Outer call. - assert_eq!(to_gas_for_fee(&TrackedResource::SierraGas, 8, &inner_calls).0, 2); + assert_eq!(gas_consumed_without_inner_calls(&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(RunnableCairo1::Casm)); + 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::>(), + ); + let call_versions = [ + CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1(RunnableCairo1::Casm)), + CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1(RunnableCairo1::Casm)), + 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); } diff --git a/crates/blockifier/src/execution/entry_point_test.rs b/crates/blockifier/src/execution/entry_point_test.rs index 6e7230742c..845fec006a 100644 --- a/crates/blockifier/src/execution/entry_point_test.rs +++ b/crates/blockifier/src/execution/entry_point_test.rs @@ -17,7 +17,7 @@ use crate::state::cached_state::CachedState; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; -use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, RunnableCairo1, BALANCE}; +use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, BALANCE}; use crate::versioned_constants::VersionedConstants; #[test] @@ -513,8 +513,8 @@ fn test_storage_related_members() { } #[test] -fn test_cairo1_entry_point_segment_arena() { - let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm)); +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![]; diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs index 37c7cf55db..343d37c219 100644 --- a/crates/blockifier/src/execution/native/entry_point_execution.rs +++ b/crates/blockifier/src/execution/native/entry_point_execution.rs @@ -2,7 +2,6 @@ use cairo_native::execution_result::ContractExecutionResult; use cairo_native::utils::BuiltinCosts; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use stacker; -use starknet_api::execution_resources::GasAmount; use crate::execution::call_info::{CallExecution, CallInfo, ChargedResources, Retdata}; use crate::execution::contract_class::TrackedResource; @@ -11,6 +10,7 @@ use crate::execution::entry_point::{ EntryPointExecutionContext, EntryPointExecutionResult, }; +use crate::execution::entry_point_execution::gas_consumed_without_inner_calls; use crate::execution::errors::{EntryPointExecutionError, PostExecutionError}; use crate::execution::native::contract_class::NativeCompiledClassV1; use crate::execution::native::syscall_handler::NativeSyscallHandler; @@ -98,8 +98,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: gas_consumed_without_inner_calls( + &TrackedResource::SierraGas, + gas_consumed, + &syscall_handler.base.inner_calls, + ), }; let charged_resources = &charged_resources_without_inner_calls + &CallInfo::summarize_charged_resources(syscall_handler.base.inner_calls.iter()); diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs index 1017a5f664..654850934c 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs +++ b/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; @@ -15,17 +13,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, - RunnableCairo1, - BALANCE, -}; +use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, RunnableCairo1, BALANCE}; use crate::versioned_constants::VersionedConstants; #[cfg_attr(feature = "cairo_native", test_case(RunnableCairo1::Native; "Native"))] @@ -157,17 +148,7 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) { ..nested_storage_entry_point }; - let mut first_storage_entry_point_resources = - ChargedResources { gas_for_fee: GasAmount(0), ..Default::default() }; - if runnable_version == RunnableCairo1::Casm { - first_storage_entry_point_resources.vm_resources = ExecutionResources { - n_steps: 244, - n_memory_holes: 0, - builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 7)]), - }; - } - - let storage_entry_point_resources = first_storage_entry_point_resources.clone(); + let storage_entry_point_gas = GasAmount(16990); // The default VersionedConstants is used in the execute_directly call bellow. let tracked_resource = test_contract.get_runnable_class().tracked_resource( @@ -182,24 +163,13 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) { 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 mut library_call_resources = - ChargedResources { gas_for_fee: GasAmount(0), ..Default::default() }; - if runnable_version == RunnableCairo1::Casm { - library_call_resources.vm_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 { @@ -207,7 +177,7 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) { gas_consumed: REQUIRED_GAS_LIBRARY_CALL_TEST, ..CallExecution::default() }, - charged_resources: library_call_resources, + charged_resources: ChargedResources::from_gas(GasAmount(117970)), inner_calls: vec![nested_storage_call_info], tracked_resource, ..Default::default() @@ -220,33 +190,22 @@ fn test_nested_library_call(runnable_version: RunnableCairo1) { 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 mut main_call_resources = - ChargedResources { gas_for_fee: GasAmount(0), ..Default::default() }; - if runnable_version == RunnableCairo1::Casm { - main_call_resources.vm_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 main_gas_consumed = 325110; let expected_call_info = CallInfo { call: main_entry_point.clone(), execution: CallExecution { retdata: retdata![felt!(value)], - gas_consumed: 325110, + gas_consumed: main_gas_consumed, ..CallExecution::default() }, - charged_resources: main_call_resources, + charged_resources: ChargedResources::from_gas(GasAmount(main_gas_consumed)), inner_calls: vec![library_call_info, storage_call_info], tracked_resource, ..Default::default() diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index 2677bfd4c6..95e461807f 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -10,6 +10,7 @@ pub mod transfers_generator; use std::collections::HashMap; use std::fs; use std::path::PathBuf; +use std::slice::Iter; use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; @@ -29,6 +30,8 @@ use starknet_api::transaction::fields::{ use starknet_api::transaction::TransactionVersion; use starknet_api::{contract_address, felt}; use starknet_types_core::felt::Felt; +use strum::EnumCount; +use strum_macros::EnumCount as EnumCountMacro; use crate::abi::constants; use crate::execution::call_info::ExecutionSummary; @@ -106,7 +109,7 @@ impl CairoVersion { } } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, EnumCountMacro, PartialEq, Eq, Debug)] pub enum CompilerBasedVersion { CairoVersion(CairoVersion), OldCairo1, @@ -130,6 +133,17 @@ impl CompilerBasedVersion { Self::CairoVersion(CairoVersion::Cairo1(_)) => TrackedResource::SierraGas, } } + + /// Returns an iterator over all of the enum variants. + pub fn iter() -> Iter<'static, Self> { + assert_eq!(Self::COUNT, 2); + static VERSIONS: [CompilerBasedVersion; 3] = [ + CompilerBasedVersion::CairoVersion(CairoVersion::Cairo0), + CompilerBasedVersion::OldCairo1, + CompilerBasedVersion::CairoVersion(CairoVersion::Cairo1(RunnableCairo1::Casm)), + ]; + VERSIONS.iter() + } } // Storage keys. diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 9cccd6e89a..a56d69cfcd 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -146,7 +146,9 @@ fn test_circuit(block_context: BlockContext, default_all_resource_bounds: ValidR } #[rstest] -fn test_rc96_holes(block_context: BlockContext, default_all_resource_bounds: ValidResourceBounds) { +#[case::vm(default_l1_resource_bounds())] +#[case::gas(default_all_resource_bounds())] +fn test_rc96_holes(block_context: BlockContext, #[case] resource_bounds: ValidResourceBounds) { let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm)); let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1(RunnableCairo1::Casm)); @@ -170,18 +172,21 @@ fn test_rc96_holes(block_context: BlockContext, default_all_resource_bounds: Val state, &block_context, invoke_tx_args! { - resource_bounds: default_all_resource_bounds, + resource_bounds: resource_bounds, ..tx_args }, ) .unwrap(); assert!(!tx_execution_info.is_reverted()); - assert_eq!( - tx_execution_info.receipt.resources.computation.vm_resources.builtin_instance_counter - [&BuiltinName::range_check96], - 24 - ); + if tx_execution_info.validate_call_info.unwrap().tracked_resource == TrackedResource::CairoSteps + { + assert_eq!( + tx_execution_info.receipt.resources.computation.vm_resources.builtin_instance_counter + [&BuiltinName::range_check96], + 24 + ); + } } #[rstest] @@ -866,7 +871,9 @@ fn test_reverted_reach_steps_limit( ) .unwrap(); let n_steps_0 = result.receipt.resources.computation.total_charged_steps(); + let gas_0 = result.receipt.resources.computation.sierra_gas; let actual_fee_0 = result.receipt.fee.0; + // Ensure the transaction was not reverted. assert!(!result.is_reverted()); @@ -882,12 +889,26 @@ fn test_reverted_reach_steps_limit( ) .unwrap(); let n_steps_1 = result.receipt.resources.computation.total_charged_steps(); + let gas_1 = result.receipt.resources.computation.sierra_gas; let actual_fee_1 = result.receipt.fee.0; // Ensure the transaction was not reverted. assert!(!result.is_reverted()); // Make sure that the n_steps and actual_fee are higher as the recursion depth increases. - assert!(n_steps_1 > n_steps_0); + let tracked_resource = result.validate_call_info.unwrap().tracked_resource; + match cairo_version { + CairoVersion::Cairo0 => { + assert_eq!(tracked_resource, TrackedResource::CairoSteps); + assert!(n_steps_1 > n_steps_0); + } + CairoVersion::Cairo1(_) => { + assert_eq!(tracked_resource, TrackedResource::SierraGas); + assert!(gas_1 > gas_0); + // TODO(Tzahi): adjust the steps in the test to gas for the SierraGas run (after a + // validate run gas limit is introduced to the code). + return; + } + } assert!(actual_fee_1 > actual_fee_0); // Calculate a recursion depth where the transaction will surely fail (not a minimal depth, as diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index e844e680e1..3aa944dce7 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1,4 +1,3 @@ -use std::cmp::min; use std::collections::{HashMap, HashSet}; use std::sync::{Arc, LazyLock}; @@ -182,7 +181,6 @@ struct ExpectedResultTestInvokeTx { resources: ExecutionResources, validate_gas_consumed: u64, execute_gas_consumed: u64, - inner_call_initial_gas: u64, } fn user_initial_gas_from_bounds( @@ -215,52 +213,70 @@ fn expected_validate_call_info( } }; // Extra range check in regular (invoke) validate call, due to passing the calldata as an array. - let n_range_checks = match cairo_version { - CairoVersion::Cairo0 => { - usize::from(entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME) - } - CairoVersion::Cairo1(RunnableCairo1::Casm) => { - if entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME { 7 } else { 2 } - } - #[cfg(feature = "cairo_native")] - CairoVersion::Cairo1(RunnableCairo1::Native) => { - if entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME { 7 } else { 2 } + let charged_resources = match tracked_resource { + TrackedResource::SierraGas => ChargedResources { + vm_resources: ExecutionResources::default(), + gas_for_fee: GasAmount(gas_consumed), + }, + TrackedResource::CairoSteps => { + let n_range_checks = match cairo_version { + CairoVersion::Cairo0 => { + usize::from(entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME) + } + CairoVersion::Cairo1(RunnableCairo1::Casm) => { + if entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME { + 7 + } else { + 2 + } + } + #[cfg(feature = "cairo_native")] + CairoVersion::Cairo1(RunnableCairo1::Native) => { + if entry_point_selector_name == constants::VALIDATE_ENTRY_POINT_NAME { + 7 + } else { + 2 + } + } + }; + let n_steps = match (entry_point_selector_name, cairo_version) { + (constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 13_usize, + ( + constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, + CairoVersion::Cairo1(RunnableCairo1::Casm), + ) => 32_usize, + (constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 12_usize, + ( + constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, + CairoVersion::Cairo1(RunnableCairo1::Casm), + ) => 28_usize, + (constants::VALIDATE_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 21_usize, + ( + constants::VALIDATE_ENTRY_POINT_NAME, + CairoVersion::Cairo1(RunnableCairo1::Casm), + ) => 100_usize, + (selector, _) => panic!("Selector {selector} is not a known validate selector."), + }; + let resources = ExecutionResources { + n_steps, + n_memory_holes: 0, + builtin_instance_counter: HashMap::from([( + BuiltinName::range_check, + n_range_checks, + )]), + } + .filter_unused_builtins(); + ChargedResources::from_execution_resources(resources) } }; - let n_steps = match (entry_point_selector_name, cairo_version) { - (constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 13_usize, - ( - constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME, - CairoVersion::Cairo1(RunnableCairo1::Casm), - ) => 32_usize, - (constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 12_usize, - ( - constants::VALIDATE_DECLARE_ENTRY_POINT_NAME, - CairoVersion::Cairo1(RunnableCairo1::Casm), - ) => 28_usize, - (constants::VALIDATE_ENTRY_POINT_NAME, CairoVersion::Cairo0) => 21_usize, - (constants::VALIDATE_ENTRY_POINT_NAME, CairoVersion::Cairo1(RunnableCairo1::Casm)) => { - 100_usize + let initial_gas = match tracked_resource { + TrackedResource::CairoSteps => default_initial_gas_cost(), + TrackedResource::SierraGas => { + user_initial_gas + .unwrap_or(initial_gas_amount_from_block_context(None)) + .min(VERSIONED_CONSTANTS.validate_max_sierra_gas) + .0 } - (selector, _) => panic!("Selector {selector} is not a known validate selector."), - }; - let resources = ExecutionResources { - n_steps, - n_memory_holes: 0, - builtin_instance_counter: HashMap::from([(BuiltinName::range_check, n_range_checks)]), - } - .filter_unused_builtins(); - let initial_gas = match cairo_version { - CairoVersion::Cairo0 => default_initial_gas_cost(), - CairoVersion::Cairo1(_) => match tracked_resource { - TrackedResource::CairoSteps => initial_gas_amount_from_block_context(None).0, - TrackedResource::SierraGas => { - user_initial_gas - .unwrap_or(initial_gas_amount_from_block_context(None)) - .min(VERSIONED_CONSTANTS.validate_max_sierra_gas) - .0 - } - }, }; Some(CallInfo { @@ -276,7 +292,7 @@ fn expected_validate_call_info( initial_gas, }, // The account contract we use for testing has trivial `validate` functions. - charged_resources: ChargedResources::from_execution_resources(resources), + charged_resources, execution: CallExecution { retdata, gas_consumed, ..Default::default() }, tracked_resource, ..Default::default() @@ -449,7 +465,6 @@ fn add_kzg_da_resources_to_resources_mapping( }, validate_gas_consumed: 0, execute_gas_consumed: 0, - inner_call_initial_gas: versioned_constants_for_account_testing().inifite_gas_for_vm_mode(), }, CairoVersion::Cairo0)] #[case::with_cairo1_account( @@ -462,7 +477,6 @@ fn add_kzg_da_resources_to_resources_mapping( validate_gas_consumed: 4740, // The gas consumption results from parsing the input // arguments. execute_gas_consumed: 112080, - inner_call_initial_gas: versioned_constants_for_account_testing().inifite_gas_for_vm_mode(), }, CairoVersion::Cairo1(RunnableCairo1::Casm))] // TODO(Tzahi): Add calls to cairo1 test contracts (where gas flows to and from the inner call). @@ -537,26 +551,33 @@ fn test_invoke_tx( .execute_max_sierra_gas .min(initial_gas - GasAmount(expected_arguments.validate_gas_consumed)) .0; - if account_cairo_version == CairoVersion::Cairo0 { - expected_arguments.inner_call_initial_gas = versioned_constants.inifite_gas_for_vm_mode(); - } let expected_execute_call = CallEntryPoint { entry_point_selector: selector_from_name(constants::EXECUTE_ENTRY_POINT_NAME), - initial_gas: if account_cairo_version == CairoVersion::Cairo0 { - versioned_constants.inifite_gas_for_vm_mode() - } else { - expected_initial_execution_gas + initial_gas: match account_cairo_version { + CairoVersion::Cairo0 => versioned_constants.inifite_gas_for_vm_mode(), + CairoVersion::Cairo1(_) => expected_initial_execution_gas, }, ..expected_validated_call }; - if tracked_resource == TrackedResource::CairoSteps { - expected_arguments.inner_call_initial_gas -= expected_arguments.validate_gas_consumed; - expected_arguments.inner_call_initial_gas = min( - expected_arguments.inner_call_initial_gas, - versioned_constants.execute_max_sierra_gas.0, - ); - expected_arguments.inner_call_initial_gas -= expected_arguments.execute_gas_consumed; - } + + let expected_inner_call_charged_resources = + ChargedResources::from_execution_resources(ExecutionResources { + n_steps: 23, + n_memory_holes: 0, + ..Default::default() + }); + let (expected_validate_gas_for_fee, expected_execute_gas_for_fee) = match tracked_resource { + TrackedResource::CairoSteps => (GasAmount::default(), GasAmount::default()), + TrackedResource::SierraGas => { + expected_arguments.resources = + expected_inner_call_charged_resources.vm_resources.clone(); + ( + expected_arguments.validate_gas_consumed.into(), + expected_arguments.execute_gas_consumed.into(), + ) + } + }; + let expected_return_result_call = CallEntryPoint { entry_point_selector: selector_from_name("return_result"), class_hash: Some(test_contract.get_class_hash()), @@ -566,31 +587,29 @@ fn test_invoke_tx( storage_address: test_contract_address, caller_address: sender_address, call_type: CallType::Call, - initial_gas: if account_cairo_version == CairoVersion::Cairo0 { - versioned_constants.inifite_gas_for_vm_mode() - } else { - expected_arguments.inner_call_initial_gas - }, + initial_gas: versioned_constants.inifite_gas_for_vm_mode(), }; + let expected_return_result_retdata = Retdata(expected_return_result_calldata); + let expected_inner_calls = vec![CallInfo { + call: expected_return_result_call, + execution: CallExecution::from_retdata(expected_return_result_retdata.clone()), + charged_resources: expected_inner_call_charged_resources, + ..Default::default() + }]; + let expected_execute_call_info = Some(CallInfo { call: expected_execute_call, execution: CallExecution { - retdata: Retdata(expected_return_result_retdata.0.clone()), + retdata: Retdata(expected_return_result_retdata.0), gas_consumed: expected_arguments.execute_gas_consumed, ..Default::default() }, - charged_resources: ChargedResources::from_execution_resources(expected_arguments.resources), - inner_calls: vec![CallInfo { - call: expected_return_result_call, - execution: CallExecution::from_retdata(expected_return_result_retdata), - charged_resources: ChargedResources::from_execution_resources(ExecutionResources { - n_steps: 23, - n_memory_holes: 0, - ..Default::default() - }), - ..Default::default() - }], + charged_resources: ChargedResources { + vm_resources: expected_arguments.resources, + gas_for_fee: expected_execute_gas_for_fee, + }, + inner_calls: expected_inner_calls, tracked_resource, ..Default::default() }); @@ -613,10 +632,12 @@ fn test_invoke_tx( &starknet_resources, vec![&expected_validate_call_info, &expected_execute_call_info], ); + let mut expected_actual_resources = TransactionResources { starknet_resources, computation: ComputationResources { vm_resources: expected_cairo_resources, + sierra_gas: expected_validate_gas_for_fee + expected_execute_gas_for_fee, ..Default::default() }, }; @@ -2339,6 +2360,7 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { // Build the expected call info. let accessed_storage_key = StorageKey::try_from(key).unwrap(); + let gas_consumed = GasAmount(6120); let expected_call_info = CallInfo { call: CallEntryPoint { class_hash: Some(test_contract.get_class_hash()), @@ -2353,14 +2375,10 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { }, execution: CallExecution { retdata: Retdata(vec![value]), - gas_consumed: 6120, + gas_consumed: gas_consumed.0, ..Default::default() }, - charged_resources: ChargedResources::from_execution_resources(ExecutionResources { - n_steps: 151, - n_memory_holes: 0, - builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 6)]), - }), + charged_resources: ChargedResources::from_gas(gas_consumed), accessed_storage_keys: HashSet::from_iter(vec![accessed_storage_key]), tracked_resource: test_contract .get_runnable_class() @@ -2369,15 +2387,16 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { }; // Build the expected resource mapping. + // l2_gas is 0 in the receipt even though execution resources is not, as we're in NoL2Gas mode. // TODO(Nimrod, 1/5/2024): Change these hard coded values to match to the transaction resources // (currently matches only starknet resources). let expected_gas = match use_kzg_da { true => GasVector { - l1_gas: 17988_u32.into(), + l1_gas: 17899_u32.into(), l1_data_gas: 160_u32.into(), l2_gas: 0_u32.into(), }, - false => GasVector::from_l1_gas(19682_u32.into()), + false => GasVector::from_l1_gas(19593_u32.into()), }; let expected_da_gas = match use_kzg_da { @@ -2397,11 +2416,10 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { ( BuiltinName::range_check, get_tx_resources(TransactionType::L1Handler).builtin_instance_counter - [&BuiltinName::range_check] - + 6, + [&BuiltinName::range_check], ), ]), - n_steps: get_tx_resources(TransactionType::L1Handler).n_steps + 164, + n_steps: get_tx_resources(TransactionType::L1Handler).n_steps + 13, n_memory_holes: 0, }; @@ -2417,18 +2435,19 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) { starknet_resources: actual_execution_info.receipt.resources.starknet_resources.clone(), computation: ComputationResources { vm_resources: expected_execution_resources, + sierra_gas: gas_consumed.into(), ..Default::default() }, }; assert_eq!(actual_execution_info.receipt.resources, expected_tx_resources); assert_eq!( - expected_gas, actual_execution_info.receipt.resources.to_gas_vector( versioned_constants, use_kzg_da, &gas_mode, - ) + ), + expected_gas ); let total_gas = expected_tx_resources.to_gas_vector(