From ee3cae41ba47bcef3b4a08d6bedf34cd53a7d62d Mon Sep 17 00:00:00 2001 From: Aner Ben Efraim Date: Fri, 6 Sep 2024 08:09:33 +0300 Subject: [PATCH] chore(blockifier): check sum of products covers balance for new resource bounds --- crates/blockifier/src/fee/fee_utils.rs | 54 ++++++++++++++----- .../src/transaction/account_transaction.rs | 3 +- crates/blockifier/src/transaction/errors.rs | 19 ++++++- 3 files changed, 61 insertions(+), 15 deletions(-) diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index 8a972678155..abcbaf05e92 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -4,7 +4,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; @@ -115,11 +116,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, }; @@ -130,12 +146,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 { diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 097edd2c733..08147696ebf 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -12,6 +12,7 @@ use starknet_api::transaction::{ ResourceBounds, TransactionHash, TransactionVersion, + ValidResourceBounds, }; use starknet_types_core::felt::Felt; @@ -255,7 +256,7 @@ impl AccountTransaction { match tx_info { TransactionInfo::Current(context) => { match &context.resource_bounds { - starknet_api::transaction::ValidResourceBounds::L1Gas(ResourceBounds { + ValidResourceBounds::L1Gas(ResourceBounds { max_amount: max_l1_gas_amount, max_price_per_unit: max_l1_gas_price, }) => { diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index bdfe491646d..ba9183c568a 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -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,