refactor(blockifier): remove enforce fee from actual fee

crates/blockifier/src/blockifier/stateful_validator.rs

@@ -115,7 +115,7 @@ impl<S: StateReader> StatefulValidator<S> {
         let mut execution_resources = ExecutionResources::default();
         let tx_context = Arc::new(self.tx_executor.block_context.to_tx_context(tx));
 
-        let limit_steps_by_resources = true;
+        let limit_steps_by_resources = tx.create_tx_info().enforce_fee();
         let validate_call_info = tx.validate_tx(
             self.tx_executor.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR),
             &mut execution_resources,

crates/blockifier/src/concurrency/fee_utils.rs

@@ -56,11 +56,8 @@ pub fn complete_fee_transfer_flow(
             sequencer_balance,
         );
     } else {
-        assert_eq!(
-            tx_execution_info.receipt.fee,
-            Fee(0),
-            "Transaction with no fee transfer info must have zero fee."
-        )
+        let charge_fee = tx_context.tx_info.enforce_fee();
+        assert!(!charge_fee, "Transaction with no fee transfer info must have zero fee.")
     }
 }
 

crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs

@@ -488,7 +488,7 @@ fn test_tx_info(#[values(false, true)] only_query: bool) {
         },
         max_fee,
     });
-    let limit_steps_by_resources = true;
+    let limit_steps_by_resources = tx_info.enforce_fee();
     let result = entry_point_call
         .execute_directly_given_tx_info(&mut state, tx_info, limit_steps_by_resources)
         .unwrap();

crates/blockifier/src/execution/entry_point.rs

@@ -187,7 +187,7 @@ impl EntryPointExecutionContext {
                 .expect("Failed to convert invoke_tx_max_n_steps (u32) to usize."),
         };
 
-        if !limit_steps_by_resources || !tx_info.enforce_fee() {
+        if !limit_steps_by_resources {
             return block_upper_bound;
         }
 

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs

@@ -241,7 +241,6 @@ fn test_get_execution_info(
         ),
         ..trivial_external_entry_point_with_address(test_contract_address)
     };
-
     let result = match execution_mode {
         ExecutionMode::Validate => {
             entry_point_call.execute_directly_given_tx_info_in_validate_mode(state, tx_info, false)

crates/blockifier/src/fee/receipt.rs

@@ -1,6 +1,6 @@
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
 use starknet_api::core::ContractAddress;
-use starknet_api::transaction::Fee;
+use starknet_api::transaction::{Fee, TransactionVersion};
 
 use crate::context::TransactionContext;
 use crate::execution::call_info::ExecutionSummary;
@@ -92,12 +92,15 @@ impl TransactionReceipt {
             &tx_context.get_gas_vector_computation_mode(),
         );
 
-        // L1 handler transactions are not charged an L2 fee but it is compared to the L1 fee.
-        let fee = if tx_context.tx_info.enforce_fee() || tx_type == TransactionType::L1Handler {
-            tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas)
-        } else {
+        // To be backward compatible with the meaning of enforce fee in Declare V0.
+        let fee = if tx_type == TransactionType::Declare
+            && tx_context.tx_info.signed_version() == TransactionVersion::ZERO
+        {
             Fee(0)
+        } else {
+            tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas)
         };
+
         let da_gas = tx_resources
             .starknet_resources
             .get_state_changes_cost(tx_context.block_context.block_info.use_kzg_da);

crates/blockifier/src/test_utils/struct_impls.rs

@@ -50,11 +50,9 @@ impl CallEntryPoint {
     /// Executes the call directly, without account context. Limits the number of steps by resource
     /// bounds.
     pub fn execute_directly(self, state: &mut dyn State) -> EntryPointExecutionResult<CallInfo> {
-        self.execute_directly_given_tx_info(
-            state,
-            TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
-            true,
-        )
+        let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default());
+        let limit_steps_by_resources = tx_info.enforce_fee();
+        self.execute_directly_given_tx_info(state, tx_info, limit_steps_by_resources)
     }
 
     pub fn execute_directly_given_tx_info(
@@ -76,10 +74,12 @@ impl CallEntryPoint {
         self,
         state: &mut dyn State,
     ) -> EntryPointExecutionResult<CallInfo> {
+        let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default());
+        let limit_steps_by_resources = tx_info.enforce_fee();
         self.execute_directly_given_tx_info_in_validate_mode(
             state,
-            TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
-            true,
+            tx_info,
+            limit_steps_by_resources,
         )
     }
 

crates/blockifier/src/transaction/account_transaction.rs

@@ -500,7 +500,7 @@ impl AccountTransaction {
         charge_fee: bool,
         concurrency_mode: bool,
     ) -> TransactionExecutionResult<Option<CallInfo>> {
-        if !charge_fee || actual_fee == Fee(0) {
+        if !charge_fee {
             // Fee charging is not enforced in some transaction simulations and tests.
             return Ok(None);
         }
@@ -549,8 +549,9 @@ impl AccountTransaction {
                 .gas_costs
                 .default_initial_gas_cost,
         };
-
-        let mut context = EntryPointExecutionContext::new_invoke(tx_context, true);
+        let limit_steps_by_resources = tx_info.enforce_fee();
+        let mut context =
+            EntryPointExecutionContext::new_invoke(tx_context, limit_steps_by_resources);
 
         Ok(fee_transfer_call
             .execute(state, &mut ExecutionResources::default(), &mut context)

crates/blockifier/src/transaction/account_transactions_test.rs

@@ -203,7 +203,6 @@ fn test_enforce_fee_false_works(block_context: BlockContext, #[case] version: Tr
     )
     .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.
@@ -541,6 +540,7 @@ fn test_revert_invoke(
         &block_context,
         invoke_tx_args! {
             max_fee,
+            resource_bounds: max_l1_resource_bounds(),
             sender_address: account_address,
             calldata: create_calldata(
                 test_contract_address,

crates/blockifier/src/transaction/transaction_execution.rs

@@ -135,12 +135,13 @@ impl<U: UpdatableState> ExecutableTransaction<U> for L1HandlerTransaction {
         &self,
         state: &mut TransactionalState<'_, U>,
         block_context: &BlockContext,
-        _execution_flags: ExecutionFlags,
+        execution_flags: ExecutionFlags,
     ) -> TransactionExecutionResult<TransactionExecutionInfo> {
         let tx_context = Arc::new(block_context.to_tx_context(self));
-
+        let limit_steps_by_resources = execution_flags.charge_fee;
         let mut execution_resources = ExecutionResources::default();
-        let mut context = EntryPointExecutionContext::new_invoke(tx_context.clone(), true);
+        let mut context =
+            EntryPointExecutionContext::new_invoke(tx_context.clone(), limit_steps_by_resources);
         let mut remaining_gas = block_context.versioned_constants.tx_default_initial_gas();
         let execute_call_info =
             self.run_execute(state, &mut execution_resources, &mut context, &mut remaining_gas)?;

crates/blockifier/src/transaction/transactions_test.rs

@@ -1345,10 +1345,11 @@ fn test_declare_tx(
         None,
         ExecutionSummary::default(),
     );
-
+    // For V0 transactions, the max fee is always 0.
+    let max_fee = if tx_version == TransactionVersion::ZERO { Fee(0) } else { Fee(MAX_FEE) };
     let account_tx = declare_tx(
         declare_tx_args! {
-            max_fee: Fee(MAX_FEE),
+            max_fee,
             sender_address,
             version: tx_version,
             resource_bounds: max_l1_resource_bounds,
@@ -1367,7 +1368,8 @@ fn test_declare_tx(
     );
     let fee_type = &account_tx.fee_type();
     let tx_context = &block_context.to_tx_context(&account_tx);
-    let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
+    let actual_execution_info =
+        account_tx.execute(state, block_context, tx_context.tx_info.enforce_fee(), true).unwrap(); //aviv: changed from true, true
 
     // Build expected validate call info.
     let expected_validate_call_info = declare_validate_callinfo(
@@ -1382,14 +1384,24 @@ fn test_declare_tx(
     );
 
     // Build expected fee transfer call info.
-    let expected_actual_fee =
-        actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type);
-    let expected_fee_transfer_call_info = expected_fee_transfer_call_info(
-        tx_context,
-        sender_address,
-        expected_actual_fee,
-        FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
-    );
+    // aviv added 'if' condition. not pretty solution:
+    let expected_actual_fee = if tx_context.tx_info.signed_version() == TransactionVersion::ZERO {
+        Fee(0)
+    } else {
+        actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type)
+    };
+    // aviv added 'if' condition. not pretty solution:
+    let expected_fee_transfer_call_info =
+        if tx_context.tx_info.signed_version() == TransactionVersion::ZERO {
+            None
+        } else {
+            expected_fee_transfer_call_info(
+                tx_context,
+                sender_address,
+                expected_actual_fee,
+                FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
+            )
+        };
 
     let da_gas = starknet_resources.get_state_changes_cost(use_kzg_da);
     let expected_cairo_resources = get_expected_cairo_resources(

crates/papyrus_execution/src/lib.rs

@@ -249,14 +249,13 @@ pub fn execute_call(
         execution_config,
         override_kzg_da_to_false,
     )?;
+    // TODO(yair): fix when supporting v3 transactions
+    let tx_info = TransactionInfo::Deprecated(DeprecatedTransactionInfo::default());
+    let limit_steps_by_resources = tx_info.enforce_fee();
 
     let mut context = EntryPointExecutionContext::new_invoke(
-        // TODO(yair): fix when supporting v3 transactions
-        Arc::new(TransactionContext {
-            block_context,
-            tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
-        }),
-        true, // limit_steps_by_resources
+        Arc::new(TransactionContext { block_context, tx_info }),
+        limit_steps_by_resources,
     );
 
     let res = call_entry_point

crates/starknet_api/src/transaction.rs

@@ -1088,7 +1088,14 @@ impl ValidResourceBounds {
     // TODO(Nimrod): Default testing bounds should probably be AllResourceBounds variant.
     #[cfg(any(feature = "testing", test))]
     pub fn create_for_testing() -> Self {
-        Self::L1Gas(ResourceBounds { max_amount: 0, max_price_per_unit: 1 })
+        // Self::L1Gas(ResourceBounds { max_amount: 0, max_price_per_unit: 1 })
+        // Aviv: suggestion?
+        let resource_bounds = AllResourceBounds {
+            l1_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
+            l2_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
+            l1_data_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
+        };
+        Self::AllResources(resource_bounds)
     }
 }