diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 5dbba123a93..1ba2d9e7fcf 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -12,7 +12,7 @@ use crate::blockifier::transaction_executor::{ BLOCK_STATE_ACCESS_ERR, }; use crate::context::{BlockContext, TransactionContext}; -use crate::execution::call_info::CallInfo; +use crate::execution::call_info::{CallInfo, ChargedResources}; use crate::fee::fee_checks::PostValidationReport; use crate::fee::receipt::TransactionReceipt; use crate::state::cached_state::CachedState; @@ -124,6 +124,11 @@ impl StatefulValidator { limit_steps_by_resources, )?; + let charged_resources = match &validate_call_info { + Some(call_info) => &call_info.charged_resources, + None => &ChargedResources::default(), + }; + let tx_receipt = TransactionReceipt::from_account_tx( tx, &tx_context, @@ -133,7 +138,7 @@ impl StatefulValidator { .as_mut() .expect(BLOCK_STATE_ACCESS_ERR) .get_actual_state_changes()?, - &execution_resources, + charged_resources, CallInfo::summarize_many(validate_call_info.iter()), 0, ); diff --git a/crates/blockifier/src/execution/call_info.rs b/crates/blockifier/src/execution/call_info.rs index 42398e82fe6..e28f018d9e5 100644 --- a/crates/blockifier/src/execution/call_info.rs +++ b/crates/blockifier/src/execution/call_info.rs @@ -112,6 +112,26 @@ impl ChargedResources { } } +/// Returns the total gas_for_fee used in the given validate and execute calls. +pub fn gas_for_fee_from_call_infos( + validate: &Option, + execute: &Option, +) -> GasAmount { + let validate_gas_amount = validate + .as_ref() + .map(|call_info| call_info.charged_resources.gas_for_fee) + .unwrap_or(GasAmount(0)); + let execute_gas_amount = execute + .as_ref() + .map(|call_info| call_info.charged_resources.gas_for_fee) + .unwrap_or(GasAmount(0)); + validate_gas_amount.checked_add(execute_gas_amount).unwrap_or_else(|| { + panic!( + "Gas for fee overflowed: tried to add {execute_gas_amount} to \ + {validate_gas_amount}", + ) + }) +} /// Represents the full effects of executing an entry point, including the inner calls it invoked. #[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))] diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index 153062234f0..d08a57697c1 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -362,12 +362,17 @@ fn test_gas_computation_regression_test( 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 (sierra_gas, reverted_sierra_gas) = match gas_vector_computation_mode { + GasVectorComputationMode::NoL2Gas => (GasAmount(0), GasAmount(0)), + GasVectorComputationMode::All => (GasAmount(13), GasAmount(7)), + }; + let computation_resources = + ComputationResources { vm_resources, n_reverted_steps, sierra_gas, reverted_sierra_gas }; 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)), + GasVectorComputationMode::All => GasVector::from_l2_gas(GasAmount(1033354)), }; assert_eq!( actual_computation_resources_gas_vector, expected_computation_resources_gas_vector, @@ -393,12 +398,12 @@ fn test_gas_computation_regression_test( true => GasVector { l1_gas: GasAmount(21543), l1_data_gas: GasAmount(2720), - l2_gas: GasAmount(1120374), + l2_gas: GasAmount(1120394), }, false => GasVector { l1_gas: GasAmount(62834), l1_data_gas: GasAmount(0), - l2_gas: GasAmount(1120374), + l2_gas: GasAmount(1120394), }, }, }; diff --git a/crates/blockifier/src/fee/receipt.rs b/crates/blockifier/src/fee/receipt.rs index 05a403b0804..0188ba7de93 100644 --- a/crates/blockifier/src/fee/receipt.rs +++ b/crates/blockifier/src/fee/receipt.rs @@ -1,10 +1,9 @@ -use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use starknet_api::core::ContractAddress; -use starknet_api::execution_resources::GasVector; +use starknet_api::execution_resources::{GasAmount, GasVector}; use starknet_api::transaction::fields::Fee; use crate::context::TransactionContext; -use crate::execution::call_info::ExecutionSummary; +use crate::execution::call_info::{ChargedResources, ExecutionSummary}; use crate::fee::resources::{ ComputationResources, StarknetResources, @@ -30,7 +29,7 @@ struct TransactionReceiptParameters<'a> { sender_address: Option, l1_handler_payload_size: Option, execution_summary_without_fee_transfer: ExecutionSummary, - execution_resources: &'a ExecutionResources, + charged_resources: &'a ChargedResources, tx_type: TransactionType, reverted_steps: usize, } @@ -58,7 +57,7 @@ impl TransactionReceipt { sender_address, l1_handler_payload_size, execution_summary_without_fee_transfer, - execution_resources, + charged_resources, tx_type, reverted_steps, } = tx_receipt_params; @@ -72,7 +71,9 @@ impl TransactionReceipt { execution_summary_without_fee_transfer, ); - let cairo_resources = (execution_resources + // Transaction overhead ('additional') resources are computed in VM resources no matter what + // the tracked resources of the transaction are. + let cairo_resources = (&charged_resources.vm_resources + &tx_context.block_context.versioned_constants.get_additional_os_tx_resources( tx_type, &starknet_resources, @@ -85,6 +86,8 @@ impl TransactionReceipt { computation: ComputationResources { vm_resources: cairo_resources, n_reverted_steps: reverted_steps, + sierra_gas: charged_resources.gas_for_fee, + reverted_sierra_gas: GasAmount(0), // TODO(tzahi): compute value. }, }; @@ -113,8 +116,8 @@ impl TransactionReceipt { tx_context: &'a TransactionContext, l1_handler_payload_size: usize, execution_summary_without_fee_transfer: ExecutionSummary, + charged_resources: &'a ChargedResources, state_changes: &'a StateChanges, - execution_resources: &'a ExecutionResources, ) -> Self { Self::from_params(TransactionReceiptParameters { tx_context, @@ -125,7 +128,7 @@ impl TransactionReceipt { sender_address: None, // L1 handlers have no sender address. l1_handler_payload_size: Some(l1_handler_payload_size), execution_summary_without_fee_transfer, - execution_resources, + charged_resources, tx_type: TransactionType::L1Handler, reverted_steps: 0, }) @@ -136,7 +139,7 @@ impl TransactionReceipt { account_tx: &'a AccountTransaction, tx_context: &'a TransactionContext, state_changes: &'a StateChanges, - execution_resources: &'a ExecutionResources, + charged_resources: &'a ChargedResources, execution_summary_without_fee_transfer: ExecutionSummary, reverted_steps: usize, ) -> Self { @@ -149,7 +152,7 @@ impl TransactionReceipt { sender_address: Some(tx_context.tx_info.sender_address()), l1_handler_payload_size: None, execution_summary_without_fee_transfer, - execution_resources, + charged_resources, tx_type: account_tx.tx_type(), reverted_steps, }) diff --git a/crates/blockifier/src/fee/resources.rs b/crates/blockifier/src/fee/resources.rs index b92e50bf0ee..691effdebea 100644 --- a/crates/blockifier/src/fee/resources.rs +++ b/crates/blockifier/src/fee/resources.rs @@ -57,7 +57,8 @@ impl TransactionResources { pub struct ComputationResources { pub vm_resources: ExecutionResources, pub n_reverted_steps: usize, - // TODO(Tzahi): add sierra_gas here. + pub sierra_gas: GasAmount, + pub reverted_sierra_gas: GasAmount, } impl ComputationResources { @@ -66,12 +67,19 @@ impl ComputationResources { versioned_constants: &VersionedConstants, computation_mode: &GasVectorComputationMode, ) -> GasVector { - get_vm_resources_cost( + let vm_cost = get_vm_resources_cost( versioned_constants, &self.vm_resources, self.n_reverted_steps, computation_mode, - ) + ); + let sierra_gas_cost = GasVector::from_l2_gas(self.sierra_gas + self.reverted_sierra_gas); + vm_cost.checked_add(sierra_gas_cost).unwrap_or_else(|| { + panic!( + "Computation resources to gas vector overflowed: tried to add {sierra_gas_cost:?} \ + to {vm_cost:?}", + ) + }) } #[cfg(test)] diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 18152788d2b..1bf6e246a58 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -28,7 +28,7 @@ use starknet_types_core::felt::Felt; use crate::abi::abi_utils::selector_from_name; use crate::context::{BlockContext, TransactionContext}; -use crate::execution::call_info::CallInfo; +use crate::execution::call_info::{gas_for_fee_from_call_infos, CallInfo, ChargedResources}; use crate::execution::contract_class::RunnableContractClass; use crate::execution::entry_point::{CallEntryPoint, CallType, EntryPointExecutionContext}; use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace; @@ -595,11 +595,13 @@ impl AccountTransaction { self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; } + let gas_for_fee = gas_for_fee_from_call_infos(&validate_call_info, &execute_call_info); + let tx_receipt = TransactionReceipt::from_account_tx( self, &tx_context, &state.get_actual_state_changes()?, - &resources, + &ChargedResources { vm_resources: resources, gas_for_fee }, CallInfo::summarize_many(validate_call_info.iter().chain(execute_call_info.iter())), 0, ); @@ -624,13 +626,13 @@ impl AccountTransaction { validate: bool, charge_fee: bool, ) -> TransactionExecutionResult { - let mut resources = ExecutionResources::default(); + let mut validate_resources = ExecutionResources::default(); let mut execution_context = EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee); // Run the validation, and if execution later fails, only keep the validation diff. let validate_call_info = self.handle_validate_tx( state, - &mut resources, + &mut validate_resources, tx_context.clone(), remaining_gas, validate, @@ -647,9 +649,9 @@ impl AccountTransaction { // resource and fee calculation. let validate_state_changes = state.get_actual_state_changes()?; - // Create copies of state and resources for the execution. + // Create copies of state and validate_resources for the execution. // Both will be rolled back if the execution is reverted or committed upon success. - let mut execution_resources = resources.clone(); + let mut execution_resources = validate_resources.clone(); let mut execution_state = TransactionalState::create_transactional(state); let execution_result = self.run_execute( @@ -658,15 +660,20 @@ impl AccountTransaction { &mut execution_context, remaining_gas, ); + let validate_charged_resources = match &validate_call_info { + Some(call_info) => &call_info.charged_resources, + None => &ChargedResources::default(), + }; // Pre-compute cost in case of revert. + // TODO(tzahi): add reverted_l2_gas to the receipt. let execution_steps_consumed = n_allotted_execution_steps - execution_context.n_remaining_steps(); - let revert_cost = TransactionReceipt::from_account_tx( + let revert_receipt = TransactionReceipt::from_account_tx( self, &tx_context, &validate_state_changes, - &resources, + validate_charged_resources, CallInfo::summarize_many(validate_call_info.iter()), execution_steps_consumed, ); @@ -682,7 +689,13 @@ impl AccountTransaction { validate_state_changes, execution_state.get_actual_state_changes()?, ]), - &execution_resources, + &ChargedResources { + vm_resources: execution_resources, + gas_for_fee: gas_for_fee_from_call_infos( + &validate_call_info, + &execute_call_info, + ), + }, CallInfo::summarize_many( validate_call_info.iter().chain(execute_call_info.iter()), ), @@ -707,7 +720,7 @@ impl AccountTransaction { post_execution_error.to_string(), TransactionReceipt { fee: post_execution_report.recommended_fee(), - ..revert_cost + ..revert_receipt }, )) } @@ -726,13 +739,13 @@ impl AccountTransaction { // Error during execution. Revert, even if the error is sequencer-related. execution_state.abort(); let post_execution_report = - PostExecutionReport::new(state, &tx_context, &revert_cost, charge_fee)?; + PostExecutionReport::new(state, &tx_context, &revert_receipt, charge_fee)?; Ok(ValidateExecuteCallInfo::new_reverted( validate_call_info, execution_error.to_string(), TransactionReceipt { fee: post_execution_report.recommended_fee(), - ..revert_cost + ..revert_receipt }, )) } diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index e077370b86c..e47974c7205 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -15,7 +15,7 @@ use starknet_api::transaction::{Transaction as StarknetApiTransaction, Transacti use crate::bouncer::verify_tx_weights_within_max_capacity; use crate::context::BlockContext; -use crate::execution::call_info::CallInfo; +use crate::execution::call_info::{gas_for_fee_from_call_infos, CallInfo, ChargedResources}; use crate::execution::entry_point::EntryPointExecutionContext; use crate::fee::receipt::TransactionReceipt; use crate::state::cached_state::TransactionalState; @@ -153,7 +153,7 @@ impl ExecutableTransaction for L1HandlerTransaction { let execute_call_info = self.run_execute(state, &mut execution_resources, &mut context, &mut remaining_gas)?; let l1_handler_payload_size = self.payload_size(); - + let gas_for_fee = gas_for_fee_from_call_infos(&None, &execute_call_info); let TransactionReceipt { fee: actual_fee, da_gas, @@ -163,8 +163,8 @@ impl ExecutableTransaction for L1HandlerTransaction { &tx_context, l1_handler_payload_size, CallInfo::summarize_many(execute_call_info.iter()), + &ChargedResources { vm_resources: execution_resources, gas_for_fee }, &state.get_actual_state_changes()?, - &execution_resources, ); let paid_fee = self.paid_fee_on_l1;