From c505f5896b0abba27abf8304a4b3d6fc4b259c0b Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Tue, 27 Aug 2024 16:25:08 +0200 Subject: [PATCH] Feat: refactore gas charge logic form EVM exit reason (#935) ### Description To be fully compatible with EVM, we should: :arrow_right: Success| Revert :arrow_right: ExitError - Execution errors should charge gas from users :arrow_right: ExitFatal - shouldn't charge user gas :arrow_right: Transactions validation errors should not charge user gas This PR changed the logic for EVM exit reason gas charge for users, which includes EVM execution errors gas charge. Previously transaction just panicked, and user's gas was not charged. #### EVM Related to EVM changes: aurora-is-near/sputnikvm/pull/52 --------- Co-authored-by: Oleksandr Anyshchenko --- Cargo.lock | 16 +- Cargo.toml | 8 +- engine-hashchain/src/hashchain.rs | 7 +- engine-tests/src/tests/one_inch.rs | 2 +- engine-tests/src/tests/repro.rs | 12 +- .../src/tests/res/transaction_status.borsh | Bin 0 -> 41 bytes engine-tests/src/tests/sanity.rs | 4 +- .../src/tests/standalone/call_tracer.rs | 9 +- .../src/tests/standard_precompiles.rs | 3 +- engine-tests/src/tests/uniswap.rs | 2 +- engine-tests/src/utils/mod.rs | 8 +- engine-types/src/parameters/engine.rs | 128 ++++++++++-- engine/src/engine.rs | 196 ++++++++++++++---- 13 files changed, 308 insertions(+), 87 deletions(-) create mode 100644 engine-tests/src/tests/res/transaction_status.borsh diff --git a/Cargo.lock b/Cargo.lock index faffae615..4bdc0b483 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2249,8 +2249,8 @@ dependencies = [ [[package]] name = "evm" -version = "0.43.0" -source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.43.1-aurora#a201e15e56eddac80872045e6b5015ee3859c900" +version = "0.44.1" +source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.45.0-aurora#f3bf27581eb6e16798f792a84275dfc4dbaf7f04" dependencies = [ "auto_impl", "environmental", @@ -2269,8 +2269,8 @@ dependencies = [ [[package]] name = "evm-core" -version = "0.43.0" -source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.43.1-aurora#a201e15e56eddac80872045e6b5015ee3859c900" +version = "0.44.1" +source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.45.0-aurora#f3bf27581eb6e16798f792a84275dfc4dbaf7f04" dependencies = [ "parity-scale-codec", "primitive-types 0.12.2", @@ -2280,8 +2280,8 @@ dependencies = [ [[package]] name = "evm-gasometer" -version = "0.43.0" -source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.43.1-aurora#a201e15e56eddac80872045e6b5015ee3859c900" +version = "0.44.1" +source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.45.0-aurora#f3bf27581eb6e16798f792a84275dfc4dbaf7f04" dependencies = [ "environmental", "evm-core", @@ -2291,8 +2291,8 @@ dependencies = [ [[package]] name = "evm-runtime" -version = "0.43.0" -source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.43.1-aurora#a201e15e56eddac80872045e6b5015ee3859c900" +version = "0.44.1" +source = "git+https://github.com/aurora-is-near/sputnikvm.git?tag=v0.45.0-aurora#f3bf27581eb6e16798f792a84275dfc4dbaf7f04" dependencies = [ "auto_impl", "environmental", diff --git a/Cargo.toml b/Cargo.toml index 7962fd27c..93c454ab8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,10 +31,10 @@ byte-slice-cast = { version = "1", default-features = false } criterion = "0.5" digest = "0.10" ethabi = { version = "18", default-features = false } -evm = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false } -evm-core = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std"] } -evm-gasometer = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std", "tracing"] } -evm-runtime = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std", "tracing"] } +evm = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false } +evm-core = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std"] } +evm-gasometer = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std", "tracing"] } +evm-runtime = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std", "tracing"] } fixed-hash = { version = "0.8", default-features = false } function_name = "0.3" git2 = "0.19" diff --git a/engine-hashchain/src/hashchain.rs b/engine-hashchain/src/hashchain.rs index b5133bc77..b1191e79c 100644 --- a/engine-hashchain/src/hashchain.rs +++ b/engine-hashchain/src/hashchain.rs @@ -57,10 +57,9 @@ impl Hashchain { } /// Moves to the indicated block height if it is bigger than the current block height: - /// -Updates the previous block hashchain computing the hash. - /// -Updates the current block height. - /// -Clears the transactions. - /// -Clears the transactions. + /// - Updates the previous block hashchain computing the hash. + /// - Updates the current block height. + /// - Clears the transactions. /// Returns an error in other case. pub fn move_to_block( &mut self, diff --git a/engine-tests/src/tests/one_inch.rs b/engine-tests/src/tests/one_inch.rs index 75b03b552..c2150b767 100644 --- a/engine-tests/src/tests/one_inch.rs +++ b/engine-tests/src/tests/one_inch.rs @@ -99,7 +99,7 @@ fn test_1_inch_limit_order_deploy() { // more than 3.5 million Ethereum gas used assert!(result.gas_used > 3_500_000); - // less than 11 NEAR TGas used + // less than 12 NEAR TGas used assert_gas_bound(profile.all_gas(), 12); // at least 45% of which is from wasm execution let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas(); diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs index 7c1aa29cd..d2f01ec95 100644 --- a/engine-tests/src/tests/repro.rs +++ b/engine-tests/src/tests/repro.rs @@ -26,7 +26,7 @@ fn repro_GdASJ3KESs() { block_timestamp: 1_645_717_564_644_206_730, input_path: "src/tests/res/input_GdASJ3KESs.hex", evm_gas_used: 706_713, - near_gas_used: 113, + near_gas_used: 114, }); } @@ -51,7 +51,7 @@ fn repro_8ru7VEA() { block_timestamp: 1_648_829_935_343_349_589, input_path: "src/tests/res/input_8ru7VEA.hex", evm_gas_used: 1_732_181, - near_gas_used: 204, + near_gas_used: 205, }); } @@ -71,7 +71,7 @@ fn repro_FRcorNv() { block_timestamp: 1_650_960_438_774_745_116, input_path: "src/tests/res/input_FRcorNv.hex", evm_gas_used: 1_239_721, - near_gas_used: 165, + near_gas_used: 167, }); } @@ -88,7 +88,7 @@ fn repro_5bEgfRQ() { block_timestamp: 1_651_073_772_931_594_646, input_path: "src/tests/res/input_5bEgfRQ.hex", evm_gas_used: 6_414_105, - near_gas_used: 645, + near_gas_used: 649, }); } @@ -106,7 +106,7 @@ fn repro_D98vwmi() { block_timestamp: 1_651_753_443_421_003_245, input_path: "src/tests/res/input_D98vwmi.hex", evm_gas_used: 1_035_348, - near_gas_used: 167, + near_gas_used: 168, }); } @@ -125,7 +125,7 @@ fn repro_Emufid2() { block_timestamp: 1_662_118_048_636_713_538, input_path: "src/tests/res/input_Emufid2.hex", evm_gas_used: 1_156_364, - near_gas_used: 291, + near_gas_used: 293, }); } diff --git a/engine-tests/src/tests/res/transaction_status.borsh b/engine-tests/src/tests/res/transaction_status.borsh new file mode 100644 index 0000000000000000000000000000000000000000..f8b129521e0e00f2b0d0f667d747dfd9a84c5217 GIT binary patch literal 41 ncmWe;fC5Gk&BV;Y%Er#Y$;HjX%fQDkASlEN6iqEE$}a){6rKX^ literal 0 HcmV?d00001 diff --git a/engine-tests/src/tests/sanity.rs b/engine-tests/src/tests/sanity.rs index 8601267c9..d82756106 100644 --- a/engine-tests/src/tests/sanity.rs +++ b/engine-tests/src/tests/sanity.rs @@ -1067,7 +1067,8 @@ fn test_block_hash_contract() { }) .unwrap(); - utils::panic_on_fail(result.status); + let res = utils::panic_on_fail(result.status); + assert!(res.is_none(), "Status: {res:?}"); } #[cfg(not(feature = "ext-connector"))] @@ -1245,6 +1246,7 @@ mod workspace { Some(utils::AuroraRunner::default().chain_id), &signer.secret_key, ); + let result = aurora .submit(rlp::encode(&signed_tx).to_vec()) .transact() diff --git a/engine-tests/src/tests/standalone/call_tracer.rs b/engine-tests/src/tests/standalone/call_tracer.rs index 8fc026965..e3f54798d 100644 --- a/engine-tests/src/tests/standalone/call_tracer.rs +++ b/engine-tests/src/tests/standalone/call_tracer.rs @@ -2,6 +2,7 @@ use crate::prelude::{H160, H256}; use crate::utils::solidity::erc20::{ERC20Constructor, ERC20}; use crate::utils::{self, standalone, Signer}; use aurora_engine_modexp::AuroraModExp; +use aurora_engine_types::parameters::engine::TransactionStatus; use aurora_engine_types::{ parameters::{CrossContractCallArgs, PromiseArgs, PromiseCreateArgs}, storage, @@ -296,10 +297,10 @@ fn test_contract_create_too_large() { let standalone_result = sputnik::traced_call(&mut listener, || { runner.submit_transaction(&signer.secret_key, tx) }); - assert!( - standalone_result.is_err(), - "Expected contract too large error" - ); + assert!(matches!( + standalone_result.unwrap().status, + TransactionStatus::CreateContractLimit + )); } #[allow(clippy::too_many_lines)] diff --git a/engine-tests/src/tests/standard_precompiles.rs b/engine-tests/src/tests/standard_precompiles.rs index 069d21b9e..1d571448a 100644 --- a/engine-tests/src/tests/standard_precompiles.rs +++ b/engine-tests/src/tests/standard_precompiles.rs @@ -24,7 +24,8 @@ fn test_standard_precompiles() { .submit_with_signer(&mut signer, |nonce| contract.call_method("test_all", nonce)) .unwrap(); - utils::panic_on_fail(outcome.status); + let res = utils::panic_on_fail(outcome.status); + assert!(res.is_none(), "Status: {res:?}"); } #[test] diff --git a/engine-tests/src/tests/uniswap.rs b/engine-tests/src/tests/uniswap.rs index 0a3ab4ca2..4664d8550 100644 --- a/engine-tests/src/tests/uniswap.rs +++ b/engine-tests/src/tests/uniswap.rs @@ -58,7 +58,7 @@ fn test_uniswap_exact_output() { let (_amount_in, profile) = context.exact_output_single(&token_a, &token_b, OUTPUT_AMOUNT.into()); - utils::assert_gas_bound(profile.all_gas(), 16); + utils::assert_gas_bound(profile.all_gas(), 17); let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas(); assert!( (40..=50).contains(&wasm_fraction), diff --git a/engine-tests/src/utils/mod.rs b/engine-tests/src/utils/mod.rs index 7c36bc40a..523f643c4 100644 --- a/engine-tests/src/utils/mod.rs +++ b/engine-tests/src/utils/mod.rs @@ -1000,10 +1000,12 @@ pub fn unwrap_revert_slice(result: &SubmitResult) -> &[u8] { } } -pub fn panic_on_fail(status: TransactionStatus) { +pub fn panic_on_fail(status: TransactionStatus) -> Option { match status { - TransactionStatus::Succeed(_) => (), - TransactionStatus::Revert(message) => panic!("{}", String::from_utf8_lossy(&message)), + TransactionStatus::Succeed(_) => None, + TransactionStatus::Revert(message) => { + Some(format!("Revert: {}", String::from_utf8_lossy(&message))) + } other => panic!("{}", String::from_utf8_lossy(other.as_ref())), } } diff --git a/engine-types/src/parameters/engine.rs b/engine-types/src/parameters/engine.rs index 037aaaa4d..f914cdb7b 100644 --- a/engine-types/src/parameters/engine.rs +++ b/engine-types/src/parameters/engine.rs @@ -177,16 +177,51 @@ pub struct ResultLog { pub data: Vec, } -/// The status of a transaction. +/// The status of a transaction representing EVM error kinds. +/// !!! THE ORDER OF VARIANTS MUSTN'T BE CHANGED FOR SAVING BACKWARD COMPATIBILITY !!! +/// !!! NEW VARIANTS SHOULD BE ADDED IN THE END OF THE ENUM ONLY !!! #[derive(Debug, Clone, BorshSerialize, BorshDeserialize, PartialEq, Eq)] #[cfg_attr(feature = "impl-serde", derive(Serialize, Deserialize))] pub enum TransactionStatus { + /// The transaction succeeded. Succeed(Vec), + /// The transaction reverted. Revert(Vec), + /// Execution runs out of gas. OutOfGas, + /// Not enough fund to start the execution. OutOfFund, + /// An opcode accesses external information, but the request is off offset limit. OutOfOffset, + /// Call stack is too deep. CallTooDeep, + /// Trying to pop from an empty stack. + StackUnderflow, + /// Trying to push into a stack over stack limit. + StackOverflow, + /// Jump destination is invalid. + InvalidJump, + /// An opcode accesses memory region, but the region is invalid. + InvalidRange, + /// Encountered the designated invalid opcode. + DesignatedInvalid, + /// Create opcode encountered collision. + CreateCollision, + /// Create init code exceeds limit. + CreateContractLimit, + /// Invalid opcode during execution or starting byte is 0xef. See [EIP-3541](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-3541.md). + InvalidCode(u8), + /// PC underflow (unused). + #[allow(clippy::upper_case_acronyms)] + PCUnderflow, + /// Attempt to create an empty account (unused). + CreateEmpty, + /// Nonce reached maximum value of 2^64-1 + MaxNonce, + /// `usize` casting overflow + UsizeOverflow, + /// Other normal errors. + Other(crate::Cow<'static, str>), } impl TransactionStatus { @@ -201,11 +236,8 @@ impl TransactionStatus { } #[must_use] - pub fn is_fail(&self) -> bool { - *self == Self::OutOfGas - || *self == Self::OutOfFund - || *self == Self::OutOfOffset - || *self == Self::CallTooDeep + pub const fn is_fail(&self) -> bool { + !matches!(*self, Self::Succeed(_) | Self::Revert(_)) } } @@ -218,6 +250,19 @@ impl AsRef<[u8]> for TransactionStatus { Self::OutOfGas => errors::ERR_OUT_OF_GAS, Self::OutOfOffset => errors::ERR_OUT_OF_OFFSET, Self::CallTooDeep => errors::ERR_CALL_TOO_DEEP, + Self::StackUnderflow => errors::ERR_STACK_UNDERFLOW, + Self::StackOverflow => errors::ERR_STACK_OVERFLOW, + Self::InvalidJump => errors::ERR_INVALID_JUMP, + Self::InvalidRange => errors::ERR_INVALID_RANGE, + Self::DesignatedInvalid => errors::ERR_DESIGNATED_INVALID, + Self::CreateCollision => errors::ERR_CREATE_COLLISION, + Self::CreateContractLimit => errors::ERR_CREATE_CONTRACT_LIMIT, + Self::InvalidCode(_) => errors::ERR_INVALID_CODE, + Self::PCUnderflow => errors::ERR_PC_UNDERFLOW, + Self::CreateEmpty => errors::ERR_CREATE_EMPTY, + Self::MaxNonce => errors::ERR_MAX_NONCE, + Self::UsizeOverflow => errors::ERR_USIZE_OVERFLOW, + Self::Other(e) => e.as_bytes(), } } } @@ -237,7 +282,7 @@ impl SubmitResult { /// Must be incremented when making breaking changes to the `SubmitResult` ABI. /// The current value of 7 is chosen because previously a `TransactionStatus` object /// was first in the serialization, which is an enum with less than 7 variants. - /// Therefore, no previous `SubmitResult` would have began with a leading 7 byte, + /// Therefore, no previous `SubmitResult` would have begun with a leading 7 byte, /// and this can be used to distinguish the new ABI (with version byte) from the old. const VERSION: u8 = 7; @@ -372,12 +417,24 @@ mod chain_id_deserialize { pub mod errors { use crate::{account_id::ParseAccountError, String, ToString}; - pub const ERR_REVERT: &[u8; 10] = b"ERR_REVERT"; - pub const ERR_NOT_ALLOWED: &[u8; 15] = b"ERR_NOT_ALLOWED"; - pub const ERR_OUT_OF_FUNDS: &[u8; 16] = b"ERR_OUT_OF_FUNDS"; - pub const ERR_CALL_TOO_DEEP: &[u8; 17] = b"ERR_CALL_TOO_DEEP"; - pub const ERR_OUT_OF_OFFSET: &[u8; 17] = b"ERR_OUT_OF_OFFSET"; - pub const ERR_OUT_OF_GAS: &[u8; 14] = b"ERR_OUT_OF_GAS"; + pub const ERR_REVERT: &[u8] = b"ERR_REVERT"; + pub const ERR_NOT_ALLOWED: &[u8] = b"ERR_NOT_ALLOWED"; + pub const ERR_OUT_OF_FUNDS: &[u8] = b"ERR_OUT_OF_FUNDS"; + pub const ERR_CALL_TOO_DEEP: &[u8] = b"ERR_CALL_TOO_DEEP"; + pub const ERR_OUT_OF_OFFSET: &[u8] = b"ERR_OUT_OF_OFFSET"; + pub const ERR_OUT_OF_GAS: &[u8] = b"ERR_OUT_OF_GAS"; + pub const ERR_STACK_UNDERFLOW: &[u8] = b"STACK_UNDERFLOW"; + pub const ERR_STACK_OVERFLOW: &[u8] = b"STACK_OVERFLOW"; + pub const ERR_INVALID_JUMP: &[u8] = b"INVALID_JUMP"; + pub const ERR_INVALID_RANGE: &[u8] = b"INVALID_RANGE"; + pub const ERR_DESIGNATED_INVALID: &[u8] = b"DESIGNATED_INVALID"; + pub const ERR_CREATE_COLLISION: &[u8] = b"CREATE_COLLISION"; + pub const ERR_CREATE_CONTRACT_LIMIT: &[u8] = b"CREATE_CONTRACT_LIMIT"; + pub const ERR_INVALID_CODE: &[u8] = b"INVALID_CODE"; + pub const ERR_PC_UNDERFLOW: &[u8] = b"PC_UNDERFLOW"; + pub const ERR_CREATE_EMPTY: &[u8] = b"CREATE_EMPTY"; + pub const ERR_MAX_NONCE: &[u8] = b"MAX_NONCE"; + pub const ERR_USIZE_OVERFLOW: &[u8] = b"USIZE_OVERFLOW"; #[derive(Debug)] pub enum ParseArgsError { @@ -511,4 +568,49 @@ mod tests { let arguments = NewCallArgs::deserialize(&serde_json::to_vec(&outdated).unwrap()); assert!(arguments.is_err()); } + + #[test] + fn test_serialization_transaction_status_regression() { + let bytes = + std::fs::read("../engine-tests/src/tests/res/transaction_status.borsh").unwrap(); + let actual = Vec::::try_from_slice(&bytes).unwrap(); + let expected = transaction_status_variants(); + + assert_eq!(actual, expected); + } + + #[allow(dead_code)] + fn generate_borsh_bytes() { + let variants = transaction_status_variants(); + + std::fs::write( + "../engine-tests/src/tests/res/transaction_status.borsh", + borsh::to_vec(&variants).unwrap(), + ) + .unwrap(); + } + + fn transaction_status_variants() -> Vec { + vec![ + TransactionStatus::Succeed(Vec::new()), + TransactionStatus::Revert(Vec::new()), + TransactionStatus::OutOfGas, + TransactionStatus::OutOfFund, + TransactionStatus::OutOfOffset, + TransactionStatus::CallTooDeep, + TransactionStatus::StackUnderflow, + TransactionStatus::StackOverflow, + TransactionStatus::InvalidJump, + TransactionStatus::InvalidRange, + TransactionStatus::DesignatedInvalid, + TransactionStatus::CreateCollision, + TransactionStatus::CreateContractLimit, + TransactionStatus::InvalidCode(0), + TransactionStatus::PCUnderflow, + TransactionStatus::CreateEmpty, + TransactionStatus::MaxNonce, + TransactionStatus::UsizeOverflow, + TransactionStatus::Other("error".into()), + ] + } } diff --git a/engine/src/engine.rs b/engine/src/engine.rs index 7b799f526..fe615a3fe 100644 --- a/engine/src/engine.rs +++ b/engine/src/engine.rs @@ -5,7 +5,7 @@ use aurora_engine_types::public_key::PublicKey; use aurora_engine_types::PhantomData; use core::mem; use evm::backend::{Apply, ApplyBackend, Backend, Basic, Log}; -use evm::executor; +use evm::{executor, Opcode}; use evm::{Config, CreateScheme, ExitError, ExitFatal, ExitReason}; use crate::map::BijectionMap; @@ -177,6 +177,11 @@ trait ExitIntoResult { } impl ExitIntoResult for ExitReason { + /// We should be aligned to Ethereum's gas charging: + /// - `Success` | `Revert` + /// - `ExitError` - Execution errors should charge gas from users + /// - `ExitFatal` - shouldn't charge user gas + /// NOTE: Transactions validation errors should not charge user gas fn into_result(self, data: Vec) -> Result { match self { Self::Succeed(_) => Ok(TransactionStatus::Succeed(data)), @@ -184,7 +189,24 @@ impl ExitIntoResult for ExitReason { Self::Error(ExitError::OutOfOffset) => Ok(TransactionStatus::OutOfOffset), Self::Error(ExitError::OutOfFund) => Ok(TransactionStatus::OutOfFund), Self::Error(ExitError::OutOfGas) => Ok(TransactionStatus::OutOfGas), - Self::Error(e) => Err(e.into()), + Self::Error(ExitError::CallTooDeep) => Ok(TransactionStatus::CallTooDeep), + Self::Error(ExitError::StackUnderflow) => Ok(TransactionStatus::StackUnderflow), + Self::Error(ExitError::StackOverflow) => Ok(TransactionStatus::StackOverflow), + Self::Error(ExitError::InvalidJump) => Ok(TransactionStatus::InvalidJump), + Self::Error(ExitError::InvalidRange) => Ok(TransactionStatus::InvalidRange), + Self::Error(ExitError::DesignatedInvalid) => Ok(TransactionStatus::DesignatedInvalid), + Self::Error(ExitError::CreateCollision) => Ok(TransactionStatus::CreateCollision), + Self::Error(ExitError::CreateContractLimit) => { + Ok(TransactionStatus::CreateContractLimit) + } + Self::Error(ExitError::InvalidCode(opcode)) => { + Ok(TransactionStatus::InvalidCode(opcode.0)) + } + Self::Error(ExitError::PCUnderflow) => Ok(TransactionStatus::PCUnderflow), + Self::Error(ExitError::CreateEmpty) => Ok(TransactionStatus::CreateEmpty), + Self::Error(ExitError::MaxNonce) => Ok(TransactionStatus::MaxNonce), + Self::Error(ExitError::UsizeOverflow) => Ok(TransactionStatus::UsizeOverflow), + Self::Error(ExitError::Other(msg)) => Ok(TransactionStatus::Other(msg)), Self::Fatal(e) => Err(e.into()), } } @@ -785,37 +807,7 @@ impl<'env, I: IO + Copy, E: Env, M: ModExpAlgorithm> Engine<'env, I, E, M> { Vec::new(), // TODO: are there values we should put here? handler, ) - .and_then(|submit_result| match submit_result.status { - TransactionStatus::Succeed(_) => Ok(submit_result), - TransactionStatus::Revert(bytes) => { - let error_message = crate::prelude::format!( - "Reverted with message: {}", - crate::prelude::String::from_utf8_lossy(&bytes) - ); - Err(EngineError { - kind: EngineErrorKind::EvmError(ExitError::Other( - crate::prelude::Cow::from(error_message), - )), - gas_used: submit_result.gas_used, - }) - } - TransactionStatus::OutOfFund => Err(EngineError { - kind: EngineErrorKind::EvmError(ExitError::OutOfFund), - gas_used: submit_result.gas_used, - }), - TransactionStatus::OutOfOffset => Err(EngineError { - kind: EngineErrorKind::EvmError(ExitError::OutOfOffset), - gas_used: submit_result.gas_used, - }), - TransactionStatus::OutOfGas => Err(EngineError { - kind: EngineErrorKind::EvmError(ExitError::OutOfGas), - gas_used: submit_result.gas_used, - }), - TransactionStatus::CallTooDeep => Err(EngineError { - kind: EngineErrorKind::EvmError(ExitError::CallTooDeep), - gas_used: submit_result.gas_used, - }), - }) + .and_then(submit_result_or_err) .inspect_err(|_e| { sdk::log!("{:?}", _e); self.io.return_output(output_on_fail); @@ -1920,19 +1912,14 @@ impl<'env, I: IO + Copy, E: Env, M: ModExpAlgorithm> Backend for Engine<'env, I, result } - /// Check is storage empty for the address - /// EIP-7610: non-empty storage + /// Check if the storage of the address is empty. + /// Related to EIP-7610: non-empty storage fn is_empty_storage(&self, address: H160) -> bool { - let address = Address::new(address); // As we can't read all storage data for account we assuming that if storage exists // `index = 0` always true let index = H256::zero(); - let generation = *self - .generation_cache - .borrow_mut() - .entry(address) - .or_insert_with(|| get_generation(&self.io, &address)); // First we're checking cache to not produce read-storage operation + let address = Address::new(address); if self .contract_storage_cache .borrow() @@ -1940,6 +1927,11 @@ impl<'env, I: IO + Copy, E: Env, M: ModExpAlgorithm> Backend for Engine<'env, I, { return false; } + let generation = *self + .generation_cache + .borrow_mut() + .entry(address) + .or_insert_with(|| get_generation(&self.io, &address)); !storage_has_key(&self.io, &address, &index, generation) } @@ -2087,6 +2079,88 @@ impl<'env, J: IO + Copy, E: Env, M: ModExpAlgorithm> ApplyBackend for Engine<'en } } +fn submit_result_or_err(submit_result: SubmitResult) -> Result { + match submit_result.status { + TransactionStatus::Succeed(_) => Ok(submit_result), + TransactionStatus::Revert(bytes) => { + let error_message = crate::prelude::format!( + "Reverted with message: {}", + crate::prelude::String::from_utf8_lossy(&bytes) + ); + Err(engine_error( + ExitError::Other(error_message.into()), + submit_result.gas_used, + )) + } + TransactionStatus::OutOfFund => { + Err(engine_error(ExitError::OutOfFund, submit_result.gas_used)) + } + TransactionStatus::OutOfOffset => { + Err(engine_error(ExitError::OutOfOffset, submit_result.gas_used)) + } + TransactionStatus::OutOfGas => { + Err(engine_error(ExitError::OutOfGas, submit_result.gas_used)) + } + TransactionStatus::CallTooDeep => { + Err(engine_error(ExitError::CallTooDeep, submit_result.gas_used)) + } + TransactionStatus::StackUnderflow => Err(engine_error( + ExitError::StackUnderflow, + submit_result.gas_used, + )), + TransactionStatus::StackOverflow => Err(engine_error( + ExitError::StackOverflow, + submit_result.gas_used, + )), + TransactionStatus::InvalidJump => { + Err(engine_error(ExitError::InvalidJump, submit_result.gas_used)) + } + TransactionStatus::InvalidRange => Err(engine_error( + ExitError::InvalidRange, + submit_result.gas_used, + )), + TransactionStatus::DesignatedInvalid => Err(engine_error( + ExitError::DesignatedInvalid, + submit_result.gas_used, + )), + TransactionStatus::CreateCollision => Err(engine_error( + ExitError::CreateCollision, + submit_result.gas_used, + )), + TransactionStatus::CreateContractLimit => Err(engine_error( + ExitError::CreateContractLimit, + submit_result.gas_used, + )), + TransactionStatus::InvalidCode(code) => Err(engine_error( + ExitError::InvalidCode(Opcode(code)), + submit_result.gas_used, + )), + TransactionStatus::PCUnderflow => { + Err(engine_error(ExitError::PCUnderflow, submit_result.gas_used)) + } + TransactionStatus::CreateEmpty => { + Err(engine_error(ExitError::CreateEmpty, submit_result.gas_used)) + } + TransactionStatus::MaxNonce => { + Err(engine_error(ExitError::MaxNonce, submit_result.gas_used)) + } + TransactionStatus::UsizeOverflow => Err(engine_error( + ExitError::UsizeOverflow, + submit_result.gas_used, + )), + TransactionStatus::Other(e) => { + Err(engine_error(ExitError::Other(e), submit_result.gas_used)) + } + } +} + +const fn engine_error(exit_error: ExitError, gas_used: u64) -> EngineError { + EngineError { + kind: EngineErrorKind::EvmError(exit_error), + gas_used, + } +} + #[cfg(test)] mod tests { use super::*; @@ -2573,6 +2647,46 @@ mod tests { assert_eq!(expected_value, actual_value); } + #[test] + fn test_storage_is_empty_with_cache() { + let origin = Address::zero(); + let current_account_id = AccountId::default(); + let env = Fixed::default(); + let storage = RefCell::new(Storage::default()); + let mut io = StoragePointer(&storage); + let engine: Engine<_, _> = + Engine::new_with_state(EngineState::default(), origin, current_account_id, io, &env); + + let expected_value = H256::from_low_u64_le(64); + let index = H256::zero(); + // Check that storage is empty + assert!(engine.is_empty_storage(origin.raw())); + let generation = get_generation(&io, &origin); + set_storage(&mut io, &origin, &index, &expected_value, generation); + // It will read without cache + assert!(!engine.is_empty_storage(origin.raw())); + // Cache should be empty + let cache_val = engine + .contract_storage_cache + .borrow() + .contains_key(&(origin, index)); + assert!(!cache_val); + // Check the storage value and hit the cache + let actual_value = engine.storage(origin.raw(), index); + assert_eq!(expected_value, actual_value); + // Cache should exists + let cache_val = engine + .contract_storage_cache + .borrow() + .contains_key(&(origin, index)); + assert!(cache_val); + remove_storage(&mut io, &origin, &index, generation); + // Value should still be in the cache + let actual_value = engine.storage(origin.raw(), index); + assert_eq!(expected_value, actual_value); + assert!(!engine.is_empty_storage(origin.raw())); + } + #[test] fn test_loading_engine_from_storage_loads_stored_state() { let origin = Address::zero();