diff --git a/src/kernel_evm/evm_execution/src/account_storage.rs b/src/kernel_evm/evm_execution/src/account_storage.rs index c0c90d605bea6d470cbfaaa479d3d9ca8e1dc253..003364ee1370e1b67380709da61af8e17edd3fc3 100644 --- a/src/kernel_evm/evm_execution/src/account_storage.rs +++ b/src/kernel_evm/evm_execution/src/account_storage.rs @@ -18,11 +18,8 @@ pub const WORD_SIZE: usize = 32_usize; /// All errors that may happen as result of using the Ethereum account /// interface. -#[derive(Error, Copy, Eq, PartialEq, Clone, Debug)] +#[derive(Error, Eq, PartialEq, Clone, Debug)] pub enum AccountStorageError { - /// The account does not hold enough Ether for a transaction - #[error("Insufficient Ether")] - NotEnoughEther, /// Some runtime error happened while using durable storage #[error("Runtime error: {0:?}")] RuntimeError(host::runtime::RuntimeError), @@ -39,9 +36,11 @@ pub enum AccountStorageError { #[error("Account balance overflow")] BalanceOverflow, /// Some account balance became less than zero. Only positive - /// integers in 256 bit range can be stored. + /// integers in 256 bit range can be stored. Error parameter is + /// the string representation of the path to the account where + /// underflow happened. #[error("Account balance underflow")] - BalanceUnderflow, + BalanceUnderflow(String), /// Technically, the Ethereum account nonce can overflow if /// an account does an incredible number of transactions. #[error("Nonce overflow")] @@ -289,7 +288,7 @@ impl EthereumAccount { host.store_write(&path, &new_value_bytes, 0) .map_err(AccountStorageError::from) } else { - Err(AccountStorageError::BalanceUnderflow) + Err(AccountStorageError::BalanceUnderflow(self.path.to_string())) } } @@ -668,7 +667,7 @@ mod test { .expect("Could not add first value to balance"); assert_eq!( a1.balance_remove(&mut host, v2).unwrap_err(), - AccountStorageError::BalanceUnderflow + AccountStorageError::BalanceUnderflow("/eth_accounts.2/dfkjd".to_string()) ); storage diff --git a/src/kernel_evm/evm_execution/src/handler.rs b/src/kernel_evm/evm_execution/src/handler.rs index 5e533d107346b060aa1a6977cbbcb20aff32ea9b..3da3b260f4bfd3a24d741f410071fd591bcfea10 100644 --- a/src/kernel_evm/evm_execution/src/handler.rs +++ b/src/kernel_evm/evm_execution/src/handler.rs @@ -82,17 +82,14 @@ type CallOutcome = (ExitReason, Vec); /// address will be `None`. type CreateOutcome = (ExitReason, Option, Vec); +/// Wrap ethereum errors in the SputnikVM errors +/// +/// This function wraps critical errors that indicate something is wrong +/// with the kernel or rollup node into errors that can be passed on to +/// SputnikVM execution. This is needed if an error occurs in a callback +/// called by SputnikVM. fn ethereum_error_to_exit_reason(exit_reason: EthereumError) -> ExitReason { - match exit_reason { - EthereumError::MachineExitError(err) => { - ExitReason::Fatal(ExitFatal::CallErrorAsFatal(err)) - } - EthereumError::FatalMachineError(err) => ExitReason::Fatal(err), - EthereumError::CallRevert => ExitReason::Revert(ExitRevert::Reverted), - _ => ExitReason::Fatal(ExitFatal::Other(Cow::from( - "Internal error in EVM interpreter", - ))), - } + ExitReason::Fatal(ExitFatal::Other(Cow::from(format!("{:?}", exit_reason)))) } /// Data related to the current transaction layer @@ -187,18 +184,7 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { runtime: &mut evm::Runtime, ) -> Result { match runtime.run(self) { - Capture::Exit(ExitReason::Succeed(exit_succeed)) => { - Ok(ExitReason::Succeed(exit_succeed)) - } - Capture::Exit(ExitReason::Revert(exit_revert)) => { - Ok(ExitReason::Revert(exit_revert)) - } - Capture::Exit(ExitReason::Error(error)) => { - Err(EthereumError::MachineExitError(error)) - } - Capture::Exit(ExitReason::Fatal(error)) => { - Err(EthereumError::FatalMachineError(error)) - } + Capture::Exit(reason) => Ok(reason), Capture::Trap(_) => Err(EthereumError::InternalTrapError), } } @@ -229,13 +215,21 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { } /// Execute a transfer between two accounts + /// + /// In case the transfer succeeds, the function returns + /// `Ok(ExitReason::Succeed(ExitSucceed::Returned))`. In case the + /// transaction fails, but execution doesn't encounter non-contract or + /// -account errors, it returns `Ok(ExitReason::Error(err))`, where `err` + /// indicates what went wrong (insufficient balance, etc.). In case of + /// critical errors in the rollup node or kernel, an `Err(err)` is returned, + /// where `err` indicates what went wrong, eg, a storage error. fn execute_transfer( &mut self, from: H160, to: H160, value: U256, gas_limit: Option, - ) -> Result<(), EthereumError> { + ) -> Result { debug_msg!(self.host, "Executing a transfer"); // TODO let transfers cost gas @@ -243,21 +237,36 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { if value == U256::zero() { // Nothing to transfer so succeeds by default - Ok(()) + Ok(ExitReason::Succeed(ExitSucceed::Returned)) } else if let Some(mut from_account) = self.get_account(from) { let mut to_account = self.get_or_create_account(to)?; - from_account.balance_remove(self.host, value)?; - to_account - .balance_add(self.host, value) - .map_err(EthereumError::from) + match from_account.balance_remove(self.host, value) { + Ok(_) => { + to_account + .balance_add(self.host, value) + .map_err(EthereumError::from)?; + Ok(ExitReason::Succeed(ExitSucceed::Returned)) + } + Err(AccountStorageError::BalanceUnderflow(_)) => { + debug_msg!( + self.host, + "Transaction failture - balance underflow on account {:?} - withdraw {:?}", + from_account, + value + ); + + Ok(ExitReason::Fatal(ExitFatal::CallErrorAsFatal( + ExitError::OutOfFund, + ))) + } + Err(err) => Err(EthereumError::from(err)), + } } else { debug_msg!(self.host, "'from' account {:?} is empty", from); // Accounts of zero balance by default, so this must be // an underflow. - Err(EthereumError::EthereumAccountError( - AccountStorageError::BalanceUnderflow, - )) + Ok(ExitReason::Error(ExitError::OutOfFund)) } } @@ -282,7 +291,11 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { // issue: https://gitlab.com/tezos/tezos/-/issues/4866 if self.evm_account_storage.stack_depth() >= MAXIMUM_TRANSACTION_DEPTH { - return Err(EthereumError::MachineExitError(ExitError::CallTooDeep)); + return Ok(( + ExitReason::Fatal(ExitFatal::CallErrorAsFatal(ExitError::CallTooDeep)), + None, + vec![], + )); } let context = Context { @@ -359,7 +372,12 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { if self.evm_account_storage.stack_depth() > MAXIMUM_TRANSACTION_DEPTH { debug_msg!(self.host, "Execution beyond the call limit of 1024"); - return Err(EthereumError::MachineExitError(ExitError::CallTooDeep)); + + return Ok(( + ExitReason::Fatal(ExitFatal::CallErrorAsFatal(ExitError::CallTooDeep)), + None, + vec![], + )); } // TODO: check gas @@ -372,12 +390,29 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { // issue: https://gitlab.com/tezos/tezos/-/issues/4866 if let Some(transfer) = transfer { - self.execute_transfer( + match self.execute_transfer( transaction_context.context.caller, address, transfer.value, gas_limit, - )?; + )? { + r @ ExitReason::Fatal(_) => { + return Ok((r, None, vec![])); + } + r @ ExitReason::Error(_) => { + return Ok((r, None, vec![])); + } + r @ ExitReason::Revert(_) => { + // A transfer cannot revert - this implies internal error in + // EVM execution + return Err(EthereumError::InconsistentState(Cow::from( + "Transfer returned revert", + ))); + } + ExitReason::Succeed(_) => { + // Otherwise result is ok and we do nothing and continue + } + } } self.increment_nonce(transaction_context.context.caller)?; @@ -484,15 +519,16 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { let transfer_cost = self.config.gas_transaction_call; - let gas_result = self - .gasometer - .record_cost(transfer_cost) - .map_err(EthereumError::from); + let gas_result = self.gasometer.record_cost(transfer_cost); - let tx_result = gas_result.and_then(|_| { - self.execute_transfer(from, to, value, gas_limit)?; - Ok((ExitReason::Succeed(ExitSucceed::Returned), None, vec![])) - }); + let tx_result = match gas_result { + Ok(()) => Ok(( + self.execute_transfer(from, to, value, gas_limit)?, + None, + vec![], + )), + Err(err) => Ok((ExitReason::Error(err), None, vec![])), + }; self.end_initial_transaction(tx_result) } @@ -762,7 +798,13 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { execution_result: Result, ) -> Result { match execution_result { - Ok((r @ ExitReason::Succeed(_), new_address, result)) => { + Ok((ExitReason::Succeed(r), new_address, result)) => { + debug_msg!( + self.host, + "The initial transaction ended with success: {:?}", + r + ); + self.commit_initial_transaction(new_address, result) } Ok((ExitReason::Revert(ExitRevert::Reverted), _, _)) => { @@ -775,8 +817,11 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { error ); + self.rollback_initial_transaction() + } + Ok((ExitReason::Fatal(ExitFatal::Other(cow_str)), _, _)) => { self.rollback_initial_transaction()?; - Err(EthereumError::MachineExitError(error)) + Err(EthereumError::WrappedError(cow_str)) } Ok((ExitReason::Fatal(fatal_error), _, _)) => { debug_msg!( @@ -785,23 +830,19 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { fatal_error ); - self.rollback_initial_transaction()?; - Err(EthereumError::FatalMachineError(fatal_error)) + self.rollback_initial_transaction() } Err(EthereumError::EthereumAccountError( - AccountStorageError::BalanceUnderflow, - )) => self.rollback_initial_transaction(), - Err(EthereumError::MachineExitError(ExitError::OutOfGas)) => { + AccountStorageError::BalanceUnderflow(path), + )) => { debug_msg!( self.host, - "The initial transaction ended because it ran out of gas" + "Initial transaction failed because of account underflow at {:?}", + path ); self.rollback_initial_transaction() } - Err(EthereumError::FatalMachineError(ExitFatal::CallErrorAsFatal( - ExitError::OutOfGas, - ))) => self.rollback_initial_transaction(), Err(err) => { debug_msg!( self.host, @@ -920,9 +961,10 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { fn end_inter_transaction( &mut self, execution_result: Result, + promote_error: bool, ) -> Capture { if let Ok((ref r @ ExitReason::Succeed(_), _, _)) = execution_result { - debug_msg!(self.host, "Intermediate transaction with: {:?}", r); + debug_msg!(self.host, "Intermediate transaction ended with: {:?}", r); if let Err(err) = self.commit_inter_transaction() { debug_msg!( @@ -931,7 +973,6 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { err ); - // Beware that we may not be able to recover from this error. return Capture::Exit((ethereum_error_to_exit_reason(err), None, vec![])); } } else if let Err(err) = self.rollback_inter_transaction() { @@ -941,11 +982,21 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { err ); - // Beware that we may not be able to recover from this error. return Capture::Exit((ethereum_error_to_exit_reason(err), None, vec![])); } match execution_result { + Ok((ExitReason::Error(err), _, _)) => { + if promote_error { + Capture::Exit(( + ExitReason::Fatal(ExitFatal::CallErrorAsFatal(err)), + None, + vec![], + )) + } else { + Capture::Exit((ExitReason::Error(err), None, vec![])) + } + } Ok(res) => Capture::Exit(res), Err(err) => Capture::Exit((ethereum_error_to_exit_reason(err), None, vec![])), } @@ -1139,7 +1190,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { let result = self.execute_create(caller, scheme, value, init_code, target_gas); - self.end_inter_transaction(result) + self.end_inter_transaction(result, false) } } @@ -1152,17 +1203,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { is_static: bool, context: Context, ) -> Capture { - fn remap_capture( - capture: Capture, - ) -> Capture { - match capture { - Capture::Exit((reason, _, value)) => Capture::Exit((reason, value)), - Capture::Trap(x) => Capture::Trap(x), - } - } - if let Err(err) = self.begin_inter_transaction(is_static) { - // Beware that we may not be able to recover from this error. return Capture::Exit((ethereum_error_to_exit_reason(err), vec![])); } @@ -1174,7 +1215,13 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { TransactionContext::from_context(context), ); - remap_capture(self.end_inter_transaction(result)) + match self.end_inter_transaction(result, true) { + Capture::Exit((reason, _, value)) => { + debug_msg!(self.host, "Call ended with reason: {:?}", reason); + Capture::Exit((reason, value)) + } + Capture::Trap(x) => Capture::Trap(x), + } } fn pre_validate( @@ -1564,7 +1611,7 @@ mod test { input_value.to_big_endian(&mut input); let address = H160::from_low_u64_be(118); - let gas_limit: Option = None; + let gas_limit: Option = Some(10_000_000_000_u64); let transaction_context = TransactionContext::new(caller, address, U256::zero()); let transfer: Option = None; let code: Vec = vec![ @@ -1621,15 +1668,20 @@ mod test { match result { Ok(result) => { - panic!("Expected to fail, but got Ok({:?})", result); - } - Err(EthereumError::FatalMachineError(ExitFatal::CallErrorAsFatal( - ExitError::CallTooDeep, - ))) => { - assert_eq!(handler.gas_used(), 5); + let expected_result = ( + ExitReason::Fatal(ExitFatal::CallErrorAsFatal( + ExitError::CallTooDeep, + )), + None, + vec![], + ); + assert_eq!(result, expected_result); } Err(err) => { - panic!("Expected a too-many-calls error, but got {:?}", err); + panic!( + "Expected call to fail because of call depth, but got {:?}", + err + ); } } } @@ -1967,11 +2019,12 @@ mod test { match result { Ok(result) => { - panic!("Got Ok, but this was supposed to fail (got {:?})", result); - } - Err(EthereumError::EthereumAccountError( - AccountStorageError::BalanceUnderflow, - )) => { + let expected_result = ( + ExitReason::Fatal(ExitFatal::CallErrorAsFatal(ExitError::OutOfFund)), + None, + vec![], + ); + assert_eq!(result, expected_result); assert_eq!(get_balance(&mut handler, &caller), U256::from(99_u32)); assert_eq!(get_balance(&mut handler, &address), U256::zero()); assert_eq!(handler.gas_used(), 0); diff --git a/src/kernel_evm/evm_execution/src/lib.rs b/src/kernel_evm/evm_execution/src/lib.rs index bc5984243d420c12d174f02ff76343f69b1fc769..f9b65a6e3afc314b75e254a8bc4269d62cef028b 100644 --- a/src/kernel_evm/evm_execution/src/lib.rs +++ b/src/kernel_evm/evm_execution/src/lib.rs @@ -12,7 +12,7 @@ use alloc::borrow::Cow; use alloc::collections::TryReserveError; use debug::debug_msg; use evm::executor::stack::PrecompileFailure; -use evm::{Config, ExitError, ExitFatal}; +use evm::Config; use host::runtime::Runtime; use primitive_types::{H160, U256}; use tezos_ethereum::block::BlockConstants; @@ -39,19 +39,15 @@ use precompiles::PrecompileSet; /// transfer for instance. #[derive(Error, Debug, Eq, PartialEq)] pub enum EthereumError { - /// EVM returned with a machine error - #[error("Internal machine error when running contract")] - MachineExitError(ExitError), - /// A fatal error from executing EVM. - #[error("Fatal machine error when running contract")] - FatalMachineError(ExitFatal), + /// An ethereum error happened inside a callback and we had to print it to + /// a string so we could wrap it in a `ExitFatal` error. We have lost the + /// exact variant, but we can retain the message. + #[error("Wrapped Ethereum error: {0}")] + WrappedError(Cow<'static, str>), /// Calling a precompiled failed (implies there was a precompiled contract /// at the call address. #[error("Precompile call failed")] PrecompileFailed(PrecompileFailure), - /// Contract did revert - #[error("Contract call reverted")] - CallRevert, /// The SputnikVM runtime returned a Trap. This should be impossible. #[error("Internal SputnikVM trap")] InternalTrapError, @@ -92,12 +88,6 @@ pub enum EthereumError { InconsistentState(Cow<'static, str>), } -impl From for EthereumError { - fn from(err: ExitError) -> Self { - EthereumError::MachineExitError(err) - } -} - /// Execute an Ethereum Transaction /// /// The function returns `Err` only if something is wrong with the kernel and/or the diff --git a/src/kernel_evm/kernel/src/apply.rs b/src/kernel_evm/kernel/src/apply.rs index e108f6c1494711b9b6e4177725c165b282a7fff9..3eef344ef72b9b41e9ade8a00746c22014081508 100644 --- a/src/kernel_evm/kernel/src/apply.rs +++ b/src/kernel_evm/kernel/src/apply.rs @@ -9,9 +9,6 @@ use evm_execution::account_storage::EthereumAccountStorage; use evm_execution::handler::ExecutionOutcome; use evm_execution::precompiles::PrecompileBTreeMap; use evm_execution::run_transaction; -use evm_execution::EthereumError::{ - EthereumAccountError, EthereumStorageError, InternalTrapError, -}; use primitive_types::{H160, H256, U256}; use tezos_ethereum::block::BlockConstants; use tezos_ethereum::signatures::EthereumTransactionCommon; @@ -230,14 +227,13 @@ pub fn apply_transaction( Some(value), ) { Ok(outcome) => Some(outcome), - Err(InternalTrapError | EthereumAccountError(_) | EthereumStorageError(_)) => { + Err(err) => { // TODO: https://gitlab.com/tezos/tezos/-/issues/5665 // Because the proposal's state is unclear, and we do not have a sequencer // if an error that leads to a durable storage corruption is caught, we // invalidate the entire proposal. - return Err(Error::InvalidRunTransaction); + return Err(Error::InvalidRunTransaction(err)); } - Err(_) => None, }; let gas_used = match &execution_outcome { diff --git a/src/kernel_evm/kernel/src/error.rs b/src/kernel_evm/kernel/src/error.rs index a4f2343afca1888ae1753b3c55ec2148a229dd1a..10c8015f0a67d605d7bf2b3528a3a7763e2c02b6 100644 --- a/src/kernel_evm/kernel/src/error.rs +++ b/src/kernel_evm/kernel/src/error.rs @@ -42,7 +42,7 @@ pub enum Error { Transfer(TransferError), Storage(StorageError), InvalidConversion, - InvalidRunTransaction, + InvalidRunTransaction(EthereumError), Simulation(EthereumError), UpgradeError(UpgradeProcessError), InvalidSignature(SigError),