chore(blockifier): check max price of l1_data_gas and l2_gas

starkware-libs · Sep 5, 2024 · b9ee9bc · b9ee9bc
1 parent 5f2a0ff
commit b9ee9bc
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 31 deletions.
diff --git a/crates/blockifier/src/blockifier/block.rs b/crates/blockifier/src/blockifier/block.rs
@@ -37,6 +37,13 @@ pub struct GasPrices {
     strk_l2_gas_price: NonZeroU128,      // In fri.
 }
 
+#[derive(Debug)]
+pub struct GasPricesForFeeType {
+    pub l1_gas_price: NonZeroU128,
+    pub l1_data_gas_price: NonZeroU128,
+    pub l2_gas_price: NonZeroU128,
+}
+
 impl GasPrices {
     pub fn new(
         eth_l1_gas_price: NonZeroU128,
@@ -94,6 +101,14 @@ impl GasPrices {
             FeeType::Eth => self.eth_l2_gas_price,
         }
     }
+
+    pub fn get_gas_prices_by_fee_type(&self, fee_type: &FeeType) -> GasPricesForFeeType {
+        GasPricesForFeeType {
+            l1_gas_price: self.get_l1_data_gas_price_by_fee_type(fee_type),
+            l1_data_gas_price: self.get_l1_data_gas_price_by_fee_type(fee_type),
+            l2_gas_price: self.get_l2_gas_price_by_fee_type(fee_type),
+        }
+    }
 }
 
 // Block pre-processing.

diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs
@@ -4,7 +4,9 @@ use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
 use starknet_api::calldata;
 use starknet_api::core::{ContractAddress, EntryPointSelector};
 use starknet_api::deprecated_contract_class::EntryPointType;
+use starknet_api::transaction::Resource::{L1DataGas, L1Gas, L2Gas};
 use starknet_api::transaction::{
+    AllResourceBounds,
     Calldata,
     Fee,
     ResourceBounds,
@@ -14,6 +16,7 @@ use starknet_api::transaction::{
 use starknet_types_core::felt::Felt;
 
 use crate::abi::abi_utils::selector_from_name;
+use crate::blockifier::block::GasPricesForFeeType;
 use crate::context::{BlockContext, TransactionContext};
 use crate::execution::call_info::{CallInfo, Retdata};
 use crate::execution::contract_class::ContractClass;
@@ -228,6 +231,7 @@ impl AccountTransaction {
         &self,
         tx_context: &TransactionContext,
     ) -> TransactionPreValidationResult<()> {
+        // TODO(Aner): seprate to cases based on context.resource_bounds type
         let minimal_l1_gas_amount_vector =
             estimate_minimal_gas_vector(&tx_context.block_context, self)?;
         // TODO(Aner, 30/01/24): modify once data gas limit is enforced.
@@ -239,30 +243,68 @@ impl AccountTransaction {
         let fee_type = &tx_info.fee_type();
         match 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();
-
-                let max_l1_gas_amount_as_u128: u128 = max_l1_gas_amount.into();
-                if max_l1_gas_amount_as_u128 < minimal_l1_gas_amount {
-                    return Err(TransactionFeeError::MaxL1GasAmountTooLow {
-                        max_l1_gas_amount,
-                        // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why
-                        // the convertion works.
-                        minimal_l1_gas_amount: (minimal_l1_gas_amount
-                            .try_into()
-                            .expect("Failed to convert u128 to u64.")),
-                    })?;
-                }
-
-                let actual_l1_gas_price =
-                    block_info.gas_prices.get_l1_gas_price_by_fee_type(fee_type);
-                if max_l1_gas_price < actual_l1_gas_price.into() {
-                    return Err(TransactionFeeError::MaxL1GasPriceTooLow {
-                        max_l1_gas_price,
-                        actual_l1_gas_price: actual_l1_gas_price.into(),
-                    })?;
+                match &context.resource_bounds {
+                    starknet_api::transaction::ValidResourceBounds::L1Gas(ResourceBounds {
+                        max_amount: max_l1_gas_amount,
+                        max_price_per_unit: max_l1_gas_price,
+                    }) => {
+                        let max_l1_gas_amount_as_u128: u128 = (*max_l1_gas_amount).into();
+                        if max_l1_gas_amount_as_u128 < minimal_l1_gas_amount {
+                            return Err(TransactionFeeError::MaxL1GasAmountTooLow {
+                                max_l1_gas_amount: *max_l1_gas_amount,
+                                // TODO(Ori, 1/2/2024): Write an indicative expect message
+                                // explaining why the convertion
+                                // works.
+                                minimal_l1_gas_amount: (minimal_l1_gas_amount
+                                    .try_into()
+                                    .expect("Failed to convert u128 to u64.")),
+                            })?;
+                        }
+
+                        let actual_l1_gas_price =
+                            block_info.gas_prices.get_l1_gas_price_by_fee_type(fee_type);
+                        if *max_l1_gas_price < actual_l1_gas_price.into() {
+                            return Err(TransactionFeeError::MaxGasPriceTooLow {
+                                gas_type: L1Gas,
+                                max_gas_price: *max_l1_gas_price,
+                                actual_gas_price: actual_l1_gas_price.into(),
+                            })?;
+                        }
+                    }
+                    starknet_api::transaction::ValidResourceBounds::AllResources(
+                        AllResourceBounds { l1_gas, l2_gas, l1_data_gas },
+                    ) => {
+                        let max_l1_gas_amount_as_u128: u128 = l1_gas.max_amount.into();
+                        if max_l1_gas_amount_as_u128 < minimal_l1_gas_amount {
+                            return Err(TransactionFeeError::MaxL1GasAmountTooLow {
+                                max_l1_gas_amount: l1_gas.max_amount,
+                                // TODO(Ori, 1/2/2024): Write an indicative expect message
+                                // explaining why the convertion
+                                // works.
+                                minimal_l1_gas_amount: (minimal_l1_gas_amount
+                                    .try_into()
+                                    .expect("Failed to convert u128 to u64.")),
+                            })?;
+                        }
+                        // TODO(Aner): add checks for minimal_l1_data_gas and minimal_l2_gas
+
+                        let GasPricesForFeeType { l1_gas_price, l1_data_gas_price, l2_gas_price } =
+                            block_info.gas_prices.get_gas_prices_by_fee_type(fee_type);
+                        // TODO!(Aner): add tests for l1_data_gas_price and l2_gas_price
+                        for (gas_type, max_gas_price, actual_gas_price) in [
+                            (L1Gas, l1_gas.max_price_per_unit, l1_gas_price.into()),
+                            (L1DataGas, l1_data_gas.max_price_per_unit, l1_data_gas_price.into()),
+                            (L2Gas, l2_gas.max_price_per_unit, l2_gas_price.into()),
+                        ] {
+                            if max_gas_price < actual_gas_price {
+                                return Err(TransactionFeeError::MaxGasPriceTooLow {
+                                    gas_type,
+                                    max_gas_price,
+                                    actual_gas_price,
+                                })?;
+                            }
+                        }
+                    }
                 }
             }
             TransactionInfo::Deprecated(context) => {

diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs
@@ -1,7 +1,7 @@
 use cairo_vm::types::errors::program_errors::ProgramError;
 use num_bigint::BigUint;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce};
-use starknet_api::transaction::{Fee, TransactionVersion};
+use starknet_api::transaction::{Fee, Resource, TransactionVersion};
 use starknet_api::StarknetApiError;
 use starknet_types_core::felt::FromStrError;
 use thiserror::Error;
@@ -33,10 +33,10 @@ pub enum TransactionFeeError {
     #[error("Max fee ({}) is too low. Minimum fee: {}.", max_fee.0, min_fee.0)]
     MaxFeeTooLow { min_fee: Fee, max_fee: Fee },
     #[error(
-        "Max L1 gas price ({max_l1_gas_price}) is lower than the actual gas price: \
-         {actual_l1_gas_price}."
+        "Max {gas_type} gas price ({max_gas_price}) is lower than the actual gas price: \
+         {actual_gas_price}."
     )]
-    MaxL1GasPriceTooLow { max_l1_gas_price: u128, actual_l1_gas_price: u128 },
+    MaxGasPriceTooLow { gas_type: Resource, max_gas_price: u128, actual_gas_price: u128 },
     #[error(
         "Max L1 gas amount ({max_l1_gas_amount}) is lower than the minimal gas amount: \
          {minimal_l1_gas_amount}."

diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs
@@ -329,7 +329,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
                 result.unwrap_err(),
                 TransactionExecutionError::TransactionPreValidationError(
                     TransactionPreValidationError::TransactionFeeError(
-                        TransactionFeeError::MaxL1GasPriceTooLow { .. }
+                        TransactionFeeError::MaxGasPriceTooLow { .. }
                     )
                 )
             );

diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs
@@ -11,6 +11,7 @@ use rstest::{fixture, rstest};
 use starknet_api::core::{ChainId, ClassHash, ContractAddress, EthAddress, Nonce, PatriciaKey};
 use starknet_api::deprecated_contract_class::EntryPointType;
 use starknet_api::state::StorageKey;
+use starknet_api::transaction::Resource::L1Gas;
 use starknet_api::transaction::{
     Calldata,
     EventContent,
@@ -965,10 +966,12 @@ fn test_insufficient_resource_bounds(
         execution_error,
         TransactionExecutionError::TransactionPreValidationError(
             TransactionPreValidationError::TransactionFeeError(
-                TransactionFeeError::MaxL1GasPriceTooLow{ max_l1_gas_price, actual_l1_gas_price }))
+                TransactionFeeError::MaxGasPriceTooLow{ gas_type: L1Gas ,max_gas_price: max_l1_gas_price, actual_gas_price:actual_l1_gas_price }))
         if max_l1_gas_price == insufficient_max_l1_gas_price &&
         actual_l1_gas_price == actual_strk_l1_gas_price.into()
     );
+
+    // TODO(Aner): add test for low Max L1 data gas price and L2 gas price
 }
 
 // TODO(Aner, 21/01/24) modify test for 4844.

diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs
@@ -867,7 +867,18 @@ impl From<Tip> for Felt {
 
 /// Execution resource.
 #[derive(
-    Clone, Copy, Debug, Deserialize, EnumIter, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize,
+    Clone,
+    Copy,
+    Debug,
+    Deserialize,
+    Display,
+    EnumIter,
+    Eq,
+    Hash,
+    Ord,
+    PartialEq,
+    PartialOrd,
+    Serialize,
 )]
 pub enum Resource {
     #[serde(rename = "L1_GAS")]