From 23894e72d410063ad6b7c08b5f48bf6f4e05e6d8 Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Sun, 1 Dec 2024 15:05:42 +0200 Subject: [PATCH] chore(blockifier): ues account_transaction::ExecutionFlags instead of transaction::ExecutionFlags --- .../src/blockifier/transaction_executor.rs | 6 +- .../blockifier/transaction_executor_test.rs | 18 ++++-- .../blockifier/src/concurrency/test_utils.rs | 2 +- .../src/concurrency/worker_logic.rs | 10 +-- .../src/execution/stack_trace_test.rs | 6 ++ .../src/transaction/account_transaction.rs | 12 ++-- .../transaction/account_transactions_test.rs | 12 ++-- .../src/transaction/execution_flavors_test.rs | 63 +++++++++++++------ .../blockifier/src/transaction/test_utils.rs | 16 +++-- .../src/transaction/transaction_execution.rs | 8 +-- .../src/transaction/transactions.rs | 8 +-- .../src/transaction/transactions_test.rs | 12 +++- .../state_reader/reexecution_state_reader.rs | 8 ++- .../native_blockifier/src/py_transaction.rs | 23 ++++++- crates/papyrus_execution/src/lib.rs | 18 +++++- crates/starknet_batcher/src/block_builder.rs | 9 ++- .../src/stateful_transaction_validator.rs | 10 +-- 17 files changed, 164 insertions(+), 77 deletions(-) diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 4ad8bfb2857..f693daf8fff 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 377fb47f9fa..1e9cf7299a1 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -41,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, @@ -131,7 +132,9 @@ fn test_declare( }, calculate_class_info_for_testing(declared_contract.get_class()), ); - let execution_flags = ExecutionFlags::default(); + 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); } @@ -153,13 +156,17 @@ fn test_deploy_account( }, &mut NonceManager::default(), ); - let tx = AccountTransaction { tx: deploy_account_tx, execution_flags: ExecutionFlags::default() }.into(); + 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 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); } @@ -224,7 +231,10 @@ fn test_invoke( calldata, version, }); - let tx = AccountTransaction { tx: invoke_tx, only_query: false }.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 96e8d20261d..a14e96de6f8 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/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index af2ec35c36b..dee56aa6aa3 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/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index 80e77478a6e..16d2cdb5d97 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/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index a0151dbdc33..fabeac7e7a8 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -721,7 +721,7 @@ impl ExecutableTransaction for AccountTransaction { &self, state: &mut TransactionalState<'_, U>, block_context: &BlockContext, - execution_flags: TransactionExecutionFlags, + execution_flags_: TransactionExecutionFlags, ) -> TransactionExecutionResult { let tx_context = Arc::new(block_context.to_tx_context(self)); self.verify_tx_version(tx_context.tx_info.version())?; @@ -731,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, )?; @@ -752,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 { diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 14380cff44e..3a946a3d33f 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -1372,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(); @@ -1543,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()); @@ -1639,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()); @@ -1771,7 +1768,8 @@ fn test_revert_in_execute( resource_bounds: default_all_resource_bounds, ..tx_args }); - let account_tx = AccountTransaction { tx, only_query: false }; + 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()); diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs index ca91f29210e..2c2c6409abf 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -38,7 +38,7 @@ use crate::test_utils::{ DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE, }; -use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, @@ -225,7 +225,8 @@ fn test_invalid_nonce_pre_validate( let invalid_nonce = nonce!(7_u8); let account_nonce = state.get_nonce_at(account_address).unwrap(); let tx = invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args}); - let account_tx = AccountTransaction { tx, only_query }; + 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(), @@ -267,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) @@ -306,7 +308,10 @@ fn test_simulate_validate_pre_validate_with_charge_fee( ..pre_validation_base_args.clone() }); - let account_tx = AccountTransaction { tx, only_query }; + 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); @@ -339,7 +344,10 @@ fn test_simulate_validate_pre_validate_with_charge_fee( ..pre_validation_base_args }); - let account_tx = AccountTransaction { tx, only_query }; + 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); @@ -377,10 +385,12 @@ fn test_simulate_validate_pre_validate_not_charge_fee( nonce: nonce_manager.next(account_address), ..pre_validation_base_args.clone() }); - let account_tx = AccountTransaction { tx, only_query }; + 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 @@ -401,10 +411,12 @@ fn test_simulate_validate_pre_validate_not_charge_fee( ..pre_validation_base_args.clone() }); - let account_tx = AccountTransaction { tx, only_query }; + 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, @@ -469,7 +481,10 @@ fn execute_fail_validation( version, nonce: nonce_manager.next(faulty_account_address), }); - let account_tx = AccountTransaction { tx, only_query }; + let account_tx = AccountTransaction { + tx, + execution_flags: ExecutionFlags { only_query, charge_fee, validate }, + }; account_tx.execute(&mut falliable_state, &block_context, charge_fee, validate) } @@ -592,7 +607,10 @@ fn test_simulate_validate_charge_fee_mid_execution( only_query, ..execution_base_args.clone() }); - let account_tx = AccountTransaction { tx, only_query }; + 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); @@ -643,10 +661,12 @@ fn test_simulate_validate_charge_fee_mid_execution( ..execution_base_args.clone() }); - let account_tx = AccountTransaction { tx, only_query }; + 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!( @@ -691,6 +711,7 @@ fn test_simulate_validate_charge_fee_mid_execution( GasVector::from_l1_gas(block_limit_gas), &fee_type, ); + let tx = invoke_tx(invoke_tx_args! { max_fee: huge_fee, resource_bounds: l1_resource_bounds(huge_gas_limit, gas_price.into()), @@ -698,10 +719,12 @@ fn test_simulate_validate_charge_fee_mid_execution( nonce: nonce_manager.next(account_address), ..execution_base_args }); - let account_tx = AccountTransaction { tx, only_query }; + 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") ); @@ -786,10 +809,12 @@ fn test_simulate_validate_charge_fee_post_execution( version, only_query, }); - let account_tx = AccountTransaction { tx, only_query }; + 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 = @@ -849,10 +874,12 @@ fn test_simulate_validate_charge_fee_post_execution( only_query, }); - let account_tx = AccountTransaction { tx, only_query }; + 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 593fd95d5ab..3ab7b4796ad 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -21,7 +21,6 @@ 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 crate::context::{BlockContext, ChainInfo}; use crate::state::cached_state::CachedState; use crate::state::state_api::State; @@ -43,10 +42,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}; use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult}; use crate::transaction::transaction_types::TransactionType; -use crate::transaction::transactions::ExecutableTransaction; +use crate::transaction::transactions::{enforce_fee, ExecutableTransaction}; // Corresponding constants to the ones in faulty_account. pub const VALID: u64 = 0; @@ -294,6 +293,7 @@ pub fn create_account_tx_for_validate_test( calldata: execute_calldata, version: tx_version, nonce: nonce_manager.next(sender_address), + }); AccountTransaction { tx, execution_flags } } @@ -314,8 +314,9 @@ pub fn run_invoke_tx( ) -> TransactionExecutionResult { let only_query = invoke_args.only_query; let tx = invoke_tx(invoke_args); - let account_tx = AccountTransaction { tx, only_query }; - let charge_fee = account_tx.enforce_fee(); + 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 }; account_tx.execute(state, block_context, charge_fee, true) } @@ -392,6 +393,9 @@ 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, only_query: false } + AccountTransaction { tx, execution_flags } } diff --git a/crates/blockifier/src/transaction/transaction_execution.rs b/crates/blockifier/src/transaction/transaction_execution.rs index 103db62607c..1351abb18be 100644 --- a/crates/blockifier/src/transaction/transaction_execution.rs +++ b/crates/blockifier/src/transaction/transaction_execution.rs @@ -84,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) => { @@ -127,11 +129,7 @@ impl Transaction { }; Ok(AccountTransaction { tx: executable_tx, - execution_flags: AccountExecutionFlags { - only_query, - charge_fee: false, - validate: false, - }, + 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 effa40574f1..1214845b1b1 100644 --- a/crates/blockifier/src/transaction/transactions.rs +++ b/crates/blockifier/src/transaction/transactions.rs @@ -53,8 +53,6 @@ mod test; #[derive(Clone, Copy, Debug)] pub struct ExecutionFlags { - pub charge_fee: bool, - pub validate: bool, pub concurrency_mode: bool, } @@ -65,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); diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 459c564b663..1bcb9e4d553 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1971,11 +1971,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, @@ -1983,8 +1988,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,); @@ -2155,7 +2158,10 @@ fn test_valid_flag( calldata: create_trivial_calldata(test_contract.get_instance_address(0)), resource_bounds: default_all_resource_bounds, }); - let account_tx = AccountTransaction { tx, only_query: false }; + 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(); 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 3e3c3f030d4..52729935832 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 6e72697b7da..b050e9d0294 100644 --- a/crates/native_blockifier/src/py_transaction.rs +++ b/crates/native_blockifier/src/py_transaction.rs @@ -3,6 +3,7 @@ use std::collections::BTreeMap; 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,21 +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."); let tx = ExecutableTransaction::Declare(py_declare(tx, non_optional_py_class_info)?); - AccountTransaction { tx, execution_flags: ExecutionFlags::default() }.into() + let execution_flags = ExecutionFlags { + only_query, + charge_fee: enforce_fee(&tx, only_query), + ..ExecutionFlags::default() + }; + AccountTransaction { tx, execution_flags }.into() } TransactionType::DeployAccount => { let tx = ExecutableTransaction::DeployAccount(py_deploy_account(tx)?); - AccountTransaction { tx, execution_flags: ExecutionFlags::default() }.into() + let execution_flags = ExecutionFlags { + only_query, + charge_fee: enforce_fee(&tx, only_query), + ..ExecutionFlags::default() + }; + AccountTransaction { tx, execution_flags }.into() } TransactionType::InvokeFunction => { let tx = ExecutableTransaction::Invoke(py_invoke_function(tx)?); - AccountTransaction { tx, execution_flags: ExecutionFlags::default() }.into() + 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 369222aec56..4b405eda7a3 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 580914fb05d..7b3b750e218 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -18,6 +18,7 @@ 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)] @@ -164,10 +165,16 @@ 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 { // TODO(yair): Avoid this clone. tx: account_tx.clone(), - execution_flags: ExecutionFlags::default(), + execution_flags: ExecutionFlags { + only_query, + charge_fee, + validate: true, + }, }) } Transaction::L1Handler(l1_handler_tx) => { diff --git a/crates/starknet_gateway/src/stateful_transaction_validator.rs b/crates/starknet_gateway/src/stateful_transaction_validator.rs index 6e0549e50dc..d09080c94e5 100644 --- a/crates/starknet_gateway/src/stateful_transaction_validator.rs +++ b/crates/starknet_gateway/src/stateful_transaction_validator.rs @@ -6,6 +6,7 @@ use blockifier::bouncer::BouncerConfig; use blockifier::context::{BlockContext, ChainInfo}; use blockifier::state::cached_state::CachedState; use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; +use blockifier::transaction::transactions::enforce_fee; use blockifier::versioned_constants::VersionedConstants; #[cfg(test)] use mockall::automock; @@ -75,10 +76,11 @@ impl StatefulTransactionValidator { mut validator: V, ) -> StatefulTransactionValidatorResult<()> { let skip_validate = skip_stateful_validations(executable_tx, account_nonce); - let account_tx = AccountTransaction { - tx: executable_tx.clone(), - execution_flags: ExecutionFlags::default(), - }; + 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() })?;