Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

refactor: remove f64 usage at blockifier #1519

Merged
merged 1 commit into from
Feb 21, 2024
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
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ log = "0.4"
num-bigint = "0.4"
num-integer = "0.1.45"
num-traits = "0.2"
num-rational = { version = "0.4", features = ["serde"] }
once_cell = "1.19.0"
papyrus_storage = "0.3.0-rc.0"
phf = { version = "0.11", features = ["macros"] }
Expand Down
1 change: 1 addition & 0 deletions crates/blockifier/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ log.workspace = true
num-bigint.workspace = true
num-integer.workspace = true
num-traits.workspace = true
num-rational.workspace = true
once_cell.workspace = true
phf.workspace = true
rstest = { workspace = true, optional = true }
Expand Down
45 changes: 36 additions & 9 deletions crates/blockifier/resources/versioned_constants.json
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,41 @@
},
"validate_max_n_steps": 1000000,
"vm_resource_fee_cost": {
"bitwise_builtin": 0.16,
"ec_op_builtin": 2.56,
"ecdsa_builtin": 5.12,
"keccak_builtin": 5.12,
"n_steps": 0.0025,
"output_builtin": 0,
"pedersen_builtin": 0.08,
"poseidon_builtin": 0.08,
"range_check_builtin": 0.04
"bitwise_builtin": [
16,
100
],
"ec_op_builtin": [
256,
100
],
"ecdsa_builtin": [
512,
100
],
"keccak_builtin": [
512,
100
],
"n_steps": [
25,
10000
],
"output_builtin": [
0,
1
],
"pedersen_builtin": [
8,
100
],
"poseidon_builtin": [
8,
100
],
"range_check_builtin": [
4,
100
]
}
}
13 changes: 11 additions & 2 deletions crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::cmp::min;
use std::sync::Arc;

use cairo_vm::vm::runners::cairo_runner::{ExecutionResources, ResourceTracker, RunResources};
use num_traits::{Inv, Zero};
use serde::Serialize;
use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
use starknet_api::deprecated_contract_class::EntryPointType;
Expand All @@ -19,7 +20,7 @@ use crate::execution::execution_utils::execute_entry_point_call;
use crate::state::state_api::State;
use crate::transaction::objects::{HasRelatedFeeType, TransactionExecutionResult, TransactionInfo};
use crate::transaction::transaction_types::TransactionType;
use crate::utils::usize_from_u128;
use crate::utils::{u128_from_usize, usize_from_u128};
use crate::versioned_constants::VersionedConstants;

#[cfg(test)]
Expand Down Expand Up @@ -244,7 +245,15 @@ 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 = (tx_gas_upper_bound as f64 / gas_per_step).floor() as u128;

let upper_bound_u128 = if gas_per_step.is_zero() {
u128::MAX
} else {
(gas_per_step.inv()
* u128_from_usize(tx_gas_upper_bound)
.expect("Conversion from usize to u128 should not fail."))
.to_integer()
};
let tx_upper_bound = usize_from_u128(upper_bound_u128).unwrap_or_else(|_| {
log::warn!(
"Failed to convert u128 to usize: {upper_bound_u128}. Upper bound from tx is \
Expand Down
10 changes: 7 additions & 3 deletions crates/blockifier/src/fee/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,16 @@ pub fn calculate_l1_gas_by_vm_usage(
let vm_l1_gas_usage = vm_resource_fee_costs
.iter()
.map(|(key, resource_val)| {
(*resource_val) * vm_resource_usage.0.get(key).cloned().unwrap_or_default() as f64
((*resource_val)
* u128_from_usize(vm_resource_usage.0.get(key).cloned().unwrap_or_default())
.expect("Conversion from usize to u128 should not fail."))
.ceil()
.to_integer()
})
.fold(f64::NAN, f64::max);
.fold(0, u128::max);

// TODO(Dori, 1/5/2024): Check this conversion.
Ok(GasVector { l1_gas: vm_l1_gas_usage.ceil() as u128, l1_data_gas: 0 })
Ok(GasVector { l1_gas: vm_l1_gas_usage, l1_data_gas: 0 })
}

/// Computes and returns the total L1 gas consumption.
Expand Down
44 changes: 34 additions & 10 deletions crates/blockifier/src/versioned_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::sync::Arc;
use cairo_vm::vm::runners::builtin_runner;
use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use indexmap::{IndexMap, IndexSet};
use num_rational::Ratio;
use once_cell::sync::Lazy;
use serde::de::Error as DeserializationError;
use serde::{Deserialize, Deserializer};
Expand All @@ -30,6 +31,8 @@ static DEFAULT_CONSTANTS: Lazy<VersionedConstants> = Lazy::new(|| {
.expect("Versioned constants JSON file is malformed")
});

pub type ResourceCost = Ratio<u128>;

/// Contains constants for the Blockifier that may vary between versions.
/// Additional constants in the JSON file, not used by Blockifier but included for transparency, are
/// automatically ignored during deserialization.
Expand All @@ -55,7 +58,7 @@ pub struct VersionedConstants {
// Fee related.
// TODO: Consider making this a struct, this will require change the way we access these
// values.
vm_resource_fee_cost: Arc<HashMap<String, f64>>,
vm_resource_fee_cost: Arc<HashMap<String, ResourceCost>>,
}

impl VersionedConstants {
Expand All @@ -71,7 +74,7 @@ impl VersionedConstants {
os_consts.gas_costs["initial_gas_cost"] - os_consts.gas_costs["transaction_gas_cost"]
}

pub fn vm_resource_fee_cost(&self) -> &HashMap<String, f64> {
pub fn vm_resource_fee_cost(&self) -> &HashMap<String, ResourceCost> {
&self.vm_resource_fee_cost
}

Expand Down Expand Up @@ -140,14 +143,35 @@ impl VersionedConstants {
#[cfg(any(feature = "testing", test))]
pub fn create_for_account_testing() -> Self {
let vm_resource_fee_cost = Arc::new(HashMap::from([
(crate::abi::constants::N_STEPS_RESOURCE.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::HASH_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::RANGE_CHECK_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::SIGNATURE_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::BITWISE_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::POSEIDON_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::OUTPUT_BUILTIN_NAME.to_string(), 1_f64),
(cairo_vm::vm::runners::builtin_runner::EC_OP_BUILTIN_NAME.to_string(), 1_f64),
(crate::abi::constants::N_STEPS_RESOURCE.to_string(), ResourceCost::from_integer(1)),
(
cairo_vm::vm::runners::builtin_runner::HASH_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::RANGE_CHECK_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::SIGNATURE_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::BITWISE_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::POSEIDON_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::OUTPUT_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
(
cairo_vm::vm::runners::builtin_runner::EC_OP_BUILTIN_NAME.to_string(),
ResourceCost::from_integer(1),
),
]));

Self { vm_resource_fee_cost, ..Self::create_for_testing() }
Expand Down
Loading