From ccb64a07ff13d022390c42d83eb819fd17e7d418 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Sun, 6 Oct 2024 10:08:26 +0300 Subject: [PATCH] refactor(blockifier): resource cost ratio type changed to u64 ratio from u128 ratio --- .../blockifier/src/execution/entry_point.rs | 17 +++++++++++------ crates/blockifier/src/fee/fee_test.rs | 8 ++++---- crates/blockifier/src/fee/fee_utils.rs | 6 +++--- crates/blockifier/src/fee/gas_usage_test.rs | 9 ++++++--- crates/blockifier/src/fee/receipt_test.rs | 17 +++++++++-------- crates/blockifier/src/fee/resources.rs | 17 +++++++++++------ crates/blockifier/src/test_utils.rs | 4 ---- crates/blockifier/src/transaction/errors.rs | 2 ++ crates/blockifier/src/utils.rs | 12 ++++++++++++ crates/blockifier/src/versioned_constants.rs | 19 +++++++++++++++---- 10 files changed, 73 insertions(+), 38 deletions(-) diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 10c759fa17..96f9839ba8 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -27,7 +27,7 @@ use crate::execution::execution_utils::execute_entry_point_call_wrapper; use crate::state::state_api::{State, StateResult}; use crate::transaction::objects::{HasRelatedFeeType, TransactionInfo}; use crate::transaction::transaction_types::TransactionType; -use crate::utils::usize_from_u128; +use crate::utils::usize_from_u64; use crate::versioned_constants::{GasCosts, VersionedConstants}; #[cfg(test)] @@ -278,14 +278,19 @@ impl EntryPointExecutionContext { // 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_u128 = if gas_per_step.is_zero() { - u128::MAX + let upper_bound_u64 = if gas_per_step.is_zero() { + u64::MAX } else { - (gas_per_step.inv() * tx_gas_upper_bound.0).to_integer() + // 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() }; - let tx_upper_bound = usize_from_u128(upper_bound_u128).unwrap_or_else(|_| { + let tx_upper_bound = usize_from_u64(upper_bound_u64).unwrap_or_else(|_| { log::warn!( - "Failed to convert u128 to usize: {upper_bound_u128}. Upper bound from tx is \ + "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}." ); usize::MAX diff --git a/crates/blockifier/src/fee/fee_test.rs b/crates/blockifier/src/fee/fee_test.rs index b15e9d871b..bd7a310e2b 100644 --- a/crates/blockifier/src/fee/fee_test.rs +++ b/crates/blockifier/src/fee/fee_test.rs @@ -30,7 +30,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; +use crate::utils::{u128_from_usize, u64_from_usize}; use crate::versioned_constants::VersionedConstants; fn get_vm_resource_usage() -> ExecutionResources { @@ -59,7 +59,7 @@ fn test_simple_get_vm_resource_usage( // Positive flow. // Verify calculation - in our case, n_steps is the heaviest resource. let vm_usage_in_l1_gas = (versioned_constants.vm_resource_fee_cost().n_steps - * (u128_from_usize(vm_resource_usage.n_steps + n_reverted_steps))) + * (u64_from_usize(vm_resource_usage.n_steps + n_reverted_steps))) .ceil() .to_integer() .into(); @@ -114,7 +114,7 @@ fn test_float_get_vm_resource_usage( // Verify calculation - in our case, n_steps is the heaviest resource. let n_reverted_steps = 300; let vm_usage_in_l1_gas = (versioned_constants.vm_resource_fee_cost().n_steps - * u128_from_usize(vm_resource_usage.n_steps + n_reverted_steps)) + * u64_from_usize(vm_resource_usage.n_steps + n_reverted_steps)) .ceil() .to_integer() .into(); @@ -137,7 +137,7 @@ fn test_float_get_vm_resource_usage( vm_resource_usage.n_steps = 200; let vm_usage_in_l1_gas = ((*versioned_constants.vm_resource_fee_cost().builtins.get(&BuiltinName::ecdsa).unwrap()) - * u128_from_usize( + * u64_from_usize( *vm_resource_usage.builtin_instance_counter.get(&BuiltinName::ecdsa).unwrap(), )) .ceil() diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index 9a87eba0a8..3a62213528 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -17,7 +17,7 @@ use crate::fee::resources::{GasVector, TransactionFeeResult}; use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionFeeError; use crate::transaction::objects::{ExecutionResourcesTraits, FeeType, TransactionInfo}; -use crate::utils::u128_from_usize; +use crate::utils::u64_from_usize; use crate::versioned_constants::VersionedConstants; #[cfg(test)] @@ -62,8 +62,8 @@ pub fn get_vm_resources_cost( vm_resource_fee_costs.n_steps, vm_resource_usage.total_n_steps() + n_reverted_steps, )]) - .map(|(cost, usage)| (cost * u128_from_usize(usage)).ceil().to_integer()) - .fold(0, u128::max).into(); + .map(|(cost, usage)| (cost * u64_from_usize(usage)).ceil().to_integer()) + .fold(0, u64::max).into(); match computation_mode { GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas(vm_l1_gas_usage), diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index b343eba056..12e65ae417 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -15,7 +15,7 @@ use crate::state::cached_state::StateChangesCount; use crate::test_utils::{DEFAULT_ETH_L1_DATA_GAS_PRICE, DEFAULT_ETH_L1_GAS_PRICE}; use crate::transaction::objects::FeeType; use crate::transaction::test_utils::account_invoke_tx; -use crate::utils::u128_from_usize; +use crate::utils::{u128_from_usize, u64_from_usize}; use crate::versioned_constants::{ResourceCost, VersionedConstants}; #[fixture] fn versioned_constants() -> &'static VersionedConstants { @@ -88,7 +88,7 @@ fn test_get_event_gas_cost( .collect(); let execution_summary = CallInfo::summarize_many(call_infos.iter()); // 8 keys and 11 data words overall. - let expected_gas = (data_word_cost * (event_key_factor * 8_u128 + 11_u128)).to_integer().into(); + let expected_gas = (data_word_cost * (event_key_factor * 8_u64 + 11_u64)).to_integer().into(); let expected_gas_vector = match gas_vector_computation_mode { GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas(expected_gas), GasVectorComputationMode::All => GasVector::from_l2_gas(expected_gas), @@ -193,7 +193,10 @@ 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(actual_cost.0, u128_from_usize(cost_without_discount)); + let cost_ratio = ResourceCost::new( + u64::try_from(actual_cost.0).unwrap(), + u64_from_usize(cost_without_discount), + ); assert!(cost_ratio <= ResourceCost::new(9, 10)); assert!(cost_ratio >= ResourceCost::new(88, 100)); } diff --git a/crates/blockifier/src/fee/receipt_test.rs b/crates/blockifier/src/fee/receipt_test.rs index 0cde3cbd92..3d37685f01 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, usize_from_u128}; +use crate::utils::{u128_from_usize, u64_from_usize, usize_from_u128}; use crate::versioned_constants::VersionedConstants; #[fixture] @@ -82,7 +82,7 @@ fn test_calculate_tx_gas_usage_basic<'a>( .get_archival_data_gas_costs(&gas_vector_computation_mode) .gas_per_code_byte; let code_gas_cost = (gas_per_code_byte - * u128_from_usize( + * u64_from_usize( (class_info.bytecode_length() + class_info.sierra_program_length()) * eth_gas_constants::WORD_WIDTH + class_info.abi_length(), @@ -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 = (gas_per_data_felt - * u128_from_usize(calldata_length + signature_length)) - .to_integer() + let calldata_and_signature_gas_cost = u128::from( + (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,9 +163,10 @@ 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 = (gas_per_data_felt - * u128_from_usize(l1_handler_payload_size + signature_length)) - .to_integer() + let calldata_and_signature_gas_cost = u128::from( + (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 => { diff --git a/crates/blockifier/src/fee/resources.rs b/crates/blockifier/src/fee/resources.rs index 0768752636..0ded29654b 100644 --- a/crates/blockifier/src/fee/resources.rs +++ b/crates/blockifier/src/fee/resources.rs @@ -19,8 +19,12 @@ 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; -use crate::versioned_constants::{ArchivalDataGasCosts, VersionedConstants}; +use crate::utils::{u128_from_usize, u64_from_usize}; +use crate::versioned_constants::{ + resource_cost_to_u128_ratio, + ArchivalDataGasCosts, + VersionedConstants, +}; pub type TransactionFeeResult = Result; @@ -203,20 +207,21 @@ impl ArchivalDataResources { archival_gas_costs: &ArchivalDataGasCosts, ) -> GasAmount { // TODO(Avi, 20/2/2024): Calculate the number of bytes instead of the number of felts. - let total_data_size = u128_from_usize(self.calldata_length + self.signature_length); + let total_data_size = u64_from_usize(self.calldata_length + self.signature_length); (archival_gas_costs.gas_per_data_felt * total_data_size).to_integer().into() } /// Returns the cost of declared class codes in L1/L2 gas units, depending on the mode. fn get_code_gas_cost(&self, archival_gas_costs: &ArchivalDataGasCosts) -> GasAmount { - (archival_gas_costs.gas_per_code_byte * u128_from_usize(self.code_size)).to_integer().into() + (archival_gas_costs.gas_per_code_byte * u64_from_usize(self.code_size)).to_integer().into() } /// 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 { - (archival_gas_costs.gas_per_data_felt - * (archival_gas_costs.event_key_factor * self.event_summary.total_event_keys + (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() diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index c96562f848..af8e837997 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -382,10 +382,6 @@ pub fn create_trivial_calldata(test_contract_address: ContractAddress) -> Callda ) } -pub fn u64_from_usize(val: usize) -> u64 { - val.try_into().unwrap() -} - pub fn update_json_value(base: &mut serde_json::Value, update: serde_json::Value) { match (base, update) { (serde_json::Value::Object(base_map), serde_json::Value::Object(update_map)) => { diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index 52e7aa0a54..804ada40fd 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -166,4 +166,6 @@ pub enum ParseError { pub enum NumericConversionError { #[error("Conversion of {0} to u128 unsuccessful.")] U128ToUsizeError(u128), + #[error("Conversion of {0} to u64 unsuccessful.")] + U64ToUsizeError(u64), } diff --git a/crates/blockifier/src/utils.rs b/crates/blockifier/src/utils.rs index 5a75bc5e2d..9e70cf5f90 100644 --- a/crates/blockifier/src/utils.rs +++ b/crates/blockifier/src/utils.rs @@ -54,8 +54,20 @@ 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 { + val.try_into().map_err(|_| NumericConversionError::U64ToUsizeError(val)) +} + /// Conversion from usize to u128. May fail on architectures with over 128 bits /// of address space. pub fn u128_from_usize(val: usize) -> u128 { val.try_into().expect("Conversion from usize to u128 should not fail.") } + +/// Conversion from usize to u64. May fail on architectures with over 64 bits +/// of address space. +pub fn u64_from_usize(val: usize) -> u64 { + val.try_into().expect("Conversion from usize to u64 should not fail.") +} diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index c6da2022d7..9b846d190b 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -135,7 +135,12 @@ define_versioned_constants! { V0_13_3 } -pub type ResourceCost = Ratio; +pub type ResourceCost = Ratio; + +pub fn resource_cost_to_u128_ratio(cost: ResourceCost) -> Ratio { + Ratio::new((*cost.numer()).into(), (*cost.denom()).into()) +} + #[derive(Clone, Debug, Deserialize, Eq, PartialEq, PartialOrd)] pub struct CompilerVersion(pub Version); impl Default for CompilerVersion { @@ -222,18 +227,24 @@ impl VersionedConstants { /// Converts from L1 gas price to L2 gas price with **upward rounding**. pub fn convert_l1_to_l2_gas_price_round_up(&self, l1_gas_price: GasPrice) -> GasPrice { - (*(self.l1_to_l2_gas_price_ratio() * l1_gas_price.0).ceil().numer()).into() + (*(resource_cost_to_u128_ratio(self.l1_to_l2_gas_price_ratio()) * l1_gas_price.0) + .ceil() + .numer()) + .into() } /// 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. - (*(self.l1_to_l2_gas_price_ratio().inv() * l1_gas_amount.0).ceil().numer()).into() + (*(resource_cost_to_u128_ratio(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. fn l1_to_l2_gas_price_ratio(&self) -> ResourceCost { - Ratio::new(1, u128::from(self.os_constants.gas_costs.step_gas_cost)) + Ratio::new(1, self.os_constants.gas_costs.step_gas_cost) * self.vm_resource_fee_cost().n_steps }