From 3acb443f29e6fb283f189b3824371bce613fe22f Mon Sep 17 00:00:00 2001 From: meship-starkware Date: Thu, 21 Nov 2024 15:07:02 +0200 Subject: [PATCH] refactor(blockifier): split FC to groups base on their tags --- .../src/test_utils/cairo_compile.rs | 14 ++++-- crates/blockifier/src/test_utils/contracts.rs | 50 +++++++++++++++---- .../feature_contracts_compatibility_test.rs | 43 ++++++++++------ 3 files changed, 77 insertions(+), 30 deletions(-) diff --git a/crates/blockifier/src/test_utils/cairo_compile.rs b/crates/blockifier/src/test_utils/cairo_compile.rs index a0c5f77a8d..13d2c170f2 100644 --- a/crates/blockifier/src/test_utils/cairo_compile.rs +++ b/crates/blockifier/src/test_utils/cairo_compile.rs @@ -204,10 +204,9 @@ fn verify_cairo0_compiler_deps() { ); } -fn prepare_cairo1_compiler_deps(git_tag_override: Option) { - let cairo_repo_path = local_cairo1_compiler_repo_path(); +fn get_tag_and_repo_file_path(git_tag_override: Option) -> (String, PathBuf) { let tag = git_tag_override.unwrap_or(cairo1_compiler_tag()); - + let cairo_repo_path = local_cairo1_compiler_repo_path(); // Check if the path is a directory. assert!( cairo_repo_path.is_dir(), @@ -216,6 +215,11 @@ fn prepare_cairo1_compiler_deps(git_tag_override: Option) { cairo_repo_path.to_string_lossy(), ); + (tag, cairo_repo_path) +} + +pub fn prepare_group_tag_compiler_deps(git_tag_override: Option) { + let (tag, cairo_repo_path) = get_tag_and_repo_file_path(git_tag_override); // Checkout the required version in the compiler repo. run_and_verify_output(Command::new("git").args([ "-C", @@ -223,6 +227,10 @@ fn prepare_cairo1_compiler_deps(git_tag_override: Option) { "checkout", &tag, ])); +} + +fn prepare_cairo1_compiler_deps(git_tag_override: Option) { + let (tag, cairo_repo_path) = get_tag_and_repo_file_path(git_tag_override); // Verify that the checked out tag is as expected. run_and_verify_output(Command::new("git").args([ diff --git a/crates/blockifier/src/test_utils/contracts.rs b/crates/blockifier/src/test_utils/contracts.rs index 7a4bb8fa93..2af77e66b2 100644 --- a/crates/blockifier/src/test_utils/contracts.rs +++ b/crates/blockifier/src/test_utils/contracts.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use cairo_lang_starknet_classes::casm_contract_class::CasmContractClass; use starknet_api::abi::abi_utils::selector_from_name; use starknet_api::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME; @@ -86,6 +88,8 @@ const LEGACY_CONTRACT_RUST_TOOLCHAIN: &str = "2023-07-05"; const CAIRO_STEPS_TEST_CONTRACT_COMPILER_TAG: &str = "v2.7.0"; const CAIRO_STEPS_TEST_CONTRACT_RUST_TOOLCHAIN: &str = "2024-04-29"; +pub type TagToContractsMapping = HashMap<(Option, Option), Vec>; + /// Enum representing all feature contracts. /// The contracts that are implemented in both Cairo versions include a version field. #[derive(Clone, Copy, Debug, EnumIter, Hash, PartialEq, Eq)] @@ -317,7 +321,11 @@ impl FeatureContract { /// Compiles the feature contract and returns the compiled contract as a byte vector. /// Panics if the contract is ERC20, as ERC20 contract recompilation is not supported. - pub fn compile(&self) -> Vec { + pub fn compile( + &self, + tag_override: Option, + cargo_nightly_arg: Option, + ) -> Vec { if matches!(self, Self::ERC20(_)) { panic!("ERC20 contract recompilation not supported."); } @@ -340,19 +348,15 @@ impl FeatureContract { cairo0_compile(self.get_source_path(), extra_arg, false) } CairoVersion::Cairo1 => { - let (tag_override, cargo_nightly_arg) = self.fixed_tag_and_rust_toolchain(); cairo1_compile(self.get_source_path(), tag_override, cargo_nightly_arg) } #[cfg(feature = "cairo_native")] - CairoVersion::Native => { - let (tag_override, cargo_nightly_arg) = self.fixed_tag_and_rust_toolchain(); - starknet_compile( - self.get_source_path(), - tag_override, - cargo_nightly_arg, - &mut vec![], - ) - } + CairoVersion::Native => starknet_compile( + self.get_source_path(), + tag_override, + cargo_nightly_arg, + &mut vec![], + ), } } @@ -409,6 +413,30 @@ impl FeatureContract { self.get_offset(selector, EntryPointType::Constructor) } + pub fn feature_contracts_by_tag() -> TagToContractsMapping { + // EnumIter iterates over all variants with Default::default() as the cairo + // version. + let mut tag_to_contracts_map = HashMap::new(); + for contract in Self::iter() { + if matches!(contract, Self::ERC20(_)) { + continue; + } + let tag_and_version = contract.fixed_tag_and_rust_toolchain(); + let contracts_to_insert = if contract.has_two_versions() { + let mut other_contract = contract; + other_contract.set_cairo_version(contract.cairo_version().other()); + vec![contract, other_contract] + } else { + vec![contract] + }; + tag_to_contracts_map + .entry(tag_and_version) + .or_insert_with(Vec::new) + .extend(contracts_to_insert); + } + tag_to_contracts_map + } + pub fn all_contracts() -> impl Iterator { // EnumIter iterates over all variants with Default::default() as the cairo // version. diff --git a/crates/blockifier/tests/feature_contracts_compatibility_test.rs b/crates/blockifier/tests/feature_contracts_compatibility_test.rs index 845e57b318..d30a475c31 100644 --- a/crates/blockifier/tests/feature_contracts_compatibility_test.rs +++ b/crates/blockifier/tests/feature_contracts_compatibility_test.rs @@ -1,5 +1,6 @@ use std::fs; +use blockifier::test_utils::cairo_compile::prepare_group_tag_compiler_deps; use blockifier::test_utils::contracts::FeatureContract; use blockifier::test_utils::CairoVersion; use pretty_assertions::assert_eq; @@ -36,25 +37,35 @@ const FIX_COMMAND: &str = "FIX_FEATURE_TEST=1 cargo test -p blockifier --test \ // `COMPILED_CONTRACTS_SUBDIR` which equals `starknet-compile-deprecated X.cairo --no_debug_info`. fn verify_feature_contracts_compatibility(fix: bool, cairo_version: CairoVersion) { // TODO(Dori, 1/10/2024): Parallelize this test. - for contract in FeatureContract::all_feature_contracts() - .filter(|contract| contract.cairo_version() == cairo_version) + for ((tag_override, cargo_nightly_arg), feature_contracts) in + FeatureContract::feature_contracts_by_tag() { - // Compare output of cairo-file on file with existing compiled file. - let expected_compiled_output = contract.compile(); - let existing_compiled_path = contract.get_compiled_path(); - - if fix { - fs::write(&existing_compiled_path, &expected_compiled_output).unwrap(); + if cairo_version != CairoVersion::Cairo0 { + prepare_group_tag_compiler_deps(tag_override.clone()); } - let existing_compiled_contents = fs::read_to_string(&existing_compiled_path) - .unwrap_or_else(|_| panic!("Cannot read {existing_compiled_path}.")); + for contract in feature_contracts + .into_iter() + .filter(|contract| contract.cairo_version() == cairo_version) + { + // Compare output of cairo-file on file with existing compiled file. + let expected_compiled_output = + contract.compile(tag_override.clone(), cargo_nightly_arg.clone()); + let existing_compiled_path = contract.get_compiled_path(); + + if fix { + fs::write(&existing_compiled_path, &expected_compiled_output).unwrap(); + } + let existing_compiled_contents = fs::read_to_string(&existing_compiled_path) + .unwrap_or_else(|_| panic!("Cannot read {existing_compiled_path}.")); - if String::from_utf8(expected_compiled_output).unwrap() != existing_compiled_contents { - panic!( - "{} does not compile to {existing_compiled_path}.\nRun `{FIX_COMMAND}` to fix the \ - existing file according to locally installed `starknet-compile-deprecated`.\n", - contract.get_source_path() - ); + if String::from_utf8(expected_compiled_output).unwrap() != existing_compiled_contents { + panic!( + "{} does not compile to {existing_compiled_path}.\nRun `{FIX_COMMAND}` to fix \ + the existing file according to locally installed \ + `starknet-compile-deprecated`.\n", + contract.get_source_path() + ); + } } } }