From cd93544848948c21d2dc5b60c988c64f77283c9c Mon Sep 17 00:00:00 2001 From: dorimedini-starkware Date: Sun, 13 Oct 2024 16:04:48 +0300 Subject: [PATCH] feat(blockifier): implement and test max_steps computation for the L2 gas bound case (#1290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change is [Reviewable](https://reviewable.io/reviews/starkware-libs/sequencer/1290) --- .../blockifier/src/execution/entry_point.rs | 65 +++++++++++++------ .../transaction/account_transactions_test.rs | 63 ++++++++++++++++++ 2 files changed, 109 insertions(+), 19 deletions(-) diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index e3ebce173c..a5873037f9 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -9,7 +9,13 @@ use serde::Serialize; use starknet_api::contract_class::EntryPointType; use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector}; use starknet_api::state::StorageKey; -use starknet_api::transaction::{Calldata, TransactionVersion}; +use starknet_api::transaction::{ + AllResourceBounds, + Calldata, + ResourceBounds, + TransactionVersion, + ValidResourceBounds, +}; use starknet_types_core::felt::Felt; use crate::abi::abi_utils::selector_from_name; @@ -249,6 +255,7 @@ impl EntryPointExecutionContext { /// Returns the maximum number of cairo steps allowed, given the max fee, gas price and the /// execution mode. /// If fee is disabled, returns the global maximum. + /// The bound computation is saturating (no panic on overflow). fn max_steps( tx_context: &TransactionContext, mode: &ExecutionMode, @@ -270,31 +277,51 @@ impl EntryPointExecutionContext { return block_upper_bound; } - let gas_per_step = versioned_constants.vm_resource_fee_cost().n_steps; + // Deprecated transactions derive the step limit from the `max_fee`, by computing the L1 gas + // limit induced by the max fee and translating into cairo steps. + // New transactions with only L1 bounds use the L1 resource bounds directly. + // New transactions with L2 bounds use the L2 bounds directly. + let l1_gas_per_step = versioned_constants.vm_resource_fee_cost().n_steps; + let l2_gas_per_step = versioned_constants.os_constants.gas_costs.step_gas_cost; - // New transactions derive the step limit by the L1 gas resource bounds; deprecated - // transactions derive this value from the `max_fee`. - let tx_gas_upper_bound = match tx_info { + let tx_upper_bound_u64 = match tx_info { // Fee is a larger uint type than GasAmount, so we need to saturate the division. // This is just a computation of an upper bound, so it's safe to saturate. - TransactionInfo::Deprecated(context) => context.max_fee.saturating_div( - block_info.gas_prices.get_l1_gas_price_by_fee_type(&tx_info.fee_type()), - ), - TransactionInfo::Current(context) => context.l1_resource_bounds().max_amount, + TransactionInfo::Deprecated(context) => { + if l1_gas_per_step.is_zero() { + u64::MAX + } else { + let induced_l1_gas_limit = context.max_fee.saturating_div( + block_info.gas_prices.get_l1_gas_price_by_fee_type(&tx_info.fee_type()), + ); + (l1_gas_per_step.inv() * induced_l1_gas_limit.0).to_integer() + } + } + TransactionInfo::Current(context) => match context.resource_bounds { + ValidResourceBounds::L1Gas(ResourceBounds { max_amount, .. }) => { + if l1_gas_per_step.is_zero() { + u64::MAX + } else { + (l1_gas_per_step.inv() * max_amount.0).to_integer() + } + } + ValidResourceBounds::AllResources(AllResourceBounds { + l2_gas: ResourceBounds { max_amount, .. }, + .. + }) => { + if l2_gas_per_step.is_zero() { + u64::MAX + } else { + max_amount.0.saturating_div(l2_gas_per_step) + } + } + }, }; // Use saturating upper bound to avoid overflow. This is safe because the upper bound is // bounded above by the block's limit, which is a usize. - let upper_bound_u64 = if gas_per_step.is_zero() { - u64::MAX - } else { - (gas_per_step.inv() * tx_gas_upper_bound.0).to_integer() - }; - let tx_upper_bound = usize_from_u64(upper_bound_u64).unwrap_or_else(|_| { - log::warn!( - "Failed to convert u64 to usize: {upper_bound_u64}. Upper bound from tx is \ - {tx_gas_upper_bound}, gas per step is {gas_per_step}." - ); + let tx_upper_bound = usize_from_u64(tx_upper_bound_u64).unwrap_or_else(|_| { + log::warn!("Failed to convert u64 to usize: {tx_upper_bound_u64}."); usize::MAX }); min(tx_upper_bound, block_upper_bound) diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 8c6fe6db2a..333ac89810 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ResourceTracker; +use num_traits::Inv; use pretty_assertions::assert_eq; use rstest::rstest; use starknet_api::block::GasPrice; @@ -65,6 +66,7 @@ use crate::test_utils::{ CairoVersion, BALANCE, DEFAULT_STRK_L1_GAS_PRICE, + DEFAULT_STRK_L2_GAS_PRICE, MAX_FEE, }; use crate::transaction::account_transaction::AccountTransaction; @@ -989,6 +991,67 @@ fn test_n_reverted_steps( assert!(actual_fee_100 - actual_fee_0 == 100 * single_call_fee_delta); } +#[rstest] +fn test_max_fee_computation_from_tx_bounds(block_context: BlockContext) { + macro_rules! assert_max_steps_as_expected { + ($account_tx:expr, $expected_max_steps:expr $(,)?) => { + let tx_context = Arc::new(block_context.to_tx_context(&$account_tx)); + let execution_context = EntryPointExecutionContext::new_invoke(tx_context, true); + let max_steps = execution_context.vm_run_resources.get_n_steps().unwrap(); + assert_eq!(u64::try_from(max_steps).unwrap(), $expected_max_steps); + }; + } + + // V1 transaction: limit based on max fee. + // Convert max fee to L1 gas units, and then to steps. + let max_fee = Fee(100); + let account_tx_max_fee = account_invoke_tx(invoke_tx_args! { + max_fee, version: TransactionVersion::ONE + }); + let steps_per_l1_gas = block_context.versioned_constants.vm_resource_fee_cost().n_steps.inv(); + assert_max_steps_as_expected!( + account_tx_max_fee, + (steps_per_l1_gas + * max_fee + .checked_div( + block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&FeeType::Eth), + ) + .unwrap() + .0) + .to_integer(), + ); + + // V3 transaction: limit based on L1 gas bounds. + // Convert L1 gas units to steps. + let l1_gas_bound = 200_u64; + let account_tx_l1_bounds = account_invoke_tx(invoke_tx_args! { + resource_bounds: l1_resource_bounds(l1_gas_bound.into(), 1_u8.into()), + version: TransactionVersion::THREE + }); + assert_max_steps_as_expected!( + account_tx_l1_bounds, + (steps_per_l1_gas * l1_gas_bound).to_integer(), + ); + + // V3 transaction: limit based on L2 gas bounds (all resource_bounds). + // Convert L2 gas units to steps. + let l2_gas_bound = 300_u64; + let account_tx_l2_bounds = account_invoke_tx(invoke_tx_args! { + resource_bounds: ValidResourceBounds::AllResources(AllResourceBounds { + l2_gas: ResourceBounds { + max_amount: l2_gas_bound.into(), + max_price_per_unit: DEFAULT_STRK_L2_GAS_PRICE.into(), + }, + ..Default::default() + }), + version: TransactionVersion::THREE + }); + assert_max_steps_as_expected!( + account_tx_l2_bounds, + l2_gas_bound / block_context.versioned_constants.os_constants.gas_costs.step_gas_cost, + ); +} + #[rstest] /// Tests that steps are correctly limited based on max_fee. #[case(TransactionVersion::ONE)]