diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 4ad8bfb285..f693daf8ff 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -17,7 +17,7 @@ use crate::state::cached_state::{CachedState, CommitmentStateDiff, Transactional use crate::state::errors::StateError; use crate::state::state_api::{StateReader, StateResult}; use crate::transaction::errors::TransactionExecutionError; -use crate::transaction::objects::{TransactionExecutionInfo, TransactionInfoCreator}; +use crate::transaction::objects::TransactionExecutionInfo; use crate::transaction::transaction_execution::Transaction; use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; @@ -100,11 +100,9 @@ impl TransactionExecutor { let mut transactional_state = TransactionalState::create_transactional( self.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR), ); - let tx_charge_fee = tx.create_tx_info().enforce_fee(); // Executing a single transaction cannot be done in a concurrent mode. - let execution_flags = - ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: false }; + let execution_flags = ExecutionFlags { concurrency_mode: false }; let tx_execution_result = tx.execute_raw(&mut transactional_state, &self.block_context, execution_flags); match tx_execution_result { diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index 3089818a87..c01dc739b5 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -21,6 +21,7 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::declare::declare_tx; use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::initial_test_state::test_state; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::l1_handler::l1handler_tx; use crate::test_utils::{ create_calldata, @@ -29,9 +30,9 @@ use crate::test_utils::{ BALANCE, DEFAULT_STRK_L1_GAS_PRICE, }; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::test_utils::{ - account_invoke_tx, block_context, calculate_class_info_for_testing, create_test_init_data, @@ -40,6 +41,7 @@ use crate::transaction::test_utils::{ TestInitData, }; use crate::transaction::transaction_execution::Transaction; +use crate::transaction::transactions::enforce_fee; fn tx_executor_test_body( state: CachedState, block_context: BlockContext, @@ -120,7 +122,7 @@ fn test_declare( let declared_contract = FeatureContract::Empty(cairo_version); let state = test_state(&block_context.chain_info, BALANCE, &[(account_contract, 1)]); - let tx = declare_tx( + let declare_tx = declare_tx( declare_tx_args! { sender_address: account_contract.get_instance_address(0), class_hash: declared_contract.get_class_hash(), @@ -129,8 +131,11 @@ fn test_declare( resource_bounds: l1_resource_bounds(0_u8.into(), DEFAULT_STRK_L1_GAS_PRICE.into()), }, calculate_class_info_for_testing(declared_contract.get_class()), - ) - .into(); + ); + let only_query = false; + let charge_fee = enforce_fee(&declare_tx, only_query); + let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() }; + let tx = AccountTransaction { tx: declare_tx, execution_flags }.into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } @@ -143,21 +148,25 @@ fn test_deploy_account( let account_contract = FeatureContract::AccountWithoutValidations(cairo_version); let state = test_state(&block_context.chain_info, BALANCE, &[(account_contract, 0)]); - let tx = deploy_account_tx( + let deploy_tx = deploy_account_tx( deploy_account_tx_args! { class_hash: account_contract.get_class_hash(), resource_bounds: l1_resource_bounds(0_u8.into(), DEFAULT_STRK_L1_GAS_PRICE.into()), version, }, &mut NonceManager::default(), - ) - .into(); + ); + let only_query = false; + let charge_fee = enforce_fee(&deploy_tx, only_query); + let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() }; + let tx = AccountTransaction { tx: deploy_tx, execution_flags }.into(); let expected_bouncer_weights = BouncerWeights { state_diff_size: 3, message_segment_length: 0, n_events: 0, ..BouncerWeights::empty() - }; + } + .into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } @@ -217,12 +226,15 @@ fn test_invoke( let calldata = create_calldata(test_contract.get_instance_address(0), entry_point_name, &entry_point_args); - let tx = account_invoke_tx(invoke_tx_args! { + let invoke_tx = invoke_tx(invoke_tx_args! { sender_address: account_contract.get_instance_address(0), calldata, version, - }) - .into(); + }); + let only_query = false; + let charge_fee = enforce_fee(&invoke_tx, only_query); + let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() }; + let tx = AccountTransaction { tx: invoke_tx, execution_flags }.into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } diff --git a/crates/blockifier/src/concurrency/test_utils.rs b/crates/blockifier/src/concurrency/test_utils.rs index 96e8d20261..a14e96de6f 100644 --- a/crates/blockifier/src/concurrency/test_utils.rs +++ b/crates/blockifier/src/concurrency/test_utils.rs @@ -76,7 +76,7 @@ pub fn create_fee_transfer_call_info( ) -> CallInfo { let block_context = BlockContext::create_for_account_testing(); let mut transactional_state = TransactionalState::create_transactional(state); - let execution_flags = ExecutionFlags { charge_fee: true, validate: true, concurrency_mode }; + let execution_flags = ExecutionFlags { concurrency_mode }; let execution_info = account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap(); diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index c817263ddc..c83d19762a 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -44,9 +44,10 @@ use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; use crate::test_utils::{CairoVersion, BALANCE, DEFAULT_STRK_L1_GAS_PRICE}; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::objects::HasRelatedFeeType; use crate::transaction::test_utils::{default_all_resource_bounds, l1_resource_bounds}; -use crate::transaction::transactions::ExecutableTransaction; +use crate::transaction::transactions::{enforce_fee, ExecutableTransaction}; #[fixture] pub fn safe_versioned_state( @@ -225,7 +226,7 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) { let mut state_2 = TransactionalState::create_transactional(&mut versioned_state_proxy_2); // Prepare transactions - let deploy_account_tx_1 = deploy_account_tx( + let tx = deploy_account_tx( deploy_account_tx_args! { class_hash: account_without_validation.get_class_hash(), resource_bounds: l1_resource_bounds( @@ -235,7 +236,9 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) { }, &mut NonceManager::default(), ); - let enforce_fee = deploy_account_tx_1.enforce_fee(); + let enforce_fee = enforce_fee(&tx, false); + let execution_flags = ExecutionFlags { charge_fee: enforce_fee, ..ExecutionFlags::default() }; + let deploy_account_tx_1 = AccountTransaction { tx, execution_flags: execution_flags.clone() }; let class_hash = grindy_account.get_class_hash(); let ctor_storage_arg = felt!(1_u8); @@ -247,7 +250,11 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) { constructor_calldata: constructor_calldata.clone(), }; let nonce_manager = &mut NonceManager::default(); - let delpoy_account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager); + let delpoy_account_tx_2 = AccountTransaction { + tx: deploy_account_tx(deploy_tx_args, nonce_manager), + execution_flags, + }; + let account_address = delpoy_account_tx_2.sender_address(); let tx_context = block_context.to_tx_context(&delpoy_account_tx_2); let fee_type = tx_context.tx_info.fee_type(); diff --git a/crates/blockifier/src/concurrency/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index af2ec35c36..dee56aa6aa 100644 --- a/crates/blockifier/src/concurrency/worker_logic.rs +++ b/crates/blockifier/src/concurrency/worker_logic.rs @@ -17,11 +17,7 @@ use crate::concurrency::TxIndex; use crate::context::BlockContext; use crate::state::cached_state::{ContractClassMapping, StateMaps, TransactionalState}; use crate::state::state_api::{StateReader, UpdatableState}; -use crate::transaction::objects::{ - TransactionExecutionInfo, - TransactionExecutionResult, - TransactionInfoCreator, -}; +use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult}; use crate::transaction::transaction_execution::Transaction; use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; @@ -127,11 +123,9 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { fn execute_tx(&self, tx_index: TxIndex) { let mut tx_versioned_state = self.state.pin_version(tx_index); let tx = &self.chunk[tx_index]; - let tx_charge_fee = tx.create_tx_info().enforce_fee(); let mut transactional_state = TransactionalState::create_transactional(&mut tx_versioned_state); - let execution_flags = - ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: true }; + let execution_flags = ExecutionFlags { concurrency_mode: true }; let execution_result = tx.execute_raw(&mut transactional_state, self.block_context, execution_flags); diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index 057d4bafab..08b74c6a96 100644 --- a/crates/blockifier/src/concurrency/worker_logic_test.rs +++ b/crates/blockifier/src/concurrency/worker_logic_test.rs @@ -32,7 +32,7 @@ use crate::test_utils::{ BALANCE, TEST_ERC20_CONTRACT_ADDRESS2, }; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::objects::HasRelatedFeeType; use crate::transaction::test_utils::{ account_invoke_tx, @@ -561,6 +561,8 @@ fn test_deploy_before_declare( }, test_class_info.clone(), ); + let execution_flags = ExecutionFlags::default(); + let declare_account_tx = AccountTransaction { tx: declare_tx, execution_flags }; // Deploy test contract. let invoke_tx = account_invoke_tx(invoke_tx_args! { @@ -580,8 +582,10 @@ fn test_deploy_before_declare( nonce: nonce!(0_u8) }); - let txs = - [declare_tx, invoke_tx].into_iter().map(Transaction::Account).collect::>(); + let txs = [declare_account_tx, invoke_tx] + .into_iter() + .map(Transaction::Account) + .collect::>(); let mut bouncer = Bouncer::new(block_context.bouncer_config.clone()); let worker_executor = diff --git a/crates/blockifier/src/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index 80e77478a6..16d2cdb5d9 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -45,6 +45,7 @@ use crate::execution::syscalls::hint_processor::ENTRYPOINT_FAILED_ERROR; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::initial_test_state::{fund_account, test_state}; use crate::test_utils::{create_calldata, CairoVersion, BALANCE}; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::test_utils::{ account_invoke_tx, block_context, @@ -608,6 +609,11 @@ fn test_validate_trace( } } + // TODO(AvivG): Change this fixup to not create account_tx twice w wrong charge_fee. + let execution_flags = + ExecutionFlags { charge_fee: account_tx.enforce_fee(), ..ExecutionFlags::default() }; + let account_tx = AccountTransaction { tx: account_tx.tx, execution_flags }; + let contract_address = *sender_address.0.key(); let expected_error = match cairo_version { diff --git a/crates/blockifier/src/test_utils/declare.rs b/crates/blockifier/src/test_utils/declare.rs index aaaf9b8e24..4a0616a4bf 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -1,10 +1,9 @@ use starknet_api::contract_class::ClassInfo; +use starknet_api::executable_transaction::AccountTransaction; use starknet_api::test_utils::declare::{executable_declare_tx, DeclareTxArgs}; -use crate::transaction::account_transaction::AccountTransaction; - pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> AccountTransaction { let declare_tx = executable_declare_tx(declare_tx_args, class_info); - declare_tx.into() + AccountTransaction::Declare(declare_tx) } diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index e2d6ed36fe..f99ce79460 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -1,13 +1,12 @@ +use starknet_api::executable_transaction::AccountTransaction; use starknet_api::test_utils::deploy_account::{executable_deploy_account_tx, DeployAccountTxArgs}; use starknet_api::test_utils::NonceManager; -use crate::transaction::account_transaction::AccountTransaction; - pub fn deploy_account_tx( deploy_tx_args: DeployAccountTxArgs, nonce_manager: &mut NonceManager, ) -> AccountTransaction { let deploy_account_tx = executable_deploy_account_tx(deploy_tx_args, nonce_manager); - deploy_account_tx.into() + AccountTransaction::DeployAccount(deploy_account_tx) } diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index 569ca98861..ae7e4b2be5 100644 --- a/crates/blockifier/src/test_utils/invoke.rs +++ b/crates/blockifier/src/test_utils/invoke.rs @@ -1,14 +1,6 @@ -use starknet_api::executable_transaction::AccountTransaction as ExecutableTransaction; +use starknet_api::executable_transaction::AccountTransaction; use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs}; -use crate::transaction::account_transaction::AccountTransaction; - pub fn invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { - let only_query = invoke_args.only_query; - let invoke_tx = ExecutableTransaction::Invoke(executable_invoke_tx(invoke_args)); - - match only_query { - true => AccountTransaction::new_for_query(invoke_tx), - false => AccountTransaction::new(invoke_tx), - } + AccountTransaction::Invoke(executable_invoke_tx(invoke_args)) } diff --git a/crates/blockifier/src/test_utils/transfers_generator.rs b/crates/blockifier/src/test_utils/transfers_generator.rs index 0915dc82f1..6833e78a20 100644 --- a/crates/blockifier/src/test_utils/transfers_generator.rs +++ b/crates/blockifier/src/test_utils/transfers_generator.rs @@ -2,6 +2,7 @@ use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; use starknet_api::abi::abi_utils::selector_from_name; use starknet_api::core::ContractAddress; +use starknet_api::executable_transaction::AccountTransaction as ApiExecutableTransaction; use starknet_api::test_utils::NonceManager; use starknet_api::transaction::constants::TRANSFER_ENTRY_POINT_NAME; use starknet_api::transaction::fields::Fee; @@ -17,9 +18,9 @@ use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; use crate::test_utils::invoke::invoke_tx; use crate::test_utils::{CairoVersion, BALANCE, MAX_FEE}; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::transaction_execution::Transaction; - +use crate::transaction::transactions::enforce_fee; const N_ACCOUNTS: u16 = 10000; const N_TXS: usize = 1000; const RANDOMIZATION_SEED: u64 = 0; @@ -143,7 +144,14 @@ impl TransfersGenerator { let recipient_address = self.get_next_recipient(); self.sender_index = (self.sender_index + 1) % self.account_addresses.len(); - let account_tx = self.generate_transfer(sender_address, recipient_address); + let tx = self.generate_transfer(sender_address, recipient_address); + let only_query = false; + let execution_flags = ExecutionFlags { + charge_fee: enforce_fee(&tx, only_query), + only_query, + ..ExecutionFlags::default() + }; + let account_tx = AccountTransaction { tx, execution_flags }; txs.push(Transaction::Account(account_tx)); } let results = self.executor.execute_txs(&txs); @@ -159,7 +167,7 @@ impl TransfersGenerator { &mut self, sender_address: ContractAddress, recipient_address: ContractAddress, - ) -> AccountTransaction { + ) -> ApiExecutableTransaction { let nonce = self.nonce_manager.next(sender_address); let entry_point_selector = selector_from_name(TRANSFER_ENTRY_POINT_NAME); diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index ca61f75576..fabeac7e7a 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -6,12 +6,7 @@ use starknet_api::calldata; use starknet_api::contract_class::EntryPointType; use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; -use starknet_api::executable_transaction::{ - AccountTransaction as Transaction, - DeclareTransaction, - DeployAccountTransaction, - InvokeTransaction, -}; +use starknet_api::executable_transaction::AccountTransaction as Transaction; use starknet_api::transaction::fields::Resource::{L1DataGas, L1Gas, L2Gas}; use starknet_api::transaction::fields::{ AccountDeploymentData, @@ -66,7 +61,7 @@ use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{ Executable, ExecutableTransaction, - ExecutionFlags, + ExecutionFlags as TransactionExecutionFlags, ValidatableTransaction, }; @@ -82,11 +77,24 @@ mod flavors_test; #[path = "post_execution_test.rs"] mod post_execution_test; +#[derive(Clone, Debug, derive_more::From)] +pub struct ExecutionFlags { + pub only_query: bool, + pub charge_fee: bool, + pub validate: bool, +} + +impl Default for ExecutionFlags { + fn default() -> Self { + Self { only_query: false, charge_fee: true, validate: true } + } +} + /// Represents a paid Starknet transaction. #[derive(Clone, Debug, derive_more::From)] pub struct AccountTransaction { pub tx: Transaction, - only_query: bool, + pub execution_flags: ExecutionFlags, } // TODO(AvivG): create additional macro that returns a reference. macro_rules! implement_tx_getter_calls { @@ -97,30 +105,6 @@ macro_rules! implement_tx_getter_calls { }; } -impl From for AccountTransaction { - fn from(tx: Transaction) -> Self { - Self::new(tx) - } -} - -impl From for AccountTransaction { - fn from(tx: DeclareTransaction) -> Self { - Transaction::Declare(tx).into() - } -} - -impl From for AccountTransaction { - fn from(tx: DeployAccountTransaction) -> Self { - Transaction::DeployAccount(tx).into() - } -} - -impl From for AccountTransaction { - fn from(tx: InvokeTransaction) -> Self { - Transaction::Invoke(tx).into() - } -} - impl HasRelatedFeeType for AccountTransaction { fn version(&self) -> TransactionVersion { self.tx.version() @@ -144,14 +128,6 @@ impl AccountTransaction { (paymaster_data, PaymasterData) ); - pub fn new(tx: starknet_api::executable_transaction::AccountTransaction) -> Self { - AccountTransaction { tx, only_query: false } - } - - pub fn new_for_query(tx: starknet_api::executable_transaction::AccountTransaction) -> Self { - AccountTransaction { tx, only_query: true } - } - pub fn class_hash(&self) -> Option { match &self.tx { Transaction::Declare(tx) => Some(tx.tx.class_hash()), @@ -745,7 +721,7 @@ impl ExecutableTransaction for AccountTransaction { &self, state: &mut TransactionalState<'_, U>, block_context: &BlockContext, - execution_flags: ExecutionFlags, + execution_flags_: TransactionExecutionFlags, ) -> TransactionExecutionResult { let tx_context = Arc::new(block_context.to_tx_context(self)); self.verify_tx_version(tx_context.tx_info.version())?; @@ -755,7 +731,7 @@ impl ExecutableTransaction for AccountTransaction { self.perform_pre_validation_stage( state, &tx_context, - execution_flags.charge_fee, + self.execution_flags.charge_fee, strict_nonce_check, )?; @@ -776,15 +752,15 @@ impl ExecutableTransaction for AccountTransaction { state, &mut remaining_gas, tx_context.clone(), - execution_flags.validate, - execution_flags.charge_fee, + self.execution_flags.validate, + self.execution_flags.charge_fee, )?; let fee_transfer_call_info = Self::handle_fee( state, tx_context, final_fee, - execution_flags.charge_fee, - execution_flags.concurrency_mode, + self.execution_flags.charge_fee, + execution_flags_.concurrency_mode, )?; let tx_execution_info = TransactionExecutionInfo { @@ -805,11 +781,7 @@ impl ExecutableTransaction for AccountTransaction { impl TransactionInfoCreator for AccountTransaction { fn create_tx_info(&self) -> TransactionInfo { - match &self.tx { - Transaction::Declare(tx) => tx.create_tx_info(self.only_query), - Transaction::DeployAccount(tx) => tx.create_tx_info(self.only_query), - Transaction::Invoke(tx) => tx.create_tx_info(self.only_query), - } + self.tx.create_tx_info(self.execution_flags.only_query) } } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index f677ada05f..3a946a3d33 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -66,6 +66,7 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::declare::declare_tx; use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::initial_test_state::{fund_account, test_state}; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::syscall::build_recurse_calldata; use crate::test_utils::{ create_calldata, @@ -83,7 +84,10 @@ use crate::test_utils::{ DEFAULT_STRK_L2_GAS_PRICE, MAX_FEE, }; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{ + AccountTransaction, + ExecutionFlags as AccountExecutionFlags, +}; use crate::transaction::objects::{HasRelatedFeeType, TransactionInfoCreator}; use crate::transaction::test_utils::{ account_invoke_tx, @@ -104,7 +108,7 @@ use crate::transaction::test_utils::{ INVALID, }; use crate::transaction::transaction_types::TransactionType; -use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; +use crate::transaction::transactions::{enforce_fee, ExecutableTransaction, ExecutionFlags}; use crate::utils::u64_from_usize; #[rstest] @@ -190,7 +194,7 @@ fn test_fee_enforcement( ) { let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0); let state = &mut test_state(&block_context.chain_info, BALANCE, &[(account, 1)]); - let deploy_account_tx = deploy_account_tx( + let tx = deploy_account_tx( deploy_account_tx_args! { class_hash: account.get_class_hash(), max_fee: Fee(if zero_bounds { 0 } else { MAX_FEE.0 }), @@ -212,6 +216,11 @@ fn test_fee_enforcement( }, &mut NonceManager::default(), ); + let only_query = false; + let charge_fee = enforce_fee(&tx, only_query); + let execution_flags = + AccountExecutionFlags { only_query, charge_fee, ..AccountExecutionFlags::default() }; + let deploy_account_tx = AccountTransaction { tx, execution_flags }; let enforce_fee = deploy_account_tx.enforce_fee(); assert_ne!(zero_bounds, enforce_fee); @@ -451,7 +460,7 @@ fn test_max_fee_limit_validate( let class_info = calculate_class_info_for_testing(grindy_validate_account.get_class()); // Declare the grindy-validation account. - let account_tx = declare_tx( + let tx = declare_tx( declare_tx_args! { class_hash: grindy_class_hash, sender_address: account_address, @@ -460,6 +469,8 @@ fn test_max_fee_limit_validate( }, class_info, ); + let execution_flags = AccountExecutionFlags::default(); + let account_tx = AccountTransaction { tx, execution_flags }; account_tx.execute(&mut state, &block_context, true, true).unwrap(); // Deploy grindy account with a lot of grind in the constructor. @@ -775,12 +786,15 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { state.set_contract_class(class_hash, contract_class.clone().try_into().unwrap()).unwrap(); state.set_compiled_class_hash(class_hash, declare_tx_v2.compiled_class_hash).unwrap(); let class_info = calculate_class_info_for_testing(contract_class); - let declare_account_tx: AccountTransaction = ApiExecutableDeclareTransaction { + let executable_declare = ApiExecutableDeclareTransaction { tx: DeclareTransaction::V2(DeclareTransactionV2 { nonce: next_nonce, ..declare_tx_v2 }), tx_hash: TransactionHash::default(), class_info, - } - .into(); + }; + let declare_account_tx = AccountTransaction { + tx: ApiExecutableTransaction::Declare(executable_declare), + execution_flags: AccountExecutionFlags::default(), + }; // Fail execution, assert nonce and balance are unchanged. let tx_info = declare_account_tx.create_tx_info(); @@ -1358,8 +1372,7 @@ fn test_count_actual_storage_changes( nonce: nonce_manager.next(account_address), }; let account_tx = account_invoke_tx(invoke_args.clone()); - let execution_flags = - ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: false }; + let execution_flags = ExecutionFlags { concurrency_mode: false }; let execution_info = account_tx.execute_raw(&mut state, &block_context, execution_flags).unwrap(); @@ -1529,8 +1542,7 @@ fn test_concurrency_execute_fee_transfer( // Case 1: The transaction did not read form/ write to the sequenser balance before executing // fee transfer. let mut transactional_state = TransactionalState::create_transactional(state); - let execution_flags = - ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true }; + let execution_flags = ExecutionFlags { concurrency_mode: true }; let result = account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap(); assert!(!result.is_reverted()); @@ -1625,8 +1637,7 @@ fn test_concurrent_fee_transfer_when_sender_is_sequencer( let fee_token_address = block_context.chain_info.fee_token_address(fee_type); let mut transactional_state = TransactionalState::create_transactional(state); - let execution_flags = - ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true }; + let execution_flags = ExecutionFlags { concurrency_mode: true }; let result = account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap(); assert!(!result.is_reverted()); @@ -1753,12 +1764,13 @@ fn test_revert_in_execute( // Skip validate phase, as we want to test the revert in the execute phase. let validate = false; - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { resource_bounds: default_all_resource_bounds, ..tx_args - }) - .execute(state, &block_context, true, validate) - .unwrap(); + }); + let execution_flags = AccountExecutionFlags { validate, ..AccountExecutionFlags::default() }; + let account_tx = AccountTransaction { tx, execution_flags }; + let tx_execution_info = account_tx.execute(state, &block_context, true, validate).unwrap(); assert!(tx_execution_info.is_reverted()); assert!( diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs index fbae899ebf..2c2c6409ab 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -26,6 +26,7 @@ use crate::state::state_api::StateReader; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::{ create_calldata, create_trivial_calldata, @@ -37,6 +38,7 @@ use crate::test_utils::{ DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE, }; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, @@ -222,9 +224,10 @@ fn test_invalid_nonce_pre_validate( // First scenario: invalid nonce. Regardless of flags, should fail. let invalid_nonce = nonce!(7_u8); let account_nonce = state.get_nonce_at(account_address).unwrap(); - let result = - account_invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args}) - .execute(&mut state, &block_context, charge_fee, validate); + let tx = invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args}); + let execution_flags = ExecutionFlags { only_query, charge_fee, validate }; + let account_tx = AccountTransaction { tx, execution_flags }; + let result = account_tx.execute(&mut state, &block_context, charge_fee, validate); assert_matches!( result.unwrap_err(), TransactionExecutionError::TransactionPreValidationError( @@ -265,6 +268,7 @@ fn test_simulate_validate_pre_validate_with_charge_fee( max_fee: Fee(10), resource_bounds: l1_resource_bounds(10_u8.into(), 10_u8.into()), nonce: nonce_manager.next(account_address), + ..pre_validation_base_args.clone() }) .execute(&mut state, &block_context, charge_fee, validate) @@ -294,16 +298,21 @@ fn test_simulate_validate_pre_validate_with_charge_fee( // Second scenario: resource bounds greater than balance. let gas_price = block_context.block_info.gas_prices.l1_gas_price(&fee_type); let balance_over_gas_price = BALANCE.checked_div(gas_price).unwrap(); - let result = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: Fee(BALANCE.0 + 1), resource_bounds: l1_resource_bounds( (balance_over_gas_price.0 + 10).into(), gas_price.into() ), nonce: nonce_manager.next(account_address), + ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let result = account_tx.execute(&mut state, &block_context, charge_fee, validate); nonce_manager.rollback(account_address); if is_deprecated { @@ -329,13 +338,17 @@ fn test_simulate_validate_pre_validate_with_charge_fee( // Third scenario: L1 gas price bound lower than the price on the block. if !is_deprecated { - let err = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { resource_bounds: l1_resource_bounds(DEFAULT_L1_GAS_AMOUNT, (gas_price.get().0 - 1).into()), nonce: nonce_manager.next(account_address), + ..pre_validation_base_args - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap_err(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let err = account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap_err(); nonce_manager.rollback(account_address); assert_matches!( @@ -368,12 +381,16 @@ fn test_simulate_validate_pre_validate_not_charge_fee( get_pre_validate_test_args(cairo_version, version, only_query); let account_address = pre_validation_base_args.sender_address; - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { nonce: nonce_manager.next(account_address), ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, false) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate: false }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, false).unwrap(); let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, false); assert!( base_gas @@ -387,14 +404,19 @@ fn test_simulate_validate_pre_validate_not_charge_fee( let (actual_gas_used, actual_fee) = gas_and_fee(base_gas, validate, &fee_type); macro_rules! execute_and_check_gas_and_fee { ($max_fee:expr, $resource_bounds:expr) => {{ - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: $max_fee, resource_bounds: $resource_bounds, nonce: nonce_manager.next(account_address), + ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); check_gas_and_fee( &block_context, &tx_execution_info, @@ -447,7 +469,7 @@ fn execute_fail_validation( } = create_flavors_test_state(&block_context.chain_info, cairo_version); // Validation scenario: fallible validation. - account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee, resource_bounds: max_resource_bounds, signature: TransactionSignature(vec![ @@ -458,9 +480,12 @@ fn execute_fail_validation( calldata: create_calldata(faulty_account_address, "foo", &[]), version, nonce: nonce_manager.next(faulty_account_address), - only_query, - }) - .execute(&mut falliable_state, &block_context, charge_fee, validate) + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + account_tx.execute(&mut falliable_state, &block_context, charge_fee, validate) } /// Test simulate / charge_fee flag combinations in (fallible) validation stage. @@ -576,13 +601,18 @@ fn test_simulate_validate_charge_fee_mid_execution( }; // First scenario: logic error. Should result in revert; actual fee should be shown. - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { calldata: recurse_calldata(test_contract_address, true, 3), nonce: nonce_manager.next(account_address), + only_query, ..execution_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, validate); let (revert_gas_used, revert_fee) = gas_and_fee(base_gas, validate, &fee_type); assert!( @@ -622,15 +652,21 @@ fn test_simulate_validate_charge_fee_mid_execution( validate, &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: fee_bound, resource_bounds: l1_resource_bounds(gas_bound, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 1000), nonce: nonce_manager.next(account_address), + only_query, + ..execution_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { assert!( @@ -675,15 +711,20 @@ fn test_simulate_validate_charge_fee_mid_execution( GasVector::from_l1_gas(block_limit_gas), &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + + let tx = invoke_tx(invoke_tx_args! { max_fee: huge_fee, resource_bounds: l1_resource_bounds(huge_gas_limit, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 10000), nonce: nonce_manager.next(account_address), ..execution_base_args - }) - .execute(&mut state, &low_step_block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &low_step_block_context, charge_fee, validate).unwrap(); assert!( tx_execution_info.revert_error.clone().unwrap().to_string().contains("no remaining steps") ); @@ -759,7 +800,7 @@ fn test_simulate_validate_charge_fee_post_execution( validate, &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: just_not_enough_fee_bound, resource_bounds: l1_resource_bounds(just_not_enough_gas_bound, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 1000), @@ -767,9 +808,13 @@ fn test_simulate_validate_charge_fee_post_execution( sender_address: account_address, version, only_query, - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { let expected_error_prefix = @@ -819,7 +864,7 @@ fn test_simulate_validate_charge_fee_post_execution( felt!(0_u8), ], ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: actual_fee, resource_bounds: l1_resource_bounds(success_actual_gas, gas_price.into()), calldata: transfer_calldata, @@ -827,9 +872,14 @@ fn test_simulate_validate_charge_fee_post_execution( sender_address: account_address, version, only_query, - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + + }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index 36757124a4..d03880fdec 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -21,6 +21,8 @@ use starknet_api::{calldata, declare_tx_args, deploy_account_tx_args, felt, invo use starknet_types_core::felt::Felt; use strum::IntoEnumIterator; +use super::account_transaction::ExecutionFlags; +use super::transactions::enforce_fee; use crate::context::{BlockContext, ChainInfo}; use crate::state::cached_state::CachedState; use crate::state::state_api::State; @@ -42,7 +44,7 @@ use crate::test_utils::{ DEFAULT_STRK_L2_GAS_PRICE, MAX_FEE, }; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult}; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::ExecutableTransaction; @@ -113,7 +115,10 @@ pub fn deploy_and_fund_account( deploy_tx_args: DeployAccountTxArgs, ) -> (AccountTransaction, ContractAddress) { // Deploy an account contract. - let deploy_account_tx = deploy_account_tx(deploy_tx_args, nonce_manager); + let deploy_account_tx = AccountTransaction { + tx: deploy_account_tx(deploy_tx_args, nonce_manager), + execution_flags: ExecutionFlags::default(), + }; let account_address = deploy_account_tx.sender_address(); // Update the balance of the about-to-be deployed account contract in the erc20 contract, so it @@ -162,6 +167,9 @@ pub struct FaultyAccountTxCreatorArgs { pub validate_constructor: bool, // Should be used with tx_type Declare. pub declared_contract: Option, + pub validate: bool, + pub only_query: bool, + pub charge_fee: bool, } impl Default for FaultyAccountTxCreatorArgs { @@ -178,6 +186,9 @@ impl Default for FaultyAccountTxCreatorArgs { max_fee: Fee::default(), resource_bounds: ValidResourceBounds::create_for_testing_no_fee_enforcement(), declared_contract: None, + validate: true, + only_query: false, + charge_fee: true, } } } @@ -214,6 +225,9 @@ pub fn create_account_tx_for_validate_test( max_fee, resource_bounds, declared_contract, + validate, + only_query, + charge_fee, } = faulty_account_tx_creator_args; // The first felt of the signature is used to set the scenario. If the scenario is @@ -223,7 +237,7 @@ pub fn create_account_tx_for_validate_test( signature_vector.extend(additional_data); } let signature = TransactionSignature(signature_vector); - + let execution_flags = ExecutionFlags { validate, charge_fee, only_query }; match tx_type { TransactionType::Declare => { let declared_contract = match declared_contract { @@ -235,7 +249,7 @@ pub fn create_account_tx_for_validate_test( }; let class_hash = declared_contract.get_class_hash(); let class_info = calculate_class_info_for_testing(declared_contract.get_class()); - declare_tx( + let tx = declare_tx( declare_tx_args! { max_fee, resource_bounds, @@ -247,7 +261,8 @@ pub fn create_account_tx_for_validate_test( compiled_class_hash: declared_contract.get_compiled_class_hash(), }, class_info, - ) + ); + AccountTransaction { tx, execution_flags } } TransactionType::DeployAccount => { // We do not use the sender address here because the transaction generates the actual @@ -256,7 +271,7 @@ pub fn create_account_tx_for_validate_test( true => constants::FELT_TRUE, false => constants::FELT_FALSE, })]; - deploy_account_tx( + let tx = deploy_account_tx( deploy_account_tx_args! { max_fee, resource_bounds, @@ -267,11 +282,12 @@ pub fn create_account_tx_for_validate_test( constructor_calldata, }, nonce_manager, - ) + ); + AccountTransaction { tx, execution_flags } } TransactionType::InvokeFunction => { let execute_calldata = create_calldata(sender_address, "foo", &[]); - invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee, resource_bounds, signature, @@ -279,14 +295,18 @@ pub fn create_account_tx_for_validate_test( calldata: execute_calldata, version: tx_version, nonce: nonce_manager.next(sender_address), - }) + + }); + AccountTransaction { tx, execution_flags } } _ => panic!("{tx_type:?} is not an account transaction."), } } pub fn account_invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction { - invoke_tx(invoke_args) + let only_query = invoke_args.only_query; + let execution_flags = ExecutionFlags { only_query, ..ExecutionFlags::default() }; + AccountTransaction { tx: invoke_tx(invoke_args), execution_flags } } pub fn run_invoke_tx( @@ -294,10 +314,14 @@ pub fn run_invoke_tx( block_context: &BlockContext, invoke_args: InvokeTxArgs, ) -> TransactionExecutionResult { - let tx = account_invoke_tx(invoke_args); - let charge_fee = tx.enforce_fee(); + let only_query = invoke_args.only_query; + let tx = invoke_tx(invoke_args); + let charge_fee = enforce_fee(&tx, only_query); + let execution_flags = ExecutionFlags { charge_fee, only_query, ..ExecutionFlags::default() }; + let account_tx = AccountTransaction { tx, execution_flags }; + // let charge_fee = tx.enforce_fee(); - tx.execute(state, block_context, charge_fee, true) + account_tx.execute(state, block_context, charge_fee, true) } /// Creates a `ResourceBoundsMapping` with the given `max_amount` and `max_price` for L1 gas limits. @@ -367,9 +391,13 @@ pub fn emit_n_events_tx( felt!(0_u32), // data length. ]; let calldata = create_calldata(contract_address, "test_emit_events", &entry_point_args); - account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { sender_address: account_contract, calldata, nonce - }) + }); + let only_query = false; + let charge_fee = enforce_fee(&tx, only_query); + let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() }; + AccountTransaction { tx, execution_flags } } diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 864267d8a0..1351abb18b 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -19,7 +19,10 @@ use crate::execution::entry_point::EntryPointExecutionContext; use crate::fee::receipt::TransactionReceipt; use crate::state::cached_state::TransactionalState; use crate::state::state_api::UpdatableState; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{ + AccountTransaction, + ExecutionFlags as AccountExecutionFlags, +}; use crate::transaction::errors::TransactionFeeError; use crate::transaction::objects::{ TransactionExecutionInfo, @@ -40,7 +43,10 @@ impl From for Transaction { fn from(value: starknet_api::executable_transaction::Transaction) -> Self { match value { starknet_api::executable_transaction::Transaction::Account(tx) => { - Transaction::Account(tx.into()) + Transaction::Account(AccountTransaction { + tx, + execution_flags: AccountExecutionFlags::default(), + }) } starknet_api::executable_transaction::Transaction::L1Handler(tx) => { Transaction::L1Handler(tx) @@ -78,6 +84,8 @@ impl Transaction { paid_fee_on_l1: Option, deployed_contract_address: Option, only_query: bool, + charge_fee: bool, + validate: bool, ) -> TransactionExecutionResult { let executable_tx = match tx { StarknetApiTransaction::L1Handler(l1_handler) => { @@ -119,11 +127,11 @@ impl Transaction { } _ => unimplemented!(), }; - let account_tx = match only_query { - true => AccountTransaction::new_for_query(executable_tx), - false => AccountTransaction::new(executable_tx), - }; - Ok(account_tx.into()) + Ok(AccountTransaction { + tx: executable_tx, + execution_flags: AccountExecutionFlags { only_query, charge_fee, validate }, + } + .into()) } } diff --git a/crates/blockifier/src/transaction/transactions.rs b/crates/blockifier/src/transaction/transactions.rs index 9005f73e4d..1214845b1b 100644 --- a/crates/blockifier/src/transaction/transactions.rs +++ b/crates/blockifier/src/transaction/transactions.rs @@ -4,6 +4,7 @@ use starknet_api::abi::abi_utils::selector_from_name; use starknet_api::contract_class::EntryPointType; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress}; use starknet_api::executable_transaction::{ + AccountTransaction, DeclareTransaction, DeployAccountTransaction, InvokeTransaction, @@ -52,8 +53,6 @@ mod test; #[derive(Clone, Copy, Debug)] pub struct ExecutionFlags { - pub charge_fee: bool, - pub validate: bool, pub concurrency_mode: bool, } @@ -64,12 +63,12 @@ pub trait ExecutableTransaction: Sized { &self, state: &mut U, block_context: &BlockContext, - charge_fee: bool, - validate: bool, + _charge_fee: bool, + _validate: bool, ) -> TransactionExecutionResult { log::debug!("Executing Transaction..."); let mut transactional_state = TransactionalState::create_transactional(state); - let execution_flags = ExecutionFlags { charge_fee, validate, concurrency_mode: false }; + let execution_flags = ExecutionFlags { concurrency_mode: false }; let execution_result = self.execute_raw(&mut transactional_state, block_context, execution_flags); @@ -179,6 +178,16 @@ impl TransactionInfoCreator for L1HandlerTransaction { } } +impl TransactionInfoCreatorInner for AccountTransaction { + fn create_tx_info(&self, only_query: bool) -> TransactionInfo { + match self { + Self::Declare(tx) => tx.create_tx_info(only_query), + Self::DeployAccount(tx) => tx.create_tx_info(only_query), + Self::Invoke(tx) => tx.create_tx_info(only_query), + } + } +} + impl Executable for DeclareTransaction { fn run_execute( &self, @@ -393,6 +402,11 @@ impl TransactionInfoCreatorInner for InvokeTransaction { } } +/// Determines whether the fee should be enforced for the given transaction. +pub fn enforce_fee(tx: &AccountTransaction, only_query: bool) -> bool { + tx.create_tx_info(only_query).enforce_fee() +} + /// Attempts to declare a contract class by setting the contract class in the state with the /// specified class hash. fn try_declare( diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 5933e4b076..61b56a3c72 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -116,7 +116,7 @@ use crate::test_utils::{ MAX_FEE, TEST_SEQUENCER_ADDRESS, }; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, @@ -454,11 +454,13 @@ fn test_invoke_tx( calldata: Calldata(Arc::clone(&calldata.0)), resource_bounds, }); + let execution_flags = ExecutionFlags::default(); + let invoke_account_tx = AccountTransaction { tx: invoke_tx, execution_flags }; // Extract invoke transaction fields for testing, as it is consumed when creating an account // transaction. - let calldata_length = invoke_tx.calldata_length(); - let signature_length = invoke_tx.signature_length(); + let calldata_length = invoke_account_tx.calldata_length(); + let signature_length = invoke_account_tx.signature_length(); let state_changes_for_fee = StateChangesCount { n_storage_updates: 1, n_modified_contracts: 1, @@ -472,11 +474,12 @@ fn test_invoke_tx( None, ExecutionSummary::default(), ); - let sender_address = invoke_tx.sender_address(); + let sender_address = invoke_account_tx.sender_address(); - let tx_context = block_context.to_tx_context(&invoke_tx); + let tx_context = block_context.to_tx_context(&invoke_account_tx); - let actual_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap(); + let actual_execution_info = + invoke_account_tx.execute(state, block_context, true, true).unwrap(); let tracked_resource = account_contract.get_runnable_class().tracked_resource( &versioned_constants.min_compiler_version_for_sierra_gas, @@ -967,13 +970,16 @@ fn test_max_fee_exceeds_balance( )}; // Deploy. - let invalid_tx = deploy_account_tx( - deploy_account_tx_args! { - resource_bounds, - class_hash: test_contract.get_class_hash() - }, - &mut NonceManager::default(), - ); + let invalid_tx = AccountTransaction { + tx: deploy_account_tx( + deploy_account_tx_args! { + resource_bounds, + class_hash: test_contract.get_class_hash() + }, + &mut NonceManager::default(), + ), + execution_flags: ExecutionFlags::default(), + }; assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx); // V1 Invoke. @@ -997,15 +1003,18 @@ fn test_max_fee_exceeds_balance( // Declare. let contract_to_declare = FeatureContract::Empty(CairoVersion::Cairo1); let class_info = calculate_class_info_for_testing(contract_to_declare.get_class()); - let invalid_tx = declare_tx( - declare_tx_args! { - class_hash: contract_to_declare.get_class_hash(), - compiled_class_hash: contract_to_declare.get_compiled_class_hash(), - sender_address, - resource_bounds: $invalid_resource_bounds, - }, - class_info, - ); + let invalid_tx = AccountTransaction { + tx: declare_tx( + declare_tx_args! { + class_hash: contract_to_declare.get_class_hash(), + compiled_class_hash: contract_to_declare.get_compiled_class_hash(), + sender_address, + resource_bounds: $invalid_resource_bounds, + }, + class_info, + ), + execution_flags: ExecutionFlags::default(), + }; assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx); }; } @@ -1507,7 +1516,7 @@ fn test_declare_tx( None, ExecutionSummary::default(), ); - let account_tx = declare_tx( + let tx = declare_tx( declare_tx_args! { max_fee: MAX_FEE, sender_address, @@ -1519,6 +1528,7 @@ fn test_declare_tx( }, class_info.clone(), ); + let account_tx = AccountTransaction { tx, execution_flags: ExecutionFlags::default() }; // Check state before transaction application. assert_matches!( @@ -1628,7 +1638,7 @@ fn test_declare_tx( assert_eq!(contract_class_from_state, class_info.contract_class().try_into().unwrap()); // Checks that redeclaring the same contract fails. - let account_tx2 = declare_tx( + let tx2 = declare_tx( declare_tx_args! { max_fee: MAX_FEE, sender_address, @@ -1640,6 +1650,7 @@ fn test_declare_tx( }, class_info.clone(), ); + let account_tx2 = AccountTransaction { tx: tx2, execution_flags: ExecutionFlags::default() }; let result = account_tx2.execute(state, block_context, true, true); assert_matches!( result.unwrap_err(), @@ -1662,7 +1673,7 @@ fn test_declare_tx_v0(default_l1_resource_bounds: ValidResourceBounds) { let sender_address = account.get_instance_address(0); let mut nonce_manager = NonceManager::default(); - let account_tx = declare_tx( + let tx = declare_tx( declare_tx_args! { max_fee: Fee(0), sender_address, @@ -1674,6 +1685,10 @@ fn test_declare_tx_v0(default_l1_resource_bounds: ValidResourceBounds) { }, class_info.clone(), ); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { charge_fee: false, ..ExecutionFlags::default() }, + }; let actual_execution_info = account_tx.execute(state, block_context, false, true).unwrap(); // fee not charged for declare v0. @@ -1694,13 +1709,16 @@ fn test_deploy_account_tx( let account = FeatureContract::AccountWithoutValidations(cairo_version); let account_class_hash = account.get_class_hash(); let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]); - let deploy_account = deploy_account_tx( - deploy_account_tx_args! { - resource_bounds: default_all_resource_bounds, - class_hash: account_class_hash - }, - &mut nonce_manager, - ); + let deploy_account = AccountTransaction { + tx: deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, + class_hash: account_class_hash + }, + &mut nonce_manager, + ), + execution_flags: ExecutionFlags::default(), + }; // Extract deploy account transaction fields for testing, as it is consumed when creating an // account transaction. @@ -1851,13 +1869,16 @@ fn test_deploy_account_tx( // Negative flow. // Deploy to an existing address. - let deploy_account = deploy_account_tx( - deploy_account_tx_args! { - resource_bounds: default_all_resource_bounds, - class_hash: account_class_hash - }, - &mut nonce_manager, - ); + let deploy_account = AccountTransaction { + tx: deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, + class_hash: account_class_hash + }, + &mut nonce_manager, + ), + execution_flags: ExecutionFlags::default(), + }; let error = deploy_account.execute(state, block_context, true, true).unwrap_err(); assert_matches!( error, @@ -1882,12 +1903,15 @@ fn test_fail_deploy_account_undeclared_class_hash( let state = &mut test_state(chain_info, BALANCE, &[]); let mut nonce_manager = NonceManager::default(); let undeclared_hash = class_hash!("0xdeadbeef"); - let deploy_account = deploy_account_tx( - deploy_account_tx_args! { - resource_bounds: default_all_resource_bounds, class_hash: undeclared_hash - }, - &mut nonce_manager, - ); + let deploy_account = AccountTransaction { + tx: deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, class_hash: undeclared_hash + }, + &mut nonce_manager, + ), + execution_flags: ExecutionFlags::default(), + }; let tx_context = block_context.to_tx_context(&deploy_account); let fee_type = tx_context.tx_info.fee_type(); @@ -1948,11 +1972,16 @@ fn test_validate_accounts_tx( sender_address, class_hash, validate_constructor, + validate: true, + charge_fee: false, ..Default::default() }; // Negative flows. + // We test `__validate__`, and don't care about the cahrge fee flow. + let charge_fee = false; + // Logic failure. let account_tx = create_account_tx_for_validate_test_nonce_0(FaultyAccountTxCreatorArgs { scenario: INVALID, @@ -1960,8 +1989,6 @@ fn test_validate_accounts_tx( additional_data: None, ..default_args }); - // We test `__validate__`, and don't care about the cahrge fee flow. - let charge_fee = false; let error = account_tx.execute(state, block_context, charge_fee, true).unwrap_err(); check_tx_execution_error_for_invalid_scenario!(cairo_version, error, validate_constructor,); @@ -2127,11 +2154,15 @@ fn test_valid_flag( &[(account_contract, 1), (test_contract, 1)], ); - let account_tx = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { sender_address: account_contract.get_instance_address(0), calldata: create_trivial_calldata(test_contract.get_instance_address(0)), resource_bounds: default_all_resource_bounds, }); + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { validate: false, ..ExecutionFlags::default() }, + }; let actual_execution_info = account_tx.execute(state, block_context, true, false).unwrap(); @@ -2232,8 +2263,10 @@ fn test_only_query_flag( sender_address, only_query, }); + let execution_flags = ExecutionFlags { only_query, ..Default::default() }; + let invoke_account_tx = AccountTransaction { tx: invoke_tx, execution_flags }; - let tx_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap(); + let tx_execution_info = invoke_account_tx.execute(state, block_context, true, true).unwrap(); assert_eq!(tx_execution_info.revert_error, None); } diff --git a/crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs b/crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs index 3e3c3f030d..5272993583 100644 --- a/crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs +++ b/crates/blockifier_reexecution/src/state_reader/reexecution_state_reader.rs @@ -45,7 +45,9 @@ pub trait ReexecutionStateReader { .into_iter() .map(|(tx, tx_hash)| match tx { Transaction::Invoke(_) | Transaction::DeployAccount(_) => { - Ok(BlockifierTransaction::from_api(tx, tx_hash, None, None, None, false)?) + Ok(BlockifierTransaction::from_api( + tx, tx_hash, None, None, None, false, true, true, + )?) } Transaction::Declare(ref declare_tx) => { let class_info = self @@ -58,6 +60,8 @@ pub trait ReexecutionStateReader { None, None, false, + true, + true, )?) } Transaction::L1Handler(_) => Ok(BlockifierTransaction::from_api( @@ -67,6 +71,8 @@ pub trait ReexecutionStateReader { Some(MAX_FEE), None, false, + true, + true, )?), Transaction::Deploy(_) => { diff --git a/crates/native_blockifier/src/py_transaction.rs b/crates/native_blockifier/src/py_transaction.rs index bdf28426b0..b050e9d029 100644 --- a/crates/native_blockifier/src/py_transaction.rs +++ b/crates/native_blockifier/src/py_transaction.rs @@ -1,8 +1,9 @@ use std::collections::BTreeMap; -use blockifier::transaction::account_transaction::AccountTransaction; +use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use blockifier::transaction::transaction_execution::Transaction; use blockifier::transaction::transaction_types::TransactionType; +use blockifier::transaction::transactions::enforce_fee; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use starknet_api::block::GasPrice; @@ -133,23 +134,37 @@ pub fn py_tx( let tx_type = get_py_tx_type(tx)?; let tx_type: TransactionType = tx_type.parse().map_err(NativeBlockifierInputError::ParseError)?; + let only_query = false; Ok(match tx_type { TransactionType::Declare => { let non_optional_py_class_info: PyClassInfo = optional_py_class_info .expect("A class info must be passed in a Declare transaction."); - AccountTransaction::new(ExecutableTransaction::Declare(py_declare( - tx, - non_optional_py_class_info, - )?)) - .into() + let tx = ExecutableTransaction::Declare(py_declare(tx, non_optional_py_class_info)?); + let execution_flags = ExecutionFlags { + only_query, + charge_fee: enforce_fee(&tx, only_query), + ..ExecutionFlags::default() + }; + AccountTransaction { tx, execution_flags }.into() } TransactionType::DeployAccount => { - AccountTransaction::new(ExecutableTransaction::DeployAccount(py_deploy_account(tx)?)) - .into() + let tx = ExecutableTransaction::DeployAccount(py_deploy_account(tx)?); + let execution_flags = ExecutionFlags { + only_query, + charge_fee: enforce_fee(&tx, only_query), + ..ExecutionFlags::default() + }; + AccountTransaction { tx, execution_flags }.into() } TransactionType::InvokeFunction => { - AccountTransaction::new(ExecutableTransaction::Invoke(py_invoke_function(tx)?)).into() + let tx = ExecutableTransaction::Invoke(py_invoke_function(tx)?); + let execution_flags = ExecutionFlags { + only_query, + charge_fee: enforce_fee(&tx, only_query), + ..ExecutionFlags::default() + }; + AccountTransaction { tx, execution_flags }.into() } TransactionType::L1Handler => py_l1_handler(tx)?.into(), }) diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index 369222aec5..4b405eda7a 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -700,7 +700,7 @@ fn execute_transactions( ) => Some(*class_hash), _ => None, }; - let blockifier_tx = to_blockifier_tx(tx, tx_hash, transaction_index)?; + let blockifier_tx = to_blockifier_tx(tx, tx_hash, transaction_index, charge_fee, validate)?; // TODO(Yoni): use the TransactionExecutor instead. let tx_execution_info_result = blockifier_tx.execute(&mut transactional_state, &block_context, charge_fee, validate); @@ -762,6 +762,8 @@ fn to_blockifier_tx( tx: ExecutableTransactionInput, tx_hash: TransactionHash, transaction_index: usize, + charge_fee: bool, + validate: bool, ) -> ExecutionResult { // TODO(yair): support only_query version bit (enable in the RPC v0.6 and use the correct // value). @@ -774,6 +776,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -786,6 +790,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -813,6 +819,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -838,6 +846,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -862,6 +872,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -886,6 +898,8 @@ fn to_blockifier_tx( None, None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } @@ -897,6 +911,8 @@ fn to_blockifier_tx( Some(paid_fee), None, only_query, + charge_fee, + validate, ) .map_err(|err| ExecutionError::from((transaction_index, err))) } diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index cbe767fd9a..085235f832 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -15,8 +15,10 @@ use blockifier::execution::contract_class::RunnableCompiledClass; use blockifier::state::cached_state::CommitmentStateDiff; use blockifier::state::errors::StateError; use blockifier::state::global_cache::GlobalContractCache; +use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use blockifier::transaction::objects::TransactionExecutionInfo; use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction; +use blockifier::transaction::transactions::enforce_fee; use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOverrides}; use indexmap::IndexMap; #[cfg(test)] @@ -159,8 +161,26 @@ impl BlockBuilderTrait for BlockBuilder { let mut executor_input_chunk = vec![]; for tx in &next_tx_chunk { - // TODO(yair): Avoid this clone. - executor_input_chunk.push(BlockifierTransaction::from(tx.clone())); + let executable_tx = match tx { + Transaction::Account(account_tx) => { + let only_query = false; + let charge_fee = enforce_fee(&account_tx, only_query); + BlockifierTransaction::Account(AccountTransaction { + // TODO(yair): Avoid this clone. + tx: account_tx.clone(), + execution_flags: ExecutionFlags { + only_query, + charge_fee, + validate: true, + }, + }) + } + Transaction::L1Handler(l1_handler_tx) => { + // TODO(yair): Avoid this clone. + BlockifierTransaction::L1Handler(l1_handler_tx.clone()) + } + }; + executor_input_chunk.push(executable_tx); } let results = self.executor.add_txs_to_block(&executor_input_chunk); trace!("Transaction execution results: {:?}", results); diff --git a/crates/starknet_gateway/src/stateful_transaction_validator.rs b/crates/starknet_gateway/src/stateful_transaction_validator.rs index db04f52c79..d09080c94e 100644 --- a/crates/starknet_gateway/src/stateful_transaction_validator.rs +++ b/crates/starknet_gateway/src/stateful_transaction_validator.rs @@ -5,7 +5,8 @@ use blockifier::blockifier::stateful_validator::{ use blockifier::bouncer::BouncerConfig; use blockifier::context::{BlockContext, ChainInfo}; use blockifier::state::cached_state::CachedState; -use blockifier::transaction::account_transaction::AccountTransaction; +use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; +use blockifier::transaction::transactions::enforce_fee; use blockifier::versioned_constants::VersionedConstants; #[cfg(test)] use mockall::automock; @@ -75,7 +76,11 @@ impl StatefulTransactionValidator { mut validator: V, ) -> StatefulTransactionValidatorResult<()> { let skip_validate = skip_stateful_validations(executable_tx, account_nonce); - let account_tx = AccountTransaction::new(executable_tx.clone()); + let only_query = false; + let charge_fee = enforce_fee(&executable_tx, only_query); + let execution_flags = ExecutionFlags { only_query, charge_fee, validate: true }; + + let account_tx = AccountTransaction { tx: executable_tx.clone(), execution_flags }; validator .validate(account_tx, skip_validate) .map_err(|err| GatewaySpecError::ValidationFailure { data: err.to_string() })?;