From a6fdfe00202f2599c436229f4fe118f4471c6456 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 | 21 +++----- crates/blockifier/src/fee/fee_test.rs | 28 ++++++----- crates/blockifier/src/fee/fee_utils.rs | 4 +- crates/blockifier/src/fee/gas_usage.rs | 8 +-- crates/blockifier/src/fee/gas_usage_test.rs | 17 +++---- crates/blockifier/src/fee/receipt_test.rs | 33 ++++++------ crates/blockifier/src/fee/resources.rs | 50 +++++++++++++------ crates/blockifier/src/test_utils.rs | 4 +- .../src/transaction/account_transaction.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 | 6 +-- .../src/py_block_executor.rs | 8 +-- .../starknet_api/src/execution_resources.rs | 10 ++-- crates/starknet_api/src/transaction.rs | 34 +++++++------ crates/starknet_api/src/transaction_test.rs | 25 ++++++++-- 20 files changed, 173 insertions(+), 150 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 6fa7dea549..6f0c3efc2e 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -197,13 +197,13 @@ 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()) - } - TransactionInfo::Current(context) => { - GasAmount(u128::from(context.l1_resource_bounds().max_amount)) - } + TransactionInfo::Deprecated(context) => context + .max_fee + .checked_div( + block_info.gas_prices.get_l1_gas_price_by_fee_type(&tx_info.fee_type()), + ) + .unwrap_or(GasAmount::MAX), + TransactionInfo::Current(context) => GasAmount(context.l1_resource_bounds().max_amount), }; // Use saturating upper bound to avoid overflow. This is safe because the upper bound is @@ -212,12 +212,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 296c5a711f..cd79ea0edf 100644 --- a/crates/blockifier/src/fee/fee_test.rs +++ b/crates/blockifier/src/fee/fee_test.rs @@ -29,7 +29,7 @@ use crate::test_utils::{ MAX_L1_GAS_AMOUNT, }; 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; fn get_vm_resource_usage() -> ExecutionResources { @@ -57,12 +57,12 @@ 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 = GasAmount(u128::from( + let vm_usage_in_l1_gas = GasAmount( (versioned_constants.vm_resource_fee_cost().n_steps - * (u64_from_usize(vm_resource_usage.n_steps + n_reverted_steps))) + * u64_from_usize(vm_resource_usage.n_steps + n_reverted_steps)) .ceil() .to_integer(), - )); + ); let expected_gas_vector = gas_vector_from_vm_usage( vm_usage_in_l1_gas, &gas_vector_computation_mode, @@ -82,7 +82,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(); @@ -113,12 +113,12 @@ fn test_float_get_vm_resource_usage( // Positive flow. // Verify calculation - in our case, n_steps is the heaviest resource. let n_reverted_steps = 300; - let vm_usage_in_l1_gas = GasAmount(u128::from( + let vm_usage_in_l1_gas = GasAmount( (versioned_constants.vm_resource_fee_cost().n_steps * u64_from_usize(vm_resource_usage.n_steps + n_reverted_steps)) .ceil() .to_integer(), - )); + ); let expected_gas_vector = gas_vector_from_vm_usage( vm_usage_in_l1_gas, &gas_vector_computation_mode, @@ -136,14 +136,14 @@ fn test_float_get_vm_resource_usage( // Another positive flow, this time the heaviest resource is ecdsa_builtin. vm_resource_usage.n_steps = 200; - let vm_usage_in_l1_gas = GasAmount(u128::from( + let vm_usage_in_l1_gas = GasAmount( ((*versioned_constants.vm_resource_fee_cost().builtins.get(&BuiltinName::ecdsa).unwrap()) * u64_from_usize( *vm_resource_usage.builtin_instance_counter.get(&BuiltinName::ecdsa).unwrap(), )) .ceil() .to_integer(), - )); + ); let expected_gas_vector = gas_vector_from_vm_usage( vm_usage_in_l1_gas, &gas_vector_computation_mode, @@ -172,9 +172,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) = @@ -224,7 +224,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 diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index 7251e90c00..b15a1cb80d 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -51,7 +51,7 @@ pub fn get_vm_resources_cost( // Convert Cairo resource usage to L1 gas usage. // Do so by taking the maximum of the usage of each builtin + step usage. - let vm_l1_gas_usage = GasAmount(u128::from( + let vm_l1_gas_usage = GasAmount( vm_resource_fee_costs .builtins .iter() @@ -66,7 +66,7 @@ pub fn get_vm_resources_cost( )]) .map(|(cost, usage)| (cost * u64_from_usize(usage)).ceil().to_integer()) .fold(0, u64::max), - )); + ); match computation_mode { GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas(vm_l1_gas_usage), 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 395e685518..c4280287b0 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -16,7 +16,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, u64_from_usize}; +use crate::utils::u64_from_usize; use crate::versioned_constants::{ResourceCost, VersionedConstants}; #[fixture] fn versioned_constants() -> &'static VersionedConstants { @@ -90,7 +90,7 @@ fn test_get_event_gas_cost( let execution_summary = CallInfo::summarize_many(call_infos.iter()); // 8 keys and 11 data words overall. let expected_gas = - GasAmount(u128::from((data_word_cost * (event_key_factor * 8_u64 + 11_u64)).to_integer())); + GasAmount((data_word_cost * (event_key_factor * 8_u64 + 11_u64)).to_integer()); 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), @@ -146,7 +146,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 ); } @@ -195,10 +195,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)); } @@ -236,10 +233,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 + GasAmount(1)); diff --git a/crates/blockifier/src/fee/receipt_test.rs b/crates/blockifier/src/fee/receipt_test.rs index bf5d5f8c90..d486060bc4 100644 --- a/crates/blockifier/src/fee/receipt_test.rs +++ b/crates/blockifier/src/fee/receipt_test.rs @@ -31,7 +31,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] @@ -82,7 +82,7 @@ fn test_calculate_tx_gas_usage_basic<'a>( let gas_per_code_byte = versioned_constants .get_archival_data_gas_costs(&gas_vector_computation_mode) .gas_per_code_byte; - let code_gas_cost = GasAmount(u128::from( + let code_gas_cost = GasAmount( (gas_per_code_byte * u64_from_usize( (class_info.bytecode_length() + class_info.sierra_program_length()) @@ -90,7 +90,7 @@ fn test_calculate_tx_gas_usage_basic<'a>( + class_info.abi_length(), )) .to_integer(), - )); + ); let manual_gas_vector = match gas_vector_computation_mode { GasVectorComputationMode::NoL2Gas => GasVector::from_l1_gas(code_gas_cost), GasVectorComputationMode::All => GasVector::from_l2_gas(code_gas_cost), @@ -126,9 +126,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 => { @@ -165,10 +165,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 => { @@ -179,16 +178,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); @@ -247,16 +246,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() }; @@ -331,7 +330,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 401e2f3da1..e5ac59c9bf 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, @@ -208,26 +208,36 @@ impl ArchivalDataResources { ) -> GasAmount { // TODO(Avi, 20/2/2024): Calculate the number of bytes instead of the number of felts. let total_data_size = u64_from_usize(self.calldata_length + self.signature_length); - GasAmount(u128::from((archival_gas_costs.gas_per_data_felt * total_data_size).to_integer())) + GasAmount((archival_gas_costs.gas_per_data_felt * total_data_size).to_integer()) } /// 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 { - u128::from( - (archival_gas_costs.gas_per_code_byte * u64_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 { 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(), + 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(|_| { + log::warn!( + "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 + ); + u64::MAX + }), ) } } @@ -255,7 +265,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(), @@ -273,7 +283,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 @@ -374,6 +384,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 af8e837997..f3a6e8e75f 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -112,10 +112,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_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index e6173bdb9d..21da782c08 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -362,10 +362,10 @@ impl AccountTransaction { // TODO(Aner): refactor to return all amounts that are too low. // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the // conversion works. - if minimal_gas_amount > u128::from(resource_bounds.max_amount).into() { + if minimal_gas_amount > resource_bounds.max_amount.into() { return Err(TransactionFeeError::MaxGasAmountTooLow { resource, - max_gas_amount: u128::from(resource_bounds.max_amount).into(), + max_gas_amount: resource_bounds.max_amount.into(), minimal_gas_amount, })?; } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 84f6bdb015..2f2e92e800 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -86,7 +86,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) { @@ -171,7 +171,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(), @@ -965,13 +968,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( @@ -1062,7 +1065,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 48177eae9d..a91e386f3b 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -52,7 +52,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 { @@ -289,7 +289,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( @@ -373,7 +373,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 ) @@ -407,7 +407,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()) @@ -514,8 +514,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); @@ -583,8 +582,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( @@ -611,7 +609,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(GasAmount(7763), 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, @@ -660,7 +658,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 = u64::from(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), @@ -734,12 +732,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, @@ -784,7 +782,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, @@ -794,7 +792,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 06e7cadd48..b8040a00b3 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -325,7 +325,7 @@ pub fn run_invoke_tx( /// No guarantees on the values of the other resources 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, }) } @@ -358,16 +358,10 @@ 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 2c54e76c61..9da1e364d5 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -907,7 +907,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(GasAmount(balance_over_gas_price.0 + 1), MAX_L1_GAS_PRICE.into()); @@ -985,15 +985,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 0fa67976d4..356ff5c1a5 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -237,11 +237,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. - GasAmount( - *(resource_cost_to_u128_ratio(self.l1_to_l2_gas_price_ratio().inv()) * l1_gas_amount.0) - .ceil() - .numer(), - ) + GasAmount(*(self.l1_to_l2_gas_price_ratio().inv() * l1_gas_amount.0).ceil().numer()) } /// 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/execution_resources.rs b/crates/starknet_api/src/execution_resources.rs index 07c5d20f25..b769c67f79 100644 --- a/crates/starknet_api/src/execution_resources.rs +++ b/crates/starknet_api/src/execution_resources.rs @@ -18,21 +18,25 @@ use strum_macros::EnumIter; Serialize, Deserialize, )] -pub struct GasAmount(pub u128); +pub struct GasAmount(pub u64); + +impl GasAmount { + pub const MAX: Self = Self(u64::MAX); +} 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); #[derive(Debug, Default, Deserialize, Serialize, Clone, Eq, PartialEq)] pub struct GasVector { diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index 35d9574300..14ce1878e4 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}; @@ -743,12 +742,22 @@ 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 = GasAmount(result.0 + 1); + pub fn checked_div_ceil(self, rhs: NonzeroGasPrice) -> Option { + match self.checked_div(rhs) { + Some(value) => Some(if value.nonzero_saturating_mul(rhs) < self { + GasAmount(value.0 + 1) + } else { + value + }), + None => None, + } + } + + pub fn checked_div(self, rhs: NonzeroGasPrice) -> Option { + match u64::try_from(self.0 / rhs.get().0) { + Ok(value) => Some(GasAmount(value)), + Err(_) => None, } - result } } @@ -770,21 +779,14 @@ impl From for Felt { } } -impl Div for Fee { - type Output = GasAmount; - - fn div(self, rhs: NonzeroGasPrice) -> Self::Output { - GasAmount(self.0 / GasPrice::from(rhs).0) - } -} - 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/transaction_test.rs b/crates/starknet_api/src/transaction_test.rs index 7c89cfe12a..2a2ca48deb 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::new(GasPrice(1)).unwrap())); - assert_eq!(GasAmount(0), Fee(0).div_ceil(NonzeroGasPrice::new(GasPrice(1)).unwrap())); - assert_eq!(GasAmount(1), Fee(1).div_ceil(NonzeroGasPrice::new(GasPrice(2)).unwrap())); - assert_eq!(GasAmount(9), Fee(27).div_ceil(NonzeroGasPrice::new(GasPrice(3)).unwrap())); - assert_eq!(GasAmount(10), Fee(28).div_ceil(NonzeroGasPrice::new(GasPrice(3)).unwrap())); + assert_eq!( + GasAmount(1), + Fee(1).checked_div_ceil(NonzeroGasPrice::new(GasPrice(1)).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(0), + Fee(0).checked_div_ceil(NonzeroGasPrice::new(GasPrice(1)).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(1), + Fee(1).checked_div_ceil(NonzeroGasPrice::new(GasPrice(2)).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(9), + Fee(27).checked_div_ceil(NonzeroGasPrice::new(GasPrice(3)).unwrap()).unwrap() + ); + assert_eq!( + GasAmount(10), + Fee(28).checked_div_ceil(NonzeroGasPrice::new(GasPrice(3)).unwrap()).unwrap() + ); }