From b5a877719dc2ce5b1ca833f14d9473c1f1c27059 Mon Sep 17 00:00:00 2001 From: dorimedini-starkware Date: Tue, 15 Oct 2024 22:42:27 +0300 Subject: [PATCH] refactor(blockifier, starknet_api): delete blockifier's StarknetVersion (#1378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change is [Reviewable](https://reviewable.io/reviews/starkware-libs/sequencer/1378) --- crates/blockifier/src/fee/gas_usage_test.rs | 11 +- crates/blockifier/src/versioned_constants.rs | 134 +++++++----------- .../src/versioned_constants_test.rs | 23 ++- .../src/state_reader/errors.rs | 7 +- .../src/state_reader/rpc_https_test.rs | 3 +- .../src/state_reader/test_state_reader.rs | 6 +- crates/native_blockifier/src/lib.rs | 4 +- crates/papyrus_execution/src/lib.rs | 19 ++- crates/starknet_api/src/block.rs | 6 + crates/starknet_api/src/block_test.rs | 9 ++ 10 files changed, 113 insertions(+), 109 deletions(-) diff --git a/crates/blockifier/src/fee/gas_usage_test.rs b/crates/blockifier/src/fee/gas_usage_test.rs index c53eddb651..7ef5134b12 100644 --- a/crates/blockifier/src/fee/gas_usage_test.rs +++ b/crates/blockifier/src/fee/gas_usage_test.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use num_rational::Ratio; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; +use starknet_api::block::StarknetVersion; use starknet_api::execution_resources::{GasAmount, GasVector}; use starknet_api::invoke_tx_args; use starknet_api::transaction::{EventContent, EventData, EventKey, GasVectorComputationMode}; @@ -29,12 +30,7 @@ use crate::test_utils::{ use crate::transaction::objects::FeeType; use crate::transaction::test_utils::account_invoke_tx; use crate::utils::u64_from_usize; -use crate::versioned_constants::{ - ResourceCost, - StarknetVersion, - VersionedConstants, - VmResourceCosts, -}; +use crate::versioned_constants::{ResourceCost, VersionedConstants, VmResourceCosts}; pub fn create_event_for_testing(keys_size: usize, data_size: usize) -> OrderedEvent { OrderedEvent { @@ -317,7 +313,8 @@ fn test_gas_computation_regression_test( // Use a constant version of the versioned constants so that version changes do not break this // test. This specific version is arbitrary. // TODO(Amos, 1/10/2024): Parameterize the version. - let mut versioned_constants = VersionedConstants::get(StarknetVersion::V0_13_2_1).clone(); + let mut versioned_constants = + VersionedConstants::get(&StarknetVersion::V0_13_2_1).unwrap().clone(); // Change the VM resource fee cost so that the L2 / L1 gas ratio is a fraction. let vm_resource_fee_cost = VmResourceCosts { diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index 1a179bd184..d6433df961 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -15,11 +15,10 @@ use semver::Version; use serde::de::Error as DeserializationError; use serde::{Deserialize, Deserializer, Serialize}; use serde_json::{Map, Number, Value}; -use starknet_api::block::GasPrice; +use starknet_api::block::{GasPrice, StarknetVersion}; use starknet_api::execution_resources::GasAmount; use starknet_api::transaction::GasVectorComputationMode; use strum::IntoEnumIterator; -use strum_macros::{EnumCount, EnumIter}; use thiserror::Error; use crate::execution::deprecated_syscalls::hint_processor::SyscallCounter; @@ -34,35 +33,7 @@ pub mod test; /// Auto-generate getters for listed versioned constants versions. macro_rules! define_versioned_constants { - ($(($variant:ident, $path_to_json:expr, $version_str:expr)),*, $latest_variant:ident) => { - /// Enum of all the Starknet versions supporting versioned constants. - #[derive(Clone, Debug, EnumCount, EnumIter, Hash, Eq, PartialEq)] - pub enum StarknetVersion { - $($variant,)* - } - - impl StarknetVersion { - pub fn path_to_versioned_constants_json(&self) -> &'static str { - match self { - $(StarknetVersion::$variant => $path_to_json,)* - } - } - - pub fn latest() -> Self { - StarknetVersion::$latest_variant - } - } - - impl From for String { - fn from(version: StarknetVersion) -> Self { - match version { - $(StarknetVersion::$variant => String::from( - stringify!($variant) - ).to_lowercase().replace("_", "."),)* - } - } - } - + ($(($variant:ident, $path_to_json:expr)),* $(,)?) => { // Static (lazy) instances of the versioned constants. // For internal use only; for access to a static instance use the `StarknetVersion` enum. paste! { @@ -77,64 +48,70 @@ macro_rules! define_versioned_constants { } /// API to access a static instance of the versioned constants. - impl From for &'static VersionedConstants { - fn from(version: StarknetVersion) -> Self { + impl TryFrom for &'static VersionedConstants { + type Error = VersionedConstantsError; + + fn try_from(version: StarknetVersion) -> VersionedConstantsResult { match version { $( StarknetVersion::$variant => { - & paste! { [] } + Ok(& paste! { [] }) } )* + _ => Err(VersionedConstantsError::InvalidStarknetVersion(version)), } } } - pub static VERSIONED_CONSTANTS_LATEST_JSON: LazyLock = LazyLock::new(|| { - let latest_variant = StarknetVersion::$latest_variant; - let path_to_json: PathBuf = [ - env!("CARGO_MANIFEST_DIR"), "src", latest_variant.path_to_versioned_constants_json() - ].iter().collect(); - fs::read_to_string(path_to_json.clone()) - .expect(&format!("Failed to read file {}.", path_to_json.display())) - }); + impl VersionedConstants { + pub fn path_to_json(version: &StarknetVersion) -> VersionedConstantsResult<&'static str> { + match version { + $(StarknetVersion::$variant => Ok($path_to_json),)* + _ => Err(VersionedConstantsError::InvalidStarknetVersion(*version)), + } + } - impl TryFrom<&str> for StarknetVersion { - type Error = VersionedConstantsError; - fn try_from(raw_version: &str) -> Result { - match raw_version { + /// Gets the constants that shipped with the current version of the Blockifier. + /// To use custom constants, initialize the struct from a file using `from_path`. + pub fn latest_constants() -> &'static Self { + Self::get(&StarknetVersion::LATEST) + .expect("Latest version should support VC.") + } + + /// Gets the constants for the specified Starknet version. + pub fn get(version: &StarknetVersion) -> VersionedConstantsResult<&'static Self> { + match version { $( - $version_str => Ok(StarknetVersion::$variant), + StarknetVersion::$variant => Ok( + & paste! { [] } + ), )* - _ => Err(VersionedConstantsError::InvalidVersion { version: raw_version.to_string()}), + _ => Err(VersionedConstantsError::InvalidStarknetVersion(*version)), } } } - #[cfg(test)] - mod tests { - use crate::versioned_constants::StarknetVersion; - - #[test] - fn test_variant_name_string_consistency() { - $( - assert_eq!( - "v".to_owned() + $version_str, - String::from(StarknetVersion::$variant) - ); - )* - } - } + pub static VERSIONED_CONSTANTS_LATEST_JSON: LazyLock = LazyLock::new(|| { + let latest_variant = StarknetVersion::LATEST; + let path_to_json: PathBuf = [ + env!("CARGO_MANIFEST_DIR"), + "src", + VersionedConstants::path_to_json(&latest_variant) + .expect("Latest variant should have a path to json.") + ].iter().collect(); + fs::read_to_string(path_to_json.clone()) + .expect(&format!("Failed to read file {}.", path_to_json.display())) + }); }; } define_versioned_constants! { - (V0_13_0, "../resources/versioned_constants_0_13_0.json", "0.13.0"), - (V0_13_1, "../resources/versioned_constants_0_13_1.json", "0.13.1"), - (V0_13_1_1, "../resources/versioned_constants_0_13_1_1.json", "0.13.1.1"), - (V0_13_2, "../resources/versioned_constants_0_13_2.json", "0.13.2"), - (V0_13_2_1, "../resources/versioned_constants_0_13_2_1.json", "0.13.2.1"), - (V0_13_3, "../resources/versioned_constants_0_13_3.json", "0.13.3"), - V0_13_3 + (V0_13_0, "../resources/versioned_constants_0_13_0.json"), + (V0_13_1, "../resources/versioned_constants_0_13_1.json"), + (V0_13_1_1, "../resources/versioned_constants_0_13_1_1.json"), + (V0_13_2, "../resources/versioned_constants_0_13_2.json"), + (V0_13_2_1, "../resources/versioned_constants_0_13_2_1.json"), + (V0_13_3, "../resources/versioned_constants_0_13_3.json"), } pub type ResourceCost = Ratio; @@ -215,18 +192,7 @@ pub struct VersionedConstants { } impl VersionedConstants { - /// Gets the constants that shipped with the current version of the Blockifier. - /// To use custom constants, initialize the struct from a file using `from_path`. - pub fn latest_constants() -> &'static Self { - Self::get(StarknetVersion::latest()) - } - - /// Gets the constants for the specified Starknet version. - pub fn get(version: StarknetVersion) -> &'static Self { - version.into() - } - - pub fn from_path(path: &Path) -> Result { + pub fn from_path(path: &Path) -> VersionedConstantsResult { Ok(serde_json::from_reader(std::fs::File::open(path)?)?) } @@ -769,8 +735,12 @@ pub enum VersionedConstantsError { ParseError(#[from] serde_json::Error), #[error("Invalid version: {version:?}")] InvalidVersion { version: String }, + #[error("Invalid Starknet version: {0}")] + InvalidStarknetVersion(StarknetVersion), } +pub type VersionedConstantsResult = Result; + #[derive(Debug, Error)] pub enum OsConstantsSerdeError { #[error("Value cannot be cast into u64: {0}")] @@ -808,7 +778,7 @@ struct ResourceParamsRaw { impl TryFrom for ResourcesParams { type Error = VersionedConstantsError; - fn try_from(mut json_data: ResourceParamsRaw) -> Result { + fn try_from(mut json_data: ResourceParamsRaw) -> VersionedConstantsResult { let constant_value = json_data.raw_resource_params_as_dict.remove("constant"); let calldata_factor_value = json_data.raw_resource_params_as_dict.remove("calldata_factor"); diff --git a/crates/blockifier/src/versioned_constants_test.rs b/crates/blockifier/src/versioned_constants_test.rs index 32ebe68d76..7947afc52a 100644 --- a/crates/blockifier/src/versioned_constants_test.rs +++ b/crates/blockifier/src/versioned_constants_test.rs @@ -144,5 +144,26 @@ fn test_old_json_parsing() { #[test] fn test_all_jsons_in_enum() { - assert_eq!(StarknetVersion::iter().count(), all_jsons_in_dir().count()); + let all_jsons: Vec = all_jsons_in_dir().map(Result::unwrap).collect(); + + // Check that the number of new starknet versions (versions supporting VC) is equal to the + // number of JSON files. + assert_eq!( + StarknetVersion::iter().filter(|version| version >= &StarknetVersion::V0_13_0).count(), + all_jsons.len() + ); + + // Check that all JSON files are in the enum and can be loaded. + for file in all_jsons { + let filename = file.file_stem().unwrap().to_str().unwrap().to_string(); + assert!(filename.starts_with("versioned_constants_")); + let version_str = filename.trim_start_matches("versioned_constants_").replace("_", "."); + let version = StarknetVersion::try_from(version_str).unwrap(); + assert!(VersionedConstants::get(&version).is_ok()); + } +} + +#[test] +fn test_latest_no_panic() { + VersionedConstants::latest_constants(); } diff --git a/crates/blockifier_reexecution/src/state_reader/errors.rs b/crates/blockifier_reexecution/src/state_reader/errors.rs index 0cc7d35729..e2d5a0c1dc 100644 --- a/crates/blockifier_reexecution/src/state_reader/errors.rs +++ b/crates/blockifier_reexecution/src/state_reader/errors.rs @@ -2,19 +2,22 @@ use blockifier::state::errors::StateError; use blockifier::transaction::errors::TransactionExecutionError; use blockifier::versioned_constants::VersionedConstantsError; use serde_json::Error as SerdeError; +use starknet_api::StarknetApiError; use starknet_gateway::errors::RPCStateReaderError; use thiserror::Error; #[derive(Debug, Error)] #[allow(clippy::enum_variant_names)] pub enum ReexecutionError { - #[error(transparent)] - State(#[from] StateError), #[error(transparent)] Rpc(#[from] RPCStateReaderError), #[error(transparent)] Serde(#[from] SerdeError), #[error(transparent)] + StarknetApi(#[from] StarknetApiError), + #[error(transparent)] + State(#[from] StateError), + #[error(transparent)] TransactionExecutionError(#[from] TransactionExecutionError), #[error(transparent)] VersionedConstants(#[from] VersionedConstantsError), diff --git a/crates/blockifier_reexecution/src/state_reader/rpc_https_test.rs b/crates/blockifier_reexecution/src/state_reader/rpc_https_test.rs index 28e7abf5b3..73b546cde2 100644 --- a/crates/blockifier_reexecution/src/state_reader/rpc_https_test.rs +++ b/crates/blockifier_reexecution/src/state_reader/rpc_https_test.rs @@ -1,9 +1,8 @@ use assert_matches::assert_matches; use blockifier::blockifier::block::BlockInfo; -use blockifier::versioned_constants::StarknetVersion; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; -use starknet_api::block::BlockNumber; +use starknet_api::block::{BlockNumber, StarknetVersion}; use starknet_api::core::ClassHash; use starknet_api::test_utils::read_json_file; use starknet_api::transaction::Transaction; diff --git a/crates/blockifier_reexecution/src/state_reader/test_state_reader.rs b/crates/blockifier_reexecution/src/state_reader/test_state_reader.rs index a798d518c2..64462c996b 100644 --- a/crates/blockifier_reexecution/src/state_reader/test_state_reader.rs +++ b/crates/blockifier_reexecution/src/state_reader/test_state_reader.rs @@ -7,9 +7,9 @@ use blockifier::execution::contract_class::ContractClass as BlockifierContractCl use blockifier::state::cached_state::{CachedState, CommitmentStateDiff}; use blockifier::state::errors::StateError; use blockifier::state::state_api::{StateReader, StateResult}; -use blockifier::versioned_constants::{StarknetVersion, VersionedConstants}; +use blockifier::versioned_constants::VersionedConstants; use serde_json::{json, to_value}; -use starknet_api::block::BlockNumber; +use starknet_api::block::{BlockNumber, StarknetVersion}; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; use starknet_api::state::StorageKey; use starknet_api::transaction::{Transaction, TransactionHash}; @@ -158,7 +158,7 @@ impl TestStateReader { } pub fn get_versioned_constants(&self) -> ReexecutionResult<&'static VersionedConstants> { - Ok(self.get_starknet_version()?.into()) + Ok(VersionedConstants::get(&self.get_starknet_version()?)?) } pub fn get_block_context(&self) -> ReexecutionResult { diff --git a/crates/native_blockifier/src/lib.rs b/crates/native_blockifier/src/lib.rs index 082826e745..5b94780628 100644 --- a/crates/native_blockifier/src/lib.rs +++ b/crates/native_blockifier/src/lib.rs @@ -20,12 +20,12 @@ pub mod state_readers; pub mod storage; pub mod test_utils; -use blockifier::versioned_constants::StarknetVersion; use errors::{add_py_exceptions, UndeclaredClassHashError}; use py_block_executor::PyBlockExecutor; use py_objects::PyExecutionResources; use py_validator::PyValidator; use pyo3::prelude::*; +use starknet_api::block::StarknetVersion; use storage::StorageConfig; use crate::py_objects::PyVersionedConstantsOverrides; @@ -80,5 +80,5 @@ pub fn blockifier_version() -> PyResult { /// Returns the latest Starknet version for versioned constants. #[pyfunction] pub fn starknet_version() -> PyResult { - Ok(StarknetVersion::latest().into()) + Ok(StarknetVersion::LATEST.into()) } diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index 4e83607822..97338954c8 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -42,10 +42,7 @@ use blockifier::transaction::objects::{ }; use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction; use blockifier::transaction::transactions::ExecutableTransaction; -use blockifier::versioned_constants::{ - StarknetVersion as BlockifierStarknetVersion, - VersionedConstants, -}; +use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsError}; use cairo_lang_starknet_classes::casm_contract_class::CasmContractClass; use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; @@ -187,6 +184,8 @@ pub enum ExecutionError { TransactionHashCalculationFailed(StarknetApiError), #[error("Unknown builtin name: {builtin_name}")] UnknownBuiltin { builtin_name: BuiltinName }, + #[error(transparent)] + VersionedConstants(#[from] VersionedConstantsError), } /// Whether the only-query bit of the transaction version is on. @@ -868,16 +867,16 @@ fn get_versioned_constants( let versioned_constants = match starknet_version { Some(starknet_version) => { let version = starknet_version.to_string(); - let blockifier_starknet_version = if version == STARKNET_VERSION_O_13_0 { - BlockifierStarknetVersion::V0_13_0 + let starknet_api_starknet_version = if version == STARKNET_VERSION_O_13_0 { + StarknetVersion::V0_13_0 } else if version == STARKNET_VERSION_O_13_1 { - BlockifierStarknetVersion::V0_13_1 + StarknetVersion::V0_13_1 } else if version == STARKNET_VERSION_O_13_2 { - BlockifierStarknetVersion::V0_13_2 + StarknetVersion::V0_13_2 } else { - BlockifierStarknetVersion::V0_13_3 + StarknetVersion::V0_13_3 }; - VersionedConstants::get(blockifier_starknet_version) + VersionedConstants::get(&starknet_api_starknet_version)? } None => VersionedConstants::latest_constants(), }; diff --git a/crates/starknet_api/src/block.rs b/crates/starknet_api/src/block.rs index c0ea73291b..4704dc6757 100644 --- a/crates/starknet_api/src/block.rs +++ b/crates/starknet_api/src/block.rs @@ -118,6 +118,12 @@ impl Display for StarknetVersion { } } +impl From for String { + fn from(version: StarknetVersion) -> Self { + format!("{version}") + } +} + impl TryFrom for StarknetVersion { type Error = StarknetApiError; diff --git a/crates/starknet_api/src/block_test.rs b/crates/starknet_api/src/block_test.rs index 175034e6ac..80c02c412a 100644 --- a/crates/starknet_api/src/block_test.rs +++ b/crates/starknet_api/src/block_test.rs @@ -71,3 +71,12 @@ fn test_version_byte_vec_order() { assert!(Vec::::from(versions[i]) <= Vec::::from(versions[i + 1])); } } + +#[test] +fn test_latest_version() { + let latest = StarknetVersion::LATEST; + assert_eq!(StarknetVersion::default(), latest); + for version in StarknetVersion::iter() { + assert!(version <= latest); + } +}