diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index 1353e26e0a..dd70cc6a7c 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -96,7 +96,9 @@ impl StatefulValidator { ) -> StatefulValidatorResult<()> { let strict_nonce_check = false; // Run pre-validation in charge fee mode to perform fee and balance related checks. - let charge_fee = true; + // TODO (Meshi, 1/10/2024): When resource bound is supported in FaultyAccountTxCreatorArgs + // Change charge_fee to true. + let charge_fee = tx_context.tx_info.enforce_fee().unwrap(); tx.perform_pre_validation_stage( self.tx_executor.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR), tx_context, diff --git a/crates/blockifier/src/blockifier/stateful_validator_test.rs b/crates/blockifier/src/blockifier/stateful_validator_test.rs index 9ea08fe50f..2060c67696 100644 --- a/crates/blockifier/src/blockifier/stateful_validator_test.rs +++ b/crates/blockifier/src/blockifier/stateful_validator_test.rs @@ -69,7 +69,7 @@ fn test_transaction_validator( // Test the stateful validator. let mut stateful_validator = StatefulValidator::create(state, block_context); - let charge_fee = true; + let charge_fee = false; let skip_validation = false; let result = stateful_validator.perform_validations(tx, skip_validation, charge_fee); assert!(result.is_ok(), "Validation failed: {:?}", result.unwrap_err()); diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 9fb717125f..4ae8a759e6 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -91,6 +91,7 @@ impl TransactionExecutor { let mut transactional_state = TransactionalState::create_transactional( self.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR), ); + // Executing a single transaction cannot be done in a concurrent mode. let execution_flags = ExecutionFlags { charge_fee, validate: true, concurrency_mode: false }; diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index 62364d401f..4e00962933 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -52,7 +52,7 @@ fn tx_executor_test_body( // TODO(Arni, 30/03/2024): Consider adding a test for the transaction execution info. If A test // should not be added, rename the test to `test_bouncer_info`. // TODO(Arni, 30/03/2024): Test all bouncer weights. - let charge_fee = true; + let charge_fee = false; let _tx_execution_info = tx_executor.execute(&tx, charge_fee).unwrap(); let bouncer_weights = tx_executor.bouncer.get_accumulated_weights(); assert_eq!(bouncer_weights.state_diff_size, expected_bouncer_weights.state_diff_size); @@ -272,7 +272,7 @@ fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_even contract_address, nonce_manager.next(account_address), )), - true, // charge_fee + false, // charge_fee ) .map_err(|error| panic!("{error:?}: {error}")) .unwrap(); @@ -309,7 +309,7 @@ fn test_execute_txs_bouncing() { .collect(); // Run. - let charge_fee = true; + let charge_fee = false; let results = tx_executor.execute_txs(&txs, charge_fee); // Check execution results. diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index 79cac9ee12..247638f658 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -41,7 +41,7 @@ use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; use crate::test_utils::{CairoVersion, NonceManager, BALANCE, DEFAULT_STRK_L1_GAS_PRICE}; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::objects::{HasRelatedFeeType, TransactionInfoCreator}; +use crate::transaction::objects::HasRelatedFeeType; use crate::transaction::test_utils::{l1_resource_bounds, max_resource_bounds}; use crate::transaction::transactions::ExecutableTransaction; use crate::{compiled_class_hash, deploy_account_tx_args, nonce, storage_key}; @@ -233,7 +233,6 @@ fn test_run_parallel_txs(max_resource_bounds: ResourceBoundsMapping) { &mut NonceManager::default(), ); let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1); - let enforce_fee = account_tx_1.create_tx_info().enforce_fee().unwrap(); let class_hash = grindy_account.get_class_hash(); let ctor_storage_arg = felt!(1_u8); @@ -262,8 +261,7 @@ fn test_run_parallel_txs(max_resource_bounds: ResourceBoundsMapping) { // Execute transactions thread::scope(|s| { s.spawn(move || { - let result = account_tx_1.execute(&mut state_1, &block_context_1, true, true); - assert_eq!(result.is_err(), enforce_fee); + account_tx_1.execute(&mut state_1, &block_context_1, false, true).unwrap(); }); s.spawn(move || { account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap(); diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index 986e40a563..35bb3d0654 100644 --- a/crates/blockifier/src/concurrency/worker_logic_test.rs +++ b/crates/blockifier/src/concurrency/worker_logic_test.rs @@ -775,7 +775,7 @@ fn test_worker_commit_phase_with_halt() { // `ReadyToExecute` and not `Executing` as expected). assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(0)); assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(1)); - let charge_fee = true; + let charge_fee = false; // Execute both transactions. worker_executor.execute(0, charge_fee); worker_executor.execute(1, charge_fee); diff --git a/crates/blockifier/src/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index 6aa7523507..b6aa6cdddb 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -79,6 +79,7 @@ fn test_stack_trace( invoke_tx_args! { sender_address: account_address, calldata, + max_fee: Fee(BALANCE), version: TransactionVersion::ZERO, }, ) @@ -198,6 +199,7 @@ fn test_trace_callchain_ends_with_regular_call( sender_address: account_address, calldata, version: TransactionVersion::ZERO, + max_fee: Fee(BALANCE), }, ) .unwrap_err(); @@ -334,6 +336,7 @@ fn test_trace_call_chain_with_syscalls( sender_address: account_address, calldata, version: TransactionVersion::ZERO, + max_fee: Fee(BALANCE), }, ) .unwrap_err(); @@ -526,7 +529,7 @@ Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid s // Clean pc locations from the trace. let re = Regex::new(r"pc=0:[0-9]+").unwrap(); let cleaned_expected_error = &re.replace_all(&expected_error, "pc=0:*"); - let actual_error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let actual_error = account_tx.execute(state, block_context, false, true).unwrap_err(); let actual_error_str = actual_error.to_string(); let cleaned_actual_error = &re.replace_all(&actual_error_str, "pc=0:*"); // Compare actual trace to the expected trace (sans pc locations). @@ -589,7 +592,7 @@ An ASSERT_EQ instruction failed: 1 != 0. }; // Compare expected and actual error. - let error = deploy_account_tx.execute(state, &block_context, true, true).unwrap_err(); + let error = deploy_account_tx.execute(state, &block_context, false, true).unwrap_err(); assert_eq!(error.to_string(), expected_error); } diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs index d09c96de80..643f931c2f 100644 --- a/crates/blockifier/src/fee/fee_checks.rs +++ b/crates/blockifier/src/fee/fee_checks.rs @@ -192,7 +192,7 @@ impl PostExecutionReport { let TransactionReceipt { fee, .. } = tx_receipt; // If fee is not enforced, no need to check post-execution. - if !charge_fee || !tx_context.tx_info.enforce_fee()? { + if !charge_fee { return Ok(Self(FeeCheckReport::success_report(*fee))); } diff --git a/crates/blockifier/src/test_utils/transfers_generator.rs b/crates/blockifier/src/test_utils/transfers_generator.rs index 88bbca7ebf..5c6d1115cd 100644 --- a/crates/blockifier/src/test_utils/transfers_generator.rs +++ b/crates/blockifier/src/test_utils/transfers_generator.rs @@ -155,7 +155,7 @@ impl TransfersGenerator { let account_tx = self.generate_transfer(sender_address, recipient_address); txs.push(Transaction::AccountTransaction(account_tx)); } - let charge_fee = true; + let charge_fee = false; let results = self.executor.execute_txs(&txs, charge_fee); assert_eq!(results.len(), self.config.n_txs); for result in results { diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 1cc97e9ed1..9043bc761c 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -178,7 +178,7 @@ impl AccountTransaction { let tx_info = &tx_context.tx_info; Self::handle_nonce(state, tx_info, strict_nonce_check)?; - if charge_fee && tx_info.enforce_fee()? { + if charge_fee { self.check_fee_bounds(tx_context)?; verify_can_pay_committed_bounds(state, tx_context)?; diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 3f48297d15..d9a2c3981e 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -35,7 +35,6 @@ use crate::state::cached_state::{StateChangesCount, TransactionalState}; use crate::state::state_api::{State, StateReader}; 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::InvokeTxArgs; use crate::test_utils::{ @@ -47,7 +46,6 @@ use crate::test_utils::{ CairoVersion, NonceManager, BALANCE, - DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE, }; use crate::transaction::account_transaction::AccountTransaction; @@ -152,54 +150,6 @@ fn test_rc96_holes(block_context: BlockContext, max_resource_bounds: ResourceBou assert_eq!(tx_execution_info.receipt.gas, GasVector::from_l1_gas(6598)); } -#[rstest] -fn test_fee_enforcement( - block_context: BlockContext, - #[values(TransactionVersion::ONE, TransactionVersion::THREE)] version: TransactionVersion, - #[values(true, false)] zero_bounds: bool, -) { - 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( - deploy_account_tx_args! { - class_hash: account.get_class_hash(), - max_fee: Fee(u128::from(!zero_bounds)), - resource_bounds: l1_resource_bounds(u64::from(!zero_bounds), DEFAULT_STRK_L1_GAS_PRICE), - version, - }, - &mut NonceManager::default(), - ); - - let account_tx = AccountTransaction::DeployAccount(deploy_account_tx); - let enforce_fee = account_tx.create_tx_info().enforce_fee().unwrap(); - let result = account_tx.execute(state, &block_context, true, true); - assert_eq!(result.is_err(), enforce_fee); -} - -#[rstest] -#[case(TransactionVersion::ZERO)] -#[case(TransactionVersion::ONE)] -#[case(TransactionVersion::THREE)] -fn test_enforce_fee_false_works(block_context: BlockContext, #[case] version: TransactionVersion) { - let TestInitData { mut state, account_address, contract_address, mut nonce_manager } = - create_test_init_data(&block_context.chain_info, CairoVersion::Cairo0); - let tx_execution_info = run_invoke_tx( - &mut state, - &block_context, - invoke_tx_args! { - max_fee: Fee(0), - resource_bounds: l1_resource_bounds(0, DEFAULT_STRK_L1_GAS_PRICE), - sender_address: account_address, - calldata: create_trivial_calldata(contract_address), - version, - nonce: nonce_manager.next(account_address), - }, - ) - .unwrap(); - assert!(!tx_execution_info.is_reverted()); - assert_eq!(tx_execution_info.receipt.fee, Fee(0)); -} - // TODO(Dori, 15/9/2023): Convert version variance to attribute macro. #[rstest] fn test_account_flow_test( @@ -517,6 +467,7 @@ fn test_recursion_depth_exceeded( fn test_revert_invoke( block_context: BlockContext, max_fee: Fee, + max_resource_bounds: ResourceBoundsMapping, #[case] transaction_version: TransactionVersion, #[case] fee_type: FeeType, ) { @@ -536,6 +487,7 @@ fn test_revert_invoke( invoke_tx_args! { max_fee, sender_address: account_address, + resource_bounds: max_resource_bounds, calldata: create_calldata( test_contract_address, "write_and_revert", @@ -606,7 +558,7 @@ fn test_fail_deploy_account( let initial_balance = state.get_fee_token_balance(deploy_address, fee_token_address).unwrap(); - let error = deploy_account_tx.execute(state, &block_context, true, true).unwrap_err(); + let error = deploy_account_tx.execute(state, &block_context, false, true).unwrap_err(); // Check the error is as expected. Assure the error message is not nonce or fee related. check_transaction_execution_error_for_invalid_scenario!(cairo_version, error, false); diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index d6b1724e3a..1e60f5a244 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -140,6 +140,7 @@ pub fn create_test_init_data(chain_info: &ChainInfo, cairo_version: CairoVersion } } +// TODO(Meshi, 1/10/2024): add resource bounds to the argumaents. pub struct FaultyAccountTxCreatorArgs { pub tx_type: TransactionType, pub tx_version: TransactionVersion, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index a8198cfa9b..3f74aee778 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -777,7 +777,7 @@ fn test_state_get_fee_token_balance( version: tx_version, nonce: Nonce::default(), }); - account_tx.execute(state, block_context, true, true).unwrap(); + account_tx.execute(state, block_context, false, true).unwrap(); // Get balance from state, and validate. let (low, high) = @@ -1538,7 +1538,7 @@ fn test_validate_accounts_tx( additional_data: None, ..default_args }); - let error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let error = account_tx.execute(state, block_context, false, true).unwrap_err(); check_transaction_execution_error_for_invalid_scenario!( cairo_version, error, @@ -1554,7 +1554,7 @@ fn test_validate_accounts_tx( contract_address_salt: salt_manager.next_salt(), ..default_args }); - let error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let error = account_tx.execute(state, block_context, false, true).unwrap_err(); check_transaction_execution_error_for_custom_hint!( &error, "Unauthorized syscall call_contract in execution mode Validate.", @@ -1569,7 +1569,7 @@ fn test_validate_accounts_tx( additional_data: None, ..default_args }); - let error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let error = account_tx.execute(state, block_context, false, true).unwrap_err(); check_transaction_execution_error_for_custom_hint!( &error, "Unauthorized syscall get_block_hash in execution mode Validate.", @@ -1583,7 +1583,7 @@ fn test_validate_accounts_tx( contract_address_salt: salt_manager.next_salt(), ..default_args }); - let error = account_tx.execute(state, block_context, true, true).unwrap_err(); + let error = account_tx.execute(state, block_context, false, true).unwrap_err(); check_transaction_execution_error_for_custom_hint!( &error, "Unauthorized syscall get_sequencer_address in execution mode Validate.", @@ -1606,7 +1606,7 @@ fn test_validate_accounts_tx( ..default_args }, ); - let result = account_tx.execute(state, block_context, true, true); + let result = account_tx.execute(state, block_context, false, true); assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err()); if tx_type != TransactionType::DeployAccount { @@ -1622,7 +1622,7 @@ fn test_validate_accounts_tx( ..default_args }, ); - let result = account_tx.execute(state, block_context, true, true); + let result = account_tx.execute(state, block_context, false, true); assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err()); } @@ -1641,7 +1641,7 @@ fn test_validate_accounts_tx( ..default_args }, ); - let result = account_tx.execute(state, block_context, true, true); + let result = account_tx.execute(state, block_context, false, true); assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err()); // Call the syscall get_block_timestamp and assert the returned timestamp was modified @@ -1656,7 +1656,7 @@ fn test_validate_accounts_tx( ..default_args }, ); - let result = account_tx.execute(state, block_context, true, true); + let result = account_tx.execute(state, block_context, false, true); assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err()); } @@ -1677,7 +1677,7 @@ fn test_validate_accounts_tx( ..default_args }, ); - let result = account_tx.execute(state, block_context, true, true); + let result = account_tx.execute(state, block_context, false, true); assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err()); } }