Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading