From 3e2d1b9cd6a259daf128ab86bb5d460382aa8dcf Mon Sep 17 00:00:00 2001 From: Gilad Chase Date: Mon, 22 Jan 2024 08:57:51 +0200 Subject: [PATCH] refactor: make `AccountTransactionContext` hold BlockContext - Rename `AccountTransactionContext` into `TransactionInfo`: i want to use `Context` for something else, and `Account` is a misnomer, since L1Transactions also generate this instance. - Create a new `AccountTransactionContext`, which holds `BlockContext` and `TransactionInfo`: This mirrors `BlockContext`, which holds `BlockInfo` as well as higher level contexts. - Remove all unnecessary `get_account_tx` calls. - Replace all functions that take both `block_context` and `tx_info` with a single `TransactionContext`. - Make `EntryPointExecutionContext` hold an `Arc` instead of both a block_context and `tx_info`: previously every entry point cloned the blockContext and generated a new tx_info, now they all share the same (identical) one. --- crates/blockifier/src/context.rs | 23 +- .../deprecated_syscalls_test.rs | 30 +- .../deprecated_syscalls/hint_processor.rs | 20 +- .../src/execution/deprecated_syscalls/mod.rs | 13 +- .../blockifier/src/execution/entry_point.rs | 64 ++--- .../src/execution/execution_utils.rs | 10 +- .../src/execution/syscalls/hint_processor.rs | 41 +-- .../blockifier/src/execution/syscalls/mod.rs | 3 +- .../src/execution/syscalls/syscalls_test.rs | 44 ++- crates/blockifier/src/fee/actual_cost.rs | 41 ++- crates/blockifier/src/fee/fee_checks.rs | 72 ++--- crates/blockifier/src/fee/fee_utils.rs | 40 ++- crates/blockifier/src/test_utils/prices.rs | 5 +- .../blockifier/src/test_utils/struct_impls.rs | 26 +- .../src/transaction/account_transaction.rs | 235 ++++++---------- .../transaction/account_transactions_test.rs | 34 +-- crates/blockifier/src/transaction/objects.rs | 36 +-- .../src/transaction/post_execution_test.rs | 13 +- .../src/transaction/transaction_execution.rs | 22 +- .../src/transaction/transactions.rs | 264 +++++++++--------- .../src/transaction/transactions_test.rs | 40 +-- crates/native_blockifier/src/py_validator.rs | 31 +- .../src/transaction_executor.rs | 12 +- 23 files changed, 520 insertions(+), 599 deletions(-) diff --git a/crates/blockifier/src/context.rs b/crates/blockifier/src/context.rs index fc6695d12e..b65440ea7f 100644 --- a/crates/blockifier/src/context.rs +++ b/crates/blockifier/src/context.rs @@ -1,9 +1,15 @@ use starknet_api::core::{ChainId, ContractAddress}; use crate::block::BlockInfo; -use crate::transaction::objects::FeeType; +use crate::transaction::objects::{FeeType, TransactionInfo, TransactionInfoCreator}; use crate::versioned_constants::VersionedConstants; +#[derive(Clone, Debug)] +pub struct TransactionContext { + pub block_context: BlockContext, + pub tx_info: TransactionInfo, +} + #[derive(Clone, Debug)] pub struct BlockContext { pub block_info: BlockInfo, @@ -11,6 +17,18 @@ pub struct BlockContext { pub versioned_constants: VersionedConstants, } +impl BlockContext { + pub fn to_tx_context( + &self, + tx_info_creator: &impl TransactionInfoCreator, + ) -> TransactionContext { + TransactionContext { + block_context: self.clone(), + tx_info: tx_info_creator.create_tx_info(), + } + } +} + #[derive(Clone, Debug)] pub struct ChainInfo { pub chain_id: ChainId, @@ -18,6 +36,9 @@ pub struct ChainInfo { } impl ChainInfo { + // TODO(Gilad): since fee_type is comes from TransactionInfo, we can make move this method into + // TransactionContext, which has both the chain_info (through BlockContext) and the tx_info. + // That is, add to BlockContext with the signature `pub fn fee_token_address(&self)`. pub fn fee_token_address(&self, fee_type: &FeeType) -> ContractAddress { self.fee_token_addresses.get_by_fee_type(fee_type) } diff --git a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs index 9b65f887dd..06e2611ee5 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs @@ -33,7 +33,7 @@ use crate::test_utils::{ }; use crate::transaction::constants::QUERY_VERSION_BASE_BIT; use crate::transaction::objects::{ - AccountTransactionContext, CommonAccountFields, DeprecatedAccountTransactionContext, + CommonAccountFields, DeprecatedTransactionInfo, TransactionInfo, }; use crate::{check_entry_point_execution_error_for_custom_hint, retdata}; @@ -451,21 +451,19 @@ fn test_tx_info(#[case] only_query: bool) { calldata: expected_tx_info, ..trivial_external_entry_point() }; - let account_tx_context = - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields: CommonAccountFields { - transaction_hash: tx_hash, - version: TransactionVersion::ONE, - nonce, - sender_address, - only_query, - ..Default::default() - }, - max_fee, - }); - let result = entry_point_call - .execute_directly_given_account_context(&mut state, account_tx_context, true) - .unwrap(); + let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields: CommonAccountFields { + transaction_hash: tx_hash, + version: TransactionVersion::ONE, + nonce, + sender_address, + only_query, + ..Default::default() + }, + max_fee, + }); + let result = + entry_point_call.execute_directly_given_account_context(&mut state, tx_info, true).unwrap(); assert!(!result.execution.failed) } diff --git a/crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs b/crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs index 936a1ede6e..8b35f06240 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs @@ -25,6 +25,7 @@ use starknet_api::StarknetApiError; use thiserror::Error; use crate::abi::constants; +use crate::context::TransactionContext; use crate::execution::call_info::{CallInfo, OrderedEvent, OrderedL2ToL1Message}; use crate::execution::common_hints::{ extended_builtin_hint_processor, ExecutionMode, HintExecutionResult, @@ -295,7 +296,7 @@ impl<'a> DeprecatedSyscallHintProcessor<'a> { &mut self, vm: &mut VirtualMachine, ) -> DeprecatedSyscallResult { - let signature = &self.context.account_tx_context.signature().0; + let signature = &self.context.tx_context.tx_info.signature().0; let signature = signature.iter().map(|&x| MaybeRelocatable::from(stark_felt_to_felt(x))).collect(); let signature_segment_start_ptr = self.read_only_segments.allocate(vm, &signature)?; @@ -308,18 +309,17 @@ impl<'a> DeprecatedSyscallHintProcessor<'a> { vm: &mut VirtualMachine, ) -> DeprecatedSyscallResult { let tx_signature_start_ptr = self.get_or_allocate_tx_signature_segment(vm)?; - let account_tx_context = &self.context.account_tx_context; - let tx_signature_length = account_tx_context.signature().0.len(); + let TransactionContext { block_context, tx_info } = self.context.tx_context.as_ref(); + let tx_signature_length = tx_info.signature().0.len(); let tx_info: Vec = vec![ - stark_felt_to_felt(account_tx_context.signed_version().0).into(), - stark_felt_to_felt(*account_tx_context.sender_address().0.key()).into(), - max_fee_for_execution_info(account_tx_context).into(), + stark_felt_to_felt(tx_info.signed_version().0).into(), + stark_felt_to_felt(*tx_info.sender_address().0.key()).into(), + max_fee_for_execution_info(tx_info).into(), tx_signature_length.into(), tx_signature_start_ptr.into(), - stark_felt_to_felt(account_tx_context.transaction_hash().0).into(), - Felt252::from_bytes_be(self.context.block_context.chain_info.chain_id.0.as_bytes()) - .into(), - stark_felt_to_felt(account_tx_context.nonce().0).into(), + stark_felt_to_felt(tx_info.transaction_hash().0).into(), + Felt252::from_bytes_be(block_context.chain_info.chain_id.0.as_bytes()).into(), + stark_felt_to_felt(tx_info.nonce().0).into(), ]; let tx_info_start_ptr = self.read_only_segments.allocate(vm, &tx_info)?; diff --git a/crates/blockifier/src/execution/deprecated_syscalls/mod.rs b/crates/blockifier/src/execution/deprecated_syscalls/mod.rs index 6e6bf6e252..29c7385753 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/mod.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/mod.rs @@ -398,7 +398,7 @@ pub fn get_block_number( ) -> DeprecatedSyscallResult { // TODO(Yoni, 1/5/2024): disable for validate. Ok(GetBlockNumberResponse { - block_number: syscall_handler.context.block_context.block_info.block_number, + block_number: syscall_handler.context.tx_context.block_context.block_info.block_number, }) } @@ -425,7 +425,12 @@ pub fn get_block_timestamp( ) -> DeprecatedSyscallResult { // TODO(Yoni, 1/5/2024): disable for validate. Ok(GetBlockTimestampResponse { - block_timestamp: syscall_handler.context.block_context.block_info.block_timestamp, + block_timestamp: syscall_handler + .context + .tx_context + .block_context + .block_info + .block_timestamp, }) } @@ -478,7 +483,7 @@ pub fn get_sequencer_address( ) -> DeprecatedSyscallResult { syscall_handler.verify_not_in_validate_mode("get_sequencer_address")?; Ok(GetSequencerAddressResponse { - address: syscall_handler.context.block_context.block_info.sequencer_address, + address: syscall_handler.context.tx_context.block_context.block_info.sequencer_address, }) } @@ -518,7 +523,7 @@ pub fn get_tx_signature( syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>, ) -> DeprecatedSyscallResult { let start_ptr = syscall_handler.get_or_allocate_tx_signature_segment(vm)?; - let length = syscall_handler.context.account_tx_context.signature().0.len(); + let length = syscall_handler.context.tx_context.tx_info.signature().0.len(); Ok(GetTxSignatureResponse { segment: ReadOnlySegment { start_ptr, length } }) } diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 2979321fe8..f4134df4a4 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -12,7 +12,7 @@ use starknet_api::transaction::{Calldata, TransactionVersion}; use crate::abi::abi_utils::selector_from_name; use crate::abi::constants; -use crate::context::BlockContext; +use crate::context::TransactionContext; use crate::execution::call_info::CallInfo; use crate::execution::common_hints::ExecutionMode; use crate::execution::deprecated_syscalls::hint_processor::SyscallCounter; @@ -20,9 +20,7 @@ use crate::execution::errors::{EntryPointExecutionError, PreExecutionError}; use crate::execution::execution_utils::execute_entry_point_call; use crate::fee::os_resources::OS_RESOURCES; use crate::state::state_api::State; -use crate::transaction::objects::{ - AccountTransactionContext, HasRelatedFeeType, TransactionExecutionResult, -}; +use crate::transaction::objects::{HasRelatedFeeType, TransactionExecutionResult, TransactionInfo}; use crate::transaction::transaction_types::TransactionType; use crate::versioned_constants::VersionedConstants; @@ -68,9 +66,10 @@ impl CallEntryPoint { resources: &mut ExecutionResources, context: &mut EntryPointExecutionContext, ) -> EntryPointExecutionResult { + let tx_context = &context.tx_context; let mut decrement_when_dropped = RecursionDepthGuard::new( context.current_recursion_depth.clone(), - context.block_context.versioned_constants.max_recursion_depth, + tx_context.block_context.versioned_constants.max_recursion_depth, ); decrement_when_dropped.try_increment_and_check_depth()?; @@ -86,7 +85,7 @@ impl CallEntryPoint { None => storage_class_hash, // If not given, take the storage contract class hash. }; // Hack to prevent version 0 attack on argent accounts. - if context.account_tx_context.version() == TransactionVersion::ZERO + if tx_context.tx_info.version() == TransactionVersion::ZERO && class_hash == ClassHash( StarkFelt::try_from(FAULTY_CLASS_HASH).expect("A class hash must be a felt."), @@ -142,8 +141,7 @@ pub struct ExecutionResources { #[derive(Debug)] pub struct EntryPointExecutionContext { - pub block_context: BlockContext, - pub account_tx_context: AccountTransactionContext, + pub tx_context: Arc, // VM execution limits. pub vm_run_resources: RunResources, /// Used for tracking events order during the current execution. @@ -162,60 +160,45 @@ pub struct EntryPointExecutionContext { impl EntryPointExecutionContext { pub fn new( - block_context: &BlockContext, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, mode: ExecutionMode, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { - let max_steps = - Self::max_steps(block_context, account_tx_context, &mode, limit_steps_by_resources)?; + let max_steps = Self::max_steps(&tx_context, &mode, limit_steps_by_resources)?; Ok(Self { vm_run_resources: RunResources::new(max_steps), n_emitted_events: 0, n_sent_messages_to_l1: 0, error_stack: vec![], - account_tx_context: account_tx_context.clone(), + tx_context: tx_context.clone(), current_recursion_depth: Default::default(), - block_context: block_context.clone(), execution_mode: mode, }) } pub fn new_validate( - block_context: &BlockContext, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { - Self::new( - block_context, - account_tx_context, - ExecutionMode::Validate, - limit_steps_by_resources, - ) + Self::new(tx_context, ExecutionMode::Validate, limit_steps_by_resources) } pub fn new_invoke( - block_context: &BlockContext, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { - Self::new( - block_context, - account_tx_context, - ExecutionMode::Execute, - limit_steps_by_resources, - ) + Self::new(tx_context, ExecutionMode::Execute, limit_steps_by_resources) } /// Returns the maximum number of cairo steps allowed, given the max fee, gas price and the /// execution mode. /// If fee is disabled, returns the global maximum. fn max_steps( - block_context: &BlockContext, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, mode: &ExecutionMode, limit_steps_by_resources: bool, ) -> TransactionExecutionResult { + let block_context = &tx_context.block_context; let constants = &block_context.versioned_constants; let default_versioned_constants = VersionedConstants::latest_constants(); let block_upper_bound = match mode { @@ -233,7 +216,8 @@ impl EntryPointExecutionContext { let block_upper_bound = usize::try_from(block_upper_bound).expect("Failed to convert u32 to usize."); - if !limit_steps_by_resources || !account_tx_context.enforce_fee()? { + let tx_info = &tx_context.tx_info; + if !limit_steps_by_resources || !tx_info.enforce_fee()? { return Ok(block_upper_bound); } @@ -244,16 +228,18 @@ impl EntryPointExecutionContext { // New transactions derive the step limit by the L1 gas resource bounds; deprecated // transactions derive this value from the `max_fee`. - let tx_gas_upper_bound = match account_tx_context { - AccountTransactionContext::Deprecated(context) => { - (context.max_fee.0 + let tx_gas_upper_bound = match tx_info { + TransactionInfo::Deprecated(context) => { + let max_cairo_steps = context.max_fee.0 / block_context .block_info .gas_prices - .get_gas_price_by_fee_type(&account_tx_context.fee_type())) - as usize + .get_gas_price_by_fee_type(&tx_info.fee_type()); + // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the + // conversion works. + usize::try_from(max_cairo_steps).expect("Failed to convert u128 to usize") } - AccountTransactionContext::Current(context) => { + TransactionInfo::Current(context) => { // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the // convertion works. context diff --git a/crates/blockifier/src/execution/execution_utils.rs b/crates/blockifier/src/execution/execution_utils.rs index 44858904ef..d5498f6a57 100644 --- a/crates/blockifier/src/execution/execution_utils.rs +++ b/crates/blockifier/src/execution/execution_utils.rs @@ -28,7 +28,7 @@ use crate::execution::errors::PostExecutionError; use crate::execution::{deprecated_entry_point_execution, entry_point_execution}; use crate::state::errors::StateError; use crate::state::state_api::State; -use crate::transaction::objects::AccountTransactionContext; +use crate::transaction::objects::TransactionInfo; pub type Args = Vec; @@ -269,10 +269,10 @@ pub fn write_maybe_relocatable>( Ok(()) } -pub fn max_fee_for_execution_info(account_tx_context: &AccountTransactionContext) -> Felt252 { - match account_tx_context { - AccountTransactionContext::Current(_) => 0, - AccountTransactionContext::Deprecated(context) => context.max_fee.0, +pub fn max_fee_for_execution_info(tx_info: &TransactionInfo) -> Felt252 { + match tx_info { + TransactionInfo::Current(_) => 0, + TransactionInfo::Deprecated(tx_info) => tx_info.max_fee.0, } .into() } diff --git a/crates/blockifier/src/execution/syscalls/hint_processor.rs b/crates/blockifier/src/execution/syscalls/hint_processor.rs index 55c3540704..5d5a6b545b 100644 --- a/crates/blockifier/src/execution/syscalls/hint_processor.rs +++ b/crates/blockifier/src/execution/syscalls/hint_processor.rs @@ -49,7 +49,7 @@ use crate::execution::syscalls::{ }; use crate::state::errors::StateError; use crate::state::state_api::State; -use crate::transaction::objects::{AccountTransactionContext, CurrentAccountTransactionContext}; +use crate::transaction::objects::{CurrentTransactionInfo, TransactionInfo}; use crate::transaction::transaction_utils::update_remaining_gas; pub type SyscallCounter = HashMap; @@ -319,7 +319,7 @@ impl<'a> SyscallHintProcessor<'a> { fn allocate_tx_resource_bounds_segment( &mut self, vm: &mut VirtualMachine, - context: &CurrentAccountTransactionContext, + context: &CurrentTransactionInfo, ) -> SyscallResult<(Relocatable, Relocatable)> { let l1_gas = StarkFelt::try_from(L1_GAS).map_err(SyscallExecutionError::from)?; let l2_gas = StarkFelt::try_from(L2_GAS).map_err(SyscallExecutionError::from)?; @@ -434,7 +434,7 @@ impl<'a> SyscallHintProcessor<'a> { &mut self, vm: &mut VirtualMachine, ) -> SyscallResult { - let block_info = &self.context.block_context.block_info; + let block_info = &self.context.tx_context.block_context.block_info; let block_timestamp = StarkFelt::from(block_info.block_timestamp.0); let block_number = StarkFelt::from(block_info.block_number.0); let block_data: Vec = if self.is_validate_mode() { @@ -465,24 +465,27 @@ impl<'a> SyscallHintProcessor<'a> { } fn allocate_tx_info_segment(&mut self, vm: &mut VirtualMachine) -> SyscallResult { + let tx_info = &mut self.context.tx_context.tx_info.clone(); let (tx_signature_start_ptr, tx_signature_end_ptr) = - &self.allocate_data_segment(vm, self.context.account_tx_context.signature().0)?; - let account_tx_context = self.context.account_tx_context.clone(); + &self.allocate_data_segment(vm, tx_info.signature().0)?; + let tx_info = tx_info.clone(); - let mut tx_info: Vec = vec![ - stark_felt_to_felt(self.context.account_tx_context.signed_version().0).into(), - stark_felt_to_felt(*self.context.account_tx_context.sender_address().0.key()).into(), - max_fee_for_execution_info(&account_tx_context).into(), + let mut tx_data: Vec = vec![ + stark_felt_to_felt(tx_info.signed_version().0).into(), + stark_felt_to_felt(*tx_info.sender_address().0.key()).into(), + max_fee_for_execution_info(&tx_info).into(), tx_signature_start_ptr.into(), tx_signature_end_ptr.into(), - stark_felt_to_felt((self.context.account_tx_context).transaction_hash().0).into(), - Felt252::from_bytes_be(self.context.block_context.chain_info.chain_id.0.as_bytes()) - .into(), - stark_felt_to_felt((self.context.account_tx_context).nonce().0).into(), + stark_felt_to_felt((tx_info).transaction_hash().0).into(), + Felt252::from_bytes_be( + self.context.tx_context.block_context.chain_info.chain_id.0.as_bytes(), + ) + .into(), + stark_felt_to_felt((tx_info).nonce().0).into(), ]; - match account_tx_context { - AccountTransactionContext::Current(context) => { + match tx_info { + TransactionInfo::Current(context) => { let (tx_resource_bounds_start_ptr, tx_resource_bounds_end_ptr) = &self.allocate_tx_resource_bounds_segment(vm, &context)?; @@ -492,7 +495,7 @@ impl<'a> SyscallHintProcessor<'a> { let (tx_account_deployment_data_start_ptr, tx_account_deployment_data_end_ptr) = &self.allocate_data_segment(vm, context.account_deployment_data.0)?; - tx_info.extend_from_slice(&[ + tx_data.extend_from_slice(&[ tx_resource_bounds_start_ptr.into(), tx_resource_bounds_end_ptr.into(), Felt252::from(context.tip.0).into(), @@ -504,9 +507,9 @@ impl<'a> SyscallHintProcessor<'a> { tx_account_deployment_data_end_ptr.into(), ]); } - AccountTransactionContext::Deprecated(_) => { + TransactionInfo::Deprecated(_) => { let zero_felt: MaybeRelocatable = Felt252::zero().into(); - tx_info.extend_from_slice(&[ + tx_data.extend_from_slice(&[ zero_felt.clone(), // Empty segment of resource bounds (start ptr). zero_felt.clone(), // Empty segment of resource bounds (end ptr). zero_felt.clone(), // Tip. @@ -520,7 +523,7 @@ impl<'a> SyscallHintProcessor<'a> { } }; - let tx_info_start_ptr = self.read_only_segments.allocate(vm, &tx_info)?; + let tx_info_start_ptr = self.read_only_segments.allocate(vm, &tx_data)?; Ok(tx_info_start_ptr) } diff --git a/crates/blockifier/src/execution/syscalls/mod.rs b/crates/blockifier/src/execution/syscalls/mod.rs index 41817a0bc9..ff5a98546d 100644 --- a/crates/blockifier/src/execution/syscalls/mod.rs +++ b/crates/blockifier/src/execution/syscalls/mod.rs @@ -352,7 +352,8 @@ pub fn get_block_hash( } let requested_block_number = request.block_number.0; - let current_block_number = syscall_handler.context.block_context.block_info.block_number.0; + let current_block_number = + syscall_handler.context.tx_context.block_context.block_info.block_number.0; if current_block_number < constants::STORED_BLOCK_HASH_BUFFER || requested_block_number > current_block_number - constants::STORED_BLOCK_HASH_BUFFER diff --git a/crates/blockifier/src/execution/syscalls/syscalls_test.rs b/crates/blockifier/src/execution/syscalls/syscalls_test.rs index 4ebb052dde..e03ec7fdab 100644 --- a/crates/blockifier/src/execution/syscalls/syscalls_test.rs +++ b/crates/blockifier/src/execution/syscalls/syscalls_test.rs @@ -47,8 +47,7 @@ use crate::test_utils::{ }; use crate::transaction::constants::QUERY_VERSION_BASE_BIT; use crate::transaction::objects::{ - AccountTransactionContext, CommonAccountFields, CurrentAccountTransactionContext, - DeprecatedAccountTransactionContext, + CommonAccountFields, CurrentTransactionInfo, DeprecatedTransactionInfo, TransactionInfo, }; use crate::{check_entry_point_execution_error_for_custom_hint, retdata}; @@ -320,7 +319,7 @@ fn test_get_execution_info( let expected_tx_info: Vec; let mut expected_resource_bounds: Vec = vec![]; - let account_tx_context: AccountTransactionContext; + let tx_info: TransactionInfo; if version == TransactionVersion::ONE { expected_tx_info = vec![ version.0, // Transaction version. @@ -336,18 +335,17 @@ fn test_get_execution_info( stark_felt!(0_u16), // Length of resource bounds array. ]; } - account_tx_context = - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields: CommonAccountFields { - transaction_hash: tx_hash, - version: TransactionVersion::ONE, - nonce, - sender_address, - only_query, - ..Default::default() - }, - max_fee, - }); + tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields: CommonAccountFields { + transaction_hash: tx_hash, + version: TransactionVersion::ONE, + nonce, + sender_address, + only_query, + ..Default::default() + }, + max_fee, + }); } else { let max_amount = Fee(13); let max_price_per_unit = Fee(61); @@ -371,7 +369,7 @@ fn test_get_execution_info( StarkFelt::ZERO, // Max price per unit. ]; } - account_tx_context = AccountTransactionContext::Current(CurrentAccountTransactionContext { + tx_info = TransactionInfo::Current(CurrentTransactionInfo { common_fields: CommonAccountFields { transaction_hash: tx_hash, version: TransactionVersion::THREE, @@ -428,16 +426,10 @@ fn test_get_execution_info( let result = match execution_mode { ExecutionMode::Validate => entry_point_call - .execute_directly_given_account_context_in_validate_mode( - state, - account_tx_context, - false, - ), - ExecutionMode::Execute => entry_point_call.execute_directly_given_account_context( - state, - account_tx_context, - false, - ), + .execute_directly_given_account_context_in_validate_mode(state, tx_info, false), + ExecutionMode::Execute => { + entry_point_call.execute_directly_given_account_context(state, tx_info, false) + } }; assert!(!result.unwrap().execution.failed); diff --git a/crates/blockifier/src/fee/actual_cost.rs b/crates/blockifier/src/fee/actual_cost.rs index 7c10519271..480d5de1c0 100644 --- a/crates/blockifier/src/fee/actual_cost.rs +++ b/crates/blockifier/src/fee/actual_cost.rs @@ -1,15 +1,17 @@ +use std::sync::Arc; + use starknet_api::core::ContractAddress; use starknet_api::transaction::Fee; use super::gas_usage::calculate_tx_gas_usage; use crate::abi::constants as abi_constants; -use crate::context::BlockContext; +use crate::context::TransactionContext; use crate::execution::call_info::CallInfo; use crate::execution::entry_point::ExecutionResources; use crate::state::cached_state::{CachedState, StateChanges, StateChangesCount}; use crate::state::state_api::{StateReader, StateResult}; use crate::transaction::objects::{ - AccountTransactionContext, HasRelatedFeeType, ResourcesMapping, TransactionExecutionResult, + HasRelatedFeeType, ResourcesMapping, TransactionExecutionResult, }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transaction_utils::calculate_tx_resources; @@ -22,12 +24,11 @@ pub struct ActualCost { } impl ActualCost { - pub fn builder_for_l1_handler( - block_context: &BlockContext, - tx_context: AccountTransactionContext, + pub fn builder_for_l1_handler<'a>( + tx_context: Arc, l1_handler_payload_size: usize, - ) -> ActualCostBuilder<'_> { - ActualCostBuilder::new(block_context, tx_context, TransactionType::L1Handler) + ) -> ActualCostBuilder<'a> { + ActualCostBuilder::new(tx_context, TransactionType::L1Handler) .without_sender_address() .with_l1_payload_size(l1_handler_payload_size) } @@ -36,9 +37,8 @@ impl ActualCost { #[derive(Debug, Clone)] // Invariant: private fields initialized after `new` is called via dedicated methods. pub struct ActualCostBuilder<'a> { - pub account_tx_context: AccountTransactionContext, + pub account_tx_context: Arc, pub tx_type: TransactionType, - pub block_context: BlockContext, validate_call_info: Option<&'a CallInfo>, execute_call_info: Option<&'a CallInfo>, state_changes: StateChanges, @@ -49,15 +49,10 @@ pub struct ActualCostBuilder<'a> { impl<'a> ActualCostBuilder<'a> { // Recommendation: use constructor from account transaction, or from actual cost, to build this. - pub fn new( - block_context: &BlockContext, - account_tx_context: AccountTransactionContext, - tx_type: TransactionType, - ) -> Self { + pub fn new(tx_context: Arc, tx_type: TransactionType) -> Self { Self { - block_context: block_context.clone(), - sender_address: Some(account_tx_context.sender_address()), - account_tx_context, + sender_address: Some(tx_context.tx_info.sender_address()), + account_tx_context: tx_context, tx_type, validate_call_info: None, execute_call_info: None, @@ -97,8 +92,11 @@ impl<'a> ActualCostBuilder<'a> { mut self, state: &mut CachedState, ) -> StateResult { - let fee_token_address = - self.block_context.chain_info.fee_token_address(&self.account_tx_context.fee_type()); + let fee_token_address = self + .account_tx_context + .block_context + .chain_info + .fee_token_address(&self.account_tx_context.tx_info.fee_type()); let new_state_changes = state .get_actual_state_changes_for_fee_charge(fee_token_address, self.sender_address)?; @@ -139,11 +137,12 @@ impl<'a> ActualCostBuilder<'a> { *actual_resources.0.get_mut(&abi_constants::N_STEPS_RESOURCE.to_string()).unwrap() += n_reverted_steps; - let actual_fee = if self.account_tx_context.enforce_fee()? + let tx_info = &self.account_tx_context.tx_info; + let actual_fee = if tx_info.enforce_fee()? // L1 handler transactions are not charged an L2 fee but it is compared to the L1 fee. || self.tx_type == TransactionType::L1Handler { - self.account_tx_context.calculate_tx_fee(&actual_resources, &self.block_context)? + tx_info.calculate_tx_fee(&actual_resources, &self.account_tx_context.block_context)? } else { Fee(0) }; diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs index 5f66fe0b8f..17ef300179 100644 --- a/crates/blockifier/src/fee/fee_checks.rs +++ b/crates/blockifier/src/fee/fee_checks.rs @@ -2,16 +2,14 @@ use starknet_api::hash::StarkFelt; use starknet_api::transaction::Fee; use thiserror::Error; -use crate::block::BlockInfo; -use crate::context::{BlockContext, ChainInfo}; +use crate::context::TransactionContext; use crate::fee::actual_cost::ActualCost; use crate::fee::fee_utils::{ calculate_tx_l1_gas_usage, get_balance_and_if_covers_fee, get_fee_by_l1_gas_usage, }; use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionExecutionError; -use crate::transaction::objects::{AccountTransactionContext, FeeType, TransactionExecutionResult}; -use crate::versioned_constants::VersionedConstants; +use crate::transaction::objects::{FeeType, TransactionExecutionResult, TransactionInfo}; #[derive(Clone, Copy, Debug, Error)] pub enum FeeCheckError { @@ -57,8 +55,7 @@ impl FeeCheckReport { pub fn from_fee_check_error( actual_fee: Fee, error: FeeCheckError, - block_info: &BlockInfo, - account_tx_context: &AccountTransactionContext, + account_tx_context: &TransactionContext, ) -> TransactionExecutionResult { let recommended_fee = match error { // If the error is insufficient balance, the recommended fee is the actual fee. @@ -70,13 +67,13 @@ impl FeeCheckReport { // If the transaction passed pre-validation checks (i.e. balance initially covered the // resource bounds), the sender should be able to pay this fee. FeeCheckError::MaxFeeExceeded { .. } | FeeCheckError::MaxL1GasAmountExceeded { .. } => { - match account_tx_context { - AccountTransactionContext::Current(context) => get_fee_by_l1_gas_usage( - block_info, + match &account_tx_context.tx_info { + TransactionInfo::Current(context) => get_fee_by_l1_gas_usage( + &account_tx_context.block_context.block_info, context.l1_resource_bounds()?.max_amount.into(), &FeeType::Strk, ), - AccountTransactionContext::Deprecated(context) => context.max_fee, + TransactionInfo::Deprecated(context) => context.max_fee, } } }; @@ -86,16 +83,17 @@ impl FeeCheckReport { /// If the actual cost exceeds the resource bounds on the transaction, returns a fee check /// error. fn check_actual_cost_within_bounds( - versioned_constants: &VersionedConstants, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, actual_cost: &ActualCost, ) -> TransactionExecutionResult<()> { let ActualCost { actual_fee, actual_resources } = actual_cost; + let versioned_constants = &tx_context.block_context.versioned_constants; + // First, compare the actual resources used against the upper bound(s) defined by the // sender. - match account_tx_context { - AccountTransactionContext::Current(context) => { + match &tx_context.tx_info { + TransactionInfo::Current(context) => { // Check L1 gas limit. let max_l1_gas = context.l1_resource_bounds()?.max_amount.into(); let actual_used_l1_gas = @@ -107,7 +105,7 @@ impl FeeCheckReport { })?; } } - AccountTransactionContext::Deprecated(context) => { + TransactionInfo::Deprecated(context) => { // Check max fee. let max_fee = context.max_fee; if actual_fee > &max_fee { @@ -125,18 +123,17 @@ impl FeeCheckReport { /// If the actual cost exceeds the sender's balance, returns a fee check error. fn check_can_pay_fee( state: &mut S, - chain_info: &ChainInfo, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, actual_cost: &ActualCost, ) -> TransactionExecutionResult<()> { - let ActualCost { actual_fee, .. } = actual_cost; + let ActualCost { actual_fee, .. } = *actual_cost; let (balance_low, balance_high, can_pay) = - get_balance_and_if_covers_fee(state, account_tx_context, chain_info, *actual_fee)?; + get_balance_and_if_covers_fee(state, tx_context, actual_fee)?; if can_pay { return Ok(()); } Err(FeeCheckError::InsufficientFeeTokenBalance { - fee: *actual_fee, + fee: actual_fee, balance_low, balance_high, })? @@ -168,20 +165,15 @@ impl PostValidationReport { /// Note: the balance cannot be changed in `__validate__` (which cannot call other contracts), /// so there is no need to recheck that balance >= actual_cost. pub fn verify( - versioned_constants: &VersionedConstants, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, actual_cost: &ActualCost, ) -> TransactionExecutionResult<()> { // If fee is not enforced, no need to check post-execution. - if !account_tx_context.enforce_fee()? { + if !tx_context.tx_info.enforce_fee()? { return Ok(()); } - FeeCheckReport::check_actual_cost_within_bounds( - versioned_constants, - account_tx_context, - actual_cost, - ) + FeeCheckReport::check_actual_cost_within_bounds(tx_context, actual_cost) } } @@ -190,35 +182,26 @@ impl PostExecutionReport { /// that should be charged in revert flow. pub fn new( state: &mut S, - block_context: &BlockContext, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, actual_cost: &ActualCost, charge_fee: bool, ) -> TransactionExecutionResult { let ActualCost { actual_fee, .. } = actual_cost; // If fee is not enforced, no need to check post-execution. - if !charge_fee || !account_tx_context.enforce_fee()? { + if !charge_fee || !tx_context.tx_info.enforce_fee()? { return Ok(Self(FeeCheckReport::success_report(*actual_fee))); } // First, compare the actual resources used against the upper bound(s) defined by the // sender. - let cost_with_bounds_result = FeeCheckReport::check_actual_cost_within_bounds( - &block_context.versioned_constants, - account_tx_context, - actual_cost, - ); + let cost_with_bounds_result = + FeeCheckReport::check_actual_cost_within_bounds(tx_context, actual_cost); // Next, verify the actual cost is covered by the account balance, which may have changed // after execution. If the above check passes, the pre-execution balance covers the actual // cost for sure. - let can_pay_fee_result = FeeCheckReport::check_can_pay_fee( - state, - &block_context.chain_info, - account_tx_context, - actual_cost, - ); + let can_pay_fee_result = FeeCheckReport::check_can_pay_fee(state, tx_context, actual_cost); for fee_check_result in [cost_with_bounds_result, can_pay_fee_result] { match fee_check_result { @@ -229,14 +212,13 @@ impl PostExecutionReport { return Ok(Self(FeeCheckReport::from_fee_check_error( *actual_fee, fee_check_error, - &block_context.block_info, - account_tx_context, + tx_context, )?)); } Err(other_error) => return Err(other_error), } } - Ok(Self(FeeCheckReport::success_report(actual_cost.actual_fee))) + Ok(Self(FeeCheckReport::success_report(*actual_fee))) } } diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index 2086e79819..6bd2a18723 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -5,11 +5,11 @@ use starknet_api::transaction::Fee; use crate::abi::constants; use crate::block::BlockInfo; -use crate::context::{BlockContext, ChainInfo}; +use crate::context::{BlockContext, TransactionContext}; use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionFeeError; use crate::transaction::objects::{ - AccountTransactionContext, FeeType, HasRelatedFeeType, ResourcesMapping, TransactionFeeResult, + FeeType, HasRelatedFeeType, ResourcesMapping, TransactionFeeResult, TransactionInfo, }; use crate::versioned_constants::VersionedConstants; @@ -85,13 +85,13 @@ pub fn calculate_tx_fee( /// Returns the current fee balance and a boolean indicating whether the balance covers the fee. pub fn get_balance_and_if_covers_fee( state: &mut dyn StateReader, - account_tx_context: &AccountTransactionContext, - chain_info: &ChainInfo, + tx_context: &TransactionContext, fee: Fee, ) -> TransactionFeeResult<(StarkFelt, StarkFelt, bool)> { + let tx_info = &tx_context.tx_info; let (balance_low, balance_high) = state.get_fee_token_balance( - account_tx_context.sender_address(), - chain_info.fee_token_address(&account_tx_context.fee_type()), + tx_info.sender_address(), + tx_context.block_context.chain_info.fee_token_address(&tx_info.fee_type()), )?; Ok(( balance_low, @@ -106,26 +106,26 @@ pub fn get_balance_and_if_covers_fee( /// Error may indicate insufficient balance, or some other error. pub fn verify_can_pay_committed_bounds( state: &mut dyn StateReader, - account_tx_context: &AccountTransactionContext, - chain_info: &ChainInfo, + tx_context: &TransactionContext, ) -> TransactionFeeResult<()> { - let committed_fee = match account_tx_context { - AccountTransactionContext::Current(context) => { + let tx_info = &tx_context.tx_info; + let committed_fee = match tx_info { + TransactionInfo::Current(context) => { let l1_bounds = context.l1_resource_bounds()?; let max_amount: u128 = l1_bounds.max_amount.into(); // Sender will not be charged by `max_price_per_unit`, but this check should not depend // on the current gas price. Fee(max_amount * l1_bounds.max_price_per_unit) } - AccountTransactionContext::Deprecated(context) => context.max_fee, + TransactionInfo::Deprecated(context) => context.max_fee, }; let (balance_low, balance_high, can_pay) = - get_balance_and_if_covers_fee(state, account_tx_context, chain_info, committed_fee)?; + get_balance_and_if_covers_fee(state, tx_context, committed_fee)?; if can_pay { Ok(()) } else { - Err(match account_tx_context { - AccountTransactionContext::Current(context) => { + Err(match tx_info { + TransactionInfo::Current(context) => { let l1_bounds = context.l1_resource_bounds()?; TransactionFeeError::L1GasBoundsExceedBalance { max_amount: l1_bounds.max_amount, @@ -134,13 +134,11 @@ pub fn verify_can_pay_committed_bounds( balance_high, } } - AccountTransactionContext::Deprecated(context) => { - TransactionFeeError::MaxFeeExceedsBalance { - max_fee: context.max_fee, - balance_low, - balance_high, - } - } + TransactionInfo::Deprecated(context) => TransactionFeeError::MaxFeeExceedsBalance { + max_fee: context.max_fee, + balance_low, + balance_high, + }, }) } } diff --git a/crates/blockifier/src/test_utils/prices.rs b/crates/blockifier/src/test_utils/prices.rs index 1b8994ecfd..291a4a12ec 100644 --- a/crates/blockifier/src/test_utils/prices.rs +++ b/crates/blockifier/src/test_utils/prices.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use cached::proc_macro::cached; use cairo_vm::vm::runners::cairo_runner::ExecutionResources as VmExecutionResources; use starknet_api::core::ContractAddress; @@ -73,8 +75,7 @@ fn fee_transfer_resources( state, &mut ExecutionResources::default(), &mut EntryPointExecutionContext::new( - block_context, - &account_invoke_tx(InvokeTxArgs::default()).get_account_tx_context(), + Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))), ExecutionMode::Execute, false, ) diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs index 39524586c5..775842258d 100644 --- a/crates/blockifier/src/test_utils/struct_impls.rs +++ b/crates/blockifier/src/test_utils/struct_impls.rs @@ -17,7 +17,7 @@ use super::{ }; use crate::abi::constants; use crate::block::{BlockInfo, GasPrices}; -use crate::context::{BlockContext, ChainInfo, FeeTokenAddresses}; +use crate::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionContext}; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::{ContractClassV0, ContractClassV1}; use crate::execution::entry_point::{ @@ -25,7 +25,7 @@ use crate::execution::entry_point::{ }; use crate::state::state_api::State; use crate::test_utils::get_raw_contract_class; -use crate::transaction::objects::{AccountTransactionContext, DeprecatedAccountTransactionContext}; +use crate::transaction::objects::{DeprecatedTransactionInfo, TransactionInfo}; use crate::versioned_constants::VersionedConstants; impl CallEntryPoint { @@ -34,7 +34,7 @@ impl CallEntryPoint { pub fn execute_directly(self, state: &mut dyn State) -> EntryPointExecutionResult { self.execute_directly_given_account_context( state, - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext::default()), + TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), true, ) } @@ -42,16 +42,14 @@ impl CallEntryPoint { pub fn execute_directly_given_account_context( self, state: &mut dyn State, - account_tx_context: AccountTransactionContext, + tx_info: TransactionInfo, limit_steps_by_resources: bool, ) -> EntryPointExecutionResult { let block_context = BlockContext::create_for_testing(); - let mut context = EntryPointExecutionContext::new_invoke( - &block_context, - &account_tx_context, - limit_steps_by_resources, - ) - .unwrap(); + let tx_context = TransactionContext { block_context, tx_info }; + let mut context = + EntryPointExecutionContext::new_invoke(Arc::new(tx_context), limit_steps_by_resources) + .unwrap(); self.execute(state, &mut ExecutionResources::default(), &mut context) } @@ -63,7 +61,7 @@ impl CallEntryPoint { ) -> EntryPointExecutionResult { self.execute_directly_given_account_context_in_validate_mode( state, - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext::default()), + TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), true, ) } @@ -71,13 +69,13 @@ impl CallEntryPoint { pub fn execute_directly_given_account_context_in_validate_mode( self, state: &mut dyn State, - account_tx_context: AccountTransactionContext, + tx_info: TransactionInfo, limit_steps_by_resources: bool, ) -> EntryPointExecutionResult { let block_context = BlockContext::create_for_testing(); + let tx_context = TransactionContext { block_context, tx_info }; let mut context = EntryPointExecutionContext::new_validate( - &block_context, - &account_tx_context, + Arc::new(tx_context), limit_steps_by_resources, ) .unwrap(); diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index c72d3330bc..1913ccde51 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use itertools::concat; use starknet_api::calldata; use starknet_api::core::{ContractAddress, EntryPointSelector}; @@ -7,7 +9,7 @@ use starknet_api::transaction::{Calldata, Fee, ResourceBounds, TransactionVersio use crate::abi::abi_utils::selector_from_name; use crate::abi::constants as abi_constants; -use crate::context::BlockContext; +use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::{CallInfo, Retdata}; use crate::execution::contract_class::ContractClass; use crate::execution::entry_point::{ @@ -25,8 +27,8 @@ use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, TransactionPreValidationError, }; use crate::transaction::objects::{ - AccountTransactionContext, HasRelatedFeeType, TransactionExecutionInfo, - TransactionExecutionResult, TransactionPreValidationResult, + HasRelatedFeeType, TransactionExecutionInfo, TransactionExecutionResult, TransactionInfo, + TransactionInfoCreator, TransactionPreValidationResult, }; use crate::transaction::transaction_execution::Transaction; use crate::transaction::transaction_types::TransactionType; @@ -94,7 +96,7 @@ impl AccountTransaction { } // Calldata for validation contains transaction fields that cannot be obtained by calling - // `get_tx_info()`. + // `et_tx_info()`. fn validate_entrypoint_calldata(&self) -> Calldata { match self { Self::Declare(tx) => calldata![tx.class_hash().0], @@ -110,14 +112,6 @@ impl AccountTransaction { } } - pub fn get_account_tx_context(&self) -> AccountTransactionContext { - match self { - Self::Declare(tx) => tx.get_account_tx_context(), - Self::DeployAccount(tx) => tx.get_account_tx_context(), - Self::Invoke(tx) => tx.get_account_tx_context(), - } - } - fn verify_tx_version(&self, version: TransactionVersion) -> TransactionExecutionResult<()> { let allowed_versions: Vec = match self { // Support `Declare` of version 0 in order to allow bootstrapping of a new system. @@ -148,17 +142,17 @@ impl AccountTransaction { pub fn perform_pre_validation_stage( &self, state: &mut S, - account_tx_context: &AccountTransactionContext, - block_context: &BlockContext, + tx_context: &TransactionContext, charge_fee: bool, strict_nonce_check: bool, ) -> TransactionPreValidationResult<()> { - Self::handle_nonce(state, account_tx_context, strict_nonce_check)?; + let tx_info = &tx_context.tx_info; + Self::handle_nonce(state, tx_info, strict_nonce_check)?; - if charge_fee && account_tx_context.enforce_fee()? { - self.check_fee_bounds(account_tx_context, block_context)?; + if charge_fee && tx_info.enforce_fee()? { + self.check_fee_bounds(tx_context)?; - verify_can_pay_committed_bounds(state, account_tx_context, &block_context.chain_info)?; + verify_can_pay_committed_bounds(state, tx_context)?; } Ok(()) @@ -166,15 +160,17 @@ impl AccountTransaction { fn check_fee_bounds( &self, - account_tx_context: &AccountTransactionContext, - block_context: &BlockContext, + tx_context: &TransactionContext, ) -> TransactionPreValidationResult<()> { + let block_context = &tx_context.block_context; let minimal_l1_gas_amount = estimate_minimal_l1_gas(&block_context.versioned_constants, self)?; - let block_info = &block_context.block_info; - match account_tx_context { - AccountTransactionContext::Current(context) => { + let block_info = &block_context.block_info; + let tx_info = &tx_context.tx_info; + let fee_type = &tx_info.fee_type(); + match tx_info { + TransactionInfo::Current(context) => { let ResourceBounds { max_amount: max_l1_gas_amount, max_price_per_unit: max_l1_gas_price, @@ -192,8 +188,7 @@ impl AccountTransaction { })?; } - let actual_l1_gas_price = - block_info.gas_prices.get_gas_price_by_fee_type(&account_tx_context.fee_type()); + let actual_l1_gas_price = block_info.gas_prices.get_gas_price_by_fee_type(fee_type); if max_l1_gas_price < actual_l1_gas_price { return Err(TransactionFeeError::MaxL1GasPriceTooLow { max_l1_gas_price, @@ -201,13 +196,9 @@ impl AccountTransaction { })?; } } - AccountTransactionContext::Deprecated(context) => { + TransactionInfo::Deprecated(context) => { let max_fee = context.max_fee; - let min_fee = get_fee_by_l1_gas_usage( - block_info, - minimal_l1_gas_amount, - &account_tx_context.fee_type(), - ); + let min_fee = get_fee_by_l1_gas_usage(block_info, minimal_l1_gas_amount, fee_type); if max_fee < min_fee { return Err(TransactionFeeError::MaxFeeTooLow { min_fee, max_fee })?; } @@ -218,16 +209,16 @@ impl AccountTransaction { fn handle_nonce( state: &mut dyn State, - account_tx_context: &AccountTransactionContext, + tx_info: &TransactionInfo, strict: bool, ) -> TransactionPreValidationResult<()> { - if account_tx_context.is_v0() { + if tx_info.is_v0() { return Ok(()); } - let address = account_tx_context.sender_address(); + let address = tx_info.sender_address(); let account_nonce = state.get_nonce_at(address)?; - let incoming_tx_nonce = account_tx_context.nonce(); + let incoming_tx_nonce = tx_info.nonce(); let valid_nonce = if strict { account_nonce == incoming_tx_nonce } else { @@ -243,26 +234,17 @@ impl AccountTransaction { }) } - #[allow(clippy::too_many_arguments)] fn handle_validate_tx( &self, state: &mut dyn State, resources: &mut ExecutionResources, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, remaining_gas: &mut u64, - block_context: &BlockContext, validate: bool, limit_steps_by_resources: bool, ) -> TransactionExecutionResult> { if validate { - self.validate_tx( - state, - resources, - account_tx_context, - remaining_gas, - block_context, - limit_steps_by_resources, - ) + self.validate_tx(state, resources, tx_context, remaining_gas, limit_steps_by_resources) } else { Ok(None) } @@ -271,7 +253,7 @@ impl AccountTransaction { fn handle_fee( &self, state: &mut dyn State, - block_context: &BlockContext, + tx_context: Arc, actual_fee: Fee, charge_fee: bool, ) -> TransactionExecutionResult> { @@ -281,17 +263,14 @@ impl AccountTransaction { } // Charge fee. - let account_tx_context = self.get_account_tx_context(); - let fee_transfer_call_info = - Self::execute_fee_transfer(state, block_context, account_tx_context, actual_fee)?; + let fee_transfer_call_info = Self::execute_fee_transfer(state, tx_context, actual_fee)?; Ok(Some(fee_transfer_call_info)) } fn execute_fee_transfer( state: &mut dyn State, - block_context: &BlockContext, - account_tx_context: AccountTransactionContext, + tx_context: Arc, actual_fee: Fee, ) -> TransactionExecutionResult { // The least significant 128 bits of the amount transferred. @@ -299,9 +278,10 @@ impl AccountTransaction { // The most significant 128 bits of the amount transferred. let msb_amount = StarkFelt::from(0_u8); + let TransactionContext { block_context, tx_info } = tx_context.as_ref(); + // TODO(Gilad): add test that correct fee address is taken, once we add V3 test support. - let storage_address = - block_context.chain_info.fee_token_address(&account_tx_context.fee_type()); + let storage_address = block_context.chain_info.fee_token_address(&tx_info.fee_type()); let fee_transfer_call = CallEntryPoint { class_hash: None, code_address: None, @@ -313,14 +293,13 @@ impl AccountTransaction { msb_amount ], storage_address, - caller_address: account_tx_context.sender_address(), + caller_address: tx_info.sender_address(), call_type: CallType::Call, // The fee-token contract is a Cairo 0 contract, hence the initial gas is irrelevant. initial_gas: abi_constants::INITIAL_GAS_COST, }; - let mut context = - EntryPointExecutionContext::new_invoke(block_context, &account_tx_context, true)?; + let mut context = EntryPointExecutionContext::new_invoke(tx_context, true)?; Ok(fee_transfer_call .execute(state, &mut ExecutionResources::default(), &mut context) @@ -344,9 +323,8 @@ impl AccountTransaction { fn run_non_revertible( &self, state: &mut TransactionalState<'_, S>, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, remaining_gas: &mut u64, - block_context: &BlockContext, validate: bool, charge_fee: bool, ) -> TransactionExecutionResult { @@ -357,34 +335,26 @@ impl AccountTransaction { // Handle `DeployAccount` transactions separately, due to different order of things. // Also, the execution context required form the `DeployAccount` execute phase is // validation context. - let mut execution_context = EntryPointExecutionContext::new_validate( - block_context, - account_tx_context, - charge_fee, - )?; + let mut execution_context = + EntryPointExecutionContext::new_validate(tx_context.clone(), charge_fee)?; execute_call_info = self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?; validate_call_info = self.handle_validate_tx( state, &mut resources, - account_tx_context, + tx_context.clone(), remaining_gas, - block_context, validate, charge_fee, )?; } else { - let mut execution_context = EntryPointExecutionContext::new_invoke( - block_context, - account_tx_context, - charge_fee, - )?; + let mut execution_context = + EntryPointExecutionContext::new_invoke(tx_context.clone(), charge_fee)?; validate_call_info = self.handle_validate_tx( state, &mut resources, - account_tx_context, + tx_context.clone(), remaining_gas, - block_context, validate, charge_fee, )?; @@ -393,19 +363,14 @@ impl AccountTransaction { } let actual_cost = self - .to_actual_cost_builder(block_context) + .to_actual_cost_builder(tx_context.clone()) .with_validate_call_info(&validate_call_info) .with_execute_call_info(&execute_call_info) .try_add_state_changes(state)? .build(&resources)?; - let post_execution_report = PostExecutionReport::new( - state, - block_context, - account_tx_context, - &actual_cost, - charge_fee, - )?; + let post_execution_report = + PostExecutionReport::new(state, &tx_context, &actual_cost, charge_fee)?; match post_execution_report.error() { Some(error) => Err(error.into()), None => Ok(ValidateExecuteCallInfo::new_accepted( @@ -416,27 +381,23 @@ impl AccountTransaction { } } - #[allow(clippy::too_many_arguments)] fn run_revertible( &self, state: &mut TransactionalState<'_, S>, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, remaining_gas: &mut u64, - block_context: &BlockContext, validate: bool, charge_fee: bool, ) -> TransactionExecutionResult { let mut resources = ExecutionResources::default(); let mut execution_context = - EntryPointExecutionContext::new_invoke(block_context, account_tx_context, charge_fee)?; - let account_tx_context = self.get_account_tx_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, - &account_tx_context, + tx_context.clone(), remaining_gas, - block_context, validate, charge_fee, )?; @@ -447,7 +408,7 @@ impl AccountTransaction { // Save the state changes resulting from running `validate_tx`, to be used later for // resource and fee calculation. let actual_cost_builder_with_validation_changes = self - .to_actual_cost_builder(block_context) + .to_actual_cost_builder(tx_context.clone()) .with_validate_call_info(&validate_call_info) .try_add_state_changes(state)?; @@ -486,8 +447,7 @@ impl AccountTransaction { // Post-execution checks. let post_execution_report = PostExecutionReport::new( &mut execution_state, - block_context, - &account_tx_context, + &tx_context, &actual_cost, charge_fee, )?; @@ -521,13 +481,8 @@ impl AccountTransaction { Err(_) => { // Error during execution. Revert, even if the error is sequencer-related. execution_state.abort(); - let post_execution_report = PostExecutionReport::new( - state, - block_context, - &account_tx_context, - &revert_cost, - charge_fee, - )?; + let post_execution_report = + PostExecutionReport::new(state, &tx_context, &revert_cost, charge_fee)?; Ok(ValidateExecuteCallInfo::new_reverted( validate_call_info, execution_context.error_trace(), @@ -540,7 +495,7 @@ impl AccountTransaction { } } - fn is_non_revertible(&self) -> bool { + fn is_non_revertible(&self, tx_info: &TransactionInfo) -> bool { // Reverting a Declare or Deploy transaction is not currently supported in the OS. match self { Self::Declare(_) => true, @@ -548,7 +503,7 @@ impl AccountTransaction { Self::Invoke(_) => { // V0 transactions do not have validation; we cannot deduct fee for execution. Thus, // invoke transactions of are non-revertible iff they are of version 0. - self.get_account_tx_context().is_v0() + tx_info.is_v0() } } } @@ -558,35 +513,22 @@ impl AccountTransaction { &self, state: &mut TransactionalState<'_, S>, remaining_gas: &mut u64, - block_context: &BlockContext, + tx_context: Arc, validate: bool, charge_fee: bool, ) -> TransactionExecutionResult { - let account_tx_context = self.get_account_tx_context(); - - if self.is_non_revertible() { - return self.run_non_revertible( - state, - &account_tx_context, - remaining_gas, - block_context, - validate, - charge_fee, - ); + if self.is_non_revertible(&tx_context.tx_info) { + return self.run_non_revertible(state, tx_context, remaining_gas, validate, charge_fee); } - self.run_revertible( - state, - &account_tx_context, - remaining_gas, - block_context, - validate, - charge_fee, - ) + self.run_revertible(state, tx_context, remaining_gas, validate, charge_fee) } - pub fn to_actual_cost_builder(&self, block_context: &BlockContext) -> ActualCostBuilder<'_> { - ActualCostBuilder::new(block_context, self.get_account_tx_context(), self.tx_type()) + pub fn to_actual_cost_builder( + &self, + tx_context: Arc, + ) -> ActualCostBuilder<'_> { + ActualCostBuilder::new(tx_context, self.tx_type()) } } @@ -598,19 +540,12 @@ impl ExecutableTransaction for AccountTransaction { charge_fee: bool, validate: bool, ) -> TransactionExecutionResult { - let account_tx_context = self.get_account_tx_context(); - - self.verify_tx_version(account_tx_context.version())?; + let tx_context = Arc::new(block_context.to_tx_context(&self)); + self.verify_tx_version(tx_context.tx_info.version())?; // Nonce and fee check should be done before running user code. let strict_nonce_check = true; - self.perform_pre_validation_stage( - state, - &account_tx_context, - block_context, - charge_fee, - strict_nonce_check, - )?; + self.perform_pre_validation_stage(state, &tx_context, charge_fee, strict_nonce_check)?; // Run validation and execution. let mut remaining_gas = Transaction::initial_gas(); @@ -619,10 +554,15 @@ impl ExecutableTransaction for AccountTransaction { execute_call_info, revert_error, final_cost: ActualCost { actual_fee: final_fee, actual_resources: final_resources }, - } = self.run_or_revert(state, &mut remaining_gas, block_context, validate, charge_fee)?; + } = self.run_or_revert( + state, + &mut remaining_gas, + tx_context.clone(), + validate, + charge_fee, + )?; - let fee_transfer_call_info = - self.handle_fee(state, block_context, final_fee, charge_fee)?; + let fee_transfer_call_info = self.handle_fee(state, tx_context, final_fee, charge_fee)?; let tx_execution_info = TransactionExecutionInfo { validate_call_info, @@ -636,6 +576,16 @@ impl ExecutableTransaction for AccountTransaction { } } +impl TransactionInfoCreator for AccountTransaction { + fn create_tx_info(&self) -> TransactionInfo { + match self { + Self::Declare(tx) => tx.create_tx_info(), + Self::DeployAccount(tx) => tx.create_tx_info(), + Self::Invoke(tx) => tx.create_tx_info(), + } + } +} + /// Represents a bundle of validate-execute stage execution effects. struct ValidateExecuteCallInfo { validate_call_info: Option, @@ -672,21 +622,18 @@ impl ValidatableTransaction for AccountTransaction { &self, state: &mut dyn State, resources: &mut ExecutionResources, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, remaining_gas: &mut u64, - block_context: &BlockContext, limit_steps_by_resources: bool, ) -> TransactionExecutionResult> { - let mut context = EntryPointExecutionContext::new_validate( - block_context, - account_tx_context, - limit_steps_by_resources, - )?; - if context.account_tx_context.is_v0() { + let mut context = + EntryPointExecutionContext::new_validate(tx_context, limit_steps_by_resources)?; + let tx_info = &context.tx_context.tx_info; + if tx_info.is_v0() { return Ok(None); } - let storage_address = account_tx_context.sender_address(); + let storage_address = tx_info.sender_address(); let validate_call = CallEntryPoint { entry_point_type: EntryPointType::External, entry_point_selector: self.validate_entry_point_selector(), diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 5230fad98d..73608f4a37 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::sync::Arc; use assert_matches::assert_matches; use cairo_felt::Felt252; @@ -41,7 +42,7 @@ use crate::test_utils::{ use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::constants::TRANSFER_ENTRY_POINT_NAME; use crate::transaction::errors::TransactionExecutionError; -use crate::transaction::objects::{FeeType, HasRelatedFeeType}; +use crate::transaction::objects::{FeeType, HasRelatedFeeType, TransactionInfoCreator}; use crate::transaction::test_utils::{ account_invoke_tx, block_context, create_account_tx_for_validate_test, create_test_init_data, deploy_and_fund_account, l1_resource_bounds, max_fee, max_resource_bounds, run_invoke_tx, @@ -73,7 +74,7 @@ fn test_fee_enforcement( ); let account_tx = AccountTransaction::DeployAccount(deploy_account_tx); - let enforce_fee = account_tx.get_account_tx_context().enforce_fee().unwrap(); + let enforce_fee = account_tx.create_tx_info().enforce_fee().unwrap(); let result = account_tx.execute(state, &block_context, true, true); assert_eq!(result.is_err(), enforce_fee); } @@ -107,7 +108,7 @@ fn test_enforce_fee_false_works(block_context: BlockContext, #[case] version: Tr } // TODO(Dori, 15/9/2023): Convert version variance to attribute macro. -// TODO(Dori, 10/10/2023): Add V3 case once `get_account_tx_context` is supported for V3. +// TODO(Dori, 10/10/2023): Add V3 case once `create_tx_info` is supported for V3. #[rstest] fn test_account_flow_test( block_context: BlockContext, @@ -141,7 +142,7 @@ fn test_account_flow_test( #[rstest] #[case(TransactionVersion::ZERO)] #[case(TransactionVersion::ONE)] -// TODO(Nimrod, 10/10/2023): Add V3 case once `get_account_tx_context` is supported for V3. +// TODO(Nimrod, 10/10/2023): Add V3 case once `get_tx_info` is supported for V3. fn test_invoke_tx_from_non_deployed_account( block_context: BlockContext, max_fee: Fee, @@ -570,12 +571,9 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { ); // Fail execution, assert nonce and balance are unchanged. - let account_tx_context = declare_account_tx.get_account_tx_context(); + let tx_info = declare_account_tx.create_tx_info(); let initial_balance = state - .get_fee_token_balance( - account_address, - chain_info.fee_token_address(&account_tx_context.fee_type()), - ) + .get_fee_token_balance(account_address, chain_info.fee_token_address(&tx_info.fee_type())) .unwrap(); declare_account_tx.execute(&mut state, &block_context, true, true).unwrap_err(); @@ -584,7 +582,7 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { state .get_fee_token_balance( account_address, - chain_info.fee_token_address(&account_tx_context.fee_type()) + chain_info.fee_token_address(&tx_info.fee_type()) ) .unwrap(), initial_balance @@ -849,12 +847,8 @@ fn test_max_fee_to_max_steps_conversion( resource_bounds: l1_resource_bounds(actual_gas_used, actual_strk_gas_price), nonce: nonce_manager.next(account_address), }); - let execution_context1 = EntryPointExecutionContext::new_invoke( - &block_context, - &account_tx1.get_account_tx_context(), - true, - ) - .unwrap(); + let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1)); + let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true).unwrap(); let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps(); let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap(); let n_steps1 = tx_execution_info1.actual_resources.n_steps(); @@ -872,12 +866,8 @@ fn test_max_fee_to_max_steps_conversion( resource_bounds: l1_resource_bounds(2 * actual_gas_used, actual_strk_gas_price), nonce: nonce_manager.next(account_address), }); - let execution_context2 = EntryPointExecutionContext::new_invoke( - &block_context, - &account_tx2.get_account_tx_context(), - true, - ) - .unwrap(); + let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2)); + let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true).unwrap(); let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps(); let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap(); let n_steps2 = tx_execution_info2.actual_resources.n_steps(); diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index 9b4efedc4e..5db86f9043 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -36,20 +36,14 @@ macro_rules! implement_getters { }; } -#[derive(Clone, Copy, Hash, EnumIter, Eq, PartialEq)] -pub enum FeeType { - Strk, - Eth, -} - /// Contains the account information of the transaction (outermost call). #[derive(Clone, Debug, Eq, PartialEq)] -pub enum AccountTransactionContext { - Current(CurrentAccountTransactionContext), - Deprecated(DeprecatedAccountTransactionContext), +pub enum TransactionInfo { + Current(CurrentTransactionInfo), + Deprecated(DeprecatedTransactionInfo), } -impl AccountTransactionContext { +impl TransactionInfo { implement_getters!( (transaction_hash, TransactionHash), (version, TransactionVersion), @@ -82,17 +76,17 @@ impl AccountTransactionContext { pub fn enforce_fee(&self) -> TransactionFeeResult { match self { - AccountTransactionContext::Current(context) => { + TransactionInfo::Current(context) => { let l1_bounds = context.l1_resource_bounds()?; let max_amount: u128 = l1_bounds.max_amount.into(); Ok(max_amount * l1_bounds.max_price_per_unit > 0) } - AccountTransactionContext::Deprecated(context) => Ok(context.max_fee != Fee(0)), + TransactionInfo::Deprecated(context) => Ok(context.max_fee != Fee(0)), } } } -impl HasRelatedFeeType for AccountTransactionContext { +impl HasRelatedFeeType for TransactionInfo { fn version(&self) -> TransactionVersion { self.version() } @@ -103,7 +97,7 @@ impl HasRelatedFeeType for AccountTransactionContext { } #[derive(Clone, Debug, Eq, PartialEq)] -pub struct CurrentAccountTransactionContext { +pub struct CurrentTransactionInfo { pub common_fields: CommonAccountFields, pub resource_bounds: ResourceBoundsMapping, pub tip: Tip, @@ -113,7 +107,7 @@ pub struct CurrentAccountTransactionContext { pub account_deployment_data: AccountDeploymentData, } -impl CurrentAccountTransactionContext { +impl CurrentTransactionInfo { /// Fetch the L1 resource bounds, if they exist. pub fn l1_resource_bounds(&self) -> TransactionFeeResult { match self.resource_bounds.0.get(&Resource::L1Gas).copied() { @@ -124,7 +118,7 @@ impl CurrentAccountTransactionContext { } #[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct DeprecatedAccountTransactionContext { +pub struct DeprecatedTransactionInfo { pub common_fields: CommonAccountFields, pub max_fee: Fee, } @@ -224,3 +218,13 @@ pub trait HasRelatedFeeType { Ok(calculate_tx_fee(resources, block_context, &self.fee_type())?) } } + +#[derive(Clone, Copy, Hash, EnumIter, Eq, PartialEq)] +pub enum FeeType { + Strk, + Eth, +} + +pub trait TransactionInfoCreator { + fn create_tx_info(&self) -> TransactionInfo; +} diff --git a/crates/blockifier/src/transaction/post_execution_test.rs b/crates/blockifier/src/transaction/post_execution_test.rs index 9d55825564..414d3f40fb 100644 --- a/crates/blockifier/src/transaction/post_execution_test.rs +++ b/crates/blockifier/src/transaction/post_execution_test.rs @@ -17,7 +17,7 @@ use crate::test_utils::initial_test_state::test_state; use crate::test_utils::{create_calldata, CairoVersion, BALANCE, MAX_L1_GAS_PRICE}; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::TransactionExecutionError; -use crate::transaction::objects::{FeeType, HasRelatedFeeType}; +use crate::transaction::objects::{FeeType, HasRelatedFeeType, TransactionInfoCreator}; use crate::transaction::test_utils::{ account_invoke_tx, block_context, l1_resource_bounds, max_fee, max_resource_bounds, run_invoke_tx, TestInitData, @@ -108,7 +108,7 @@ fn test_revert_on_overdraft( resource_bounds: max_resource_bounds.clone(), nonce: nonce_manager.next(account_address), }); - let account_tx_context = approve_tx.get_account_tx_context(); + let tx_info = approve_tx.create_tx_info(); let approval_execution_info = approve_tx.execute(&mut state, &block_context, true, true).unwrap(); assert!(!approval_execution_info.is_reverted()); @@ -141,10 +141,7 @@ fn test_revert_on_overdraft( // Check the current balance, before next transaction. let (balance, _) = state - .get_fee_token_balance( - account_address, - chain_info.fee_token_address(&account_tx_context.fee_type()), - ) + .get_fee_token_balance(account_address, chain_info.fee_token_address(&tx_info.fee_type())) .unwrap(); // Attempt to transfer the entire balance, such that no funds remain to pay transaction fee. @@ -187,7 +184,7 @@ fn test_revert_on_overdraft( state .get_fee_token_balance( account_address, - chain_info.fee_token_address(&account_tx_context.fee_type()), + chain_info.fee_token_address(&tx_info.fee_type()), ) .unwrap(), (expected_new_balance, stark_felt!(0_u8)) @@ -196,7 +193,7 @@ fn test_revert_on_overdraft( state .get_fee_token_balance( recipient_address, - chain_info.fee_token_address(&account_tx_context.fee_type()) + chain_info.fee_token_address(&tx_info.fee_type()) ) .unwrap(), (final_received_amount, stark_felt!(0_u8)) diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 94f609b6ef..1cad7c8924 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use starknet_api::core::{calculate_contract_address, ContractAddress}; use starknet_api::transaction::{Fee, Transaction as StarknetApiTransaction, TransactionHash}; @@ -10,12 +12,15 @@ use crate::state::cached_state::TransactionalState; use crate::state::state_api::StateReader; use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::TransactionFeeError; -use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult}; +use crate::transaction::objects::{ + TransactionExecutionInfo, TransactionExecutionResult, TransactionInfo, TransactionInfoCreator, +}; use crate::transaction::transactions::{ DeclareTransaction, DeployAccountTransaction, Executable, ExecutableTransaction, InvokeTransaction, L1HandlerTransaction, }; +// TODO: Move into transaction.rs, makes more sense to be defined there. #[derive(Debug, derive_more::From)] pub enum Transaction { AccountTransaction(AccountTransaction), @@ -88,6 +93,15 @@ impl Transaction { } } +impl TransactionInfoCreator for Transaction { + fn create_tx_info(&self) -> TransactionInfo { + match self { + Self::AccountTransaction(account_tx) => account_tx.create_tx_info(), + Self::L1HandlerTransaction(l1_handler_tx) => l1_handler_tx.create_tx_info(), + } + } +} + impl ExecutableTransaction for L1HandlerTransaction { fn execute_raw( self, @@ -96,10 +110,10 @@ impl ExecutableTransaction for L1HandlerTransaction { _charge_fee: bool, _validate: bool, ) -> TransactionExecutionResult { - let tx_context = self.get_account_tx_context(); + let tx_context = Arc::new(block_context.to_tx_context(&self)); let mut execution_resources = ExecutionResources::default(); - let mut context = EntryPointExecutionContext::new_invoke(block_context, &tx_context, true)?; + let mut context = EntryPointExecutionContext::new_invoke(tx_context.clone(), true)?; let mut remaining_gas = Transaction::initial_gas(); let execute_call_info = self.run_execute(state, &mut execution_resources, &mut context, &mut remaining_gas)?; @@ -107,7 +121,7 @@ impl ExecutableTransaction for L1HandlerTransaction { let l1_handler_payload_size = self.tx.calldata.0.len() - 1; let ActualCost { actual_fee, actual_resources } = - ActualCost::builder_for_l1_handler(block_context, tx_context, l1_handler_payload_size) + ActualCost::builder_for_l1_handler(tx_context, l1_handler_payload_size) .with_execute_call_info(&execute_call_info) .try_add_state_changes(state)? .build(&execution_resources)?; diff --git a/crates/blockifier/src/transaction/transactions.rs b/crates/blockifier/src/transaction/transactions.rs index 662a54f4b2..1049c61d03 100644 --- a/crates/blockifier/src/transaction/transactions.rs +++ b/crates/blockifier/src/transaction/transactions.rs @@ -8,7 +8,7 @@ use starknet_api::transaction::{ }; use crate::abi::abi_utils::selector_from_name; -use crate::context::BlockContext; +use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::CallInfo; use crate::execution::contract_class::ContractClass; use crate::execution::entry_point::{ @@ -21,9 +21,8 @@ use crate::state::state_api::{State, StateReader}; use crate::transaction::constants; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::objects::{ - AccountTransactionContext, CommonAccountFields, CurrentAccountTransactionContext, - DeprecatedAccountTransactionContext, HasRelatedFeeType, TransactionExecutionInfo, - TransactionExecutionResult, + CommonAccountFields, CurrentTransactionInfo, DeprecatedTransactionInfo, HasRelatedFeeType, + TransactionExecutionInfo, TransactionExecutionResult, TransactionInfo, TransactionInfoCreator, }; use crate::transaction::transaction_utils::{update_remaining_gas, verify_contract_class_version}; @@ -68,6 +67,7 @@ pub trait ExecutableTransaction: Sized { } } + // FIXME: not transactional, maybe remove from trait. /// Executes the transaction in a transactional manner /// (if it fails, given state might become corrupted; i.e., changes until failure will appear). fn execute_raw( @@ -95,9 +95,8 @@ pub trait ValidatableTransaction { &self, state: &mut dyn State, resources: &mut ExecutionResources, - account_tx_context: &AccountTransactionContext, + tx_context: Arc, remaining_gas: &mut u64, - block_context: &BlockContext, limit_steps_by_resources: bool, ) -> TransactionExecutionResult>; } @@ -153,45 +152,6 @@ impl DeclareTransaction { self.contract_class.clone() } - pub fn get_account_tx_context(&self) -> AccountTransactionContext { - // TODO(Nir, 01/11/2023): Consider to move this (from all get_account_tx_context methods). - let common_fields = CommonAccountFields { - transaction_hash: self.tx_hash(), - version: self.tx.version(), - signature: self.tx.signature(), - nonce: self.tx.nonce(), - sender_address: self.tx.sender_address(), - only_query: self.only_query, - }; - - match &self.tx { - starknet_api::transaction::DeclareTransaction::V0(tx) - | starknet_api::transaction::DeclareTransaction::V1(tx) => { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields, - max_fee: tx.max_fee, - }) - } - starknet_api::transaction::DeclareTransaction::V2(tx) => { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields, - max_fee: tx.max_fee, - }) - } - starknet_api::transaction::DeclareTransaction::V3(tx) => { - AccountTransactionContext::Current(CurrentAccountTransactionContext { - common_fields, - resource_bounds: tx.resource_bounds.clone(), - tip: tx.tip, - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - account_deployment_data: tx.account_deployment_data.clone(), - }) - } - } - } - pub fn only_query(&self) -> bool { self.only_query } @@ -241,6 +201,46 @@ impl Executable for DeclareTransaction { } } +impl TransactionInfoCreator for DeclareTransaction { + fn create_tx_info(&self) -> TransactionInfo { + // TODO(Nir, 01/11/2023): Consider to move this (from all get_tx_info methods). + let common_fields = CommonAccountFields { + transaction_hash: self.tx_hash(), + version: self.tx.version(), + signature: self.tx.signature(), + nonce: self.tx.nonce(), + sender_address: self.tx.sender_address(), + only_query: self.only_query, + }; + + match &self.tx { + starknet_api::transaction::DeclareTransaction::V0(tx) + | starknet_api::transaction::DeclareTransaction::V1(tx) => { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields, + max_fee: tx.max_fee, + }) + } + starknet_api::transaction::DeclareTransaction::V2(tx) => { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields, + max_fee: tx.max_fee, + }) + } + starknet_api::transaction::DeclareTransaction::V3(tx) => { + TransactionInfo::Current(CurrentTransactionInfo { + common_fields, + resource_bounds: tx.resource_bounds.clone(), + tip: tx.tip, + nonce_data_availability_mode: tx.nonce_data_availability_mode, + fee_data_availability_mode: tx.fee_data_availability_mode, + paymaster_data: tx.paymaster_data.clone(), + account_deployment_data: tx.account_deployment_data.clone(), + }) + } + } + } +} #[derive(Debug, Clone)] pub struct DeployAccountTransaction { pub tx: starknet_api::transaction::DeployAccountTransaction, @@ -278,37 +278,6 @@ impl DeployAccountTransaction { pub fn tx(&self) -> &starknet_api::transaction::DeployAccountTransaction { &self.tx } - - pub fn get_account_tx_context(&self) -> AccountTransactionContext { - let common_fields = CommonAccountFields { - transaction_hash: self.tx_hash, - version: self.tx.version(), - signature: self.tx.signature(), - nonce: self.tx.nonce(), - sender_address: self.contract_address, - only_query: self.only_query, - }; - - match &self.tx { - starknet_api::transaction::DeployAccountTransaction::V1(tx) => { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields, - max_fee: tx.max_fee, - }) - } - starknet_api::transaction::DeployAccountTransaction::V3(tx) => { - AccountTransactionContext::Current(CurrentAccountTransactionContext { - common_fields, - resource_bounds: tx.resource_bounds.clone(), - tip: tx.tip, - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - account_deployment_data: AccountDeploymentData::default(), - }) - } - } - } } impl Executable for DeployAccountTransaction { @@ -341,6 +310,39 @@ impl Executable for DeployAccountTransaction { } } +impl TransactionInfoCreator for DeployAccountTransaction { + fn create_tx_info(&self) -> TransactionInfo { + let common_fields = CommonAccountFields { + transaction_hash: self.tx_hash, + version: self.tx.version(), + signature: self.tx.signature(), + nonce: self.tx.nonce(), + sender_address: self.contract_address, + only_query: self.only_query, + }; + + match &self.tx { + starknet_api::transaction::DeployAccountTransaction::V1(tx) => { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields, + max_fee: tx.max_fee, + }) + } + starknet_api::transaction::DeployAccountTransaction::V3(tx) => { + TransactionInfo::Current(CurrentTransactionInfo { + common_fields, + resource_bounds: tx.resource_bounds.clone(), + tip: tx.tip, + nonce_data_availability_mode: tx.nonce_data_availability_mode, + fee_data_availability_mode: tx.fee_data_availability_mode, + paymaster_data: tx.paymaster_data.clone(), + account_deployment_data: AccountDeploymentData::default(), + }) + } + } + } +} + #[derive(Debug, Clone)] pub struct InvokeTransaction { pub tx: starknet_api::transaction::InvokeTransaction, @@ -369,43 +371,6 @@ impl InvokeTransaction { (signature, TransactionSignature), (sender_address, ContractAddress) ); - - pub fn get_account_tx_context(&self) -> AccountTransactionContext { - let common_fields = CommonAccountFields { - transaction_hash: self.tx_hash, - version: self.tx.version(), - signature: self.tx.signature(), - nonce: self.tx.nonce(), - sender_address: self.tx.sender_address(), - only_query: self.only_query, - }; - - match &self.tx { - starknet_api::transaction::InvokeTransaction::V0(tx) => { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields, - max_fee: tx.max_fee, - }) - } - starknet_api::transaction::InvokeTransaction::V1(tx) => { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields, - max_fee: tx.max_fee, - }) - } - starknet_api::transaction::InvokeTransaction::V3(tx) => { - AccountTransactionContext::Current(CurrentAccountTransactionContext { - common_fields, - resource_bounds: tx.resource_bounds.clone(), - tip: tx.tip, - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - account_deployment_data: tx.account_deployment_data.clone(), - }) - } - } - } } impl Executable for InvokeTransaction { @@ -423,7 +388,7 @@ impl Executable for InvokeTransaction { selector_from_name(constants::EXECUTE_ENTRY_POINT_NAME) } }; - let storage_address = context.account_tx_context.sender_address(); + let storage_address = context.tx_context.tx_info.sender_address(); let execute_call = CallEntryPoint { entry_point_type: EntryPointType::External, entry_point_selector, @@ -445,6 +410,45 @@ impl Executable for InvokeTransaction { } } +impl TransactionInfoCreator for InvokeTransaction { + fn create_tx_info(&self) -> TransactionInfo { + let common_fields = CommonAccountFields { + transaction_hash: self.tx_hash, + version: self.tx.version(), + signature: self.tx.signature(), + nonce: self.tx.nonce(), + sender_address: self.tx.sender_address(), + only_query: self.only_query, + }; + + match &self.tx { + starknet_api::transaction::InvokeTransaction::V0(tx) => { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields, + max_fee: tx.max_fee, + }) + } + starknet_api::transaction::InvokeTransaction::V1(tx) => { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields, + max_fee: tx.max_fee, + }) + } + starknet_api::transaction::InvokeTransaction::V3(tx) => { + TransactionInfo::Current(CurrentTransactionInfo { + common_fields, + resource_bounds: tx.resource_bounds.clone(), + tip: tx.tip, + nonce_data_availability_mode: tx.nonce_data_availability_mode, + fee_data_availability_mode: tx.fee_data_availability_mode, + paymaster_data: tx.paymaster_data.clone(), + account_deployment_data: tx.account_deployment_data.clone(), + }) + } + } + } +} + #[derive(Debug)] pub struct L1HandlerTransaction { pub tx: starknet_api::transaction::L1HandlerTransaction, @@ -452,22 +456,6 @@ pub struct L1HandlerTransaction { pub paid_fee_on_l1: Fee, } -impl L1HandlerTransaction { - pub fn get_account_tx_context(&self) -> AccountTransactionContext { - AccountTransactionContext::Deprecated(DeprecatedAccountTransactionContext { - common_fields: CommonAccountFields { - transaction_hash: self.tx_hash, - version: self.tx.version, - signature: TransactionSignature::default(), - nonce: self.tx.nonce, - sender_address: self.tx.contract_address, - only_query: false, - }, - max_fee: Fee::default(), - }) - } -} - impl HasRelatedFeeType for L1HandlerTransaction { fn version(&self) -> TransactionVersion { self.tx.version @@ -506,3 +494,19 @@ impl Executable for L1HandlerTransaction { .map_err(TransactionExecutionError::ExecutionError) } } + +impl TransactionInfoCreator for L1HandlerTransaction { + fn create_tx_info(&self) -> TransactionInfo { + TransactionInfo::Deprecated(DeprecatedTransactionInfo { + common_fields: CommonAccountFields { + transaction_hash: self.tx_hash, + version: self.tx.version, + signature: TransactionSignature::default(), + nonce: self.tx.nonce, + sender_address: self.tx.contract_address, + only_query: false, + }, + max_fee: Fee::default(), + }) + } +} diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 8a7eeb12b2..bc09aa28e7 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -60,8 +60,8 @@ use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, TransactionPreValidationError, }; use crate::transaction::objects::{ - AccountTransactionContext, FeeType, HasRelatedFeeType, ResourcesMapping, - TransactionExecutionInfo, + FeeType, HasRelatedFeeType, ResourcesMapping, TransactionExecutionInfo, TransactionInfo, + TransactionInfoCreator, }; use crate::transaction::test_utils::{ account_invoke_tx, create_account_tx_for_validate_test, l1_resource_bounds, @@ -703,8 +703,8 @@ fn assert_failure_if_resource_bounds_exceed_balance( block_context: &BlockContext, invalid_tx: AccountTransaction, ) { - match invalid_tx.get_account_tx_context() { - AccountTransactionContext::Deprecated(context) => { + match invalid_tx.create_tx_info() { + TransactionInfo::Deprecated(context) => { assert_matches!( invalid_tx.execute(state, block_context, true, true).unwrap_err(), TransactionExecutionError::TransactionPreValidationError( @@ -713,7 +713,7 @@ fn assert_failure_if_resource_bounds_exceed_balance( if max_fee == context.max_fee ); } - AccountTransactionContext::Current(context) => { + TransactionInfo::Current(context) => { let l1_bounds = context.l1_resource_bounds().unwrap(); assert_matches!( invalid_tx.execute(state, block_context, true, true).unwrap_err(), @@ -929,15 +929,9 @@ fn test_invalid_nonce(account_cairo_version: CairoVersion) { let invalid_nonce = Nonce(stark_felt!(1_u8)); let invalid_tx = account_invoke_tx(invoke_tx_args! { nonce: invalid_nonce, ..valid_invoke_tx_args.clone() }); - let invalid_tx_context = invalid_tx.get_account_tx_context(); + let invalid_tx_context = block_context.to_tx_context(&invalid_tx); let pre_validation_err = invalid_tx - .perform_pre_validation_stage( - &mut transactional_state, - &invalid_tx_context, - block_context, - false, - true, - ) + .perform_pre_validation_stage(&mut transactional_state, &invalid_tx_context, false, true) .unwrap_err(); // Test error. @@ -954,29 +948,19 @@ fn test_invalid_nonce(account_cairo_version: CairoVersion) { let valid_nonce = Nonce(stark_felt!(1_u8)); let valid_tx = account_invoke_tx(invoke_tx_args! { nonce: valid_nonce, ..valid_invoke_tx_args.clone() }); - let valid_tx_context = valid_tx.get_account_tx_context(); + + let valid_tx_context = block_context.to_tx_context(&valid_tx); valid_tx - .perform_pre_validation_stage( - &mut transactional_state, - &valid_tx_context, - block_context, - false, - false, - ) + .perform_pre_validation_stage(&mut transactional_state, &valid_tx_context, false, false) .unwrap(); // Negative flow: account nonce = 1, incoming tx nonce = 0. let invalid_nonce = Nonce(stark_felt!(0_u8)); let invalid_tx = account_invoke_tx(invoke_tx_args! { nonce: invalid_nonce, ..valid_invoke_tx_args.clone() }); + let invalid_tx_context = block_context.to_tx_context(&invalid_tx); let pre_validation_err = invalid_tx - .perform_pre_validation_stage( - &mut transactional_state, - &invalid_tx.get_account_tx_context(), - block_context, - false, - false, - ) + .perform_pre_validation_stage(&mut transactional_state, &invalid_tx_context, false, false) .unwrap_err(); // Test error. diff --git a/crates/native_blockifier/src/py_validator.rs b/crates/native_blockifier/src/py_validator.rs index f3def7f7f8..729f30b559 100644 --- a/crates/native_blockifier/src/py_validator.rs +++ b/crates/native_blockifier/src/py_validator.rs @@ -1,10 +1,11 @@ +use blockifier::context::TransactionContext; use blockifier::execution::call_info::CallInfo; use blockifier::fee::actual_cost::ActualCost; use blockifier::fee::fee_checks::PostValidationReport; use blockifier::state::cached_state::GlobalContractCache; use blockifier::state::state_api::StateReader; use blockifier::transaction::account_transaction::AccountTransaction; -use blockifier::transaction::objects::{AccountTransactionContext, TransactionExecutionResult}; +use blockifier::transaction::objects::{TransactionExecutionResult, TransactionInfo}; use blockifier::transaction::transaction_execution::Transaction; use pyo3::prelude::*; use starknet_api::core::Nonce; @@ -60,7 +61,7 @@ impl PyValidator { deploy_account_tx_hash: Option, ) -> NativeBlockifierResult<()> { let account_tx = py_account_tx(tx, raw_contract_class)?; - let account_tx_context = account_tx.get_account_tx_context(); + let tx_context = self.tx_executor.block_context.to_tx_context(&account_tx); // Deploy account transactions should be fully executed, since the constructor must run // before `__validate_deploy__`. The execution already includes all necessary validations, // so they are skipped here. @@ -75,10 +76,10 @@ impl PyValidator { // processed. It is done before the pre-validations checks because, in these checks, we // change the state (more precisely, we increment the nonce). let skip_validate = self.skip_validate_due_to_unprocessed_deploy_account( - &account_tx_context, + &tx_context.tx_info, deploy_account_tx_hash, )?; - self.perform_pre_validation_stage(&account_tx)?; + self.perform_pre_validation_stage(&account_tx, &tx_context)?; if skip_validate { return Ok(()); @@ -90,7 +91,7 @@ impl PyValidator { // Post validations. // TODO(Ayelet, 09/11/2023): Check call succeeded. - self.perform_post_validation_stage(&account_tx_context, &actual_cost)?; + self.perform_post_validation_stage(&tx_context, &actual_cost)?; Ok(()) } @@ -128,16 +129,14 @@ impl PyValidator { fn perform_pre_validation_stage( &mut self, account_tx: &AccountTransaction, + tx_context: &TransactionContext, ) -> NativeBlockifierResult<()> { - let account_tx_context = account_tx.get_account_tx_context(); - let strict_nonce_check = false; // Run pre-validation in charge fee mode to perform fee and balance related checks. let charge_fee = true; account_tx.perform_pre_validation_stage( &mut self.tx_executor.state, - &account_tx_context, - &self.tx_executor.block_context, + tx_context, charge_fee, strict_nonce_check, )?; @@ -150,11 +149,11 @@ impl PyValidator { // (they will otherwise fail solely because the deploy account hasn't been processed yet). fn skip_validate_due_to_unprocessed_deploy_account( &mut self, - account_tx_context: &AccountTransactionContext, + tx_info: &TransactionInfo, deploy_account_tx_hash: Option, ) -> NativeBlockifierResult { - let nonce = self.tx_executor.state.get_nonce_at(account_tx_context.sender_address())?; - let tx_nonce = account_tx_context.nonce(); + let nonce = self.tx_executor.state.get_nonce_at(tx_info.sender_address())?; + let tx_nonce = tx_info.nonce(); let deploy_account_not_processed = deploy_account_tx_hash.is_some() && nonce == Nonce(StarkFelt::ZERO); @@ -182,13 +181,9 @@ impl PyValidator { fn perform_post_validation_stage( &mut self, - account_tx_context: &AccountTransactionContext, + tx_context: &TransactionContext, actual_cost: &ActualCost, ) -> TransactionExecutionResult<()> { - PostValidationReport::verify( - &self.tx_executor.block_context.versioned_constants, - account_tx_context, - actual_cost, - ) + PostValidationReport::verify(tx_context, actual_cost) } } diff --git a/crates/native_blockifier/src/transaction_executor.rs b/crates/native_blockifier/src/transaction_executor.rs index 0b141b9582..4d6b3ac1a3 100644 --- a/crates/native_blockifier/src/transaction_executor.rs +++ b/crates/native_blockifier/src/transaction_executor.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::sync::Arc; use blockifier::block::pre_process_block; use blockifier::context::BlockContext; @@ -27,6 +28,7 @@ use crate::py_transaction_execution_info::{ }; use crate::py_utils::PyFelt; +// TODO(Gilad): make this hold TransactionContext instead of BlockContext. pub struct TransactionExecutor { pub block_context: BlockContext, @@ -123,26 +125,26 @@ impl TransactionExecutor { mut remaining_gas: u64, ) -> NativeBlockifierResult<(Option, ActualCost)> { let mut execution_resources = ExecutionResources::default(); - let account_tx_context = account_tx.get_account_tx_context(); + let tx_context = Arc::new(self.block_context.to_tx_context(account_tx)); + let tx_info = &tx_context.tx_info; // TODO(Amos, 01/12/2023): Delete this once deprecated txs call // PyValidator.perform_validations(). // For fee charging purposes, the nonce-increment cost is taken into consideration when // calculating the fees for validation. // Note: This assumes that the state is reset between calls to validate. - self.state.increment_nonce(account_tx_context.sender_address())?; + self.state.increment_nonce(tx_info.sender_address())?; let validate_call_info = account_tx.validate_tx( &mut self.state, &mut execution_resources, - &account_tx_context, + tx_context.clone(), &mut remaining_gas, - &self.block_context, true, )?; let actual_cost = account_tx - .to_actual_cost_builder(&self.block_context) + .to_actual_cost_builder(tx_context) .with_validate_call_info(&validate_call_info) .try_add_state_changes(&mut self.state)? .build(&execution_resources)?;