diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index 1e9cf7299a1..ab894901e5e 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -30,7 +30,7 @@ use crate::test_utils::{ BALANCE, DEFAULT_STRK_L1_GAS_PRICE, }; -use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; +use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::test_utils::{ block_context, @@ -41,7 +41,6 @@ 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, @@ -132,10 +131,7 @@ fn test_declare( }, calculate_class_info_for_testing(declared_contract.get_class()), ); - 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(); + let tx = AccountTransaction::new_for_sequencing(declare_tx).into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } @@ -156,10 +152,7 @@ fn test_deploy_account( }, &mut NonceManager::default(), ); - let only_query = false; - let charge_fee = enforce_fee(&deploy_account_tx, only_query); - let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() }; - let tx = AccountTransaction { tx: deploy_account_tx, execution_flags }.into(); + let tx = AccountTransaction::new_for_sequencing(deploy_account_tx).into(); let expected_bouncer_weights = BouncerWeights { state_diff_size: 3, message_segment_length: 0, @@ -231,10 +224,7 @@ fn test_invoke( calldata, version, }); - 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(); + let tx = AccountTransaction::new_for_sequencing(invoke_tx).into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index 87727a1c362..f6c9e5b897f 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -44,10 +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::account_transaction::AccountTransaction; use crate::transaction::objects::HasRelatedFeeType; use crate::transaction::test_utils::{default_all_resource_bounds, l1_resource_bounds}; -use crate::transaction::transactions::{enforce_fee, ExecutableTransaction}; +use crate::transaction::transactions::ExecutableTransaction; #[fixture] pub fn safe_versioned_state( @@ -236,9 +236,8 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) { }, &mut NonceManager::default(), ); - 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 deploy_account_tx_1 = AccountTransaction::new_for_sequencing(tx); + let enforce_fee = deploy_account_tx_1.enforce_fee(); let class_hash = grindy_account.get_class_hash(); let ctor_storage_arg = felt!(1_u8); @@ -250,10 +249,8 @@ 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 = AccountTransaction { - tx: deploy_account_tx(deploy_tx_args, nonce_manager), - execution_flags, - }; + let tx = deploy_account_tx(deploy_tx_args, nonce_manager); + let delpoy_account_tx_2 = AccountTransaction::new_for_sequencing(tx); let account_address = delpoy_account_tx_2.sender_address(); let tx_context = block_context.to_tx_context(&delpoy_account_tx_2); diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index ee64bb82104..55a604223db 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, ExecutionFlags}; +use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::objects::HasRelatedFeeType; use crate::transaction::test_utils::{ account_invoke_tx, @@ -549,7 +549,7 @@ fn test_deploy_before_declare( let test_class_hash = test_contract.get_class_hash(); let test_class_info = calculate_class_info_for_testing(test_contract.get_class()); let test_compiled_class_hash = test_contract.get_compiled_class_hash(); - let tx = declare_tx( + let declare_tx = AccountTransaction::new_with_default_flags(declare_tx( declare_tx_args! { sender_address: account_address_0, resource_bounds: default_all_resource_bounds, @@ -560,9 +560,7 @@ fn test_deploy_before_declare( nonce: nonce!(0_u8), }, test_class_info.clone(), - ); - let execution_flags = ExecutionFlags::default(); - let declare_tx = AccountTransaction { tx, execution_flags }; + )); // Deploy test contract. let invoke_tx = account_invoke_tx(invoke_tx_args! { diff --git a/crates/blockifier/src/test_utils/transfers_generator.rs b/crates/blockifier/src/test_utils/transfers_generator.rs index 6833e78a205..f66cba4aca2 100644 --- a/crates/blockifier/src/test_utils/transfers_generator.rs +++ b/crates/blockifier/src/test_utils/transfers_generator.rs @@ -18,9 +18,8 @@ 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, ExecutionFlags}; +use crate::transaction::account_transaction::AccountTransaction; 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; @@ -145,13 +144,7 @@ impl TransfersGenerator { self.sender_index = (self.sender_index + 1) % self.account_addresses.len(); 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 }; + let account_tx = AccountTransaction::new_for_sequencing(tx); txs.push(Transaction::Account(account_tx)); } let results = self.executor.execute_txs(&txs); diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index b614bfde346..e56a5310d10 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -59,6 +59,7 @@ use crate::transaction::objects::{ }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{ + enforce_fee, Executable, ExecutableTransaction, ExecutionFlags as TransactionExecutionFlags, @@ -128,6 +129,19 @@ impl AccountTransaction { (paymaster_data, PaymasterData) ); + pub fn new_with_default_flags(tx: Transaction) -> Self { + Self { tx, execution_flags: ExecutionFlags::default() } + } + + pub fn new_for_sequencing(tx: Transaction) -> Self { + let execution_flags = ExecutionFlags { + only_query: false, + charge_fee: enforce_fee(&tx, false), + validate: true, + }; + AccountTransaction { tx, execution_flags } + } + pub fn class_hash(&self) -> Option { match &self.tx { Transaction::Declare(tx) => Some(tx.tx.class_hash()), diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 73db0ab828e..d5955f4d0c5 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -108,7 +108,7 @@ use crate::transaction::test_utils::{ INVALID, }; use crate::transaction::transaction_types::TransactionType; -use crate::transaction::transactions::{enforce_fee, ExecutableTransaction, ExecutionFlags}; +use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags}; use crate::utils::u64_from_usize; #[rstest] @@ -216,11 +216,7 @@ 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 deploy_account_tx = AccountTransaction::new_for_sequencing(tx); let enforce_fee = deploy_account_tx.enforce_fee(); assert_ne!(zero_bounds, enforce_fee); @@ -469,8 +465,7 @@ fn test_max_fee_limit_validate( }, class_info, ); - let execution_flags = AccountExecutionFlags::default(); - let account_tx = AccountTransaction { tx, execution_flags }; + let account_tx = AccountTransaction::new_with_default_flags(tx); account_tx.execute(&mut state, &block_context).unwrap(); // Deploy grindy account with a lot of grind in the constructor. @@ -791,10 +786,9 @@ fn test_fail_declare(block_context: BlockContext, max_fee: Fee) { tx_hash: TransactionHash::default(), class_info, }; - let declare_account_tx = AccountTransaction { - tx: ApiExecutableTransaction::Declare(executable_declare), - execution_flags: AccountExecutionFlags::default(), - }; + let declare_account_tx = AccountTransaction::new_with_default_flags( + ApiExecutableTransaction::Declare(executable_declare), + ); // Fail execution, assert nonce and balance are unchanged. let tx_info = declare_account_tx.create_tx_info(); diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index cf88e8cad5b..982b65bc915 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -113,10 +113,10 @@ pub fn deploy_and_fund_account( deploy_tx_args: DeployAccountTxArgs, ) -> (AccountTransaction, ContractAddress) { // Deploy an account contract. - let deploy_account_tx = AccountTransaction { - tx: deploy_account_tx(deploy_tx_args, nonce_manager), - execution_flags: ExecutionFlags::default(), - }; + let deploy_account_tx = AccountTransaction::new_with_default_flags(deploy_account_tx( + deploy_tx_args, + nonce_manager, + )); 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 @@ -315,8 +315,8 @@ pub fn run_invoke_tx( ) -> TransactionExecutionResult { 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 execution_flags = + ExecutionFlags { only_query, charge_fee: enforce_fee(&tx, only_query), validate: true }; let account_tx = AccountTransaction { tx, execution_flags }; account_tx.execute(state, block_context) @@ -394,9 +394,6 @@ pub fn emit_n_events_tx( 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 } + AccountTransaction::new_for_sequencing(tx) } diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 93c3ef5f0a8..d4b78de0485 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -43,10 +43,7 @@ impl From for Transaction { fn from(value: starknet_api::executable_transaction::Transaction) -> Self { match value { starknet_api::executable_transaction::Transaction::Account(tx) => { - Transaction::Account(AccountTransaction { - tx, - execution_flags: AccountExecutionFlags::default(), - }) + Transaction::Account(AccountTransaction::new_with_default_flags(tx)) } starknet_api::executable_transaction::Transaction::L1Handler(tx) => { Transaction::L1Handler(tx) diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 9c097c13cba..ea51654b654 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -454,13 +454,11 @@ fn test_invoke_tx( let test_contract_address = test_contract.get_instance_address(0); let account_contract_address = account_contract.get_instance_address(0); let calldata = create_trivial_calldata(test_contract_address); - let tx = invoke_tx(invoke_tx_args! { + let invoke_tx = AccountTransaction::new_with_default_flags(invoke_tx(invoke_tx_args! { sender_address: account_contract_address, calldata: Calldata(Arc::clone(&calldata.0)), resource_bounds, - }); - let execution_flags = ExecutionFlags::default(); - let invoke_tx = AccountTransaction { tx, execution_flags }; + })); // Extract invoke transaction fields for testing, as it is consumed when creating an account // transaction. @@ -973,16 +971,13 @@ fn test_max_fee_exceeds_balance( )}; // Deploy. - 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(), - }; + let invalid_tx = AccountTransaction::new_with_default_flags(deploy_account_tx( + deploy_account_tx_args! { + resource_bounds, + class_hash: test_contract.get_class_hash() + }, + &mut NonceManager::default(), + )); assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx); // V1 Invoke. @@ -1006,18 +1001,15 @@ 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 = 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(), - }; + let invalid_tx = AccountTransaction::new_with_default_flags(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, + )); assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx); }; } @@ -1520,7 +1512,7 @@ fn test_declare_tx( None, ExecutionSummary::default(), ); - let tx = declare_tx( + let account_tx = AccountTransaction::new_with_default_flags(declare_tx( declare_tx_args! { max_fee: MAX_FEE, sender_address, @@ -1531,8 +1523,7 @@ fn test_declare_tx( nonce: nonce_manager.next(sender_address), }, class_info.clone(), - ); - let account_tx = AccountTransaction { tx, execution_flags: ExecutionFlags::default() }; + )); // Check state before transaction application. assert_matches!( @@ -1641,7 +1632,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 tx2 = declare_tx( + let account_tx2 = AccountTransaction::new_with_default_flags(declare_tx( declare_tx_args! { max_fee: MAX_FEE, sender_address, @@ -1652,8 +1643,7 @@ fn test_declare_tx( nonce: nonce_manager.next(sender_address), }, class_info.clone(), - ); - let account_tx2 = AccountTransaction { tx: tx2, execution_flags: ExecutionFlags::default() }; + )); let result = account_tx2.execute(state, block_context); assert_matches!( result.unwrap_err(), @@ -1712,16 +1702,13 @@ 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 = 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 deploy_account = AccountTransaction::new_with_default_flags(deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, + class_hash: account_class_hash + }, + &mut nonce_manager, + )); // Extract deploy account transaction fields for testing, as it is consumed when creating an // account transaction. @@ -1871,16 +1858,13 @@ fn test_deploy_account_tx( // Negative flow. // Deploy to an existing address. - 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 deploy_account = AccountTransaction::new_with_default_flags(deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, + class_hash: account_class_hash + }, + &mut nonce_manager, + )); let error = deploy_account.execute(state, block_context).unwrap_err(); assert_matches!( error, @@ -1905,15 +1889,12 @@ 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 = 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 deploy_account = AccountTransaction::new_with_default_flags(deploy_account_tx( + deploy_account_tx_args! { + resource_bounds: default_all_resource_bounds, class_hash: undeclared_hash + }, + &mut nonce_manager, + )); let tx_context = block_context.to_tx_context(&deploy_account); let fee_type = tx_context.tx_info.fee_type(); diff --git a/crates/native_blockifier/src/py_transaction.rs b/crates/native_blockifier/src/py_transaction.rs index 8bc3fdf4b64..40e0f64f4ec 100644 --- a/crates/native_blockifier/src/py_transaction.rs +++ b/crates/native_blockifier/src/py_transaction.rs @@ -1,9 +1,8 @@ use std::collections::BTreeMap; -use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; +use blockifier::transaction::account_transaction::AccountTransaction; 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; @@ -137,37 +136,21 @@ 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."); 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() + AccountTransaction::new_for_sequencing(tx).into() } TransactionType::DeployAccount => { 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() + AccountTransaction::new_for_sequencing(tx).into() } TransactionType::InvokeFunction => { 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() + AccountTransaction::new_for_sequencing(tx).into() } TransactionType::L1Handler => py_l1_handler(tx)?.into(), }) diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index ab4f38ebef6..79b43509674 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -14,10 +14,9 @@ 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::account_transaction::AccountTransaction; 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)] @@ -156,17 +155,10 @@ impl BlockBuilderTrait for BlockBuilder { // 'match'. 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 { + BlockifierTransaction::Account(AccountTransaction::new_for_sequencing( // TODO(yair): Avoid this clone. - tx: account_tx.clone(), - execution_flags: ExecutionFlags { - only_query, - charge_fee, - validate: true, - }, - }) + account_tx.clone(), + )) } Transaction::L1Handler(l1_handler_tx) => { // TODO(yair): Avoid this clone.