Fail with an Err transactions whose calculated fee exceed max_fee (#892)

* Make tx fail when actual_fee exceeds max_fee

* Changed test

* Formatting

* Fix logic

* Leave fail only without charging

* Change test

* Fix test broken by better fee calc

* Fixed test fee

* Update fee on test_deploy_account

* Remove comment

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>

Author: xqft

src/transaction/fee.rs

@@ -154,16 +154,18 @@ pub fn charge_fee<S: StateReader>(
         block_context,
     )?;
 
+    if actual_fee > max_fee {
+        // TODO: Charge max_fee
+        return Err(TransactionError::ActualFeeExceedsMaxFee(
+            actual_fee, max_fee,
+        ));
+    }
+
     let actual_fee = if tx_execution_context.version != 0.into()
         && tx_execution_context.version != *QUERY_VERSION_BASE
     {
         min(actual_fee, max_fee) * FEE_FACTOR
     } else {
-        if actual_fee > max_fee {
-            return Err(TransactionError::ActualFeeExceedsMaxFee(
-                actual_fee, max_fee,
-            ));
-        }
         actual_fee
     };
 
@@ -177,6 +179,7 @@ pub fn charge_fee<S: StateReader>(
             actual_fee,
         )?)
     };
+
     Ok((fee_transfer_info, actual_fee))
 }
 
@@ -218,7 +221,7 @@ mod tests {
     }
 
     #[test]
-    fn test_charge_fee_v1_actual_fee_exceeds_max_fee_should_return_max_fee() {
+    fn test_charge_fee_v1_actual_fee_exceeds_max_fee_should_return_error() {
         let mut state = CachedState::new(Arc::new(InMemoryStateReader::default()), None, None);
         let mut tx_execution_context = TransactionExecutionContext {
             version: 1.into(),
@@ -241,8 +244,8 @@ mod tests {
             &mut tx_execution_context,
             skip_fee_transfer,
         )
-        .unwrap();
+        .unwrap_err();
 
-        assert_eq!(result.1, max_fee);
+        assert_matches!(result, TransactionError::ActualFeeExceedsMaxFee(_, _));
     }
 }

src/transaction/invoke_function.rs

@@ -771,7 +771,7 @@ mod tests {
     }
 
     #[test]
-    fn test_execute_invoke_actual_fee_exceeded_max_fee_should_charge_max_fee() {
+    fn test_execute_invoke_actual_fee_exceeded_max_fee_should_fail() {
         let max_fee = 5;
         let internal_invoke_function = InvokeFunction {
             contract_address: Address(0.into()),
@@ -824,8 +824,8 @@ mod tests {
 
         let tx = internal_invoke_function
             .execute(&mut state, &block_context, 0)
-            .unwrap();
-        assert_eq!(tx.actual_fee, max_fee);
+            .unwrap_err();
+        assert_matches!(tx, TransactionError::ActualFeeExceedsMaxFee(_, _));
     }
 
     #[test]

tests/internals.rs

@@ -1459,7 +1459,7 @@ fn test_invoke_with_declarev2_tx() {
 fn test_deploy_account() {
     let (block_context, mut state) = create_account_tx_test_state().unwrap();
 
-    let expected_fee = 3684;
+    let expected_fee = 6157;
 
     let deploy_account_tx = DeployAccount::new(
         felt_to_hash(&TEST_ACCOUNT_CONTRACT_CLASS_HASH),
@@ -1576,7 +1576,7 @@ fn expected_deploy_account_states() -> (
     CachedState<InMemoryStateReader>,
     CachedState<InMemoryStateReader>,
 ) {
-    let fee = Felt252::from(3684);
+    let fee = Felt252::from(6157);
     let mut state_before = CachedState::new(
         Arc::new(InMemoryStateReader::new(
             HashMap::from([