Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(blockifier): resource cost ratio type changed to u64 ratio from u128 ratio #1174

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions crates/blockifier/src/fee/fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/fee/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
Expand Down
9 changes: 6 additions & 3 deletions crates/blockifier/src/fee/gas_usage_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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));
}
Expand Down
17 changes: 9 additions & 8 deletions crates/blockifier/src/fee/receipt_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 => {
Expand Down
17 changes: 11 additions & 6 deletions crates/blockifier/src/fee/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = Result<T, TransactionFeeError>;

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 0 additions & 4 deletions crates/blockifier/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) => {
Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
12 changes: 12 additions & 0 deletions crates/blockifier/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,20 @@ pub fn usize_from_u128(val: u128) -> Result<usize, NumericConversionError> {
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<usize, NumericConversionError> {
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.")
}
19 changes: 15 additions & 4 deletions crates/blockifier/src/versioned_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ define_versioned_constants! {
V0_13_3
}

pub type ResourceCost = Ratio<u128>;
pub type ResourceCost = Ratio<u64>;

pub fn resource_cost_to_u128_ratio(cost: ResourceCost) -> Ratio<u128> {
Ratio::new((*cost.numer()).into(), (*cost.denom()).into())
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, PartialOrd)]
pub struct CompilerVersion(pub Version);
impl Default for CompilerVersion {
Expand Down Expand Up @@ -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
}

Expand Down
Loading