Skip to content

Commit

Permalink
chore(blockifier): check max price of l1_data_gas and l2_gas (#710)
Browse files Browse the repository at this point in the history
  • Loading branch information
aner-starkware authored Sep 24, 2024
1 parent 352521b commit 64cb254
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 26 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 @@ -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.
Expand Down
92 changes: 67 additions & 25 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ 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,
ValidResourceBounds,
};
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 @@ -241,35 +244,74 @@ impl AccountTransaction {
let block_info = &block_context.block_info;
let fee_type = &tx_info.fee_type();
match tx_info {
TransactionInfo::Current(context) => {
let ResourceBounds {
TransactionInfo::Current(context) => match &context.resource_bounds {
ValidResourceBounds::L1Gas(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: 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 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: L1Gas,
max_gas_price: *max_l1_gas_price,
actual_gas_price: actual_l1_gas_price.into(),
})?;
}
}

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(),
})?;
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()),
] {
// TODO(Aner): refactor to return all prices that are too low.
if max_gas_price < actual_gas_price {
return Err(TransactionFeeError::MaxGasPriceTooLow {
resource,
max_gas_price,
actual_gas_price,
})?;
}
}
}
}
},
TransactionInfo::Deprecated(context) => {
let max_fee = context.max_fee;
let min_fee =
Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet_api/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,14 +870,14 @@ impl From<Tip> for Felt {
Copy,
Debug,
Deserialize,
Display,
EnumIter,
Eq,
Hash,
Ord,
PartialEq,
PartialOrd,
Serialize,
Display,
)]
pub enum Resource {
#[serde(rename = "L1_GAS")]
Expand Down

0 comments on commit 64cb254

Please sign in to comment.