From 0123ab27f1ac00dbbda6f96104e5d3a23508a997 Mon Sep 17 00:00:00 2001
From: dorimedini-starkware <dori@starkware.co>
Date: Tue, 1 Oct 2024 13:54:14 +0300
Subject: [PATCH] feat(blockifier): update assert_actual_fee_in_bounds and add
 test (#1118)

Signed-off-by: Dori Medini <dori@starkware.co>
---
 .../src/transaction/account_transaction.rs    | 14 ++---
 .../transaction/account_transactions_test.rs  | 53 +++++++++++++++++++
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs
index bb8be960be..e076077bcf 100644
--- a/crates/blockifier/src/transaction/account_transaction.rs
+++ b/crates/blockifier/src/transaction/account_transaction.rs
@@ -457,15 +457,12 @@ impl AccountTransaction {
     fn assert_actual_fee_in_bounds(tx_context: &Arc<TransactionContext>, actual_fee: Fee) {
         match &tx_context.tx_info {
             TransactionInfo::Current(context) => {
-                let ResourceBounds {
-                    max_amount: max_l1_gas_amount,
-                    max_price_per_unit: max_l1_gas_price,
-                } = context.l1_resource_bounds();
-                if actual_fee > Fee(u128::from(max_l1_gas_amount) * max_l1_gas_price) {
+                let max_fee = context.resource_bounds.max_possible_fee();
+                if actual_fee > max_fee {
                     panic!(
-                        "Actual fee {:#?} exceeded bounds; max amount is {:#?}, max price is
-                         {:#?}.",
-                        actual_fee, max_l1_gas_amount, max_l1_gas_price
+                        "Actual fee {:#?} exceeded bounds; max possible fee is {:#?} (computed \
+                         from {:#?}).",
+                        actual_fee, max_fee, context.resource_bounds
                     );
                 }
             }
@@ -492,7 +489,6 @@ impl AccountTransaction {
             return Ok(None);
         }
 
-        // TODO(Amos, 8/04/2024): Add test for this assert.
         Self::assert_actual_fee_in_bounds(&tx_context, actual_fee);
 
         let fee_transfer_call_info = if concurrency_mode && !tx_context.is_sequencer_the_sender() {
diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs
index 8438f79ec1..fff5d09570 100644
--- a/crates/blockifier/src/transaction/account_transactions_test.rs
+++ b/crates/blockifier/src/transaction/account_transactions_test.rs
@@ -11,11 +11,13 @@ use starknet_api::state::StorageKey;
 use starknet_api::test_utils::invoke::InvokeTxArgs;
 use starknet_api::test_utils::NonceManager;
 use starknet_api::transaction::{
+    AllResourceBounds,
     Calldata,
     ContractAddressSalt,
     DeclareTransactionV2,
     Fee,
     Resource,
+    ResourceBounds,
     TransactionHash,
     TransactionVersion,
     ValidResourceBounds,
@@ -182,6 +184,57 @@ fn test_fee_enforcement(
     assert_eq!(result.is_err(), enforce_fee);
 }
 
+#[rstest]
+#[case::positive_case_deprecated_tx(true, true)]
+#[case::positive_case_new_tx(true, false)]
+#[should_panic(expected = "exceeded bounds; max fee is")]
+#[case::negative_case_deprecated_tx(false, true)]
+#[should_panic(expected = "exceeded bounds; max possible fee is")]
+#[case::negative_case_new_tx(false, false)]
+fn test_assert_actual_fee_in_bounds(
+    block_context: BlockContext,
+    #[case] positive_flow: bool,
+    #[case] deprecated_tx: bool,
+) {
+    let actual_fee_offset = if positive_flow { 0 } else { 1 };
+    if deprecated_tx {
+        let max_fee = 100;
+        let tx = account_invoke_tx(invoke_tx_args! {
+            max_fee: Fee(max_fee),
+            version: TransactionVersion::ONE,
+        });
+        let context = Arc::new(block_context.to_tx_context(&tx));
+        AccountTransaction::assert_actual_fee_in_bounds(&context, Fee(max_fee + actual_fee_offset));
+    } else {
+        // All resources.
+        let l1_gas = ResourceBounds { max_amount: 2, max_price_per_unit: 3 };
+        let l2_gas = ResourceBounds { max_amount: 4, max_price_per_unit: 5 };
+        let l1_data_gas = ResourceBounds { max_amount: 6, max_price_per_unit: 7 };
+        let all_resource_bounds =
+            ValidResourceBounds::AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas });
+        let all_resource_fee = u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
+            + u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit
+            + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
+            + actual_fee_offset;
+
+        // L1 resources.
+        let l1_resource_bounds = ValidResourceBounds::L1Gas(l1_gas);
+        let l1_resource_fee =
+            u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit + actual_fee_offset;
+
+        for (bounds, actual_fee) in
+            [(all_resource_bounds, all_resource_fee), (l1_resource_bounds, l1_resource_fee)]
+        {
+            let tx = account_invoke_tx(invoke_tx_args! {
+                resource_bounds: bounds,
+                version: TransactionVersion::THREE,
+            });
+            let context = Arc::new(block_context.to_tx_context(&tx));
+            AccountTransaction::assert_actual_fee_in_bounds(&context, Fee(actual_fee));
+        }
+    }
+}
+
 #[rstest]
 #[case(TransactionVersion::ZERO)]
 #[case(TransactionVersion::ONE)]