Skip to content

Commit

Permalink
chore(blockifier): check max price of l1_data_gas and l2_gas
Browse files Browse the repository at this point in the history
  • Loading branch information
aner-starkware committed Sep 4, 2024
1 parent ecc87c2 commit 958996d
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 31 deletions.
15 changes: 15 additions & 0 deletions crates/blockifier/src/blockifier/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
90 changes: 66 additions & 24 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -229,6 +232,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.
Expand All @@ -240,30 +244,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) => {
Expand Down
8 changes: 4 additions & 4 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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}."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
result.unwrap_err(),
TransactionExecutionError::TransactionPreValidationError(
TransactionPreValidationError::TransactionFeeError(
TransactionFeeError::MaxL1GasPriceTooLow { .. }
TransactionFeeError::MaxGasPriceTooLow { .. }
)
)
);
Expand Down
5 changes: 4 additions & 1 deletion crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
DeprecatedResourceBoundsMapping,
Expand Down Expand Up @@ -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.
Expand Down
13 changes: 12 additions & 1 deletion crates/starknet_api/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down

0 comments on commit 958996d

Please sign in to comment.