refactor(blockifier): feature contracts and feature compatibility tests

crates/blockifier/src/test_utils.rs

@@ -75,6 +75,18 @@ impl Default for CairoVersion {
     }
 }
 
+impl From<isize> for CairoVersion {
+    fn from(value: isize) -> Self {
+        match value {
+            0 => Self::Cairo0,
+            1 => Self::Cairo1,
+            #[cfg(feature = "cairo_native")]
+            2 => Self::Native,
+            _ => panic!("Invalid value for CairoVersion: {}", value),
+        }
+    }
+}
+
 impl CairoVersion {
     // A declare transaction of the given version, can be used to declare contracts of the returned
     // cairo version.
@@ -333,7 +345,7 @@ macro_rules! check_tx_execution_error_for_invalid_scenario {
                     $validate_constructor,
                 );
             }
-            CairoVersion::Cairo1  => {
+            CairoVersion::Cairo1 => {
                 if let $crate::transaction::errors::TransactionExecutionError::ValidateTransactionError {
                     error, ..
                 } = $error {
@@ -345,7 +357,7 @@ macro_rules! check_tx_execution_error_for_invalid_scenario {
                 }
             }
             #[cfg(feature = "cairo_native")]
-            CairoVersion::Native   => {
+            CairoVersion::Native => {
                 if let $crate::transaction::errors::TransactionExecutionError::ValidateTransactionError {
                     error, ..
                 } = $error {

crates/blockifier/src/test_utils/cairo_compile.rs

@@ -204,6 +204,17 @@ fn verify_cairo0_compiler_deps() {
     );
 }
 
+// Lock to prevent concurrent git operations that create a git index.lock file.
+// If removed some tests will fail i.e. feature contract tests with following error:
+// """
+// Another git process seems to be running in this repository, e.g.
+// an editor opened by 'git commit'. Please make sure all processes
+// are terminated then try again. If it still fails, a git process
+// may have crashed in this repository earlier:
+// remove the file manually to continue.
+// """
+static GIT_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
 fn prepare_cairo1_compiler_deps(git_tag_override: Option<String>) {
     let cairo_repo_path = local_cairo1_compiler_repo_path();
     let tag = git_tag_override.unwrap_or(cairo1_compiler_tag());
@@ -216,6 +227,8 @@ fn prepare_cairo1_compiler_deps(git_tag_override: Option<String>) {
         cairo_repo_path.to_string_lossy(),
     );
 
+    let _lock = GIT_LOCK.lock().unwrap();
+
     // Checkout the required version in the compiler repo.
     run_and_verify_output(Command::new("git").args([
         "-C",

crates/blockifier/src/test_utils/contracts.rs

@@ -119,18 +119,41 @@ impl FeatureContract {
         }
     }
 
-    fn has_two_versions(&self) -> bool {
-        match self {
-            Self::AccountWithLongValidate(_)
-            | Self::AccountWithoutValidations(_)
-            | Self::Empty(_)
-            | Self::FaultyAccount(_)
-            | Self::TestContract(_)
-            | Self::ERC20(_) => true,
-            #[cfg(feature = "cairo_native")]
-            Self::SierraExecutionInfoV1Contract => false,
-            Self::SecurityTests | Self::LegacyTestContract | Self::CairoStepsTestContract => false,
-        }
+    /// Returns a bit mask of supported Cairo versions for the feature contract.
+    ///
+    /// Each bit identifies a supported Cairo version. The least significant bit is Cairo0, the next
+    /// bit is Cairo1, and the most significant bit is the native version.
+    ///
+    /// This order is defined in [CairoVersion] enum.
+    fn supported_versions(&self) -> u32 {
+        let supports_legacy = matches!(
+            self,
+            Self::FaultyAccount(_)
+                | Self::AccountWithoutValidations(_)
+                | Self::AccountWithLongValidate(_)
+                | Self::Empty(_)
+                | Self::TestContract(_)
+                | Self::SecurityTests
+                | Self::ERC20(_)
+        );
+
+        let supports_cairo1 = matches!(
+            self,
+            Self::FaultyAccount(_)
+                | Self::AccountWithoutValidations(_)
+                | Self::AccountWithLongValidate(_)
+                | Self::Empty(_)
+                | Self::LegacyTestContract
+                | Self::TestContract(_)
+                | Self::ERC20(_)
+                | Self::CairoStepsTestContract
+        );
+
+        let supports_native = matches!(self, Self::TestContract(_) | Self::SierraExecutionInfoV1Contract);
+
+        (u32::from(supports_legacy))
+            | (u32::from(supports_cairo1)) << 1
+            | (u32::from(supports_native)) << 2
     }
 
     pub fn set_cairo_version(&mut self, version: CairoVersion) {
@@ -276,7 +299,7 @@ impl FeatureContract {
                     CairoVersion::Cairo0 => "0",
                     CairoVersion::Cairo1 => "1",
                     #[cfg(feature = "cairo_native")]
-                    CairoVersion::Native => "_native",
+                    CairoVersion::Native => "1",
                 },
                 self.get_non_erc20_base_name()
             )
@@ -296,12 +319,18 @@ impl FeatureContract {
         } else {
             let cairo_version = self.cairo_version();
             format!(
-                "feature_contracts/cairo{}/compiled/{}{}.json",
+                "feature_contracts/cairo{}/compiled{}/{}{}.json",
                 match cairo_version {
                     CairoVersion::Cairo0 => "0",
                     CairoVersion::Cairo1 => "1",
                     #[cfg(feature = "cairo_native")]
-                    CairoVersion::Native => "_native",
+                    CairoVersion::Native => "1",
+                },
+                match self.cairo_version() {
+                    CairoVersion::Cairo0 => "",
+                    CairoVersion::Cairo1 => "_casm",
+                    #[cfg(feature = "cairo_native")]
+                    CairoVersion::Native => "_sierra",
                 },
                 self.get_non_erc20_base_name(),
                 match cairo_version {
@@ -412,12 +441,29 @@ impl FeatureContract {
         // EnumIter iterates over all variants with Default::default() as the cairo
         // version.
         Self::iter().flat_map(|contract| {
-            if contract.has_two_versions() {
-                let mut other_contract = contract;
-                other_contract.set_cairo_version(contract.cairo_version().other());
-                vec![contract, other_contract].into_iter()
-            } else {
+            // If only one supported version exists, add the contract to the array as is.
+            if contract.supported_versions().is_power_of_two() {
                 vec![contract].into_iter()
+            } else {
+                let supported_versions = contract.supported_versions();
+
+                #[cfg(feature = "cairo_native")]
+                let range = 0..3isize;
+                #[cfg(not(feature = "cairo_native"))]
+                let range = 0..2isize;
+
+                // If multiple supported versions exist, add each supported version of the
+                // contract to the array.
+                range
+                    .filter(|i| supported_versions & (1u32 << i) != 0)
+                    .map(move |i| {
+                        let mut contract = contract;
+                        contract.set_cairo_version(CairoVersion::from(i));
+
+                        contract
+                    })
+                    .collect::<Vec<_>>()
+                    .into_iter()
             }
         })
     }

crates/blockifier/tests/feature_contracts_compatibility_test.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::fs;
 
 use blockifier::test_utils::contracts::FeatureContract;
@@ -8,8 +9,11 @@ use rstest::rstest;
 const CAIRO0_FEATURE_CONTRACTS_DIR: &str = "feature_contracts/cairo0";
 const CAIRO1_FEATURE_CONTRACTS_DIR: &str = "feature_contracts/cairo1";
 #[cfg(feature = "cairo_native")]
-const NATIVE_FEATURE_CONTRACTS_DIR: &str = "feature_contracts/cairo_native";
-const COMPILED_CONTRACTS_SUBDIR: &str = "compiled";
+const CAIRO_NATIVE_FEATURE_CONTRACTS_DIR: &str = "feature_contracts/cairo1";
+const COMPILED_CONTRACTS_SUBDIR_CAIRO0: &str = "compiled";
+const COMPILED_CONTRACTS_SUBDIR_CASM: &str = "compiled_casm";
+const COMPILED_CONTRACTS_SUBDIR_SIERRA: &str = "compiled_sierra";
+
 const FIX_COMMAND: &str = "FIX_FEATURE_TEST=1 cargo test -p blockifier --test \
                            feature_contracts_compatibility_test --features testing -- \
                            --include-ignored";
@@ -67,42 +71,78 @@ fn verify_and_get_files(cairo_version: CairoVersion) -> Vec<(String, String, Str
         CairoVersion::Cairo0 => CAIRO0_FEATURE_CONTRACTS_DIR,
         CairoVersion::Cairo1 => CAIRO1_FEATURE_CONTRACTS_DIR,
         #[cfg(feature = "cairo_native")]
-        CairoVersion::Native => NATIVE_FEATURE_CONTRACTS_DIR,
+        CairoVersion::Native => CAIRO_NATIVE_FEATURE_CONTRACTS_DIR,
     };
     let compiled_extension = match cairo_version {
         CairoVersion::Cairo0 => "_compiled.json",
         CairoVersion::Cairo1 => ".casm.json",
         #[cfg(feature = "cairo_native")]
         CairoVersion::Native => ".sierra.json",
     };
-    for file in fs::read_dir(directory).unwrap() {
-        let path = file.unwrap().path();
+
+    // Collect base filenames from the FeatureContract enum for the given Cairo version.
+    let contract_base_filenames: HashSet<String> = FeatureContract::all_feature_contracts()
+        .filter(|contract| contract.cairo_version() == cairo_version)
+        .map(|contract| {
+            let source_path = contract.get_source_path();
+            // Extract the base filename without extension
+            let file_stem = std::path::Path::new(&source_path)
+                .file_stem()
+                .unwrap()
+                .to_string_lossy()
+                .into_owned();
+            file_stem
+        })
+        .collect();
+
+    for entry in fs::read_dir(directory).unwrap() {
+        let path = entry.unwrap().path();
 
         // Verify `TEST_CONTRACTS` file and directory structure.
         if !path.is_file() {
             if let Some(dir_name) = path.file_name() {
-                assert_eq!(
-                    dir_name,
-                    COMPILED_CONTRACTS_SUBDIR,
-                    "Found directory '{}' in `{directory}`, which should contain only the \
-                     `{COMPILED_CONTRACTS_SUBDIR}` directory.",
-                    dir_name.to_string_lossy()
+                const ALLOWED_SUBDIRS: &[&str] = &[
+                    COMPILED_CONTRACTS_SUBDIR_CAIRO0,
+                    COMPILED_CONTRACTS_SUBDIR_CASM,
+                    COMPILED_CONTRACTS_SUBDIR_SIERRA,
+                ];
+
+                assert!(
+                    ALLOWED_SUBDIRS.contains(&dir_name.to_str().unwrap()),
+                    "Found directory '{}' in `{directory}`, which should contain only the {} \
+                     directories.",
+                    dir_name.to_string_lossy(),
+                    ALLOWED_SUBDIRS.join(" / ")
                 );
                 continue;
             }
         }
-        let path_str = path.to_string_lossy();
-        assert_eq!(
-            path.extension().unwrap(),
-            "cairo",
-            "Found a non-Cairo file '{path_str}' in `{directory}`"
-        );
 
-        let file_name = path.file_stem().unwrap().to_string_lossy();
-        let existing_compiled_path =
-            format!("{directory}/{COMPILED_CONTRACTS_SUBDIR}/{file_name}{compiled_extension}");
+        let file_name = path.file_stem().unwrap().to_string_lossy().into_owned();
 
-        paths.push((path_str.to_string(), file_name.to_string(), existing_compiled_path));
+        // Skip files not in the enum.
+        if !contract_base_filenames.contains(&file_name) {
+            continue;
+        }
+
+        // Verify the file extension.
+        if path.extension().unwrap() != "cairo" {
+            panic!("Found a non-Cairo file '{}' in `{}`", path.display(), directory);
+        }
+
+        let compiled_contracts_subdir = match cairo_version {
+            CairoVersion::Cairo0 => COMPILED_CONTRACTS_SUBDIR_CAIRO0,
+            CairoVersion::Cairo1 => COMPILED_CONTRACTS_SUBDIR_CASM,
+            #[cfg(feature = "cairo_native")]
+            CairoVersion::Native => COMPILED_CONTRACTS_SUBDIR_SIERRA,
+        };
+
+        let existing_compiled_path = format!(
+            "{}/{}/{}{}",
+            directory, compiled_contracts_subdir, file_name, compiled_extension
+        );
+
+        paths.push((path.to_string_lossy().into_owned(), file_name, existing_compiled_path));
     }
 
     paths
@@ -113,17 +153,37 @@ fn verify_feature_contracts_match_enum() {
     let mut compiled_paths_from_enum: Vec<String> = FeatureContract::all_feature_contracts()
         .map(|contract| contract.get_compiled_path())
         .collect();
+    #[cfg(feature = "cairo_native")]
     let mut compiled_paths_on_filesystem: Vec<String> = verify_and_get_files(CairoVersion::Cairo0)
         .into_iter()
         .chain(verify_and_get_files(CairoVersion::Cairo1))
+        .chain(verify_and_get_files(CairoVersion::Native))
         .map(|(_, _, compiled_path)| compiled_path)
         .collect();
+    #[cfg(not(feature = "cairo_native"))]
+    let mut compiled_paths_on_filesystem: Vec<String> = verify_and_get_files(CairoVersion::Cairo0)
+        .into_iter()
+        .chain(verify_and_get_files(CairoVersion::Cairo1))
+        .map(|(_, _, compiled_path)| compiled_path)
+        .collect();
+
     compiled_paths_from_enum.sort();
     compiled_paths_on_filesystem.sort();
     assert_eq!(compiled_paths_from_enum, compiled_paths_on_filesystem);
 }
 
-// todo(rdr): find the right way to feature verify native contracts as well
+#[cfg(feature = "cairo_native")]
+#[rstest]
+#[ignore]
+fn verify_feature_contracts(
+    #[values(CairoVersion::Cairo0, CairoVersion::Cairo1, CairoVersion::Native)]
+    cairo_version: CairoVersion,
+) {
+    let fix_features = std::env::var("FIX_FEATURE_TEST").is_ok();
+    verify_feature_contracts_compatibility(fix_features, cairo_version)
+}
+
+#[cfg(not(feature = "cairo_native"))]
 #[rstest]
 #[ignore]
 fn verify_feature_contracts(