chore(blockifier): have charge_fee flag represent enforce_fee return …

…value

crates/blockifier/src/blockifier/stateful_validator.rs

@@ -95,7 +95,7 @@ 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;
+        let charge_fee = tx.enforce_fee();
         tx.perform_pre_validation_stage(
             self.tx_executor.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR),
             tx_context,

crates/blockifier/src/blockifier/transaction_executor.rs

@@ -16,7 +16,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;
+use crate::transaction::objects::{TransactionExecutionInfo, TransactionInfoCreator};
 use crate::transaction::transaction_execution::Transaction;
 use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags};
 
@@ -98,9 +98,11 @@ impl<S: StateReader> TransactionExecutor<S> {
         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: true, validate: true, concurrency_mode: false };
+            ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: false };
         let tx_execution_result =
             tx.execute_raw(&mut transactional_state, &self.block_context, execution_flags);
         match tx_execution_result {

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -52,7 +52,7 @@ 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;
-use crate::transaction::objects::{HasRelatedFeeType, TransactionInfoCreator};
+use crate::transaction::objects::HasRelatedFeeType;
 use crate::transaction::test_utils::{l1_resource_bounds, max_l1_resource_bounds};
 use crate::transaction::transactions::ExecutableTransaction;
 
@@ -246,7 +246,7 @@ fn test_run_parallel_txs(max_l1_resource_bounds: ValidResourceBounds) {
         &mut NonceManager::default(),
     );
     let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1);
-    let enforce_fee = account_tx_1.create_tx_info().enforce_fee();
+    let enforce_fee = account_tx_1.enforce_fee();
 
     let class_hash = grindy_account.get_class_hash();
     let ctor_storage_arg = felt!(1_u8);
@@ -272,11 +272,12 @@ fn test_run_parallel_txs(max_l1_resource_bounds: ValidResourceBounds) {
 
     let block_context_1 = block_context.clone();
     let block_context_2 = block_context.clone();
+
     // 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);
+            let result = account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
+            assert_eq!(result.is_err(), enforce_fee); // Transaction fails iff we enforced the fee charge (as the acount is not funded).
         });
         s.spawn(move || {
             account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap();

crates/blockifier/src/concurrency/worker_logic.rs

@@ -22,7 +22,11 @@ use crate::state::cached_state::{
     TransactionalState,
 };
 use crate::state::state_api::{StateReader, UpdatableState};
-use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult};
+use crate::transaction::objects::{
+    TransactionExecutionInfo,
+    TransactionExecutionResult,
+    TransactionInfoCreator,
+};
 use crate::transaction::transaction_execution::Transaction;
 use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags};
 
@@ -128,10 +132,11 @@ 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: true, validate: true, concurrency_mode: true };
+            ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: true };
         let execution_result =
             tx.execute_raw(&mut transactional_state, self.block_context, execution_flags);
 

crates/blockifier/src/execution/stack_trace_test.rs

@@ -591,7 +591,8 @@ An ASSERT_EQ instruction failed: 1 != 0.
     // 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 charge_fee = account_tx.enforce_fee();
+    let actual_error = account_tx.execute(state, block_context, charge_fee, 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).
@@ -617,6 +618,7 @@ fn test_account_ctor_frame_stack_trace(
             scenario: INVALID,
             class_hash,
             max_fee: BALANCE,
+            resource_bounds: max_l1_resource_bounds(),
             validate_constructor: true,
             ..Default::default()
         });

crates/blockifier/src/fee/fee_checks.rs

@@ -268,7 +268,7 @@ impl PostExecutionReport {
         let TransactionReceipt { fee, gas, .. } = 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)));
         }
 

crates/blockifier/src/transaction/account_transaction.rs

@@ -255,6 +255,10 @@ impl AccountTransaction {
         }
     }
 
+    pub fn enforce_fee(&self) -> bool {
+        self.create_tx_info().enforce_fee()
+    }
+
     fn verify_tx_version(&self, version: TransactionVersion) -> TransactionExecutionResult<()> {
         let allowed_versions: Vec<TransactionVersion> = match self {
             // Support `Declare` of version 0 in order to allow bootstrapping of a new system.
@@ -292,7 +296,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

@@ -55,7 +55,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::{
     create_calldata,
@@ -64,7 +63,6 @@ use crate::test_utils::{
     get_tx_resources,
     CairoVersion,
     BALANCE,
-    DEFAULT_STRK_L1_GAS_PRICE,
     MAX_FEE,
 };
 use crate::transaction::account_transaction::AccountTransaction;
@@ -162,33 +160,6 @@ fn test_rc96_holes(block_context: BlockContext, max_l1_resource_bounds: ValidRes
     );
 }
 
-#[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(
-                u8::from(!zero_bounds).into(),
-                DEFAULT_STRK_L1_GAS_PRICE.into()
-            ),
-            version,
-        },
-        &mut NonceManager::default(),
-    );
-
-    let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
-    let enforce_fee = account_tx.create_tx_info().enforce_fee();
-    let result = account_tx.execute(state, &block_context, true, true);
-    assert_eq!(result.is_err(), enforce_fee);
-}
-
 #[rstest]
 #[case::positive_case_deprecated_tx(true, true)]
 #[case::positive_case_new_tx(true, false)]
@@ -238,30 +209,6 @@ fn test_assert_actual_fee_in_bounds(
     }
 }
 
-#[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_u8.into(), DEFAULT_STRK_L1_GAS_PRICE.into()),
-            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(
@@ -686,6 +633,7 @@ fn test_fail_deploy_account(
             scenario: INVALID,
             class_hash: faulty_account_feature_contract.get_class_hash(),
             max_fee: BALANCE,
+            resource_bounds: max_l1_resource_bounds(),
             ..Default::default()
         });
     let fee_token_address = chain_info.fee_token_address(&deploy_account_tx.fee_type());

crates/blockifier/src/transaction/test_utils.rs

@@ -323,7 +323,10 @@ pub fn run_invoke_tx(
     block_context: &BlockContext,
     invoke_args: InvokeTxArgs,
 ) -> TransactionExecutionResult<TransactionExecutionInfo> {
-    account_invoke_tx(invoke_args).execute(state, block_context, true, true)
+    let tx = account_invoke_tx(invoke_args);
+    let charge_fee = tx.enforce_fee();
+
+    tx.execute(state, block_context, charge_fee, true)
 }
 
 /// Creates a `ResourceBoundsMapping` with the given `max_amount` and `max_price` for L1 gas limits.

crates/blockifier/src/transaction/transactions_test.rs

@@ -120,6 +120,7 @@ use crate::transaction::objects::{
     HasRelatedFeeType,
     TransactionExecutionInfo,
     TransactionInfo,
+    TransactionInfoCreator,
 };
 use crate::transaction::test_utils::{
     account_invoke_tx,
@@ -804,6 +805,7 @@ fn test_state_get_fee_token_balance(
         create_calldata(fee_token_address, "permissionedMint", &[recipient, mint_low, mint_high]);
     let account_tx = account_invoke_tx(invoke_tx_args! {
         max_fee: MAX_FEE,
+        resource_bounds: max_l1_resource_bounds(),
         sender_address: account_address,
         calldata: execute_calldata,
         version: tx_version,
@@ -824,7 +826,9 @@ fn assert_failure_if_resource_bounds_exceed_balance(
     block_context: &BlockContext,
     invalid_tx: AccountTransaction,
 ) {
-    match block_context.to_tx_context(&invalid_tx).tx_info {
+    let tx_info = invalid_tx.create_tx_info();
+
+    match tx_info {
         TransactionInfo::Deprecated(context) => {
             assert_matches!(
                 invalid_tx.execute(state, block_context, true, true).unwrap_err(),
@@ -1758,7 +1762,10 @@ fn test_validate_accounts_tx(
         additional_data: None,
         ..default_args
     });
-    let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+    // We test `__validate__`, and don't care about the charge 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,);
 
     // Try to call another contract (forbidden).
@@ -1771,7 +1778,7 @@ fn test_validate_accounts_tx(
         resource_bounds: ValidResourceBounds::create_for_testing(),
         ..default_args
     });
-    let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+    let error = account_tx.execute(state, block_context, charge_fee, true).unwrap_err();
     check_tx_execution_error_for_custom_hint!(
         &error,
         "Unauthorized syscall call_contract in execution mode Validate.",
@@ -1787,7 +1794,7 @@ fn test_validate_accounts_tx(
             resource_bounds: ValidResourceBounds::create_for_testing(),
             ..default_args
         });
-        let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+        let error = account_tx.execute(state, block_context, charge_fee, true).unwrap_err();
         check_tx_execution_error_for_custom_hint!(
             &error,
             "Unauthorized syscall get_block_hash in execution mode Validate.",
@@ -1802,7 +1809,7 @@ fn test_validate_accounts_tx(
             resource_bounds: ValidResourceBounds::create_for_testing(),
             ..default_args
         });
-        let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+        let error = account_tx.execute(state, block_context, charge_fee, true).unwrap_err();
         check_tx_execution_error_for_custom_hint!(
             &error,
             "Unauthorized syscall get_sequencer_address in execution mode Validate.",
@@ -1826,7 +1833,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, charge_fee, true);
     assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err());
 
     if tx_type != TransactionType::DeployAccount {
@@ -1843,7 +1850,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, charge_fee, true);
         assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err());
     }
 
@@ -1863,7 +1870,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, charge_fee, true);
         assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err());
 
         // Call the syscall get_block_timestamp and assert the returned timestamp was modified
@@ -1879,7 +1886,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, charge_fee, true);
         assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err());
     }
 
@@ -1901,7 +1908,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, charge_fee, true);
         assert!(result.is_ok(), "Execution failed: {:?}", result.unwrap_err());
     }
 }
@@ -2049,8 +2056,7 @@ fn test_l1_handler(
     let key = calldata.0[1];
     let value = calldata.0[2];
     let payload_size = tx.payload_size();
-
-    let actual_execution_info = tx.execute(state, block_context, true, true).unwrap();
+    let actual_execution_info = tx.execute(state, block_context, false, true).unwrap(); // Do not charge fee as L1Handler's resource bounds (/max fee) is 0.
 
     // Build the expected call info.
     let accessed_storage_key = StorageKey::try_from(key).unwrap();
@@ -2182,7 +2188,7 @@ fn test_l1_handler(
     // always uptade the storage instad.
     state.set_storage_at(contract_address, StorageKey::try_from(key).unwrap(), Felt::ZERO).unwrap();
     let tx_no_fee = L1HandlerTransaction::create_for_testing(Fee(0), contract_address);
-    let error = tx_no_fee.execute(state, block_context, true, true).unwrap_err();
+    let error = tx_no_fee.execute(state, block_context, false, true).unwrap_err(); // Do not charge fee as L1Handler's resource bounds (/max fee) is 0.
     // Today, we check that the paid_fee is positive, no matter what was the actual fee.
     let expected_actual_fee =
         expected_execution_info.receipt.resources.calculate_tx_fee(block_context, &FeeType::Eth);