From 1a33a5f052faf534a5017fecd17cd359326d353e Mon Sep 17 00:00:00 2001 From: Nimrod Weiss Date: Mon, 26 Aug 2024 12:56:44 +0300 Subject: [PATCH] build(fee): implement methods on resource bounds --- crates/blockifier/src/fee/fee_checks.rs | 53 +++++----- crates/blockifier/src/fee/gas_usage.rs | 30 ++++-- crates/blockifier/src/fee/gas_usage_test.rs | 6 +- .../src/transaction/account_transaction.rs | 4 +- crates/blockifier/src/transaction/objects.rs | 99 ++++++++++++++++--- .../src/transaction/transactions_test.rs | 2 +- crates/starknet_api/src/transaction.rs | 16 +++ 7 files changed, 156 insertions(+), 54 deletions(-) diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs index 4cbcf6bd31..1a03fce4cf 100644 --- a/crates/blockifier/src/fee/fee_checks.rs +++ b/crates/blockifier/src/fee/fee_checks.rs @@ -5,12 +5,12 @@ use thiserror::Error; use crate::context::TransactionContext; use crate::fee::actual_cost::TransactionReceipt; use crate::fee::fee_utils::{get_balance_and_if_covers_fee, get_fee_by_gas_vector}; -use crate::fee::gas_usage::compute_discounted_gas_from_gas_vector; use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::objects::{ + BoundsChecker, FeeType, - GasVector, + HasRelatedFeeType, TransactionExecutionResult, TransactionInfo, }; @@ -19,6 +19,12 @@ use crate::transaction::objects::{ pub enum FeeCheckError { #[error("Insufficient max L1 gas: max amount: {max_amount}, actual used: {actual_amount}.")] MaxL1GasAmountExceeded { max_amount: u128, actual_amount: u128 }, + #[error("Insufficient max L2 gas: max amount: {max_amount}, actual used: {actual_amount}.")] + MaxL2GasAmountExceeded { max_amount: u128, actual_amount: u128 }, + #[error( + "Insufficient max L1 data gas: max amount: {max_amount}, actual used: {actual_amount}." + )] + MaxL1DataGasAmountExceeded { max_amount: u128, actual_amount: u128 }, #[error("Insufficient max fee: max fee: {}, actual fee: {}.", max_fee.0, actual_fee.0)] MaxFeeExceeded { max_fee: Fee, actual_fee: Fee }, #[error( @@ -71,16 +77,17 @@ impl FeeCheckReport { // If the error is resource overdraft, the recommended fee is the resource bounds. // If the transaction passed pre-validation checks (i.e. balance initially covered the // resource bounds), the sender should be able to pay this fee. - FeeCheckError::MaxFeeExceeded { .. } | FeeCheckError::MaxL1GasAmountExceeded { .. } => { - match &tx_context.tx_info { - TransactionInfo::Current(info) => get_fee_by_gas_vector( - &tx_context.block_context.block_info, - GasVector::from_l1_gas(info.l1_resource_bounds().max_amount.into()), - &FeeType::Strk, - ), - TransactionInfo::Deprecated(context) => context.max_fee, - } - } + FeeCheckError::MaxFeeExceeded { .. } + | FeeCheckError::MaxL1GasAmountExceeded { .. } + | FeeCheckError::MaxL2GasAmountExceeded { .. } + | FeeCheckError::MaxL1DataGasAmountExceeded { .. } => match &tx_context.tx_info { + TransactionInfo::Current(info) => get_fee_by_gas_vector( + &tx_context.block_context.block_info, + (&info.resource_bounds).into(), + &FeeType::Strk, + ), + TransactionInfo::Deprecated(context) => context.max_fee, + }, }; Self { recommended_fee, error: Some(error) } } @@ -98,23 +105,11 @@ impl FeeCheckReport { // sender. // TODO(Aner, 21/01/24) modify for 4844 (include check for blob_gas). match tx_info { - TransactionInfo::Current(context) => { - // Check L1 gas limit. - let max_l1_gas = context.l1_resource_bounds().max_amount.into(); - - // TODO(Dori, 1/7/2024): When data gas limit is added (and enforced) in resource - // bounds, check it here as well (separately, with a different error variant if - // limit exceeded). - let total_discounted_gas_used = - compute_discounted_gas_from_gas_vector(gas, tx_context); - - if total_discounted_gas_used > max_l1_gas { - return Err(FeeCheckError::MaxL1GasAmountExceeded { - max_amount: max_l1_gas, - actual_amount: total_discounted_gas_used, - })?; - } - } + TransactionInfo::Current(context) => context.resource_bounds.can_cover( + gas, + &tx_context.tx_info.fee_type(), + &tx_context.block_context.block_info.gas_prices, + )?, TransactionInfo::Deprecated(context) => { // Check max fee. let max_fee = context.max_fee; diff --git a/crates/blockifier/src/fee/gas_usage.rs b/crates/blockifier/src/fee/gas_usage.rs index ecbf658ba0..2760cb5439 100644 --- a/crates/blockifier/src/fee/gas_usage.rs +++ b/crates/blockifier/src/fee/gas_usage.rs @@ -2,11 +2,17 @@ use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use super::fee_utils::calculate_l1_gas_by_vm_usage; use crate::abi::constants; +use crate::blockifier::block::GasPrices; use crate::context::{BlockContext, TransactionContext}; use crate::fee::eth_gas_constants; use crate::state::cached_state::StateChangesCount; use crate::transaction::account_transaction::AccountTransaction; -use crate::transaction::objects::{GasVector, HasRelatedFeeType, TransactionPreValidationResult}; +use crate::transaction::objects::{ + FeeType, + GasVector, + HasRelatedFeeType, + TransactionPreValidationResult, +}; use crate::utils::{u128_div_ceil, u128_from_usize}; #[cfg(test)] @@ -190,14 +196,24 @@ pub fn estimate_minimal_gas_vector( /// X non-data-related gas consumption and Y bytes of data, in non-blob mode, would /// cost (X + 16*Y) units of gas. Applying the discount ratio to the data-related /// summand, we get total_gas = (X + Y * DGP / GP). -pub fn compute_discounted_gas_from_gas_vector( +pub fn compute_discounted_gas( gas_usage_vector: &GasVector, - tx_context: &TransactionContext, + fee_type: &FeeType, + gas_prices: &GasPrices, ) -> u128 { - let gas_prices = &tx_context.block_context.block_info.gas_prices; let GasVector { l1_gas: gas_usage, l1_data_gas: blob_gas_usage, .. } = gas_usage_vector; - let fee_type = tx_context.tx_info.fee_type(); - let gas_price = gas_prices.get_l1_gas_price_by_fee_type(&fee_type); - let data_gas_price = gas_prices.get_l1_data_gas_price_by_fee_type(&fee_type); + let gas_price = gas_prices.get_l1_gas_price_by_fee_type(fee_type); + let data_gas_price = gas_prices.get_l1_data_gas_price_by_fee_type(fee_type); gas_usage + u128_div_ceil(blob_gas_usage * u128::from(data_gas_price), gas_price) } + +pub fn compute_discounted_gas_from_tx_context( + gas_usage_vector: &GasVector, + tx_context: &TransactionContext, +) -> u128 { + compute_discounted_gas( + gas_usage_vector, + &tx_context.tx_info.fee_type(), + &tx_context.block_context.block_info.gas_prices, + ) +} diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index 1d89c913db..37c1c250f0 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -11,7 +11,7 @@ use crate::execution::call_info::{CallExecution, CallInfo, OrderedEvent}; use crate::fee::eth_gas_constants; use crate::fee::fee_utils::get_fee_by_gas_vector; use crate::fee::gas_usage::{ - compute_discounted_gas_from_gas_vector, + compute_discounted_gas_from_tx_context, get_da_gas_cost, get_message_segment_length, }; @@ -204,11 +204,11 @@ fn test_get_message_segment_length( } #[rstest] -fn test_compute_discounted_gas_from_gas_vector() { +fn test_compute_discounted_gas() { let tx_context = BlockContext::create_for_testing().to_tx_context(&account_invoke_tx(invoke_tx_args! {})); let gas_usage = GasVector { l1_gas: 100, l1_data_gas: 2, ..Default::default() }; - let actual_result = compute_discounted_gas_from_gas_vector(&gas_usage, &tx_context); + let actual_result = compute_discounted_gas_from_tx_context(&gas_usage, &tx_context); let result_div_ceil = gas_usage.l1_gas + u128_div_ceil( diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 0dbfe9dfc1..b0232ad701 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -25,7 +25,7 @@ use crate::fee::fee_utils::{ get_sequencer_balance_keys, verify_can_pay_committed_bounds, }; -use crate::fee::gas_usage::{compute_discounted_gas_from_gas_vector, estimate_minimal_gas_vector}; +use crate::fee::gas_usage::{compute_discounted_gas_from_tx_context, estimate_minimal_gas_vector}; use crate::retdata; use crate::state::cached_state::{StateChanges, TransactionalState}; use crate::state::state_api::{State, StateReader, UpdatableState}; @@ -232,7 +232,7 @@ impl AccountTransaction { estimate_minimal_gas_vector(&tx_context.block_context, self)?; // TODO(Aner, 30/01/24): modify once data gas limit is enforced. let minimal_l1_gas_amount = - compute_discounted_gas_from_gas_vector(&minimal_l1_gas_amount_vector, tx_context); + compute_discounted_gas_from_tx_context(&minimal_l1_gas_amount_vector, tx_context); let TransactionContext { block_context, tx_info } = tx_context; let block_info = &block_context.block_info; diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index 3d482282a3..a425324269 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -22,12 +22,14 @@ use starknet_types_core::felt::Felt; use strum_macros::EnumIter; use crate::abi::constants as abi_constants; -use crate::blockifier::block::BlockInfo; +use crate::blockifier::block::{BlockInfo, GasPrices}; use crate::execution::call_info::{CallInfo, ExecutionSummary, MessageL1CostInfo, OrderedEvent}; use crate::fee::actual_cost::TransactionReceipt; use crate::fee::eth_gas_constants; +use crate::fee::fee_checks::FeeCheckError; use crate::fee::fee_utils::{calculate_l1_gas_by_vm_usage, get_fee_by_gas_vector}; use crate::fee::gas_usage::{ + compute_discounted_gas, get_consumed_message_to_l2_emissions_cost, get_da_gas_cost, get_log_message_to_l1_emissions_cost, @@ -102,11 +104,7 @@ impl TransactionInfo { pub fn enforce_fee(&self) -> bool { match self { - TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds(); - let max_amount: u128 = l1_bounds.max_amount.into(); - max_amount * l1_bounds.max_price_per_unit > 0 - } + TransactionInfo::Current(context) => context.resource_bounds.willing_to_pay(), TransactionInfo::Deprecated(context) => context.max_fee != Fee(0), } } @@ -135,13 +133,11 @@ pub struct CurrentTransactionInfo { impl CurrentTransactionInfo { /// Fetch the L1 resource bounds, if they exist. - // TODO(Nimrod): Consider removing this function and add equivalent method to - // `ValidResourceBounds`. + // TODO(Nimrod): Delete this function once: + // 1. L2 gas price exists in block info. + // 2. tx resources can be calculated with l2 prices. pub fn l1_resource_bounds(&self) -> ResourceBounds { - match self.resource_bounds { - ValidResourceBounds::L1Gas(bounds) => bounds, - ValidResourceBounds::AllResources(AllResourceBounds { l1_gas, .. }) => l1_gas, - } + self.resource_bounds.get_l1_bounds() } } @@ -161,6 +157,23 @@ pub struct GasVector { pub l2_gas: u128, } +impl From<&ValidResourceBounds> for GasVector { + fn from(resource_bounds: &ValidResourceBounds) -> Self { + match resource_bounds { + ValidResourceBounds::L1Gas(l1_bounds) => Self::from_l1_gas(l1_bounds.max_amount.into()), + ValidResourceBounds::AllResources(AllResourceBounds { + l1_gas, + l2_gas, + l1_data_gas, + }) => Self { + l1_gas: l1_gas.max_amount.into(), + l1_data_gas: l1_data_gas.max_amount.into(), + l2_gas: l2_gas.max_amount.into(), + }, + } + } +} + impl GasVector { pub fn from_l1_gas(l1_gas: u128) -> Self { Self { l1_gas, ..Default::default() } @@ -571,3 +584,65 @@ pub enum FeeType { pub trait TransactionInfoCreator { fn create_tx_info(&self) -> TransactionInfo; } + +type FeeCheckResult = Result; +// Better name for the trait? +pub trait BoundsChecker { + /// Checks whether the given gas vector can be covered by the resource bounds. + /// Returns Ok if it can, otherwise returns an error indicates why it can't. + fn can_cover( + &self, + gas_vector: &GasVector, + fee_type: &FeeType, + gas_prices: &GasPrices, + ) -> FeeCheckResult<()>; +} + +impl BoundsChecker for ValidResourceBounds { + fn can_cover( + &self, + gas_vector: &GasVector, + fee_type: &FeeType, + gas_prices: &GasPrices, + ) -> FeeCheckResult<()> { + match self { + ValidResourceBounds::L1Gas(l1_bounds) => { + let max_l1_gas = l1_bounds.max_amount.into(); + let total_discounted_gas_used = + compute_discounted_gas(gas_vector, fee_type, gas_prices); + if total_discounted_gas_used > max_l1_gas { + return Err(FeeCheckError::MaxL1GasAmountExceeded { + max_amount: max_l1_gas, + actual_amount: total_discounted_gas_used, + }); + } + } + ValidResourceBounds::AllResources(AllResourceBounds { + l1_gas, + l2_gas, + l1_data_gas, + }) => { + // Does the order matter? + if gas_vector.l1_gas > l1_gas.max_amount.into() { + return Err(FeeCheckError::MaxL2GasAmountExceeded { + max_amount: l1_gas.max_amount.into(), + actual_amount: gas_vector.l1_gas, + }); + } + if gas_vector.l2_gas > l2_gas.max_amount.into() { + return Err(FeeCheckError::MaxL2GasAmountExceeded { + max_amount: l2_gas.max_amount.into(), + actual_amount: gas_vector.l2_gas, + }); + } + if gas_vector.l1_data_gas > l1_data_gas.max_amount.into() { + return Err(FeeCheckError::MaxL1DataGasAmountExceeded { + max_amount: l1_data_gas.max_amount.into(), + actual_amount: gas_vector.l1_data_gas, + }); + } + } + } + Ok(()) + } +} diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index b06a75b2db..6d60bfc396 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -803,7 +803,7 @@ fn assert_failure_if_resource_bounds_exceed_balance( ); } TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds(); + let l1_bounds = context.resource_bounds.get_l1_bounds(); assert_matches!( invalid_tx.execute(state, block_context, true, true).unwrap_err(), TransactionExecutionError::TransactionPreValidationError( diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index 43e58c752f..dcefdb9a84 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -955,6 +955,22 @@ impl ValidResourceBounds { Self::AllResources(AllResourceBounds { l2_gas, .. }) => *l2_gas, } } + + /// Returns whether there is a resource the user can pay for. + pub fn willing_to_pay(&self) -> bool { + match self { + Self::L1Gas(l1_bounds) => { + l1_bounds.max_price_per_unit * u128::from(l1_bounds.max_amount) > 0 + } + Self::AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas }) => { + let can_pay_l1_gas = l1_gas.max_price_per_unit * u128::from(l1_gas.max_amount) > 0; + let can_pay_l2_gas = l2_gas.max_price_per_unit * u128::from(l2_gas.max_amount) > 0; + let can_pay_l1_data_gas = + l1_data_gas.max_price_per_unit * u128::from(l1_data_gas.max_amount) > 0; + can_pay_l1_gas || can_pay_l2_gas || can_pay_l1_data_gas + } + } + } } #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize)]