From 67c4e9c57f9a611a24b471253602ebd76571bc4e Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Tue, 15 Oct 2024 20:18:33 -0400 Subject: [PATCH] refactor: use ExecutionFailed error instead of NativeExecutionError --- crates/blockifier/src/execution/errors.rs | 2 - .../execution/native/entry_point_execution.rs | 46 ++++++++----------- .../blockifier/src/execution/native/utils.rs | 26 ----------- .../src/execution/native/utils_test.rs | 36 +++++++++------ 4 files changed, 43 insertions(+), 67 deletions(-) diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index 17b5d0a639..d683c85bea 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/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)] diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs index a2fe0eaa31..38e64fffbb 100644 --- a/crates/blockifier/src/execution/native/entry_point_execution.rs +++ b/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 { 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 { - 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 { - // 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, diff --git a/crates/blockifier/src/execution/native/utils.rs b/crates/blockifier/src/execution/native/utils.rs index 2aeb969e9d..87f53f4795 100644 --- a/crates/blockifier/src/execution/native/utils.rs +++ b/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 { } 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) - } - } -} diff --git a/crates/blockifier/src/execution/native/utils_test.rs b/crates/blockifier/src/execution/native/utils_test.rs index bffd39b6ca..108de2901f 100644 --- a/crates/blockifier/src/execution/native/utils_test.rs +++ b/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'))" + ); }