chore: move compilation logic to FeatureContracts enum

Signed-off-by: Dori Medini <[email protected]>

crates/blockifier/src/test_utils.rs

@@ -49,7 +49,7 @@ pub const TEST_ERC20_CONTRACT_CLASS_HASH: &str = "0x1010";
 pub const ERC20_CONTRACT_PATH: &str =
     "./ERC20_without_some_syscalls/ERC20/erc20_contract_without_some_syscalls_compiled.json";
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum CairoVersion {
     Cairo0,
     Cairo1,

crates/blockifier/src/test_utils/contracts.rs

@@ -1,3 +1,5 @@
+use std::process::Command;
+
 use starknet_api::core::{ClassHash, ContractAddress, PatriciaKey};
 use starknet_api::deprecated_contract_class::ContractClass as DeprecatedContractClass;
 use starknet_api::hash::StarkHash;
@@ -51,8 +53,10 @@ const SECURITY_TEST_CONTRACT_NAME: &str = "security_tests_contract";
 const TEST_CONTRACT_NAME: &str = "test_contract";
 
 // ERC20 contract is in a unique location.
+const ERC20_BASE_NAME: &str = "ERC20";
 const ERC20_CONTRACT_PATH: &str =
     "./ERC20_without_some_syscalls/ERC20/erc20_contract_without_some_syscalls_compiled.json";
+const ERC20_CONTRACT_SOURCE_PATH: &str = "./ERC20_without_some_syscalls/ERC20/ERC20.cairo";
 
 /// Enum representing all feature contracts.
 /// The contracts that are implemented in both Cairo versions include a version field.
@@ -69,7 +73,7 @@ pub enum FeatureContract {
 }
 
 impl FeatureContract {
-    fn cairo_version(&self) -> CairoVersion {
+    pub fn cairo_version(&self) -> CairoVersion {
         match self {
             Self::AccountWithLongValidate(version)
             | Self::AccountWithoutValidations(version)
@@ -114,18 +118,40 @@ impl FeatureContract {
             }
     }
 
-    pub fn get_compiled_path(&self) -> String {
-        let cairo_version = self.cairo_version();
-        let contract_name = match self {
+    fn contract_base_name(&self) -> &str {
+        match self {
             Self::AccountWithLongValidate(_) => ACCOUNT_LONG_VALIDATE_NAME,
             Self::AccountWithoutValidations(_) => ACCOUNT_WITHOUT_VALIDATIONS_NAME,
             Self::Empty(_) => EMPTY_CONTRACT_NAME,
             Self::FaultyAccount(_) => FAULTY_ACCOUNT_NAME,
             Self::LegacyTestContract => LEGACY_CONTRACT_NAME,
             Self::SecurityTests => SECURITY_TEST_CONTRACT_NAME,
             Self::TestContract(_) => TEST_CONTRACT_NAME,
+            Self::ERC20 => ERC20_BASE_NAME,
+        }
+    }
+
+    pub fn get_source_path(&self) -> String {
+        match self {
+            // Special case: ERC20 contract in a different location.
+            Self::ERC20 => ERC20_CONTRACT_SOURCE_PATH.into(),
+            _not_erc20 => format!(
+                "feature_contracts/cairo{}/{}.cairo",
+                match self.cairo_version() {
+                    CairoVersion::Cairo0 => "0",
+                    CairoVersion::Cairo1 => "1",
+                },
+                self.contract_base_name()
+            ),
+        }
+    }
+
+    pub fn get_compiled_path(&self) -> String {
+        let cairo_version = self.cairo_version();
+        let contract_name = match self {
             // ERC20 is a special case - not in the feature_contracts directory.
             Self::ERC20 => return ERC20_CONTRACT_PATH.into(),
+            _not_erc20 => self.contract_base_name(),
         };
         format!(
             "feature_contracts/cairo{}/compiled/{}{}.json",
@@ -141,6 +167,36 @@ 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<u8> {
+        if matches!(self, Self::ERC20) {
+            panic!("ERC20 contract recompilation not supported.");
+        }
+        match self.cairo_version() {
+            CairoVersion::Cairo0 => {
+                let mut command = Command::new("starknet-compile-deprecated");
+                match self {
+                    // Account contracts require the account_contract flag.
+                    Self::AccountWithLongValidate(_)
+                    | Self::AccountWithoutValidations(_)
+                    | Self::FaultyAccount(_) => command.arg("--account_contract"),
+                    Self::SecurityTests => command.arg("--disable_hint_validation"),
+                    Self::Empty(_) | Self::TestContract(_) | Self::LegacyTestContract => {
+                        &mut command
+                    }
+                    Self::ERC20 => unreachable!(),
+                };
+                command.args([&self.get_source_path(), "--no_debug_info"]);
+                let compile_output = command.output().unwrap();
+                let stderr_output = String::from_utf8(compile_output.stderr).unwrap();
+                assert!(compile_output.status.success(), "{stderr_output}");
+                compile_output.stdout
+            }
+            CairoVersion::Cairo1 => todo!(),
+        }
+    }
+
     pub fn set_cairo_version(&mut self, version: CairoVersion) {
         match self {
             Self::AccountWithLongValidate(v)
@@ -203,4 +259,9 @@ impl FeatureContract {
             }
         })
     }
+
+    pub fn all_feature_contracts() -> impl Iterator<Item = Self> {
+        // ERC20 is a special case - not in the feature_contracts directory.
+        Self::all_contracts().filter(|contract| !matches!(contract, Self::ERC20))
+    }
 }

crates/blockifier/tests/feature_contracts_compatibility_test.rs

@@ -1,5 +1,4 @@
 use std::fs;
-use std::process::Command;
 
 use blockifier::test_utils::contracts::FeatureContract;
 use blockifier::test_utils::CairoVersion;
@@ -41,20 +40,12 @@ const FIX_COMMAND: &str = "FIX_FEATURE_TEST=1 cargo test -- --ignored";
 // 2. for each `X.cairo` file in `TEST_CONTRACTS` there exists an `X_compiled.json` file in
 // `COMPILED_CONTRACTS_SUBDIR` which equals `starknet-compile-deprecated X.cairo --no_debug_info`.
 fn verify_feature_contracts_compatibility(fix: bool, cairo_version: CairoVersion) {
-    for (path_str, file_name, existing_compiled_path) in verify_and_get_files(cairo_version) {
+    for contract in FeatureContract::all_feature_contracts()
+        .filter(|contract| contract.cairo_version() == cairo_version)
+    {
         // Compare output of cairo-file on file with existing compiled file.
-        let mut command = Command::new("starknet-compile-deprecated");
-        command.args([&path_str, "--no_debug_info"]);
-        if file_name.starts_with("account") {
-            command.arg("--account_contract");
-        }
-        if file_name.starts_with("security") {
-            command.arg("--disable_hint_validation");
-        }
-        let compile_output = command.output().unwrap();
-        let stderr_output = String::from_utf8(compile_output.stderr).unwrap();
-        assert!(compile_output.status.success(), "{stderr_output}");
-        let expected_compiled_output = compile_output.stdout;
+        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();
@@ -64,9 +55,9 @@ fn verify_feature_contracts_compatibility(fix: bool, cairo_version: CairoVersion
 
         if String::from_utf8(expected_compiled_output).unwrap() != existing_compiled_contents {
             panic!(
-                "{path_str} does not compile to {existing_compiled_path}.\nRun `{FIX_COMMAND}` to \
-                 fix the expected test according to locally installed \
-                 `starknet-compile-deprecated`.\n"
+                "{} does not compile to {existing_compiled_path}.\nRun `{FIX_COMMAND}` to fix the \
+                 expected test according to locally installed `starknet-compile-deprecated`.\n",
+                contract.get_source_path()
             );
         }
     }
@@ -119,9 +110,7 @@ fn verify_and_get_files(cairo_version: CairoVersion) -> Vec<(String, String, Str
 
 #[test]
 fn verify_feature_contracts_match_enum() {
-    let mut compiled_paths_from_enum: Vec<String> = FeatureContract::all_contracts()
-        // ERC20 is a special case - not in the feature_contracts directory.
-        .filter(|contract| !matches!(contract, FeatureContract::ERC20))
+    let mut compiled_paths_from_enum: Vec<String> = FeatureContract::all_feature_contracts()
         .map(|contract| contract.get_compiled_path())
         .collect();
     let mut compiled_paths_on_filesystem: Vec<String> = verify_and_get_files(CairoVersion::Cairo0)