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

Commit

Permalink
refactor: remove f64 usage at blockifier
Browse files Browse the repository at this point in the history
  • Loading branch information
nimrod-starkware committed Feb 18, 2024
1 parent ea76ae9 commit b118384
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 25 deletions.
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
47 changes: 37 additions & 10 deletions crates/blockifier/resources/versioned_constants.json
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,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": [
4,
25
],
"ec_op_builtin": [
64,
25
],
"ecdsa_builtin": [
128,
25
],
"keccak_builtin": [
128,
25
],
"n_steps": [
1,
400
],
"output_builtin": [
0,
1
],
"pedersen_builtin": [
2,
25
],
"poseidon_builtin": [
2,
25
],
"range_check_builtin": [
1,
25
]
}
}
}
4 changes: 3 additions & 1 deletion crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ 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 =
(tx_gas_upper_bound as u128 * gas_per_step.denom()) / gas_per_step.numer();
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
7 changes: 4 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,13 @@ 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) * (vm_resource_usage.0.get(key).cloned().unwrap_or_default() as u128)
})
.fold(f64::NAN, f64::max);
.max()
.unwrap();

// 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.ceil().to_integer(), l1_data_gas: 0 })
}

/// Computes and returns the total L1 gas consumption.
Expand Down
44 changes: 33 additions & 11 deletions crates/blockifier/src/versioned_constants.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::{HashMap, HashSet};
use std::io;
use std::path::Path;
use std::sync::Arc;
use std::{io, u128};

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, Serialize};
Expand Down Expand Up @@ -46,7 +47,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, Ratio<u128>>>,

// Cairo OS constants.
// Note: if loaded from a json file, there are some assumptions made on its structure.
Expand All @@ -69,7 +70,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, Ratio<u128>> {
&self.vm_resource_fee_cost
}

Expand Down Expand Up @@ -130,14 +131,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(), Ratio::from_integer(1_u128)),
(
cairo_vm::vm::runners::builtin_runner::HASH_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::RANGE_CHECK_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::SIGNATURE_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::BITWISE_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::POSEIDON_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::OUTPUT_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
(
cairo_vm::vm::runners::builtin_runner::EC_OP_BUILTIN_NAME.to_string(),
Ratio::from_integer(1_u128),
),
]));

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

0 comments on commit b118384

Please sign in to comment.