From 1ff6bd447b61d662ad1a68fc58da53a54db6ea29 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Sun, 6 Oct 2024 10:09:04 +0300 Subject: [PATCH] refactor(blockifier,starknet_api): gas amount is now a u64, not a u128 --- crates/blockifier/src/bouncer.rs | 4 +- .../blockifier/src/execution/entry_point.rs | 15 ++---- crates/blockifier/src/fee/fee_test.rs | 48 ++++++++++++++----- crates/blockifier/src/fee/gas_usage.rs | 8 ++-- crates/blockifier/src/fee/gas_usage_test.rs | 15 +++--- crates/blockifier/src/fee/receipt_test.rs | 29 ++++++----- crates/blockifier/src/fee/resources.rs | 44 +++++++++++++---- crates/blockifier/src/test_utils.rs | 4 +- .../transaction/account_transactions_test.rs | 13 +++-- .../src/transaction/execution_flavors_test.rs | 26 +++++----- .../blockifier/src/transaction/test_utils.rs | 14 ++---- .../src/transaction/transactions_test.rs | 8 ++-- crates/blockifier/src/utils.rs | 6 --- crates/blockifier/src/versioned_constants.rs | 7 ++- .../src/py_block_executor.rs | 8 ++-- crates/starknet_api/src/block.rs | 5 +- .../starknet_api/src/execution_resources.rs | 8 ++-- crates/starknet_api/src/transaction.rs | 35 ++++++++------ crates/starknet_api/src/transaction_test.rs | 25 ++++++++-- 19 files changed, 187 insertions(+), 135 deletions(-) diff --git a/crates/blockifier/src/bouncer.rs b/crates/blockifier/src/bouncer.rs index 4e778f94c2..d60731d715 100644 --- a/crates/blockifier/src/bouncer.rs +++ b/crates/blockifier/src/bouncer.rs @@ -16,7 +16,7 @@ use crate::state::cached_state::{StateChangesKeys, StorageEntry}; use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::objects::{ExecutionResourcesTraits, TransactionExecutionResult}; -use crate::utils::usize_from_u128; +use crate::utils::usize_from_u64; #[cfg(test)] #[path = "bouncer_test.rs"] @@ -296,7 +296,7 @@ pub fn get_tx_weights( state_changes_keys: &StateChangesKeys, ) -> TransactionExecutionResult { let message_resources = &tx_resources.starknet_resources.messages; - let message_starknet_gas = usize_from_u128(message_resources.get_starknet_gas_cost().l1_gas.0) + let message_starknet_gas = usize_from_u64(message_resources.get_starknet_gas_cost().l1_gas.0) .expect("This conversion should not fail as the value is a converted usize."); let mut additional_os_resources = get_casm_hash_calculation_resources(state_reader, executed_class_hashes)?; diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index c2385aa7eb..1376dba8d0 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -278,10 +278,10 @@ impl EntryPointExecutionContext { // 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 { - TransactionInfo::Deprecated(context) => { - context.max_fee - / block_info.gas_prices.get_l1_gas_price_by_fee_type(&tx_info.fee_type()) - } + // Fee is a larger uint type than GasAmount, so we need to saturate the division. + 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.into(), }; @@ -291,12 +291,7 @@ impl EntryPointExecutionContext { let upper_bound_u64 = if gas_per_step.is_zero() { u64::MAX } else { - // TODO: This panic will disappear once GasAmount is a u64. - (gas_per_step.inv() - * u64::try_from(tx_gas_upper_bound.0).unwrap_or_else(|_| { - panic!("Gas amounts cannot be more than 64 bits; got {tx_gas_upper_bound:?}.") - })) - .to_integer() + (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!( diff --git a/crates/blockifier/src/fee/fee_test.rs b/crates/blockifier/src/fee/fee_test.rs index ad06c097a5..dfe067a8b9 100644 --- a/crates/blockifier/src/fee/fee_test.rs +++ b/crates/blockifier/src/fee/fee_test.rs @@ -28,7 +28,7 @@ use crate::test_utils::{ }; use crate::transaction::objects::FeeType; use crate::transaction::test_utils::{account_invoke_tx, all_resource_bounds, l1_resource_bounds}; -use crate::utils::{u128_from_usize, u64_from_usize}; +use crate::utils::u64_from_usize; use crate::versioned_constants::VersionedConstants; #[rstest] @@ -66,7 +66,7 @@ fn test_simple_get_vm_resource_usage( let n_reverted_steps = 0; vm_resource_usage.n_steps = vm_resource_usage.builtin_instance_counter.get(&BuiltinName::range_check).unwrap() - 1; - let vm_usage_in_l1_gas = u128_from_usize( + let vm_usage_in_l1_gas = u64_from_usize( *vm_resource_usage.builtin_instance_counter.get(&BuiltinName::range_check).unwrap(), ) .into(); @@ -155,9 +155,9 @@ fn test_float_get_vm_resource_usage( fn test_discounted_gas_overdraft( #[case] gas_price: u128, #[case] data_gas_price: u128, - #[case] l1_gas_used: u128, - #[case] l1_data_gas_used: u128, - #[case] gas_bound: u128, + #[case] l1_gas_used: u64, + #[case] l1_data_gas_used: u64, + #[case] gas_bound: u64, #[case] expect_failure: bool, ) { let (l1_gas_used, l1_data_gas_used, gas_bound) = @@ -207,7 +207,9 @@ fn test_discounted_gas_overdraft( if expect_failure { let error = report.error().unwrap(); let expected_actual_amount = l1_gas_used - + (l1_data_gas_used.nonzero_checked_mul(data_gas_price).unwrap()) / gas_price; + + (l1_data_gas_used.nonzero_checked_mul(data_gas_price).unwrap()) + .checked_div(gas_price) + .unwrap(); assert_matches!( error, FeeCheckError::MaxGasAmountExceeded { resource, max_amount, actual_amount } if max_amount == gas_bound @@ -292,13 +294,10 @@ fn test_post_execution_gas_overdraft_all_resource_bounds( #[case::happy_flow_l1_gas_only(10, 0, 0, 10, 2*10)] #[case::happy_flow_no_l2_gas(10, 20, 0, 10 + 3*20, 2*10 + 4*20)] #[case::happy_flow_all(10, 20, 30, 10 + 3*20 + 5*30, 2*10 + 4*20 + 6*30)] -#[case::saturating_l1_gas(u128::MAX, 1, 1, u128::MAX, u128::MAX)] -#[case::saturating_l1_data_gas(1, u128::MAX, 1, u128::MAX, u128::MAX)] -#[case::saturating_l2_gas(1, 1, u128::MAX, u128::MAX, u128::MAX)] fn test_get_fee_by_gas_vector_regression( - #[case] l1_gas: u128, - #[case] l1_data_gas: u128, - #[case] l2_gas: u128, + #[case] l1_gas: u64, + #[case] l1_data_gas: u64, + #[case] l2_gas: u64, #[case] expected_fee_eth: u128, #[case] expected_fee_strk: u128, ) { @@ -322,3 +321,28 @@ fn test_get_fee_by_gas_vector_regression( Fee(expected_fee_strk) ); } + +#[rstest] +#[case::l1_saturates(u64::MAX, 0, 0)] +#[case::l1_data_saturates(0, u64::MAX, 0)] +#[case::l2_gas_saturates(0, 0, u64::MAX)] +// TODO: Add L2 saturation case once `get_fee_by_gas_vector` implements L2 collection. +fn test_get_fee_by_gas_vector_saturation( + #[case] l1_gas: u64, + #[case] l1_data_gas: u64, + #[case] l2_gas: u64, +) { + let huge_gas_price = NonzeroGasPrice::try_from(2_u128 * u128::from(u64::MAX)).unwrap(); + let mut block_info = BlockContext::create_for_account_testing().block_info; + block_info.gas_prices = GasPrices::new( + huge_gas_price, + huge_gas_price, + huge_gas_price, + huge_gas_price, + huge_gas_price, + huge_gas_price, + ); + let gas_vector = + GasVector { l1_gas: l1_gas.into(), l1_data_gas: l1_data_gas.into(), l2_gas: l2_gas.into() }; + assert_eq!(get_fee_by_gas_vector(&block_info, gas_vector, &FeeType::Eth), Fee(u128::MAX)); +} diff --git a/crates/blockifier/src/fee/gas_usage.rs b/crates/blockifier/src/fee/gas_usage.rs index 49d3896e78..36fbda8b59 100644 --- a/crates/blockifier/src/fee/gas_usage.rs +++ b/crates/blockifier/src/fee/gas_usage.rs @@ -8,7 +8,7 @@ use crate::fee::eth_gas_constants; use crate::fee::resources::GasVector; use crate::state::cached_state::StateChangesCount; use crate::transaction::account_transaction::AccountTransaction; -use crate::utils::u128_from_usize; +use crate::utils::u64_from_usize; #[cfg(test)] #[path = "gas_usage_test.rs"] @@ -43,7 +43,7 @@ pub fn get_da_gas_cost(state_changes_count: &StateChangesCount, use_kzg_da: bool let (l1_gas, blob_gas) = if use_kzg_da { ( 0_u8.into(), - u128_from_usize( + u64_from_usize( onchain_data_segment_length * eth_gas_constants::DATA_GAS_PER_FIELD_ELEMENT, ) .into(), @@ -71,7 +71,7 @@ pub fn get_da_gas_cost(state_changes_count: &StateChangesCount, use_kzg_da: bool naive_cost - discount }; - (u128_from_usize(gas).into(), 0_u8.into()) + (u64_from_usize(gas).into(), 0_u8.into()) }; GasVector { l1_gas, l1_data_gas: blob_gas, ..Default::default() } @@ -136,7 +136,7 @@ pub fn get_log_message_to_l1_emissions_cost(l2_to_l1_payload_lengths: &[usize]) fn get_event_emission_cost(n_topics: usize, data_length: usize) -> GasVector { GasVector::from_l1_gas( - u128_from_usize( + u64_from_usize( eth_gas_constants::GAS_PER_LOG + (n_topics + constants::N_DEFAULT_TOPICS) * eth_gas_constants::GAS_PER_LOG_TOPIC + data_length * eth_gas_constants::GAS_PER_LOG_DATA_WORD, diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index b96f6ad288..ca148f9a6f 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -29,7 +29,7 @@ use crate::test_utils::{ }; use crate::transaction::objects::FeeType; use crate::transaction::test_utils::account_invoke_tx; -use crate::utils::{u128_from_usize, u64_from_usize}; +use crate::utils::u64_from_usize; use crate::versioned_constants::{ ResourceCost, StarknetVersion, @@ -204,7 +204,7 @@ fn test_get_da_gas_cost_basic(#[case] state_changes_count: StateChangesCount) { let computed_gas_vector = get_da_gas_cost(&state_changes_count, true); assert_eq!( - GasVector::from_l1_data_gas(u128_from_usize(manual_blob_gas_usage).into()), + GasVector::from_l1_data_gas(u64_from_usize(manual_blob_gas_usage).into()), computed_gas_vector ); } @@ -253,10 +253,7 @@ fn test_onchain_data_discount() { let cost_without_discount = (state_changes_count.n_storage_updates * 2) * (512 + 100); let actual_cost = get_da_gas_cost(&state_changes_count, use_kzg_da).l1_gas; - let cost_ratio = ResourceCost::new( - u64::try_from(actual_cost.0).unwrap(), - u64_from_usize(cost_without_discount), - ); + let cost_ratio = ResourceCost::new(actual_cost.0, u64_from_usize(cost_without_discount)); assert!(cost_ratio <= ResourceCost::new(9, 10)); assert!(cost_ratio >= ResourceCost::new(88, 100)); } @@ -294,10 +291,12 @@ fn test_discounted_gas_from_gas_vector_computation() { let result_div_ceil = gas_usage.l1_gas + (gas_usage.l1_data_gas.nonzero_checked_mul(DEFAULT_ETH_L1_DATA_GAS_PRICE).unwrap()) - .div_ceil(DEFAULT_ETH_L1_GAS_PRICE); + .checked_div_ceil(DEFAULT_ETH_L1_GAS_PRICE) + .unwrap(); let result_div_floor = gas_usage.l1_gas + (gas_usage.l1_data_gas.nonzero_checked_mul(DEFAULT_ETH_L1_DATA_GAS_PRICE).unwrap()) - / DEFAULT_ETH_L1_GAS_PRICE; + .checked_div(DEFAULT_ETH_L1_GAS_PRICE) + .unwrap(); assert_eq!(actual_result, result_div_ceil); assert_eq!(actual_result, result_div_floor + 1_u8.into()); diff --git a/crates/blockifier/src/fee/receipt_test.rs b/crates/blockifier/src/fee/receipt_test.rs index 2b46e92d9a..13654e6001 100644 --- a/crates/blockifier/src/fee/receipt_test.rs +++ b/crates/blockifier/src/fee/receipt_test.rs @@ -30,7 +30,7 @@ use crate::transaction::test_utils::{ create_resource_bounds, }; use crate::transaction::transactions::ExecutableTransaction; -use crate::utils::{u128_from_usize, u64_from_usize, usize_from_u128}; +use crate::utils::{u64_from_usize, usize_from_u64}; use crate::versioned_constants::VersionedConstants; #[fixture] @@ -124,9 +124,9 @@ fn test_calculate_tx_gas_usage_basic<'a>( let gas_per_data_felt = versioned_constants .get_archival_data_gas_costs(&gas_vector_computation_mode) .gas_per_data_felt; - let calldata_and_signature_gas_cost = u128::from( - (gas_per_data_felt * u64_from_usize(calldata_length + signature_length)).to_integer(), - ) + let calldata_and_signature_gas_cost = (gas_per_data_felt + * u64_from_usize(calldata_length + signature_length)) + .to_integer() .into(); let manual_starknet_gas_usage_vector = match gas_vector_computation_mode { GasVectorComputationMode::NoL2Gas => { @@ -163,10 +163,9 @@ fn test_calculate_tx_gas_usage_basic<'a>( // Manual calculation. let message_segment_length = get_message_segment_length(&[], Some(l1_handler_payload_size)); - let calldata_and_signature_gas_cost = u128::from( - (gas_per_data_felt * u64_from_usize(l1_handler_payload_size + signature_length)) - .to_integer(), - ) + let calldata_and_signature_gas_cost = (gas_per_data_felt + * u64_from_usize(l1_handler_payload_size + signature_length)) + .to_integer() .into(); let calldata_and_signature_gas_cost_vector = match gas_vector_computation_mode { GasVectorComputationMode::NoL2Gas => { @@ -177,16 +176,16 @@ fn test_calculate_tx_gas_usage_basic<'a>( let manual_starknet_l1_gas_usage = message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD + eth_gas_constants::GAS_PER_COUNTER_DECREASE - + usize_from_u128( + + usize_from_u64( get_consumed_message_to_l2_emissions_cost(Some(l1_handler_payload_size)).l1_gas.0, ) .unwrap(); let manual_starknet_l1_gas_usage_vector = - GasVector::from_l1_gas(u128_from_usize(manual_starknet_l1_gas_usage).into()); + GasVector::from_l1_gas(u64_from_usize(manual_starknet_l1_gas_usage).into()); let manual_sharp_gas_usage = message_segment_length * eth_gas_constants::SHARP_GAS_PER_MEMORY_WORD; let manual_gas_computation = - GasVector::from_l1_gas(u128_from_usize(manual_sharp_gas_usage).into()) + GasVector::from_l1_gas(u64_from_usize(manual_sharp_gas_usage).into()) + manual_starknet_l1_gas_usage_vector + calldata_and_signature_gas_cost_vector; assert_eq!(l1_handler_gas_usage_vector, manual_gas_computation); @@ -245,16 +244,16 @@ fn test_calculate_tx_gas_usage_basic<'a>( let n_l2_to_l1_messages = l2_to_l1_payload_lengths.len(); let manual_starknet_gas_usage = message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD + n_l2_to_l1_messages * eth_gas_constants::GAS_PER_ZERO_TO_NONZERO_STORAGE_SET - + usize_from_u128(get_log_message_to_l1_emissions_cost(&l2_to_l1_payload_lengths).l1_gas.0) + + usize_from_u64(get_log_message_to_l1_emissions_cost(&l2_to_l1_payload_lengths).l1_gas.0) .unwrap(); let manual_sharp_gas_usage = message_segment_length * eth_gas_constants::SHARP_GAS_PER_MEMORY_WORD - + usize_from_u128(l2_to_l1_starknet_resources.state.to_gas_vector(use_kzg_da).l1_gas.0) + + usize_from_u64(l2_to_l1_starknet_resources.state.to_gas_vector(use_kzg_da).l1_gas.0) .unwrap(); let manual_sharp_blob_gas_usage = l2_to_l1_starknet_resources.state.to_gas_vector(use_kzg_da).l1_data_gas; let manual_gas_computation = GasVector { - l1_gas: u128_from_usize(manual_starknet_gas_usage + manual_sharp_gas_usage).into(), + l1_gas: u64_from_usize(manual_starknet_gas_usage + manual_sharp_gas_usage).into(), l1_data_gas: manual_sharp_blob_gas_usage, ..Default::default() }; @@ -329,7 +328,7 @@ fn test_calculate_tx_gas_usage_basic<'a>( + storage_writings_gas_usage_vector.l1_gas // l2_to_l1_messages_gas_usage and storage_writings_gas_usage got a discount each, while // the combined calculation got it once. - + u128_from_usize(fee_balance_discount).into(), + + u64_from_usize(fee_balance_discount).into(), // Expected blob gas usage is from data availability only. l1_data_gas: combined_cases_starknet_resources.state.to_gas_vector(use_kzg_da).l1_data_gas, l2_gas: l1_handler_gas_usage_vector.l2_gas, diff --git a/crates/blockifier/src/fee/resources.rs b/crates/blockifier/src/fee/resources.rs index 2a111b22ee..8448dc5cfd 100644 --- a/crates/blockifier/src/fee/resources.rs +++ b/crates/blockifier/src/fee/resources.rs @@ -19,7 +19,7 @@ use crate::fee::gas_usage::{ use crate::state::cached_state::{StateChanges, StateChangesCount}; use crate::transaction::errors::TransactionFeeError; use crate::transaction::objects::HasRelatedFeeType; -use crate::utils::{u128_from_usize, u64_from_usize}; +use crate::utils::u64_from_usize; use crate::versioned_constants::{ resource_cost_to_u128_ratio, ArchivalDataGasCosts, @@ -219,12 +219,24 @@ impl ArchivalDataResources { /// Returns the cost of the transaction's emmited events in L1/L2 gas units, depending on the /// mode. fn get_events_gas_cost(&self, archival_gas_costs: &ArchivalDataGasCosts) -> GasAmount { - (resource_cost_to_u128_ratio(archival_gas_costs.gas_per_data_felt) - * (resource_cost_to_u128_ratio(archival_gas_costs.event_key_factor) - * self.event_summary.total_event_keys - + self.event_summary.total_event_data_size)) - .to_integer() - .into() + u64::try_from( + (resource_cost_to_u128_ratio(archival_gas_costs.gas_per_data_felt) + * (resource_cost_to_u128_ratio(archival_gas_costs.event_key_factor) + * self.event_summary.total_event_keys + + self.event_summary.total_event_data_size)) + .to_integer(), + ) + .unwrap_or_else(|_| { + panic!( + "Events gas cost overflowed: {} event keys (factor: {}), data length {} (at {} \ + gas per felt).", + self.event_summary.total_event_keys, + archival_gas_costs.event_key_factor, + self.event_summary.total_event_data_size, + archival_gas_costs.gas_per_data_felt + ) + }) + .into() } } @@ -251,7 +263,7 @@ impl MessageResources { pub fn to_gas_vector(&self) -> GasVector { let starknet_gas_usage = self.get_starknet_gas_cost(); let sharp_gas_usage = GasVector::from_l1_gas( - u128_from_usize( + u64_from_usize( self.message_segment_length * eth_gas_constants::SHARP_GAS_PER_MEMORY_WORD, ) .into(), @@ -268,7 +280,7 @@ impl MessageResources { GasVector::from_l1_gas( // Starknet's updateState gets the message segment as an argument. - u128_from_usize( + u64_from_usize( self.message_segment_length * eth_gas_constants::GAS_PER_MEMORY_WORD // Starknet's updateState increases a (storage) counter for each L2-to-L1 message. + n_l2_to_l1_messages * eth_gas_constants::GAS_PER_ZERO_TO_NONZERO_STORAGE_SET @@ -362,6 +374,18 @@ impl GasVector { 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); - self.l1_gas + (self.l1_data_gas.nonzero_saturating_mul(data_gas_price)).div_ceil(gas_price) + self.l1_gas + + (self.l1_data_gas.nonzero_saturating_mul(data_gas_price)) + .checked_div_ceil(gas_price) + .unwrap_or_else(|| { + log::warn!( + "Discounted L1 gas cost overflowed: division of L1 data gas cost ({:?} * \ + {:?}) by regular L1 gas price ({:?}) resulted in overflow.", + self.l1_data_gas, + data_gas_price, + gas_price + ); + GasAmount::MAX + }) } } diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index efa7cdc67e..4cff55bf93 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -113,10 +113,10 @@ pub const DEFAULT_ETH_L1_DATA_GAS_PRICE: NonzeroGasPrice = NonzeroGasPrice::new_unchecked(GasPrice(u128::pow(10, 6))); // Given in units of Wei. pub const DEFAULT_STRK_L1_DATA_GAS_PRICE: NonzeroGasPrice = NonzeroGasPrice::new_unchecked(GasPrice(u128::pow(10, 9))); // Given in units of STRK. -pub const DEFAULT_L1_DATA_GAS_MAX_AMOUNT: GasAmount = GasAmount(u128::pow(10, 6)); +pub const DEFAULT_L1_DATA_GAS_MAX_AMOUNT: GasAmount = GasAmount(u64::pow(10, 6)); pub const DEFAULT_STRK_L2_GAS_PRICE: NonzeroGasPrice = NonzeroGasPrice::new_unchecked(GasPrice(u128::pow(10, 9))); -pub const DEFAULT_L2_GAS_MAX_AMOUNT: GasAmount = GasAmount(u128::pow(10, 6)); +pub const DEFAULT_L2_GAS_MAX_AMOUNT: GasAmount = GasAmount(u64::pow(10, 6)); // The block number of the BlockContext being used for testing. pub const CURRENT_BLOCK_NUMBER: u64 = 2001; diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 4f6bd05e1b..fe554f8189 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -88,7 +88,7 @@ use crate::transaction::test_utils::{ }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::{DeclareTransaction, ExecutableTransaction, ExecutionFlags}; -use crate::utils::u128_from_usize; +use crate::utils::u64_from_usize; #[rstest] fn test_circuit(block_context: BlockContext, max_l1_resource_bounds: ValidResourceBounds) { @@ -173,7 +173,10 @@ fn test_fee_enforcement( deploy_account_tx_args! { class_hash: account.get_class_hash(), max_fee: Fee(u128::from(!zero_bounds)), - resource_bounds: l1_resource_bounds(u128::from(!zero_bounds).into(), DEFAULT_STRK_L1_GAS_PRICE.into()), + resource_bounds: l1_resource_bounds( + u8::from(!zero_bounds).into(), + DEFAULT_STRK_L1_GAS_PRICE.into() + ), version, }, &mut NonceManager::default(), @@ -991,13 +994,13 @@ fn test_max_fee_to_max_steps_conversion( ) { let TestInitData { mut state, account_address, contract_address, mut nonce_manager } = create_test_init_data(&block_context.chain_info, CairoVersion::Cairo0); - let actual_gas_used: GasAmount = u128_from_usize( + let actual_gas_used: GasAmount = u64_from_usize( get_syscall_resources(SyscallSelector::CallContract).n_steps + get_tx_resources(TransactionType::InvokeFunction).n_steps + 1751, ) .into(); - let actual_fee = actual_gas_used.0 * 100000000000; + let actual_fee = u128::from(actual_gas_used.0) * 100000000000; let actual_strk_gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&FeeType::Strk); let execute_calldata = create_calldata( @@ -1088,7 +1091,7 @@ fn test_insufficient_max_fee_reverts( let actual_fee_depth1 = tx_execution_info1.receipt.fee; let gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&FeeType::Strk); - let gas_ammount = actual_fee_depth1 / gas_price; + let gas_ammount = actual_fee_depth1.checked_div(gas_price).unwrap(); // Invoke the `recurse` function with depth of 2 and the actual fee of depth 1 as max_fee. // This call should fail due to insufficient max fee (steps bound based on max_fee is not so diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs index 83ae1d7203..c1f77cc400 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -51,7 +51,7 @@ use crate::transaction::test_utils::{ }; use crate::transaction::transaction_types::TransactionType; use crate::transaction::transactions::ExecutableTransaction; -use crate::utils::u128_from_usize; +use crate::utils::u64_from_usize; const VALIDATE_GAS_OVERHEAD: GasAmount = GasAmount(21); struct FlavorTestInitialState { @@ -287,7 +287,7 @@ fn test_simulate_validate_pre_validate_with_charge_fee( // Second scenario: resource bounds greater than balance. let gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&fee_type); - let balance_over_gas_price = BALANCE / gas_price; + let balance_over_gas_price = BALANCE.checked_div(gas_price).unwrap(); let result = account_invoke_tx(invoke_tx_args! { max_fee: Fee(BALANCE.0 + 1), resource_bounds: l1_resource_bounds( @@ -371,7 +371,7 @@ fn test_simulate_validate_pre_validate_not_charge_fee( let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, false); assert!( base_gas - > u128_from_usize( + > u64_from_usize( get_syscall_resources(SyscallSelector::CallContract).n_steps + get_tx_resources(TransactionType::InvokeFunction).n_steps ) @@ -405,7 +405,7 @@ fn test_simulate_validate_pre_validate_not_charge_fee( // Second scenario: resource bounds greater than balance. let gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&fee_type); - let balance_over_gas_price = BALANCE / gas_price; + let balance_over_gas_price = BALANCE.checked_div(gas_price).unwrap(); execute_and_check_gas_and_fee!( Fee(BALANCE.0 + 1), l1_resource_bounds((balance_over_gas_price.0 + 10).into(), gas_price.into()) @@ -512,8 +512,7 @@ fn test_simulate_charge_fee_no_validation_fail_validate( let block_context = BlockContext::create_for_account_testing(); let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, validate); assert!( - base_gas - > u128_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps).into() + base_gas > u64_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps).into() ); let (actual_gas_used, actual_fee) = gas_and_fee(base_gas, validate, &fee_type); @@ -581,8 +580,7 @@ fn test_simulate_validate_charge_fee_mid_execution( let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, validate); let (revert_gas_used, revert_fee) = gas_and_fee(base_gas, validate, &fee_type); assert!( - base_gas - > u128_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps).into() + base_gas > u64_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps).into() ); assert!(tx_execution_info.is_reverted()); check_gas_and_fee( @@ -609,7 +607,7 @@ fn test_simulate_validate_charge_fee_mid_execution( // used. Otherwise, execution is limited by block bounds, so more resources will be used. let (limited_gas_used, limited_fee) = gas_and_fee(7763_u32.into(), validate, &fee_type); let (unlimited_gas_used, unlimited_fee) = gas_and_fee( - u128_from_usize( + u64_from_usize( get_syscall_resources(SyscallSelector::CallContract).n_steps + get_tx_resources(TransactionType::InvokeFunction).n_steps + 5730, @@ -658,7 +656,7 @@ fn test_simulate_validate_charge_fee_mid_execution( // lower when `validate` is true, but this is not reflected in the actual gas usage. let invoke_tx_max_n_steps_as_u64: u64 = low_step_block_context.versioned_constants.invoke_tx_max_n_steps.into(); - let block_limit_gas = u128::from(invoke_tx_max_n_steps_as_u64 + 1652).into(); + let block_limit_gas = (invoke_tx_max_n_steps_as_u64 + 1652).into(); let block_limit_fee = get_fee_by_gas_vector( &block_context.block_info, GasVector::from_l1_gas(block_limit_gas), @@ -732,12 +730,12 @@ fn test_simulate_validate_charge_fee_post_execution( // `__validate__` and overhead resources + number of reverted steps, comes out slightly more // than the gas bound. let (revert_gas_usage, revert_fee) = gas_and_fee( - (u128_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps) + 5730).into(), + (u64_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps) + 5730).into(), validate, &fee_type, ); let (unlimited_gas_used, unlimited_fee) = gas_and_fee( - u128_from_usize( + u64_from_usize( get_syscall_resources(SyscallSelector::CallContract).n_steps + get_tx_resources(TransactionType::InvokeFunction).n_steps + 5730, @@ -782,7 +780,7 @@ fn test_simulate_validate_charge_fee_post_execution( // Second scenario: balance too low. // Execute a transfer, and make sure we get the expected result. let (success_actual_gas, actual_fee) = gas_and_fee( - u128_from_usize( + u64_from_usize( get_syscall_resources(SyscallSelector::CallContract).n_steps + get_tx_resources(TransactionType::InvokeFunction).n_steps + 4260, @@ -792,7 +790,7 @@ fn test_simulate_validate_charge_fee_post_execution( &fee_type, ); let (fail_actual_gas, fail_actual_fee) = gas_and_fee( - u128_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps + 2252).into(), + u64_from_usize(get_tx_resources(TransactionType::InvokeFunction).n_steps + 2252).into(), validate, &fee_type, ); diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index b1ba7a91a7..4540acd7d2 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -331,7 +331,7 @@ pub fn run_invoke_tx( // TODO: Check usages of this function and update to using all gas bounds. pub fn l1_resource_bounds(max_amount: GasAmount, max_price: GasPrice) -> ValidResourceBounds { ValidResourceBounds::L1Gas(ResourceBounds { - max_amount: max_amount.0.try_into().unwrap(), + max_amount: max_amount.0, max_price_per_unit: max_price.0, }) } @@ -364,16 +364,10 @@ pub fn create_all_resource_bounds( l1_data_max_price: GasPrice, ) -> ValidResourceBounds { ValidResourceBounds::AllResources(AllResourceBounds { - l1_gas: ResourceBounds { - max_amount: l1_max_amount.0.try_into().unwrap(), - max_price_per_unit: l1_max_price.0, - }, - l2_gas: ResourceBounds { - max_amount: l2_max_amount.0.try_into().unwrap(), - max_price_per_unit: l2_max_price.0, - }, + l1_gas: ResourceBounds { max_amount: l1_max_amount.0, max_price_per_unit: l1_max_price.0 }, + l2_gas: ResourceBounds { max_amount: l2_max_amount.0, max_price_per_unit: l2_max_price.0 }, l1_data_gas: ResourceBounds { - max_amount: l1_data_max_amount.0.try_into().unwrap(), + max_amount: l1_data_max_amount.0, max_price_per_unit: l1_data_max_price.0, }, }) diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index cc1b30c9be..4db197658c 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -905,7 +905,7 @@ fn test_max_fee_exceeds_balance( let invalid_max_fee = Fee(BALANCE.0 + 1); // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works. - let balance_over_gas_price = BALANCE / MAX_L1_GAS_PRICE; + let balance_over_gas_price = BALANCE.checked_div(MAX_L1_GAS_PRICE).unwrap(); let invalid_resource_bounds = l1_resource_bounds((balance_over_gas_price.0 + 1).into(), MAX_L1_GAS_PRICE.into()); @@ -983,15 +983,15 @@ fn test_insufficient_new_resource_bounds( let default_resource_bounds = AllResourceBounds { l1_gas: ResourceBounds { - max_amount: minimal_gas_vector.l1_gas.0.try_into().unwrap(), + max_amount: minimal_gas_vector.l1_gas.0, max_price_per_unit: actual_strk_l1_gas_price.get().0, }, l2_gas: ResourceBounds { - max_amount: minimal_gas_vector.l2_gas.0.try_into().unwrap(), + max_amount: minimal_gas_vector.l2_gas.0, max_price_per_unit: actual_strk_l2_gas_price.get().0, }, l1_data_gas: ResourceBounds { - max_amount: minimal_gas_vector.l1_data_gas.0.try_into().unwrap(), + max_amount: minimal_gas_vector.l1_data_gas.0, max_price_per_unit: actual_strk_l1_data_gas_price.get().0, }, }; diff --git a/crates/blockifier/src/utils.rs b/crates/blockifier/src/utils.rs index 9e70cf5f90..ef9c0a6c4d 100644 --- a/crates/blockifier/src/utils.rs +++ b/crates/blockifier/src/utils.rs @@ -48,12 +48,6 @@ pub const fn const_max(a: u128, b: u128) -> u128 { [a, b][(a < b) as usize] } -/// Conversion from u128 to usize. This conversion should only be used if the value came from a -/// usize. -pub fn usize_from_u128(val: u128) -> Result { - val.try_into().map_err(|_| NumericConversionError::U128ToUsizeError(val)) -} - /// Conversion from u64 to usize. This conversion should only be used if the value came from a /// usize. pub fn usize_from_u64(val: u64) -> Result { diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index f31fc2781b..48ccc7281c 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -137,6 +137,8 @@ define_versioned_constants! { pub type ResourceCost = Ratio; +// TODO: Delete this ratio-converter function once event keys / data length are no longer 128 bits +// (no other usage is expected). pub fn resource_cost_to_u128_ratio(cost: ResourceCost) -> Ratio { Ratio::new((*cost.numer()).into(), (*cost.denom()).into()) } @@ -237,10 +239,7 @@ impl VersionedConstants { /// Converts from L1 gas amount to L2 gas amount with **upward rounding**. pub fn convert_l1_to_l2_gas_amount_round_up(&self, l1_gas_amount: GasAmount) -> GasAmount { // The amount ratio is the inverse of the price ratio. - (*(resource_cost_to_u128_ratio(self.l1_to_l2_gas_price_ratio().inv()) * l1_gas_amount.0) - .ceil() - .numer()) - .into() + (*(self.l1_to_l2_gas_price_ratio().inv() * l1_gas_amount.0).ceil().numer()).into() } /// Returns the following ratio: L2_gas_price/L1_gas_price. diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 0b9a96c552..59538aae36 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -11,7 +11,7 @@ use blockifier::fee::resources::GasVector; use blockifier::state::global_cache::GlobalContractCache; use blockifier::transaction::objects::{ExecutionResourcesTraits, TransactionExecutionInfo}; use blockifier::transaction::transaction_execution::Transaction; -use blockifier::utils::usize_from_u128; +use blockifier::utils::usize_from_u64; use blockifier::versioned_constants::VersionedConstants; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyList}; @@ -93,17 +93,17 @@ impl ThinTransactionExecutionInfo { resources.extend(HashMap::from([ ( abi_constants::L1_GAS_USAGE.to_string(), - usize_from_u128(l1_gas.0) + usize_from_u64(l1_gas.0) .expect("This conversion should not fail as the value is a converted usize."), ), ( abi_constants::BLOB_GAS_USAGE.to_string(), - usize_from_u128(l1_data_gas.0) + usize_from_u64(l1_data_gas.0) .expect("This conversion should not fail as the value is a converted usize."), ), ( abi_constants::L2_GAS_USAGE.to_string(), - usize_from_u128(l2_gas.0) + usize_from_u64(l2_gas.0) .expect("This conversion should not fail as the value is a converted usize."), ), ])); diff --git a/crates/starknet_api/src/block.rs b/crates/starknet_api/src/block.rs index 0669740b7c..069ab6f53b 100644 --- a/crates/starknet_api/src/block.rs +++ b/crates/starknet_api/src/block.rs @@ -266,11 +266,12 @@ impl From for PrefixedBytesAsHex<16_usize> { impl GasPrice { pub const fn saturating_mul(self, rhs: GasAmount) -> Fee { - Fee(self.0.saturating_mul(rhs.0)) + #[allow(clippy::as_conversions)] + Fee(self.0.saturating_mul(rhs.0 as u128)) } pub fn checked_mul(self, rhs: GasAmount) -> Option { - self.0.checked_mul(rhs.0).map(Fee) + self.0.checked_mul(u128::from(rhs.0)).map(Fee) } } diff --git a/crates/starknet_api/src/execution_resources.rs b/crates/starknet_api/src/execution_resources.rs index 4457682c62..41e17d2ade 100644 --- a/crates/starknet_api/src/execution_resources.rs +++ b/crates/starknet_api/src/execution_resources.rs @@ -21,23 +21,25 @@ use crate::transaction::Fee; Serialize, Deserialize, )] -pub struct GasAmount(pub u128); +pub struct GasAmount(pub u64); macro_rules! impl_from_uint_for_gas_amount { ($($uint:ty),*) => { $( impl From<$uint> for GasAmount { fn from(value: $uint) -> Self { - Self(u128::from(value)) + Self(u64::from(value)) } } )* }; } -impl_from_uint_for_gas_amount!(u8, u16, u32, u64, u128); +impl_from_uint_for_gas_amount!(u8, u16, u32, u64); impl GasAmount { + pub const MAX: Self = Self(u64::MAX); + pub const fn saturating_mul(self, rhs: GasPrice) -> Fee { rhs.saturating_mul(self) } diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index 3f1807b48f..b8f424aa5f 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -1,6 +1,5 @@ use std::collections::BTreeMap; use std::fmt::Display; -use std::ops::Div; use std::sync::Arc; use derive_more::{Display, From}; @@ -9,7 +8,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use starknet_types_core::felt::Felt; use strum_macros::EnumIter; -use crate::block::{BlockHash, BlockNumber, GasPrice, NonzeroGasPrice}; +use crate::block::{BlockHash, BlockNumber, NonzeroGasPrice}; use crate::core::{ ChainId, ClassHash, @@ -743,24 +742,30 @@ pub struct RevertedTransactionExecutionStatus { pub struct Fee(pub u128); impl Fee { - pub fn div_ceil(self, rhs: NonzeroGasPrice) -> GasAmount { - let mut result: GasAmount = self / rhs; - if result.0 * rhs.get().0 < self.0 { - result = (result.0 + 1).into(); - } - result - } - pub fn checked_add(self, rhs: Fee) -> Option { self.0.checked_add(rhs.0).map(Fee) } -} -impl Div for Fee { - type Output = GasAmount; + pub fn checked_div_ceil(self, rhs: NonzeroGasPrice) -> Option { + match self.checked_div(rhs) { + Some(value) => Some(if value.nonzero_saturating_mul(rhs) < self { + (value.0 + 1).into() + } else { + value + }), + None => None, + } + } + + pub fn checked_div(self, rhs: NonzeroGasPrice) -> Option { + match u64::try_from(self.0 / rhs.get().0) { + Ok(value) => Some(value.into()), + Err(_) => None, + } + } - fn div(self, rhs: NonzeroGasPrice) -> Self::Output { - (self.0 / GasPrice::from(rhs).0).into() + pub fn saturating_div(self, rhs: NonzeroGasPrice) -> GasAmount { + self.checked_div(rhs).unwrap_or(GasAmount::MAX) } } diff --git a/crates/starknet_api/src/transaction_test.rs b/crates/starknet_api/src/transaction_test.rs index 18982729a3..c69096bf72 100644 --- a/crates/starknet_api/src/transaction_test.rs +++ b/crates/starknet_api/src/transaction_test.rs @@ -4,9 +4,24 @@ use crate::transaction::Fee; #[test] fn test_fee_div_ceil() { - assert_eq!(GasAmount(1), Fee(1).div_ceil(NonzeroGasPrice::try_from(1_u8).unwrap())); - assert_eq!(GasAmount(0), Fee(0).div_ceil(NonzeroGasPrice::try_from(1_u8).unwrap())); - assert_eq!(GasAmount(1), Fee(1).div_ceil(NonzeroGasPrice::try_from(2_u8).unwrap())); - assert_eq!(GasAmount(9), Fee(27).div_ceil(NonzeroGasPrice::try_from(3_u8).unwrap())); - assert_eq!(GasAmount(10), Fee(28).div_ceil(NonzeroGasPrice::try_from(3_u8).unwrap())); + assert_eq!( + GasAmount(1), + Fee(1).checked_div_ceil(NonzeroGasPrice::try_from(1_u8).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(0), + Fee(0).checked_div_ceil(NonzeroGasPrice::try_from(1_u8).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(1), + Fee(1).checked_div_ceil(NonzeroGasPrice::try_from(2_u8).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(9), + Fee(27).checked_div_ceil(NonzeroGasPrice::try_from(3_u8).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(10), + Fee(28).checked_div_ceil(NonzeroGasPrice::try_from(3_u8).unwrap()).unwrap() + ); }