From 0d773ce82a43eae60a85e3af30998159aa4b9dd9 Mon Sep 17 00:00:00 2001 From: Arnon Hod Date: Tue, 27 Aug 2024 10:24:54 +0300 Subject: [PATCH] chore: streamline the error types in starknet_sierra_compile crate (#566) --- crates/gateway/src/compilation.rs | 5 ++--- crates/gateway/src/compilation_test.rs | 12 +++------- crates/starknet_sierra_compile/Cargo.toml | 6 ++--- .../src/cairo_lang_compiler.rs | 12 +++++----- .../src/compile_test.rs | 10 ++++----- crates/starknet_sierra_compile/src/errors.rs | 22 ++++++++++++++----- crates/starknet_sierra_compile/src/lib.rs | 4 ++++ 7 files changed, 38 insertions(+), 33 deletions(-) diff --git a/crates/gateway/src/compilation.rs b/crates/gateway/src/compilation.rs index 950ba41dca..aab1affc40 100644 --- a/crates/gateway/src/compilation.rs +++ b/crates/gateway/src/compilation.rs @@ -56,9 +56,8 @@ impl GatewayCompiler { ) -> GatewayResult { match self.sierra_to_casm_compiler.compile(cairo_lang_contract_class) { Ok(casm_contract_class) => Ok(casm_contract_class), - Err(starknet_sierra_compile::errors::CompilationUtilError::CompilationPanic) => { - // TODO(Arni): Log the panic. - error!("Compilation panicked."); + Err(starknet_sierra_compile::errors::CompilationUtilError::UnexpectedError(error)) => { + error!("Compilation panicked. Error: {:?}", error); Err(GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() }) } Err(e) => { diff --git a/crates/gateway/src/compilation_test.rs b/crates/gateway/src/compilation_test.rs index b1efcef23a..8c41913c12 100644 --- a/crates/gateway/src/compilation_test.rs +++ b/crates/gateway/src/compilation_test.rs @@ -1,7 +1,4 @@ use assert_matches::assert_matches; -use cairo_lang_sierra_to_casm::compiler::CompilationError; -use cairo_lang_starknet_classes::allowed_libfuncs::AllowedLibfuncsError; -use cairo_lang_starknet_classes::casm_contract_class::StarknetSierraCompilationError; use mempool_test_utils::starknet_api_test_utils::{ compiled_class_hash as test_contract_compiled_class_hash, declare_tx as rpc_declare_tx, @@ -67,11 +64,8 @@ fn test_compile_contract_class_bytecode_size_validation(declare_tx_v3: RpcDeclar let result = gateway_compiler.process_declare_tx(&RpcDeclareTransaction::V3(declare_tx_v3)); assert_matches!(result.unwrap_err(), GatewaySpecError::CompilationFailed); - let expected_compilation_error = CompilationUtilError::StarknetSierraCompilationError( - StarknetSierraCompilationError::CompilationError(Box::new( - CompilationError::CodeSizeLimitExceeded, - )), - ); + let expected_compilation_error = + CompilationUtilError::CompilationError("Code size limit exceeded.".to_owned()); assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str())); } @@ -90,7 +84,7 @@ fn test_compile_contract_class_bad_sierra( assert_eq!(err, GatewaySpecError::CompilationFailed); let expected_compilation_error = - CompilationUtilError::AllowedLibfuncsError(AllowedLibfuncsError::SierraProgramError); + CompilationUtilError::CompilationError("Invalid Sierra program.".to_owned()); assert!(logs_contain(format!("Compilation failed: {:?}", expected_compilation_error).as_str())); } diff --git a/crates/starknet_sierra_compile/Cargo.toml b/crates/starknet_sierra_compile/Cargo.toml index 6ff8e79d46..1bcda82fe5 100644 --- a/crates/starknet_sierra_compile/Cargo.toml +++ b/crates/starknet_sierra_compile/Cargo.toml @@ -1,9 +1,9 @@ [package] -name = "starknet_sierra_compile" -version = "0.0.0" edition.workspace = true -repository.workspace = true license.workspace = true +name = "starknet_sierra_compile" +repository.workspace = true +version = "0.0.0" [lints] workspace = true diff --git a/crates/starknet_sierra_compile/src/cairo_lang_compiler.rs b/crates/starknet_sierra_compile/src/cairo_lang_compiler.rs index 7565b29afa..b78e1bbc78 100644 --- a/crates/starknet_sierra_compile/src/cairo_lang_compiler.rs +++ b/crates/starknet_sierra_compile/src/cairo_lang_compiler.rs @@ -8,10 +8,6 @@ use crate::config::SierraToCasmCompilationConfig; use crate::errors::CompilationUtilError; use crate::SierraToCasmCompiler; -#[cfg(test)] -#[path = "compile_test.rs"] -pub mod compile_test; - /// A compiler that compiles Sierra programs to Casm. Uses the code from the /// `cairo_lang_starknet_classes` crate. #[derive(Clone)] @@ -25,8 +21,12 @@ impl SierraToCasmCompiler for CairoLangSierraToCasmCompiler { contract_class: ContractClass, ) -> Result { let catch_unwind_result = panic::catch_unwind(|| self.compile_inner(contract_class)); - let casm_contract_class = - catch_unwind_result.map_err(|_| CompilationUtilError::CompilationPanic)??; + let casm_contract_class = catch_unwind_result.map_err(|error| { + CompilationUtilError::UnexpectedError(format!( + "Compilation Paniced: Error: {:?}", + error + )) + })??; Ok(casm_contract_class) } diff --git a/crates/starknet_sierra_compile/src/compile_test.rs b/crates/starknet_sierra_compile/src/compile_test.rs index badb69f8e9..547b92c497 100644 --- a/crates/starknet_sierra_compile/src/compile_test.rs +++ b/crates/starknet_sierra_compile/src/compile_test.rs @@ -2,12 +2,12 @@ use std::env; use std::path::Path; use assert_matches::assert_matches; -use cairo_lang_starknet_classes::allowed_libfuncs::AllowedLibfuncsError; use mempool_test_utils::{get_absolute_path, FAULTY_ACCOUNT_CLASS_FILE, TEST_FILES_FOLDER}; use rstest::{fixture, rstest}; -use crate::cairo_lang_compiler::{CairoLangSierraToCasmCompiler, CompilationUtilError}; +use crate::cairo_lang_compiler::CairoLangSierraToCasmCompiler; use crate::config::SierraToCasmCompilationConfig; +use crate::errors::CompilationUtilError; use crate::test_utils::contract_class_from_file; use crate::SierraToCasmCompiler; @@ -16,6 +16,7 @@ fn compiler() -> impl SierraToCasmCompiler { CairoLangSierraToCasmCompiler { config: SierraToCasmCompilationConfig::default() } } +// TODO: use the other compiler as well. #[rstest] fn test_compile_sierra_to_casm(compiler: impl SierraToCasmCompiler) { env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER)).expect("Failed to set current dir."); @@ -40,8 +41,5 @@ fn test_negative_flow_compile_sierra_to_casm(compiler: impl SierraToCasmCompiler contract_class.sierra_program = contract_class.sierra_program[..100].to_vec(); let result = compiler.compile(contract_class); - assert_matches!( - result, - Err(CompilationUtilError::AllowedLibfuncsError(AllowedLibfuncsError::SierraProgramError)) - ); + assert_matches!(result, Err(CompilationUtilError::CompilationError(..))); } diff --git a/crates/starknet_sierra_compile/src/errors.rs b/crates/starknet_sierra_compile/src/errors.rs index 2d3d2575fa..c1f25719ec 100644 --- a/crates/starknet_sierra_compile/src/errors.rs +++ b/crates/starknet_sierra_compile/src/errors.rs @@ -4,10 +4,20 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum CompilationUtilError { - #[error(transparent)] - AllowedLibfuncsError(#[from] AllowedLibfuncsError), - #[error(transparent)] - StarknetSierraCompilationError(#[from] StarknetSierraCompilationError), - #[error("Compilation panicked")] - CompilationPanic, + #[error("Starknet Sierra compilation error: {0}")] + CompilationError(String), + #[error("Unexpected compilation error: {0}")] + UnexpectedError(String), +} + +impl From for CompilationUtilError { + fn from(error: AllowedLibfuncsError) -> Self { + CompilationUtilError::CompilationError(error.to_string()) + } +} + +impl From for CompilationUtilError { + fn from(error: StarknetSierraCompilationError) -> Self { + CompilationUtilError::CompilationError(error.to_string()) + } } diff --git a/crates/starknet_sierra_compile/src/lib.rs b/crates/starknet_sierra_compile/src/lib.rs index 449a69ce5e..e68efcc505 100644 --- a/crates/starknet_sierra_compile/src/lib.rs +++ b/crates/starknet_sierra_compile/src/lib.rs @@ -12,6 +12,10 @@ pub mod utils; #[cfg(test)] pub mod test_utils; +#[cfg(test)] +#[path = "compile_test.rs"] +pub mod compile_test; + pub trait SierraToCasmCompiler: Send + Sync { fn compile( &self,