refactor(blockifier): remove enforce fee from prevalidation checks

crates/blockifier/src/blockifier/stateful_validator.rs

@@ -96,7 +96,9 @@ impl<S: StateReader> StatefulValidator<S> {
     ) -> 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,

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 result = stateful_validator.perform_validations(tx, false, true);
+    let result = stateful_validator.perform_validations(tx, false, false);
     assert!(result.is_ok(), "Validation failed: {:?}", result.unwrap_err());
 }
 

crates/blockifier/src/blockifier/transaction_executor.rs

@@ -91,6 +91,7 @@ impl<S: StateReader> TransactionExecutor<S> {
         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 };

crates/blockifier/src/blockifier/transaction_executor_test.rs

@@ -52,7 +52,7 @@ fn tx_executor_test_body<S: StateReader>(
     // 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 _tx_execution_info = tx_executor.execute(&tx, true).unwrap();
+    let _tx_execution_info = tx_executor.execute(&tx, false).unwrap();
     let bouncer_weights = tx_executor.bouncer.get_accumulated_weights();
     assert_eq!(bouncer_weights.state_diff_size, expected_bouncer_weights.state_diff_size);
     assert_eq!(
@@ -271,7 +271,7 @@ fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_even
                 contract_address,
                 nonce_manager.next(account_address),
             )),
-            true,
+            false,
         )
         .map_err(|error| panic!("{error:?}: {error}"))
         .unwrap();
@@ -308,7 +308,7 @@ fn test_execute_txs_bouncing() {
     .collect();
 
     // Run.
-    let results = tx_executor.execute_txs(&txs, true);
+    let results = tx_executor.execute_txs(&txs, false);
 
     // Check execution results.
     let expected_offset = 3;
@@ -336,12 +336,12 @@ fn test_execute_txs_bouncing() {
 
     // Check idempotency: excess transactions should not be added.
     let remaining_txs = &txs[expected_offset..];
-    let remaining_tx_results = tx_executor.execute_txs(remaining_txs, true);
+    let remaining_tx_results = tx_executor.execute_txs(remaining_txs, false);
     assert_eq!(remaining_tx_results.len(), 0);
 
     // Reset the bouncer and add the remaining transactions.
     tx_executor.bouncer = Bouncer::new(tx_executor.block_context.bouncer_config.clone());
-    let remaining_tx_results = tx_executor.execute_txs(remaining_txs, true);
+    let remaining_tx_results = tx_executor.execute_txs(remaining_txs, false);
 
     assert_eq!(remaining_tx_results.len(), 2);
     assert!(remaining_tx_results[0].is_ok());

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -262,7 +262,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);
+            let result = account_tx_1.execute(&mut state_1, &block_context_1, false, true);
             assert_eq!(result.is_err(), enforce_fee);
         });
         s.spawn(move || {

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -765,8 +765,8 @@ fn test_worker_commit_phase_with_halt() {
     assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(1));
 
     // Execute both transactions.
-    worker_executor.execute(0, true);
-    worker_executor.execute(1, true);
+    worker_executor.execute(0, false);
+    worker_executor.execute(1, false);
 
     // Commit both transactions.
     worker_executor.commit_while_possible(true);

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);
 }
 

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 results = self.executor.execute_txs(&txs, true);
+        let results = self.executor.execute_txs(&txs, false);
         assert_eq!(results.len(), self.config.n_txs);
         for result in results {
             assert!(!result.unwrap().is_reverted());

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)?;

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);
 

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,

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());
     }
 }