From 2a86d1fbc9d58c84c7c12c8b11c23377cae5904f Mon Sep 17 00:00:00 2001
From: Dori Medini <dori@starkware.co>
Date: Fri, 11 Oct 2024 21:10:07 +0300
Subject: [PATCH] feat(blockifier): tighter post-execution fee recommendation
 for new gas bounds

Signed-off-by: Dori Medini <dori@starkware.co>
---
 crates/blockifier/src/fee/fee_checks.rs | 61 +++++++++++++++++++------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs
index b3d6ff2032..e5a8a41a87 100644
--- a/crates/blockifier/src/fee/fee_checks.rs
+++ b/crates/blockifier/src/fee/fee_checks.rs
@@ -1,3 +1,4 @@
+use starknet_api::block::GasPriceVector;
 use starknet_api::execution_resources::{GasAmount, GasVector};
 use starknet_api::transaction::Resource::{self, L1DataGas, L1Gas, L2Gas};
 use starknet_api::transaction::{AllResourceBounds, Fee, ResourceBounds, ValidResourceBounds};
@@ -59,6 +60,8 @@ impl FeeCheckReport {
     /// Given a fee error and the current context, constructs and returns a report.
     pub fn from_fee_check_error(
         actual_fee: Fee,
+        actual_gas: GasVector,
+        gas_prices: &GasPriceVector,
         error: FeeCheckError,
         tx_context: &TransactionContext,
     ) -> Self {
@@ -68,18 +71,48 @@ impl FeeCheckReport {
             // the sender initially could cover the resource bounds), and (b) the actual resources
             // are within the resource bounds set by the sender.
             FeeCheckError::InsufficientFeeTokenBalance { .. } => actual_fee,
-            // If the error is resource overdraft, the recommended fee is the resource bounds.
-            // If the transaction passed pre-validation checks (i.e. balance initially covered the
-            // resource bounds), the sender should be able to pay this fee.
-            FeeCheckError::MaxFeeExceeded { .. } | FeeCheckError::MaxGasAmountExceeded { .. } => {
-                match &tx_context.tx_info {
-                    TransactionInfo::Current(info) => get_fee_by_gas_vector(
-                        &tx_context.block_context.block_info,
-                        GasVector::from_l1_gas(info.l1_resource_bounds().max_amount),
-                        &FeeType::Strk,
-                    ),
-                    TransactionInfo::Deprecated(context) => context.max_fee,
-                }
+            // If max fee exceeded (deprecated tx), the recommended fee is the max fee. The
+            // pre-validation phase ensures the account can cover the max fee.
+            FeeCheckError::MaxFeeExceeded { .. } => {
+                let TransactionInfo::Deprecated(ref context) = tx_context.tx_info else {
+                    panic!("MaxFeeExceeded can only originate from a deprecated transaction.");
+                };
+                context.max_fee
+            }
+            // If the error is resource overdraft, charge for the minimum between (a) actual gas
+            // used and (b) the user bound, for each gas type.
+            FeeCheckError::MaxGasAmountExceeded { .. } => {
+                let TransactionInfo::Current(ref context) = tx_context.tx_info else {
+                    panic!("MaxGasAmountExceeded can only originate from a V3 transaction.");
+                };
+                let gas_for_fee_charge = match context.resource_bounds {
+                    // For deprecated resource bounds, the total L1 gas for fee charge includes the
+                    // discounted L1 data gas cost.
+                    ValidResourceBounds::L1Gas(l1_bounds) => GasVector::from_l1_gas(std::cmp::min(
+                        l1_bounds.max_amount,
+                        actual_gas.to_discounted_l1_gas(gas_prices),
+                    )),
+                    ValidResourceBounds::AllResources(all_resource_bounds) => GasVector {
+                        l1_gas: std::cmp::min(
+                            all_resource_bounds.l1_gas.max_amount,
+                            actual_gas.l1_gas,
+                        ),
+                        l2_gas: std::cmp::min(
+                            all_resource_bounds.l2_gas.max_amount,
+                            actual_gas.l2_gas,
+                        ),
+                        l1_data_gas: std::cmp::min(
+                            all_resource_bounds.l1_data_gas.max_amount,
+                            actual_gas.l1_data_gas,
+                        ),
+                    },
+                };
+
+                get_fee_by_gas_vector(
+                    &tx_context.block_context.block_info,
+                    gas_for_fee_charge,
+                    &FeeType::Strk,
+                )
             }
         };
         Self { recommended_fee, error: Some(error) }
@@ -232,7 +265,7 @@ impl PostExecutionReport {
         tx_receipt: &TransactionReceipt,
         charge_fee: bool,
     ) -> TransactionExecutionResult<Self> {
-        let TransactionReceipt { fee, .. } = tx_receipt;
+        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() {
@@ -257,6 +290,8 @@ impl PostExecutionReport {
                     // current context, and return the report.
                     return Ok(Self(FeeCheckReport::from_fee_check_error(
                         *fee,
+                        *gas,
+                        tx_context.get_gas_prices(),
                         fee_check_error,
                         tx_context,
                     )));