refactor: use ExecutionFailed error instead of NativeExecutionError

crates/blockifier/src/execution/errors.rs

@@ -88,8 +88,6 @@ pub enum EntryPointExecutionError {
     InternalError(String),
     #[error("Invalid input: {input_descriptor}; {info}")]
     InvalidExecutionInput { input_descriptor: String, info: String },
-    #[error("Native execution error: {info}")]
-    NativeExecutionError { info: String },
     #[error(transparent)]
     NativeUnexpectedError(#[from] NativeError),
     #[error(transparent)]

crates/blockifier/src/execution/native/entry_point_execution.rs

@@ -1,6 +1,4 @@
-use cairo_lang_sierra::ids::FunctionId;
 use cairo_native::execution_result::ContractExecutionResult;
-use cairo_native::executor::AotNativeExecutor;
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
 use num_traits::ToPrimitive;
 
@@ -13,7 +11,6 @@ use crate::execution::entry_point::{
 };
 use crate::execution::errors::{EntryPointExecutionError, PostExecutionError};
 use crate::execution::native::syscall_handler::NativeSyscallHandler;
-use crate::execution::native::utils::decode_felts_as_str;
 use crate::state::state_api::State;
 
 pub fn execute_entry_point_call(
@@ -25,7 +22,7 @@ pub fn execute_entry_point_call(
 ) -> EntryPointExecutionResult<CallInfo> {
     let function_id = contract_class.get_entry_point(&call)?;
 
-    let syscall_handler: NativeSyscallHandler<'_> = NativeSyscallHandler::new(
+    let mut syscall_handler: NativeSyscallHandler<'_> = NativeSyscallHandler::new(
         state,
         call.caller_address,
         call.storage_address,
@@ -34,16 +31,7 @@ pub fn execute_entry_point_call(
         context,
     );
 
-    run_native_executor(&contract_class.executor, function_id, call, syscall_handler)
-}
-
-fn run_native_executor(
-    native_executor: &AotNativeExecutor,
-    function_id: FunctionId,
-    call: CallEntryPoint,
-    mut syscall_handler: NativeSyscallHandler<'_>,
-) -> EntryPointExecutionResult<CallInfo> {
-    let execution_result = native_executor.invoke_contract_dynamic(
+    let execution_result = contract_class.executor.invoke_contract_dynamic(
         &function_id,
         &call.calldata.0,
         Some(call.initial_gas.into()),
@@ -52,13 +40,12 @@ fn run_native_executor(
 
     let call_result = match execution_result {
         Err(runner_err) => Err(EntryPointExecutionError::NativeUnexpectedError(runner_err)),
-        Ok(res) if res.failure_flag => Err(EntryPointExecutionError::NativeExecutionError {
-            info: if !res.return_values.is_empty() {
-                decode_felts_as_str(&res.return_values)
-            } else {
-                String::from("Unknown error")
-            },
-        }),
+        Ok(res)
+            if res.failure_flag
+                && !syscall_handler.context.versioned_constants().enable_reverts =>
+        {
+            Err(EntryPointExecutionError::ExecutionFailed { error_data: res.return_values })
+        }
         Ok(res) => Ok(res),
     }?;
 
@@ -70,17 +57,24 @@ fn create_callinfo(
     call_result: ContractExecutionResult,
     syscall_handler: NativeSyscallHandler<'_>,
 ) -> Result<CallInfo, EntryPointExecutionError> {
-    // todo(rodro): Even if the property is called `remaining_gas` it behaves like gas used.
-    // Update once gas works on Native side has been completed (or at least this part)
-    let gas_used =
+    let remaining_gas =
         call_result.remaining_gas.to_u64().ok_or(PostExecutionError::MalformedReturnData {
             error_message: format!(
-                "Unexpected remaining gas (bigger than u64): {}",
+                "Unexpected remaining gas. Gas value is bigger than u64: {}",
                 call_result.remaining_gas
             ),
         })?;
+    if remaining_gas > call.initial_gas {
+        return Err(PostExecutionError::MalformedReturnData {
+            error_message: format!(
+                "Unexpected remaining gas. Used gas is greater than initial gas: {} > {}",
+                remaining_gas, call.initial_gas
+            ),
+        }
+        .into());
+    }
 
-    let gas_consumed = call.initial_gas - gas_used;
+    let gas_consumed = call.initial_gas - remaining_gas;
 
     Ok(CallInfo {
         call,

crates/blockifier/src/execution/native/utils.rs

@@ -1,5 +1,4 @@
 use cairo_lang_starknet_classes::contract_class::ContractEntryPoint;
-use itertools::Itertools;
 use starknet_api::core::EntryPointSelector;
 use starknet_types_core::felt::Felt;
 
@@ -21,28 +20,3 @@ pub fn encode_str_as_felts(msg: &str) -> Vec<Felt> {
     }
     encoding
 }
-
-// Todo(rodrigo): This is an opinionated way of interpretting error messages. It's ok for now but I
-// think it can be improved; (for example) trying to make the output similar to a Cairo VM panic
-pub fn decode_felts_as_str(encoding: &[Felt]) -> String {
-    let bytes_err: Vec<_> =
-        encoding.iter().flat_map(|felt| felt.to_bytes_be()[1..32].to_vec()).collect();
-
-    match String::from_utf8(bytes_err) {
-        // If the string is utf8 make sure it is not prefixed by no null chars. Null chars in
-        // between can still happen
-        Ok(s) => s.trim_matches('\0').to_owned(),
-        // If the string is non-utf8 overall, try to decode them as utf8 chunks of it and keep the
-        // original bytes for the non-utf8 chunks
-        Err(_) => {
-            let err_msgs = encoding
-                .iter()
-                .map(|felt| match String::from_utf8(felt.to_bytes_be()[1..32].to_vec()) {
-                    Ok(s) => format!("{} ({})", s.trim_matches('\0'), felt),
-                    Err(_) => felt.to_string(),
-                })
-                .join(", ");
-            format!("[{}]", err_msgs)
-        }
-    }
-}

crates/blockifier/src/execution/native/utils_test.rs

@@ -4,9 +4,9 @@ use pretty_assertions::assert_eq;
 use starknet_api::core::EntryPointSelector;
 use starknet_types_core::felt::Felt;
 
+use crate::execution::execution_utils::format_panic_data;
 use crate::execution::native::utils::{
     contract_entrypoint_to_entrypoint_selector,
-    decode_felts_as_str,
     encode_str_as_felts,
 };
 
@@ -22,23 +22,33 @@ fn test_contract_entrypoint_to_entrypoint_selector() {
 }
 
 #[test]
-fn test_encode_decode_str() {
-    const STR: &str = "normal utf8 string:";
+fn test_encode_small_str() {
+    const STR: &str = "I fit in a felt :)";
 
     let encoded_felt_array = encode_str_as_felts(STR);
 
-    let decoded_felt_array = decode_felts_as_str(encoded_felt_array.as_slice());
+    let decoded_felt_array = format_panic_data(&encoded_felt_array);
 
-    assert_eq!(&decoded_felt_array, STR);
+    assert_eq!(
+        &decoded_felt_array,
+        "0x492066697420696e20612066656c74203a2900000000000000000000000000 ('I fit in a felt :)')"
+    );
 }
 
 #[test]
-fn test_decode_non_utf8_str() {
-    let v1 = Felt::from_dec_str("1234").unwrap();
-    let v2_msg = "i am utf8";
-    let v2 = Felt::from_bytes_be_slice(v2_msg.as_bytes());
-    let v3 = Felt::from_dec_str("13299428").unwrap();
-    let felts = [v1, v2, v3];
-
-    assert_eq!(decode_felts_as_str(&felts), format!("[{}, {} ({}), {}]", v1, v2_msg, v2, v3))
+fn test_encode_large_str() {
+    const STR: &str =
+        "Three sad tigers ate wheat. Two tigers were full. The other tiger not so much";
+
+    let encoded_felt_array = encode_str_as_felts(STR);
+
+    let decoded_felt_array = format_panic_data(&encoded_felt_array);
+
+    assert_eq!(
+        &decoded_felt_array,
+        "(0x54687265652073616420746967657273206174652077686561742e2054776f ('Three sad tigers ate \
+         wheat. Two'), 0x2074696765727320776572652066756c6c2e20546865206f74686572207469 (' tigers \
+         were full. The other ti'), \
+         0x676572206e6f7420736f206d75636800000000000000000000000000000000 ('ger not so much'))"
+    );
 }