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 (#1519)
Browse files Browse the repository at this point in the history
  • Loading branch information
nimrod-starkware committed Feb 21, 2024
1 parent e2de5ec commit 20313d7
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 24 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
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

0 comments on commit 20313d7

Please sign in to comment.