chore: streamline class info constructor

starkware-libs · Oct 7, 2024 · 955c47e · 955c47e
1 parent 5e06e14
commit 955c47e
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 80 deletions.
diff --git a/crates/blockifier/src/execution/contract_class.rs b/crates/blockifier/src/execution/contract_class.rs
@@ -43,7 +43,7 @@ use super::execution_utils::poseidon_hash_many_cost;
 use crate::abi::abi_utils::selector_from_name;
 use crate::abi::constants::{self, CONSTRUCTOR_ENTRY_POINT_NAME};
 use crate::execution::entry_point::CallEntryPoint;
-use crate::execution::errors::{ContractClassError, PreExecutionError};
+use crate::execution::errors::PreExecutionError;
 use crate::execution::execution_utils::sn_api_to_cairo_vm_program;
 use crate::fee::eth_gas_constants;
 use crate::transaction::errors::TransactionExecutionError;
@@ -53,8 +53,6 @@ use crate::versioned_constants::CompilerVersion;
 #[path = "contract_class_test.rs"]
 pub mod test;
 
-pub type ContractClassResult<T> = Result<T, ContractClassError>;
-
 /// The resource used to run a contract function.
 #[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))]
 #[derive(Clone, Copy, Default, Debug, Eq, PartialEq, Serialize)]
@@ -647,23 +645,24 @@ impl ClassInfo {
             + self.abi_length()
     }
 
-    pub fn new(
-        contract_class: &ContractClass,
+    pub fn new_v1(
+        contract_class: ContractClassV1,
         sierra_program_length: usize,
         abi_length: usize,
-    ) -> ContractClassResult<Self> {
-        let (contract_class_version, condition) = match contract_class {
-            ContractClass::V0(_) => (0, sierra_program_length == 0),
-            ContractClass::V1(_) => (1, sierra_program_length > 0),
-        };
+    ) -> Self {
+        assert!(sierra_program_length > 0, "Sierra program length must be > 0 for Cairo1");
+        Self {
+            contract_class: ContractClass::V1(contract_class),
+            sierra_program_length,
+            abi_length,
+        }
+    }
 
-        if condition {
-            Ok(Self { contract_class: contract_class.clone(), sierra_program_length, abi_length })
-        } else {
-            Err(ContractClassError::ContractClassVersionSierraProgramLengthMismatch {
-                contract_class_version,
-                sierra_program_length,
-            })
+    pub fn new_v0(contract_class: ContractClassV0, abi_length: usize) -> Self {
+        Self {
+            contract_class: ContractClass::V0(contract_class),
+            sierra_program_length: 0,
+            abi_length,
         }
     }
 }
diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs
@@ -129,6 +129,8 @@ impl ConstructorEntryPointExecutionError {
     }
 }
 
+// TODO(Arni): remove this error variant. Squash this error into other error types. At least
+// simplify it.
 #[derive(Debug, Error)]
 pub enum ContractClassError {
     #[error(

diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs
@@ -367,11 +367,14 @@ fn create_all_resource_bounds(
 }
 
 pub fn calculate_class_info_for_testing(contract_class: ContractClass) -> ClassInfo {
-    let sierra_program_length = match contract_class {
-        ContractClass::V0(_) => 0,
-        ContractClass::V1(_) => 100,
-    };
-    ClassInfo::new(&contract_class, sierra_program_length, 100).unwrap()
+    let abi_length = 100;
+    match contract_class {
+        ContractClass::V0(contract_class) => ClassInfo::new_v0(contract_class, abi_length),
+        ContractClass::V1(contract_class) => {
+            let sierra_program_length = 100;
+            ClassInfo::new_v1(contract_class, sierra_program_length, abi_length)
+        }
+    }
 }
 
 pub fn emit_n_events_tx(

diff --git a/crates/native_blockifier/src/py_transaction.rs b/crates/native_blockifier/src/py_transaction.rs
@@ -1,11 +1,6 @@
 use std::collections::BTreeMap;
 
-use blockifier::execution::contract_class::{
-    ClassInfo,
-    ContractClass,
-    ContractClassV0,
-    ContractClassV1,
-};
+use blockifier::execution::contract_class::{ClassInfo, ContractClassV0, ContractClassV1};
 use blockifier::transaction::account_transaction::AccountTransaction;
 use blockifier::transaction::transaction_execution::Transaction;
 use blockifier::transaction::transaction_types::TransactionType;
@@ -168,21 +163,27 @@ impl PyClassInfo {
         py_class_info: PyClassInfo,
         tx: &starknet_api::transaction::DeclareTransaction,
     ) -> NativeBlockifierResult<ClassInfo> {
-        let contract_class: ContractClass = match tx {
+        let class_info = match tx {
             starknet_api::transaction::DeclareTransaction::V0(_)
             | starknet_api::transaction::DeclareTransaction::V1(_) => {
-                ContractClassV0::try_from_json_string(&py_class_info.raw_contract_class)?.into()
+                let contract_class =
+                    ContractClassV0::try_from_json_string(&py_class_info.raw_contract_class)?;
+                assert_eq!(py_class_info.sierra_program_length, 0);
+
+                ClassInfo::new_v0(contract_class, py_class_info.abi_length)
             }
             starknet_api::transaction::DeclareTransaction::V2(_)
             | starknet_api::transaction::DeclareTransaction::V3(_) => {
-                ContractClassV1::try_from_json_string(&py_class_info.raw_contract_class)?.into()
+                let contract_class =
+                    ContractClassV1::try_from_json_string(&py_class_info.raw_contract_class)?;
+
+                ClassInfo::new_v1(
+                    contract_class,
+                    py_class_info.sierra_program_length,
+                    py_class_info.abi_length,
+                )
             }
         };
-        let class_info = ClassInfo::new(
-            &contract_class,
-            py_class_info.sierra_program_length,
-            py_class_info.abi_length,
-        )?;
         Ok(class_info)
     }
 }
diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs
@@ -28,7 +28,7 @@ use blockifier::blockifier::block::{pre_process_block, BlockInfo, BlockNumberHas
 use blockifier::bouncer::BouncerConfig;
 use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionContext};
 use blockifier::execution::call_info::CallExecution;
-use blockifier::execution::contract_class::{ClassInfo, ContractClass as BlockifierContractClass};
+use blockifier::execution::contract_class::ClassInfo;
 use blockifier::execution::entry_point::{
     CallEntryPoint,
     CallType as BlockifierCallType,
@@ -152,12 +152,6 @@ impl SerializeConfig for ExecutionConfig {
 /// The error type for the execution module.
 #[derive(thiserror::Error, Debug)]
 pub enum ExecutionError {
-    #[error("Bad declare tx: {tx:?}. error: {err:?}")]
-    BadDeclareTransaction {
-        tx: DeclareTransaction,
-        #[source]
-        err: blockifier::execution::errors::ContractClassError,
-    },
     #[error("Execution config file does not contain a configuration for all blocks")]
     ConfigContentError,
     #[error(transparent)]
@@ -391,8 +385,6 @@ pub type AbiSize = usize;
 /// The size of the sierra program.
 pub type SierraSize = usize;
 
-const DEPRECATED_CONTRACT_SIERRA_SIZE: SierraSize = 0;
-
 /// The transaction input to be executed.
 // TODO(yair): This should use broadcasted transactions instead of regular transactions, but the
 // blockifier expects regular transactions. Consider changing the blockifier to use broadcasted txs.
@@ -745,19 +737,15 @@ fn to_blockifier_tx(
             abi_length,
             only_query,
         ) => {
-            let class_v0 = BlockifierContractClass::V0(deprecated_class.try_into().map_err(
+            let class_v0 = deprecated_class.try_into().map_err(
                 |e: cairo_vm::types::errors::program_errors::ProgramError| {
                     ExecutionError::TransactionExecutionError {
                         transaction_index,
                         execution_error: e.to_string(),
                     }
                 },
-            )?);
-            let class_info = ClassInfo::new(&class_v0, DEPRECATED_CONTRACT_SIERRA_SIZE, abi_length)
-                .map_err(|err| ExecutionError::BadDeclareTransaction {
-                    tx: DeclareTransaction::V0(declare_tx.clone()),
-                    err,
-                })?;
+            )?;
+            let class_info = ClassInfo::new_v0(class_v0, abi_length);
             BlockifierTransaction::from_api(
                 Transaction::Declare(DeclareTransaction::V0(declare_tx)),
                 tx_hash,
@@ -774,14 +762,8 @@ fn to_blockifier_tx(
             abi_length,
             only_query,
         ) => {
-            let class_v0 = BlockifierContractClass::V0(
-                deprecated_class.try_into().map_err(BlockifierError::new)?,
-            );
-            let class_info = ClassInfo::new(&class_v0, DEPRECATED_CONTRACT_SIERRA_SIZE, abi_length)
-                .map_err(|err| ExecutionError::BadDeclareTransaction {
-                    tx: DeclareTransaction::V1(declare_tx.clone()),
-                    err,
-                })?;
+            let class_v0 = deprecated_class.try_into().map_err(BlockifierError::new)?;
+            let class_info = ClassInfo::new_v0(class_v0, abi_length);
             BlockifierTransaction::from_api(
                 Transaction::Declare(DeclareTransaction::V1(declare_tx)),
                 tx_hash,
@@ -799,16 +781,8 @@ fn to_blockifier_tx(
             abi_length,
             only_query,
         ) => {
-            let class_v1 = BlockifierContractClass::V1(
-                compiled_class.try_into().map_err(BlockifierError::new)?,
-            );
-            let class_info =
-                ClassInfo::new(&class_v1, sierra_program_length, abi_length).map_err(|err| {
-                    ExecutionError::BadDeclareTransaction {
-                        tx: DeclareTransaction::V2(declare_tx.clone()),
-                        err,
-                    }
-                })?;
+            let class_v1 = compiled_class.try_into().map_err(BlockifierError::new)?;
+            let class_info = ClassInfo::new_v1(class_v1, sierra_program_length, abi_length);
             BlockifierTransaction::from_api(
                 Transaction::Declare(DeclareTransaction::V2(declare_tx)),
                 tx_hash,
@@ -826,16 +800,8 @@ fn to_blockifier_tx(
             abi_length,
             only_query,
         ) => {
-            let class_v1 = BlockifierContractClass::V1(
-                compiled_class.try_into().map_err(BlockifierError::new)?,
-            );
-            let class_info =
-                ClassInfo::new(&class_v1, sierra_program_length, abi_length).map_err(|err| {
-                    ExecutionError::BadDeclareTransaction {
-                        tx: DeclareTransaction::V3(declare_tx.clone()),
-                        err,
-                    }
-                })?;
+            let class_v1 = compiled_class.try_into().map_err(BlockifierError::new)?;
+            let class_info = ClassInfo::new_v1(class_v1, sierra_program_length, abi_length);
             BlockifierTransaction::from_api(
                 Transaction::Declare(DeclareTransaction::V3(declare_tx)),
                 tx_hash,