diff --git a/crates/blockifier/src/fee/fee_test.rs b/crates/blockifier/src/fee/fee_test.rs index bd7a310e2b..a3b4c00b2d 100644 --- a/crates/blockifier/src/fee/fee_test.rs +++ b/crates/blockifier/src/fee/fee_test.rs @@ -1,8 +1,5 @@ -use std::collections::HashMap; - use assert_matches::assert_matches; use cairo_vm::types::builtin_name::BuiltinName; -use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use rstest::rstest; use starknet_api::block::NonzeroGasPrice; use starknet_api::execution_resources::GasAmount; @@ -19,6 +16,7 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::test_state; use crate::test_utils::{ gas_vector_from_vm_usage, + get_vm_resource_usage, CairoVersion, BALANCE, DEFAULT_ETH_L1_DATA_GAS_PRICE, @@ -33,20 +31,6 @@ use crate::transaction::test_utils::{account_invoke_tx, all_resource_bounds, l1_ use crate::utils::{u128_from_usize, u64_from_usize}; use crate::versioned_constants::VersionedConstants; -fn get_vm_resource_usage() -> ExecutionResources { - ExecutionResources { - n_steps: 10000, - n_memory_holes: 0, - builtin_instance_counter: HashMap::from([ - (BuiltinName::pedersen, 10), - (BuiltinName::range_check, 24), - (BuiltinName::ecdsa, 1), - (BuiltinName::bitwise, 1), - (BuiltinName::poseidon, 1), - ]), - } -} - #[rstest] fn test_simple_get_vm_resource_usage( #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index 12e65ae417..b96f6ad288 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -1,5 +1,9 @@ +use std::sync::Arc; + +use num_rational::Ratio; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; +use starknet_api::execution_resources::GasAmount; use starknet_api::invoke_tx_args; use starknet_api::transaction::{EventContent, EventData, EventKey, GasVectorComputationMode}; use starknet_types_core::felt::Felt; @@ -10,18 +14,75 @@ use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent}; use crate::fee::eth_gas_constants; use crate::fee::fee_utils::get_fee_by_gas_vector; use crate::fee::gas_usage::{get_da_gas_cost, get_message_segment_length}; -use crate::fee::resources::{GasVector, StarknetResources, StateResources}; +use crate::fee::resources::{ + ComputationResources, + GasVector, + StarknetResources, + StateResources, + TransactionResources, +}; use crate::state::cached_state::StateChangesCount; -use crate::test_utils::{DEFAULT_ETH_L1_DATA_GAS_PRICE, DEFAULT_ETH_L1_GAS_PRICE}; +use crate::test_utils::{ + get_vm_resource_usage, + DEFAULT_ETH_L1_DATA_GAS_PRICE, + DEFAULT_ETH_L1_GAS_PRICE, +}; use crate::transaction::objects::FeeType; use crate::transaction::test_utils::account_invoke_tx; use crate::utils::{u128_from_usize, u64_from_usize}; -use crate::versioned_constants::{ResourceCost, VersionedConstants}; +use crate::versioned_constants::{ + ResourceCost, + StarknetVersion, + VersionedConstants, + VmResourceCosts, +}; + +pub fn create_event_for_testing(keys_size: usize, data_size: usize) -> OrderedEvent { + OrderedEvent { + order: 0, + event: EventContent { + keys: vec![EventKey(Felt::ZERO); keys_size], + data: EventData(vec![Felt::ZERO; data_size]), + }, + } +} + #[fixture] fn versioned_constants() -> &'static VersionedConstants { VersionedConstants::latest_constants() } +// Starknet resources with many resources (of arbitrary values) for testing. +#[fixture] +fn starknet_resources() -> StarknetResources { + let call_info_1 = CallInfo { + execution: CallExecution { + events: vec![create_event_for_testing(1, 2), create_event_for_testing(1, 2)], + ..Default::default() + }, + ..Default::default() + }; + let call_info_2 = CallInfo { + execution: CallExecution { + events: vec![create_event_for_testing(1, 0), create_event_for_testing(0, 1)], + ..Default::default() + }, + ..Default::default() + }; + let call_infos: Vec = vec![call_info_1, call_info_2] + .into_iter() + .map(|call_info| call_info.with_some_class_hash()) + .collect(); + let execution_summary = CallInfo::summarize_many(call_infos.iter()); + let state_resources = StateResources::new_for_testing(StateChangesCount { + n_storage_updates: 7, + n_class_hash_updates: 11, + n_compiled_class_hash_updates: 13, + n_modified_contracts: 17, + }); + StarknetResources::new(2_usize, 3_usize, 4_usize, state_resources, 6.into(), execution_summary) +} + #[rstest] fn test_get_event_gas_cost( versioned_constants: &VersionedConstants, @@ -50,32 +111,31 @@ fn test_get_event_gas_cost( ) ); - let create_event = |keys_size: usize, data_size: usize| OrderedEvent { - order: 0, - event: EventContent { - keys: vec![EventKey(Felt::ZERO); keys_size], - data: EventData(vec![Felt::ZERO; data_size]), - }, - }; let call_info_1 = CallInfo { execution: CallExecution { - events: vec![create_event(1, 2), create_event(1, 2)], + events: vec![create_event_for_testing(1, 2), create_event_for_testing(1, 2)], ..Default::default() }, ..Default::default() }; let call_info_2 = CallInfo { execution: CallExecution { - events: vec![create_event(1, 0), create_event(0, 1)], + events: vec![create_event_for_testing(1, 0), create_event_for_testing(0, 1)], ..Default::default() }, ..Default::default() }; let call_info_3 = CallInfo { - execution: CallExecution { events: vec![create_event(0, 1)], ..Default::default() }, + execution: CallExecution { + events: vec![create_event_for_testing(0, 1)], + ..Default::default() + }, inner_calls: vec![ CallInfo { - execution: CallExecution { events: vec![create_event(5, 5)], ..Default::default() }, + execution: CallExecution { + events: vec![create_event_for_testing(5, 5)], + ..Default::default() + }, ..Default::default() } .with_some_class_hash(), @@ -246,3 +306,109 @@ fn test_discounted_gas_from_gas_vector_computation() { <= actual_result.nonzero_checked_mul(DEFAULT_ETH_L1_GAS_PRICE).unwrap() ); } + +#[rstest] +// Assert gas computation results are as expected. The goal of this test is to prevent unwanted +// changes to the gas computation. +fn test_gas_computation_regression_test( + starknet_resources: StarknetResources, + #[values(false, true)] use_kzg_da: bool, + #[values(GasVectorComputationMode::NoL2Gas, GasVectorComputationMode::All)] + gas_vector_computation_mode: GasVectorComputationMode, +) { + // Use a constant version of the versioned constants so that version changes do not break this + // test. This specific version is arbitrary. + // TODO(Amos, 1/10/2024): Parameterize the version. + let mut versioned_constants = VersionedConstants::get(StarknetVersion::V0_13_2_1).clone(); + + // Change the VM resource fee cost so that the L2 / L1 gas ratio is a fraction. + let vm_resource_fee_cost = VmResourceCosts { + builtins: versioned_constants.vm_resource_fee_cost.builtins.clone(), + n_steps: Ratio::new(30, 10000), + }; + versioned_constants.vm_resource_fee_cost = Arc::new(vm_resource_fee_cost); + + // Test Starknet resources. + let actual_starknet_resources_gas_vector = starknet_resources.to_gas_vector( + &versioned_constants, + use_kzg_da, + &gas_vector_computation_mode, + ); + let expected_starknet_resources_gas_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => match use_kzg_da { + true => GasVector { + l1_gas: GasAmount(21544), + l1_data_gas: GasAmount(2720), + l2_gas: GasAmount(0), + }, + false => GasVector::from_l1_gas(GasAmount(62835)), + }, + GasVectorComputationMode::All => match use_kzg_da { + true => GasVector { + l1_gas: GasAmount(21543), + l1_data_gas: GasAmount(2720), + l2_gas: GasAmount(87040), + }, + false => GasVector { + l1_gas: GasAmount(62834), + l1_data_gas: GasAmount(0), + l2_gas: GasAmount(87040), + }, + }, + }; + assert_eq!( + actual_starknet_resources_gas_vector, expected_starknet_resources_gas_vector, + "Unexpected gas computation result for starknet resources. If this is intentional please \ + fix this test." + ); + + // Test VM resources. + let mut vm_resources = get_vm_resource_usage(); + vm_resources.n_memory_holes = 2; + let n_reverted_steps = 15; + let computation_resources = ComputationResources { vm_resources, n_reverted_steps }; + let actual_computation_resources_gas_vector = + computation_resources.to_gas_vector(&versioned_constants, &gas_vector_computation_mode); + let expected_computation_resources_gas_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas(GasAmount(31)), + GasVectorComputationMode::All => GasVector::from_l2_gas(GasAmount(1033334)), + }; + assert_eq!( + actual_computation_resources_gas_vector, expected_computation_resources_gas_vector, + "Unexpected gas computation result for VM resources. If this is intentional please fix \ + this test." + ); + + // Test transaction resources + let tx_resources = + TransactionResources { starknet_resources, computation: computation_resources }; + let actual_gas_vector = + tx_resources.to_gas_vector(&versioned_constants, use_kzg_da, &gas_vector_computation_mode); + let expected_gas_vector = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => match use_kzg_da { + true => GasVector { + l1_gas: GasAmount(21575), + l1_data_gas: GasAmount(2720), + l2_gas: GasAmount(0), + }, + false => GasVector::from_l1_gas(GasAmount(62866)), + }, + GasVectorComputationMode::All => match use_kzg_da { + true => GasVector { + l1_gas: GasAmount(21543), + l1_data_gas: GasAmount(2720), + l2_gas: GasAmount(1120374), + }, + false => GasVector { + l1_gas: GasAmount(62834), + l1_data_gas: GasAmount(0), + l2_gas: GasAmount(1120374), + }, + }, + }; + assert_eq!( + actual_gas_vector, expected_gas_vector, + "Unexpected gas computation result for tx resources. If this is intentional please fix \ + this test." + ); +} diff --git a/crates/blockifier/src/fee/receipt_test.rs b/crates/blockifier/src/fee/receipt_test.rs index 3d37685f01..2b46e92d9a 100644 --- a/crates/blockifier/src/fee/receipt_test.rs +++ b/crates/blockifier/src/fee/receipt_test.rs @@ -340,8 +340,6 @@ fn test_calculate_tx_gas_usage_basic<'a>( // Test that we exclude the fee token contract modification and adds the account’s balance change // in the state changes. -// TODO(Nimrod, 1/5/2024): Test regression w.r.t. all resources (including VM). (Only starknet -// resources are taken into account). #[rstest] fn test_calculate_tx_gas_usage( #[values(false, true)] use_kzg_da: bool, diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index af8e837997..efa7cdc67e 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -12,6 +12,7 @@ use std::collections::HashMap; use std::fs; use std::path::PathBuf; +use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use starknet_api::block::{GasPrice, NonzeroGasPrice}; use starknet_api::core::{ClassHash, ContractAddress, PatriciaKey}; @@ -403,3 +404,17 @@ pub fn gas_vector_from_vm_usage( ), } } + +pub fn get_vm_resource_usage() -> ExecutionResources { + ExecutionResources { + n_steps: 10000, + n_memory_holes: 0, + builtin_instance_counter: HashMap::from([ + (BuiltinName::pedersen, 10), + (BuiltinName::range_check, 24), + (BuiltinName::ecdsa, 1), + (BuiltinName::bitwise, 1), + (BuiltinName::poseidon, 1), + ]), + } +} diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index 9b846d190b..f31fc2781b 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -199,11 +199,12 @@ pub struct VersionedConstants { // See the struct's docstring for more details. pub os_constants: Arc, + // Fee related. + pub(crate) vm_resource_fee_cost: Arc, + // Resources. os_resources: Arc, - // Fee related. - vm_resource_fee_cost: Arc, // Just to make sure the value exists, but don't use the actual values. #[allow(dead_code)] gateway: serde::de::IgnoredAny,