diff --git a/src/definitions/constants.rs b/src/definitions/constants.rs index c51bb6080..d72e8ccd7 100644 --- a/src/definitions/constants.rs +++ b/src/definitions/constants.rs @@ -22,6 +22,10 @@ lazy_static! { pub(crate) const LOG_MSG_TO_L1_ENCODED_DATA_SIZE: usize = (L2_TO_L1_MSG_HEADER_SIZE + 1) - LOG_MSG_TO_L1_N_TOPICS; +/// Fee factor used for the final fee calculation: +/// actual_fee = min(max_fee, consumed_resources) * FEE_FACTOR +pub(crate) const FEE_FACTOR: u128 = 1; + /// The (empirical) L1 gas cost of each Cairo step. pub(crate) const N_STEPS_FEE_WEIGHT: f64 = 0.01; diff --git a/src/lib.rs b/src/lib.rs index 4e7e06b32..778cbf56e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -287,7 +287,7 @@ mod test { let transaction = Transaction::InvokeFunction(invoke_function); let estimated_fee = estimate_fee(&[transaction], state, &block_context).unwrap(); - assert_eq!(estimated_fee[0], (12, 0)); + assert_eq!(estimated_fee[0], (30, 0)); } #[test] @@ -375,15 +375,10 @@ mod test { state.set_contract_classes(contract_classes).unwrap(); let mut block_context = BlockContext::default(); - block_context.cairo_resource_fee_weights = HashMap::from([ - (String::from("l1_gas_usage"), 0.into()), - (String::from("pedersen_builtin"), 16.into()), - (String::from("range_check_builtin"), 70.into()), - ]); block_context.starknet_os_config.gas_price = 1; let estimated_fee = estimate_message_fee(&l1_handler, state, &block_context).unwrap(); - assert_eq!(estimated_fee, (20081, 18471)); + assert_eq!(estimated_fee, (18484, 18471)); } #[test] @@ -921,11 +916,6 @@ mod test { .unwrap(); let mut block_context = BlockContext::default(); - block_context.cairo_resource_fee_weights = HashMap::from([ - (String::from("l1_gas_usage"), 0.into()), - (String::from("pedersen_builtin"), 16.into()), - (String::from("range_check_builtin"), 70.into()), - ]); block_context.starknet_os_config.gas_price = 1; simulate_transaction( diff --git a/src/testing/state.rs b/src/testing/state.rs index dacde9d66..1d8b16097 100644 --- a/src/testing/state.rs +++ b/src/testing/state.rs @@ -358,6 +358,7 @@ mod tests { let mut actual_resources = HashMap::new(); actual_resources.insert("l1_gas_usage".to_string(), 1224); + actual_resources.insert("n_steps".to_string(), 0); let transaction_exec_info = TransactionExecutionInfo { validate_info: None, @@ -572,6 +573,7 @@ mod tests { ) .unwrap(); let actual_resources = HashMap::from([ + ("n_steps".to_string(), 2933), ("l1_gas_usage".to_string(), 0), ("range_check_builtin".to_string(), 70), ("pedersen_builtin".to_string(), 16), diff --git a/src/transaction/declare.rs b/src/transaction/declare.rs index e9a73ea5d..cc832e071 100644 --- a/src/transaction/declare.rs +++ b/src/transaction/declare.rs @@ -18,10 +18,7 @@ use crate::{ services::api::contract_classes::deprecated_contract_class::ContractClass, state::state_api::{State, StateReader}, state::ExecutionResourcesManager, - transaction::{ - error::TransactionError, - fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo}, - }, + transaction::error::TransactionError, utils::{ calculate_tx_resources, felt_to_hash, verify_no_calls_to_other_contracts, Address, ClassHash, @@ -29,8 +26,8 @@ use crate::{ }; use cairo_vm::felt::Felt252; use num_traits::Zero; -use std::collections::HashMap; +use super::fee::charge_fee; use super::{verify_version, Transaction}; // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -176,6 +173,7 @@ impl Declare { TransactionType::Declare, changes, None, + 0, ) .map_err(|_| TransactionError::ResourcesCalculation)?; @@ -243,38 +241,6 @@ impl Declare { Ok(Some(call_info)) } - /// Calculates and charges the actual fee. - pub fn charge_fee( - &self, - state: &mut CachedState, - resources: &HashMap, - block_context: &BlockContext, - ) -> Result { - if self.max_fee.is_zero() { - return Ok((None, 0)); - } - - let actual_fee = calculate_tx_fee( - resources, - block_context.starknet_os_config.gas_price, - block_context, - )?; - - let mut tx_execution_context = - self.get_execution_context(block_context.invoke_tx_max_n_steps); - let fee_transfer_info = if self.skip_fee_transfer { - None - } else { - Some(execute_fee_transfer( - state, - block_context, - &mut tx_execution_context, - actual_fee, - )?) - }; - Ok((fee_transfer_info, actual_fee)) - } - fn handle_nonce(&self, state: &mut S) -> Result<(), TransactionError> { if self.version.is_zero() || self.version == *QUERY_VERSION_BASE { return Ok(()); @@ -304,8 +270,16 @@ impl Declare { let mut tx_exec_info = self.apply(state, block_context)?; self.handle_nonce(state)?; - let (fee_transfer_info, actual_fee) = - self.charge_fee(state, &tx_exec_info.actual_resources, block_context)?; + let mut tx_execution_context = + self.get_execution_context(block_context.invoke_tx_max_n_steps); + let (fee_transfer_info, actual_fee) = charge_fee( + state, + &tx_exec_info.actual_resources, + block_context, + self.max_fee, + &mut tx_execution_context, + self.skip_fee_transfer, + )?; state.set_contract_class(&self.class_hash, &self.contract_class)?; @@ -444,6 +418,7 @@ mod tests { }); let actual_resources = HashMap::from([ + ("n_steps".to_string(), 2348), ("l1_gas_usage".to_string(), 0), ("range_check_builtin".to_string(), 57), ("pedersen_builtin".to_string(), 15), diff --git a/src/transaction/declare_v2.rs b/src/transaction/declare_v2.rs index 3ab61e2be..88594ef30 100644 --- a/src/transaction/declare_v2.rs +++ b/src/transaction/declare_v2.rs @@ -1,3 +1,4 @@ +use super::fee::charge_fee; use super::{verify_version, Transaction}; use crate::core::contract_address::{compute_casm_class_hash, compute_sierra_class_hash}; use crate::definitions::constants::QUERY_VERSION_BASE; @@ -13,23 +14,18 @@ use crate::{ transaction_type::TransactionType, }, execution::{ - execution_entry_point::ExecutionEntryPoint, CallInfo, CallType, - TransactionExecutionContext, TransactionExecutionInfo, + execution_entry_point::ExecutionEntryPoint, CallType, TransactionExecutionContext, + TransactionExecutionInfo, }, state::state_api::{State, StateReader}, state::ExecutionResourcesManager, - transaction::{ - error::TransactionError, - fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo}, - invoke_function::verify_no_calls_to_other_contracts, - }, + transaction::{error::TransactionError, invoke_function::verify_no_calls_to_other_contracts}, utils::{calculate_tx_resources, Address}, }; use cairo_lang_starknet::casm_contract_class::CasmContractClass; use cairo_lang_starknet::contract_class::ContractClass as SierraContractClass; use cairo_vm::felt::Felt252; use num_traits::Zero; -use std::collections::HashMap; /// Represents a declare transaction in the starknet network. /// Declare creates a blueprint of a contract class that is used to deploy instances of the contract @@ -274,43 +270,6 @@ impl DeclareV2 { Vec::from([bytes]) } - /// Calculates and charges the actual fee. - /// ## Parameter: - /// - state: An state that implements the State and StateReader traits. - /// - resources: the resources that are in use by the contract - /// - block_context: The block that contains the execution context - pub fn charge_fee( - &self, - state: &mut CachedState, - resources: &HashMap, - block_context: &BlockContext, - ) -> Result { - if self.max_fee.is_zero() { - return Ok((None, 0)); - } - - let actual_fee = calculate_tx_fee( - resources, - block_context.starknet_os_config.gas_price, - block_context, - )?; - - let mut tx_execution_context = - self.get_execution_context(block_context.invoke_tx_max_n_steps); - let fee_transfer_info = if self.skip_fee_transfer { - None - } else { - Some(execute_fee_transfer( - state, - block_context, - &mut tx_execution_context, - actual_fee, - )?) - }; - - Ok((fee_transfer_info, actual_fee)) - } - // TODO: delete once used #[allow(dead_code)] fn handle_nonce(&self, state: &mut S) -> Result<(), TransactionError> { @@ -347,8 +306,8 @@ impl DeclareV2 { let mut resources_manager = ExecutionResourcesManager::default(); - let (validate_info, _remaining_gas) = if self.skip_validate { - (None, 0) + let (execution_result, _remaining_gas) = if self.skip_validate { + (ExecutionResult::default(), 0) } else { let (info, gas) = self.run_validate_entrypoint( initial_gas, @@ -356,24 +315,33 @@ impl DeclareV2 { &mut resources_manager, block_context, )?; - (Some(info), gas) + (info, gas) }; let storage_changes = state.count_actual_storage_changes(); let actual_resources = calculate_tx_resources( resources_manager, - &[validate_info.clone()], + &[execution_result.call_info.clone()], self.tx_type, storage_changes, None, + execution_result.n_reverted_steps, )?; - let (fee_transfer_info, actual_fee) = - self.charge_fee(state, &actual_resources, block_context)?; + let mut tx_execution_context = + self.get_execution_context(block_context.invoke_tx_max_n_steps); + let (fee_transfer_info, actual_fee) = charge_fee( + state, + &actual_resources, + block_context, + self.max_fee, + &mut tx_execution_context, + self.skip_fee_transfer, + )?; self.compile_and_store_casm_class(state)?; let mut tx_exec_info = TransactionExecutionInfo::new_without_fee_info( - validate_info, + execution_result.call_info, None, None, actual_resources, @@ -415,7 +383,7 @@ impl DeclareV2 { state: &mut CachedState, resources_manager: &mut ExecutionResourcesManager, block_context: &BlockContext, - ) -> Result<(CallInfo, u128), TransactionError> { + ) -> Result<(ExecutionResult, u128), TransactionError> { let calldata = [self.compiled_class_hash.clone()].to_vec(); let entry_point = ExecutionEntryPoint { @@ -433,7 +401,7 @@ impl DeclareV2 { let mut tx_execution_context = self.get_execution_context(block_context.validate_max_n_steps); - let ExecutionResult { call_info, .. } = if self.skip_execute { + let execution_result = if self.skip_execute { ExecutionResult::default() } else { entry_point.execute( @@ -445,10 +413,13 @@ impl DeclareV2 { block_context.validate_max_n_steps, )? }; - let call_info = verify_no_calls_to_other_contracts(&call_info)?; - remaining_gas -= call_info.gas_consumed; - Ok((call_info, remaining_gas)) + if execution_result.call_info.is_some() { + verify_no_calls_to_other_contracts(&execution_result.call_info)?; + remaining_gas -= execution_result.call_info.clone().unwrap().gas_consumed; + } + + Ok((execution_result, remaining_gas)) } // --------------- diff --git a/src/transaction/deploy.rs b/src/transaction/deploy.rs index 4d5fd1a56..35b0ef779 100644 --- a/src/transaction/deploy.rs +++ b/src/transaction/deploy.rs @@ -185,6 +185,7 @@ impl Deploy { self.tx_type, changes, None, + 0, )?; Ok(TransactionExecutionInfo::new_without_fee_info( @@ -230,7 +231,7 @@ impl Deploy { let ExecutionResult { call_info, revert_error, - .. + n_reverted_steps, } = call.execute( state, block_context, @@ -247,6 +248,7 @@ impl Deploy { self.tx_type, changes, None, + n_reverted_steps, )?; Ok(TransactionExecutionInfo::new_without_fee_info( diff --git a/src/transaction/deploy_account.rs b/src/transaction/deploy_account.rs index 7b161bf87..d182589aa 100644 --- a/src/transaction/deploy_account.rs +++ b/src/transaction/deploy_account.rs @@ -1,3 +1,4 @@ +use super::fee::charge_fee; use super::{invoke_function::verify_no_calls_to_other_contracts, Transaction}; use crate::definitions::constants::QUERY_VERSION_BASE; use crate::execution::execution_entry_point::ExecutionResult; @@ -27,16 +28,12 @@ use crate::{ state::state_api::{State, StateReader}, state::ExecutionResourcesManager, syscalls::syscall_handler_errors::SyscallHandlerError, - transaction::{ - error::TransactionError, - fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo}, - }, + transaction::error::TransactionError, utils::{calculate_tx_resources, Address, ClassHash}, }; use cairo_vm::felt::Felt252; use getset::Getters; use num_traits::Zero; -use std::collections::HashMap; #[derive(Clone, Debug, PartialEq, Eq)] pub struct StateSelector { @@ -161,8 +158,18 @@ impl DeployAccount { let mut tx_info = self.apply(state, block_context)?; self.handle_nonce(state)?; - let (fee_transfer_info, actual_fee) = - self.charge_fee(state, &tx_info.actual_resources, block_context)?; + + let mut tx_execution_context = + self.get_execution_context(block_context.invoke_tx_max_n_steps); + let (fee_transfer_info, actual_fee) = charge_fee( + state, + &tx_info.actual_resources, + block_context, + self.max_fee, + &mut tx_execution_context, + self.skip_fee_transfer, + )?; + tx_info.set_fee_info(actual_fee, fee_transfer_info); Ok(tx_info) @@ -209,6 +216,7 @@ impl DeployAccount { TransactionType::DeployAccount, state.count_actual_storage_changes(), None, + 0, ) .map_err::(|_| TransactionError::ResourcesCalculation)?; @@ -354,38 +362,6 @@ impl DeployAccount { Ok(call_info) } - fn charge_fee( - &self, - state: &mut CachedState, - resources: &HashMap, - block_context: &BlockContext, - ) -> Result { - if self.max_fee.is_zero() { - return Ok((None, 0)); - } - - let actual_fee = calculate_tx_fee( - resources, - block_context.starknet_os_config.gas_price, - block_context, - )?; - - let mut tx_execution_context = - self.get_execution_context(block_context.invoke_tx_max_n_steps); - let fee_transfer_info = if self.skip_fee_transfer { - None - } else { - Some(execute_fee_transfer( - state, - block_context, - &mut tx_execution_context, - actual_fee, - )?) - }; - - Ok((fee_transfer_info, actual_fee)) - } - pub(crate) fn create_for_simulation( &self, skip_validate: bool, diff --git a/src/transaction/fee.rs b/src/transaction/fee.rs index a42e5c8ce..ef1b4de93 100644 --- a/src/transaction/fee.rs +++ b/src/transaction/fee.rs @@ -1,4 +1,5 @@ use super::error::TransactionError; +use crate::definitions::constants::FEE_FACTOR; use crate::execution::execution_entry_point::ExecutionResult; use crate::services::api::contract_classes::deprecated_contract_class::EntryPointType; use crate::state::cached_state::CachedState; @@ -14,7 +15,8 @@ use crate::{ state::ExecutionResourcesManager, }; use cairo_vm::felt::Felt252; -use num_traits::ToPrimitive; +use num_traits::{ToPrimitive, Zero}; +use std::cmp::min; use std::collections::HashMap; // second element is the actual fee that the transaction uses @@ -122,3 +124,45 @@ fn max_of_keys(cairo_rsc: &HashMap, weights: &HashMap( + state: &mut CachedState, + resources: &HashMap, + block_context: &BlockContext, + max_fee: u128, + tx_execution_context: &mut TransactionExecutionContext, + skip_fee_transfer: bool, +) -> Result { + if max_fee.is_zero() { + return Ok((None, 0)); + } + + let actual_fee = calculate_tx_fee( + resources, + block_context.starknet_os_config.gas_price, + block_context, + )?; + let actual_fee = min(actual_fee, max_fee) * FEE_FACTOR; + + let fee_transfer_info = if skip_fee_transfer { + None + } else { + Some(execute_fee_transfer( + state, + block_context, + tx_execution_context, + actual_fee, + )?) + }; + Ok((fee_transfer_info, actual_fee)) +} diff --git a/src/transaction/invoke_function.rs b/src/transaction/invoke_function.rs index bf5b44a3e..3703fef7d 100644 --- a/src/transaction/invoke_function.rs +++ b/src/transaction/invoke_function.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use crate::{ core::transaction_hash::{calculate_transaction_hash_common, TransactionHashPrefix}, definitions::{ @@ -15,10 +13,7 @@ use crate::{ }, state::state_api::{State, StateReader}, state::{cached_state::CachedState, ExecutionResourcesManager}, - transaction::{ - error::TransactionError, - fee::{calculate_tx_fee, execute_fee_transfer, FeeInfo}, - }, + transaction::error::TransactionError, utils::{calculate_tx_resources, Address}, }; @@ -27,7 +22,7 @@ use cairo_vm::felt::Felt252; use getset::Getters; use num_traits::Zero; -use super::Transaction; +use super::{fee::charge_fee, Transaction}; /// Represents an InvokeFunction transaction in the starknet network. #[derive(Debug, Getters, Clone)] @@ -237,7 +232,7 @@ impl InvokeFunction { let ExecutionResult { call_info, revert_error, - .. + n_reverted_steps, } = if self.skip_execute { ExecutionResult::default() } else { @@ -255,6 +250,7 @@ impl InvokeFunction { self.tx_type, changes, None, + n_reverted_steps, )?; let transaction_execution_info = TransactionExecutionInfo::new_without_fee_info( validate_info, @@ -266,37 +262,6 @@ impl InvokeFunction { Ok(transaction_execution_info) } - fn charge_fee( - &self, - state: &mut CachedState, - resources: &HashMap, - block_context: &BlockContext, - ) -> Result { - if self.max_fee.is_zero() { - return Ok((None, 0)); - } - let actual_fee = calculate_tx_fee( - resources, - block_context.starknet_os_config.gas_price, - block_context, - )?; - - let mut tx_execution_context = - self.get_execution_context(block_context.invoke_tx_max_n_steps)?; - let fee_transfer_info = if self.skip_fee_transfer { - None - } else { - Some(execute_fee_transfer( - state, - block_context, - &mut tx_execution_context, - actual_fee, - )?) - }; - - Ok((fee_transfer_info, actual_fee)) - } - /// Calculates actual fee used by the transaction using the execution info returned by apply(), /// then updates the transaction execution info with the data of the fee. /// ## Parameters @@ -312,8 +277,17 @@ impl InvokeFunction { let mut tx_exec_info = self.apply(state, block_context, remaining_gas)?; self.handle_nonce(state)?; - let (fee_transfer_info, actual_fee) = - self.charge_fee(state, &tx_exec_info.actual_resources, block_context)?; + let mut tx_execution_context = + self.get_execution_context(block_context.invoke_tx_max_n_steps)?; + let (fee_transfer_info, actual_fee) = charge_fee( + state, + &tx_exec_info.actual_resources, + block_context, + self.max_fee, + &mut tx_execution_context, + self.skip_fee_transfer, + )?; + tx_exec_info.set_fee_info(actual_fee, fee_transfer_info); Ok(tx_exec_info) @@ -417,7 +391,9 @@ mod tests { use crate::{ services::api::contract_classes::deprecated_contract_class::ContractClass, state::cached_state::CachedState, state::in_memory_state_reader::InMemoryStateReader, + utils::calculate_sn_keccak, }; + use cairo_lang_starknet::casm_contract_class::CasmContractClass; use num_traits::Num; use std::{collections::HashMap, sync::Arc}; @@ -781,12 +757,7 @@ mod tests { .set_contract_class(&class_hash, &contract_class) .unwrap(); - let mut block_context = BlockContext::default(); - block_context.cairo_resource_fee_weights = HashMap::from([ - (String::from("l1_gas_usage"), 0.into()), - (String::from("pedersen_builtin"), 16.into()), - (String::from("range_check_builtin"), 70.into()), - ]); + let block_context = BlockContext::default(); let result = internal_invoke_function.execute(&mut state, &block_context, 0); assert!(result.is_err()); @@ -794,7 +765,8 @@ mod tests { } #[test] - fn test_execute_invoke_actual_fee_exceeded_max_fee_should_fail() { + fn test_execute_invoke_actual_fee_exceeded_max_fee_should_charge_max_fee() { + let max_fee = 5; let internal_invoke_function = InvokeFunction { contract_address: Address(0.into()), entry_point_selector: Felt252::from_str_radix( @@ -809,11 +781,11 @@ mod tests { validate_entry_point_selector: 0.into(), hash_value: 0.into(), signature: Vec::new(), - max_fee: 1000, + max_fee, nonce: Some(0.into()), skip_validation: false, skip_execute: false, - skip_fee_transfer: false, + skip_fee_transfer: true, }; // Instantiate CachedState @@ -842,19 +814,12 @@ mod tests { .unwrap(); let mut block_context = BlockContext::default(); - block_context.cairo_resource_fee_weights = HashMap::from([ - (String::from("l1_gas_usage"), 0.into()), - (String::from("pedersen_builtin"), 16.into()), - (String::from("range_check_builtin"), 70.into()), - ]); block_context.starknet_os_config.gas_price = 1; - let error = internal_invoke_function.execute(&mut state, &block_context, 0); - assert!(error.is_err()); - assert_matches!( - error.unwrap_err(), - TransactionError::ActualFeeExceedsMaxFee(_, _) - ); + let tx = internal_invoke_function + .execute(&mut state, &block_context, 0) + .unwrap(); + assert_eq!(tx.actual_fee, max_fee); } #[test] @@ -1062,4 +1027,84 @@ mod tests { ); assert!(expected_error.is_err()); } + + #[test] + fn test_reverted_transaction_wrong_entry_point() { + let internal_invoke_function = InvokeFunction { + contract_address: Address(0.into()), + entry_point_selector: Felt252::from_bytes_be(&calculate_sn_keccak( + "factorial_".as_bytes(), + )), + entry_point_type: EntryPointType::External, + calldata: vec![], + tx_type: TransactionType::InvokeFunction, + version: 0.into(), + validate_entry_point_selector: 0.into(), + hash_value: 0.into(), + signature: Vec::new(), + max_fee: 0, + nonce: Some(0.into()), + skip_validation: true, + skip_execute: false, + skip_fee_transfer: true, + }; + + let mut state_reader = InMemoryStateReader::default(); + let class_hash = [1; 32]; + let program_data = include_bytes!("../../starknet_programs/cairo1/factorial.casm"); + let contract_class: CasmContractClass = serde_json::from_slice(program_data).unwrap(); + let contract_address = Address(0.into()); + let nonce = Felt252::zero(); + + state_reader + .address_to_class_hash_mut() + .insert(contract_address.clone(), class_hash); + state_reader + .address_to_nonce + .insert(contract_address, nonce); + + let mut casm_contract_class_cache = HashMap::new(); + + casm_contract_class_cache.insert(class_hash, contract_class); + + let mut state = CachedState::new( + Arc::new(state_reader), + None, + Some(casm_contract_class_cache), + ); + + let state_before_execution = state.clone(); + + let result = internal_invoke_function + .execute(&mut state, &BlockContext::default(), 0) + .unwrap(); + + assert!(result.call_info.is_none()); + assert_eq!( + result.revert_error, + Some("Requested entry point was not found".to_string()) + ); + assert_eq!( + state.cache.class_hash_writes, + state_before_execution.cache.class_hash_writes + ); + assert_eq!( + state.cache.compiled_class_hash_writes, + state_before_execution.cache.compiled_class_hash_writes + ); + assert_eq!( + state.cache.nonce_writes, + state_before_execution.cache.nonce_writes + ); + assert_eq!( + state.cache.storage_writes, + state_before_execution.cache.storage_writes + ); + assert_eq!( + state.cache.class_hash_to_compiled_class_hash, + state_before_execution + .cache + .class_hash_to_compiled_class_hash + ); + } } diff --git a/src/transaction/l1_handler.rs b/src/transaction/l1_handler.rs index f6f8b0ad2..57e34d67a 100644 --- a/src/transaction/l1_handler.rs +++ b/src/transaction/l1_handler.rs @@ -114,7 +114,7 @@ impl L1Handler { let ExecutionResult { call_info, revert_error, - .. + n_reverted_steps, } = if self.skip_execute { ExecutionResult::default() } else { @@ -135,6 +135,7 @@ impl L1Handler { TransactionType::L1Handler, changes, Some(self.get_payload_size()), + n_reverted_steps, )?; // Enforce L1 fees. @@ -275,11 +276,6 @@ mod test { .unwrap(); let mut block_context = BlockContext::default(); - block_context.cairo_resource_fee_weights = HashMap::from([ - (String::from("l1_gas_usage"), 0.into()), - (String::from("pedersen_builtin"), 16.into()), - (String::from("range_check_builtin"), 70.into()), - ]); block_context.starknet_os_config.gas_price = 1; let tx_exec = l1_handler @@ -335,6 +331,7 @@ mod test { fee_transfer_info: None, actual_fee: 0, actual_resources: HashMap::from([ + ("n_steps".to_string(), 1229), ("pedersen_builtin".to_string(), 13), ("range_check_builtin".to_string(), 23), ("l1_gas_usage".to_string(), 18471), diff --git a/src/utils.rs b/src/utils.rs index f43218b28..5c6272740 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -11,6 +11,7 @@ use crate::{ syscalls::syscall_handler_errors::SyscallHandlerError, transaction::error::TransactionError, }; +use cairo_vm::vm::runners::builtin_runner::SEGMENT_ARENA_BUILTIN_NAME; use cairo_vm::{ felt::Felt252, serde::deserialize_program::BuiltinName, vm::runners::builtin_runner, }; @@ -166,6 +167,7 @@ pub fn calculate_tx_resources( tx_type: TransactionType, storage_changes: (usize, usize), l1_handler_payload_size: Option, + n_reverted_steps: usize, ) -> Result, TransactionError> { let (n_modified_contracts, n_storage_changes) = storage_changes; @@ -192,10 +194,21 @@ pub fn calculate_tx_resources( // Add additional Cairo resources needed for the OS to run the transaction. let additional_resources = get_additional_os_resources(tx_syscall_counter, &tx_type)?; let new_resources = &cairo_usage + &additional_resources; - let filtered_builtins = new_resources.filter_unused_builtins(); + let mut filtered_builtins = new_resources.filter_unused_builtins(); + + let n_steps = new_resources.n_steps + + n_reverted_steps + + 10 * filtered_builtins + .builtin_instance_counter + .remove(SEGMENT_ARENA_BUILTIN_NAME) + .unwrap_or(0); let mut resources: HashMap = HashMap::new(); resources.insert("l1_gas_usage".to_string(), l1_gas_usage); + resources.insert( + "n_steps".to_string(), + n_steps + filtered_builtins.n_memory_holes, + ); for (builtin, value) in filtered_builtins.builtin_instance_counter { resources.insert(builtin, value); } diff --git a/tests/deploy_account.rs b/tests/deploy_account.rs index f6551ead2..8a3dedd4e 100644 --- a/tests/deploy_account.rs +++ b/tests/deploy_account.rs @@ -93,6 +93,7 @@ fn internal_deploy_account() { None, 0, [ + ("n_steps", 3098), ("pedersen_builtin", 23), ("range_check_builtin", 74), ("l1_gas_usage", 1224) @@ -159,6 +160,16 @@ fn internal_deploy_account_cairo1() { ]; let keys: HashSet<[u8; 32]> = [accessed_keys].iter().copied().collect(); + let n_steps; + #[cfg(not(feature = "cairo_1_tests"))] + { + n_steps = 3359; + } + #[cfg(feature = "cairo_1_tests")] + { + n_steps = 3363; + } + assert_eq!( tx_info, TransactionExecutionInfo::new( @@ -242,6 +253,7 @@ fn internal_deploy_account_cairo1() { None, 0, [ + ("n_steps", n_steps), ("pedersen_builtin", 23), ("range_check_builtin", 78), ("l1_gas_usage", 3672) diff --git a/tests/internals.rs b/tests/internals.rs index 786c6cc07..d24129a95 100644 --- a/tests/internals.rs +++ b/tests/internals.rs @@ -652,9 +652,9 @@ fn expected_fib_fee_transfer_info(fee: u128) -> CallInfo { ], }], storage_read_values: vec![ - INITIAL_BALANCE.clone() - Felt252::from(10), + INITIAL_BALANCE.clone() - Felt252::from(24), Felt252::zero(), - Felt252::from(10), + Felt252::from(24), Felt252::zero(), ], accessed_storage_keys: HashSet::from([ @@ -825,6 +825,7 @@ fn test_declare_tx() { assert!(state.get_contract_class(&declare_tx.class_hash).is_ok()); let resources = HashMap::from([ + ("n_steps".to_string(), 2348), ("range_check_builtin".to_string(), 57), ("pedersen_builtin".to_string(), 15), ("l1_gas_usage".to_string(), 0), @@ -873,6 +874,7 @@ fn test_declarev2_tx() { .is_ok()); let resources = HashMap::from([ + ("n_steps".to_string(), 2348), ("range_check_builtin".to_string(), 57), ("pedersen_builtin".to_string(), 15), ("l1_gas_usage".to_string(), 0), @@ -1091,6 +1093,7 @@ fn expected_fib_validate_call_info_2() -> CallInfo { fn expected_transaction_execution_info(block_context: &BlockContext) -> TransactionExecutionInfo { let resources = HashMap::from([ + ("n_steps".to_string(), 2921), ("pedersen_builtin".to_string(), 16), ("l1_gas_usage".to_string(), 0), ("range_check_builtin".to_string(), 72), @@ -1110,7 +1113,17 @@ fn expected_transaction_execution_info(block_context: &BlockContext) -> Transact fn expected_fib_transaction_execution_info( block_context: &BlockContext, ) -> TransactionExecutionInfo { + let n_steps; + #[cfg(not(feature = "cairo_1_tests"))] + { + n_steps = 3017; + } + #[cfg(feature = "cairo_1_tests")] + { + n_steps = 3020; + } let resources = HashMap::from([ + ("n_steps".to_string(), n_steps), ("l1_gas_usage".to_string(), 4896), ("pedersen_builtin".to_string(), 16), ("range_check_builtin".to_string(), 75), @@ -1287,6 +1300,7 @@ fn test_deploy_account() { ); let resources = HashMap::from([ + ("n_steps".to_string(), 3111), ("range_check_builtin".to_string(), 74), ("pedersen_builtin".to_string(), 23), ("l1_gas_usage".to_string(), 3672), @@ -1294,14 +1308,14 @@ fn test_deploy_account() { let fee = calculate_tx_fee(&resources, *GAS_PRICE, &block_context).unwrap(); - assert_eq!(fee, expected_fee); + assert_eq!(fee, 3704); let expected_execution_info = TransactionExecutionInfo::new( expected_validate_call_info.into(), expected_execute_call_info.into(), None, expected_fee_transfer_call_info.into(), - fee, + expected_fee, // Entry **not** in blockifier. // Default::default(), resources, @@ -1316,7 +1330,7 @@ fn test_deploy_account() { let hash = TEST_ERC20_DEPLOYED_ACCOUNT_BALANCE_KEY.to_be_bytes(); - validate_final_balances(&mut state, &block_context, &hash, fee); + validate_final_balances(&mut state, &block_context, &hash, expected_fee); let class_hash_from_state = state .get_class_hash_at(deploy_account_tx.contract_address()) @@ -1554,7 +1568,7 @@ fn test_state_for_declare_tx() { ]) ); - let fee = Felt252::from(10); + let fee = Felt252::from(24); // Check state.cache assert_eq!(