From a71ffd9ad6810f744816e5c14a80f4f238ab73fc Mon Sep 17 00:00:00 2001 From: Aner Ben Efraim Date: Wed, 4 Sep 2024 15:35:09 +0300 Subject: [PATCH] chore(blockifier): check max price of l1_data_gas and l2_gas --- crates/blockifier/src/blockifier/block.rs | 15 +++ .../src/transaction/account_transaction.rs | 95 +++++++++++++------ .../src/transaction/transactions_test.rs | 2 + crates/starknet_api/src/transaction.rs | 2 +- 4 files changed, 86 insertions(+), 28 deletions(-) diff --git a/crates/blockifier/src/blockifier/block.rs b/crates/blockifier/src/blockifier/block.rs index 75e08dbc70..2bf9571fd4 100644 --- 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, @@ -96,6 +103,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_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 index 15d30dd554..00e84feb99 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -4,10 +4,11 @@ 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, - Resource, ResourceBounds, TransactionHash, TransactionVersion, @@ -15,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; @@ -234,6 +236,7 @@ impl AccountTransaction { self, &tx_context.get_gas_vector_computation_mode(), )?; + // TODO(Aner, 30/01/24): modify once data gas limit is enforced. let minimal_l1_gas_amount = minimal_l1_gas_amount_vector.to_discounted_l1_gas(tx_context); @@ -242,32 +245,70 @@ 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::MaxGasAmountTooLow { - resource: Resource::L1Gas, - max_gas_amount: max_l1_gas_amount, - // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why - // the convertion works. - minimal_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 { - resource: Resource::L1Gas, - max_gas_price: max_l1_gas_price, - actual_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::MaxGasAmountTooLow { + resource: L1Gas, + max_gas_amount: *max_l1_gas_amount, + // TODO(Ori, 1/2/2024): Write an indicative expect message + // explaining why the conversion + // works. + minimal_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 { + resource: 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::MaxGasAmountTooLow { + resource: L1Gas, + max_gas_amount: l1_gas.max_amount, + // TODO(Ori, 1/2/2024): Write an indicative expect message + // explaining why the conversion + // works. + minimal_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 (resource, 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 { + resource, + max_gas_price, + actual_gas_price, + })?; + } + } + } } } TransactionInfo::Deprecated(context) => { diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index f348039070..fa2112d279 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1021,6 +1021,8 @@ fn test_insufficient_resource_bounds( actual_gas_price == actual_strk_l1_gas_price.into() && resource == Resource::L1Gas ); + + // 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 index a5c5c16ece..0950b83a2b 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -870,6 +870,7 @@ impl From for Felt { Copy, Debug, Deserialize, + Display, EnumIter, Eq, Hash, @@ -877,7 +878,6 @@ impl From for Felt { PartialEq, PartialOrd, Serialize, - Display, )] pub enum Resource { #[serde(rename = "L1_GAS")]