Skip to content

Commit

Permalink
chore(blockifier): check sum of products covers balance for new resou…
Browse files Browse the repository at this point in the history 
…rce bounds
  • Loading branch information
@aner-starkware
aner-starkware committed Sep 26, 2024
1 parent fdd4c92 commit 1141f2a
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 40 deletions.
54 changes: 42 additions & 12 deletions crates/blockifier/src/fee/fee_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use num_bigint::BigUint;
use starknet_api::core::ContractAddress;
use starknet_api::state::StorageKey;
use starknet_api::transaction::{Fee, Resource};
use starknet_api::transaction::ValidResourceBounds::{AllResources, L1Gas};
use starknet_api::transaction::{AllResourceBounds, Fee, Resource};
use starknet_types_core::felt::Felt;

use crate::abi::abi_utils::get_fee_token_var_address;
Expand Down Expand Up @@ -117,11 +118,26 @@ pub fn verify_can_pay_committed_bounds(
let tx_info = &tx_context.tx_info;
let committed_fee = match tx_info {
TransactionInfo::Current(context) => {
let l1_bounds = context.l1_resource_bounds();
let max_amount: u128 = l1_bounds.max_amount.into();
// Sender will not be charged by `max_price_per_unit`, but this check should not depend
// on the current gas price.
Fee(max_amount * l1_bounds.max_price_per_unit)
match &context.resource_bounds {
// Old resource bounds, only L1 Gas.
L1Gas(l1_gas) =>
// Sender will not be charged by `max_price_per_unit`, but this check should not
// depend on the current gas price.
{
Fee(u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit)
}
// New resource bounds, also includes L1 Data Gas and L2 Gas.
// TODO!(Aner): add tests
AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas }) => {
// Committed fee is sum of products (resource_max_amount * resource_max_price)
// of the different resources.
// The Sender will not be charged by`max_price_per_unit`, but this check should
// not depend on the current gas price
Fee(u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
+ u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
+ u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit)
}
}
}
TransactionInfo::Deprecated(context) => context.max_fee,
};
Expand All @@ -132,12 +148,26 @@ pub fn verify_can_pay_committed_bounds(
} else {
Err(match tx_info {
TransactionInfo::Current(context) => {
let l1_bounds = context.l1_resource_bounds();
TransactionFeeError::GasBoundsExceedBalance {
resource: Resource::L1Gas,
max_amount: l1_bounds.max_amount,
max_price: l1_bounds.max_price_per_unit,
balance: balance_to_big_uint(&balance_low, &balance_high),
match &context.resource_bounds {
// Old resource bounds, only L1 Gas.
L1Gas(l1_gas) => TransactionFeeError::GasBoundsExceedBalance {
resource: Resource::L1Gas,
max_amount: l1_gas.max_amount,
max_price: l1_gas.max_price_per_unit,
balance: balance_to_big_uint(&balance_low, &balance_high),
},
// New resource bounds, also includes L1 Data Gas and L2 Gas.
AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas }) => {
TransactionFeeError::ResourcesBoundsExceedBalance {
balance: balance_to_big_uint(&balance_low, &balance_high),
l1_max_amount: l1_gas.max_amount,
l1_max_price: l1_gas.max_price_per_unit,
l1_data_max_amount: l1_data_gas.max_amount,
l1_data_max_price: l1_data_gas.max_price_per_unit,
l2_max_amount: l2_gas.max_amount,
l2_max_price: l2_gas.max_price_per_unit,
}
}
}
}
TransactionInfo::Deprecated(context) => TransactionFeeError::MaxFeeExceedsBalance {
Expand Down
40 changes: 14 additions & 26 deletions crates/blockifier/src/transaction/account_transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,78 +258,66 @@ impl AccountTransaction {
match tx_info {
TransactionInfo::Current(context) => {
let resources_amount_tuple = match &context.resource_bounds {
ValidResourceBounds::L1Gas(ResourceBounds {
max_amount: max_l1_gas_amount,
max_price_per_unit: max_l1_gas_price,
}) => vec![(
ValidResourceBounds::L1Gas(l1_gas_resource_bounds) => vec![(
L1Gas,
*max_l1_gas_amount,
l1_gas_resource_bounds,
minimal_l1_gas_amount,
*max_l1_gas_price,
u128::from(block_info.gas_prices.get_l1_gas_price_by_fee_type(fee_type)),
)],
ValidResourceBounds::AllResources(AllResourceBounds {
l1_gas,
l2_gas,
l1_data_gas,
l1_gas: l1_gas_resource_bounds,
l2_gas: l2_gas_resource_bounds,
l1_data_gas: l1_data_gas_resource_bounds,
}) => {
let GasPricesForFeeType { l1_gas_price, l1_data_gas_price, l2_gas_price } =
block_info.gas_prices.get_gas_prices_by_fee_type(fee_type);
vec![
(
L1Gas,
l1_gas.max_amount,
l1_gas_resource_bounds,
minimal_l1_gas_amount,
l1_gas.max_price_per_unit,
l1_gas_price.into(),
),
(
L1DataGas,
l1_data_gas.max_amount,
l1_data_gas_resource_bounds,
minimal_gas_amount_vector
.l1_data_gas
.try_into()
// TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works.
.expect("Failed to convert u128 to u64."),
l1_data_gas.max_price_per_unit,
l1_data_gas_price.into(),
),
(
L2Gas,
l2_gas.max_amount,
l2_gas_resource_bounds,
minimal_gas_amount_vector
.l2_gas
.try_into()
// TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works.
.expect("Failed to convert u128 to u64."),
l2_gas.max_price_per_unit,
l2_gas_price.into(),
),
]
}
};
for (
resource,
max_gas_amount,
minimal_gas_amount,
max_gas_price,
actual_gas_price,
) in resources_amount_tuple
for (resource, resource_bounds, minimal_gas_amount, actual_gas_price) in
resources_amount_tuple
{
// TODO(Aner): refactor to indicate both amount and price are too low.
// TODO(Aner): refactor to return all amounts that are too low.
if max_gas_amount < minimal_gas_amount {
if resource_bounds.max_amount < minimal_gas_amount {
return Err(TransactionFeeError::MaxGasAmountTooLow {
resource,
max_gas_amount,
max_gas_amount: resource_bounds.max_amount,
minimal_gas_amount,
})?;
}
// TODO(Aner): refactor to return all prices that are too low.
if max_gas_price < actual_gas_price {
if resource_bounds.max_price_per_unit < actual_gas_price {
return Err(TransactionFeeError::MaxGasPriceTooLow {
resource,
max_gas_price,
max_gas_price: resource_bounds.max_price_per_unit,
actual_gas_price,
})?;
}
Expand Down
19 changes: 17 additions & 2 deletions crates/blockifier/src/transaction/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,23 @@ pub enum TransactionFeeError {
#[error("Actual fee ({}) exceeded paid fee on L1 ({}).", actual_fee.0, paid_fee.0)]
InsufficientFee { paid_fee: Fee, actual_fee: Fee },
#[error(
"Resource {resource} bounds (max amount: {max_amount}, max price: {max_price}) exceed \
balance ({balance})."
"Resources bounds (l1 gas max amount: {l1_max_amount}, l1 gas max price: {l1_max_price}, \
l1 data max amount: {l1_data_max_amount}, l1 data max price: {l1_data_max_price}, l2 gas \
max amount: {l2_max_amount}, l2 gas max price: {l2_max_price}) exceed balance \
({balance})."
)]
ResourcesBoundsExceedBalance {
l1_max_amount: u64,
l1_max_price: u128,
l1_data_max_amount: u64,
l1_data_max_price: u128,
l2_max_amount: u64,
l2_max_price: u128,
balance: BigUint,
},
#[error(
"Resource {resource} Gas bounds (max amount: {max_amount}, max price): {max_price}) \
exceed balance ({balance})."
)]
GasBoundsExceedBalance {
resource: Resource,
Expand Down

0 comments on commit 1141f2a

Please sign in to comment.