From 530697ee0ecd41ab425d3b51e8008f04b7aaa743 Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Mon, 7 Oct 2024 20:22:23 +0200 Subject: [PATCH] Feat: Refactore IsPrecompileResult type (#62) * Redactore IsPrecompile * Remove type: IsPrecompileResult --- gasometer/src/lib.rs | 22 +++++++++---------- runtime/src/handler.rs | 2 +- src/executor/stack/executor.rs | 36 ++++++-------------------------- src/executor/stack/mod.rs | 3 +-- src/executor/stack/precompile.rs | 24 +++++---------------- 5 files changed, 24 insertions(+), 63 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 12bf6f137..4f46b4b4f 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -634,14 +634,14 @@ pub fn dynamic_opcode_cost( let target = stack.peek_h256(0)?.into(); storage_target = StorageTarget::Address(target); GasCost::ExtCodeSize { - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), } } Opcode::BALANCE => { let target = stack.peek_h256(0)?.into(); storage_target = StorageTarget::Address(target); GasCost::Balance { - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), } } Opcode::BLOCKHASH => GasCost::BlockHash, @@ -650,7 +650,7 @@ pub fn dynamic_opcode_cost( let target = stack.peek_h256(0)?.into(); storage_target = StorageTarget::Address(target); GasCost::ExtCodeHash { - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), } } Opcode::EXTCODEHASH => GasCost::Invalid(opcode), @@ -661,7 +661,7 @@ pub fn dynamic_opcode_cost( GasCost::CallCode { value: stack.peek(2)?, gas: stack.peek(0)?, - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), target_exists: { handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) @@ -673,7 +673,7 @@ pub fn dynamic_opcode_cost( storage_target = StorageTarget::Address(target); GasCost::StaticCall { gas: stack.peek(0)?, - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), target_exists: { handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) @@ -687,7 +687,7 @@ pub fn dynamic_opcode_cost( let target = stack.peek_h256(0)?.into(); storage_target = StorageTarget::Address(target); GasCost::ExtCodeCopy { - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), len: stack.peek(3)?, } } @@ -701,7 +701,7 @@ pub fn dynamic_opcode_cost( let index = stack.peek_h256(0)?; storage_target = StorageTarget::Slot(address, index); GasCost::SLoad { - target_is_cold: handler.is_cold(address, Some(index))?, + target_is_cold: handler.is_cold(address, Some(index)), } } @@ -710,7 +710,7 @@ pub fn dynamic_opcode_cost( storage_target = StorageTarget::Address(target); GasCost::DelegateCall { gas: stack.peek(0)?, - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), target_exists: { handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) @@ -734,7 +734,7 @@ pub fn dynamic_opcode_cost( original: handler.original_storage(address, index), current: handler.storage(address, index), new: value, - target_is_cold: handler.is_cold(address, Some(index))?, + target_is_cold: handler.is_cold(address, Some(index)), } } Opcode::LOG0 if !is_static => GasCost::Log { @@ -766,7 +766,7 @@ pub fn dynamic_opcode_cost( storage_target = StorageTarget::Address(target); GasCost::Suicide { value: handler.balance(address), - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), target_exists: { handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) @@ -780,7 +780,7 @@ pub fn dynamic_opcode_cost( GasCost::Call { value: stack.peek(2)?, gas: stack.peek(0)?, - target_is_cold: handler.is_cold(target, None)?, + target_is_cold: handler.is_cold(target, None), target_exists: { handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index c21975e5a..d73f44a3d 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -78,7 +78,7 @@ pub trait Handler { /// /// # Errors /// Return `ExitError` - fn is_cold(&mut self, address: H160, index: Option) -> Result; + fn is_cold(&mut self, address: H160, index: Option) -> bool; /// Set storage value of address at index. /// diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index a1106d1e9..597e09ae7 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -1,6 +1,6 @@ use crate::backend::Backend; use crate::executor::stack::precompile::{ - IsPrecompileResult, PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileSet, + PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileSet, }; use crate::executor::stack::tagged_runtime::{RuntimeKind, TaggedRuntime}; use crate::gasometer::{self, Gasometer, StorageTarget}; @@ -1336,30 +1336,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler } } - fn is_cold(&mut self, address: H160, maybe_index: Option) -> Result { - Ok(match maybe_index { - None => { - let is_precompile = match self - .precompile_set - .is_precompile(address, self.state.metadata().gasometer.gas()) - { - IsPrecompileResult::Answer { - is_precompile, - extra_cost, - } => { - self.state - .metadata_mut() - .gasometer - .record_cost(extra_cost)?; - is_precompile - } - IsPrecompileResult::OutOfGas => return Err(ExitError::OutOfGas), - }; - - !is_precompile && self.state.is_cold(address) - } + fn is_cold(&mut self, address: H160, maybe_index: Option) -> bool { + match maybe_index { + None => !self.precompile_set.is_precompile(address) && self.state.is_cold(address), Some(index) => self.state.is_storage_cold(address, index), - }) + } } fn gas_left(&self) -> U256 { @@ -1610,15 +1591,10 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr // Since we don't go through opcodes we need manually record the call // cost. Not doing so will make the code panic as recording the call stipend // will do an underflow. - let target_is_cold = match self.executor.is_cold(code_address, None) { - Ok(x) => x, - Err(err) => return (ExitReason::Error(err), Vec::new()), - }; - let gas_cost = gasometer::GasCost::Call { value: transfer.clone().map_or_else(U256::zero, |x| x.value), gas: U256::from(gas_limit.unwrap_or(u64::MAX)), - target_is_cold, + target_is_cold: self.executor.is_cold(code_address, None), target_exists: self.executor.exists(code_address), }; diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 2c9fca670..9908f24e2 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -12,7 +12,6 @@ pub use self::executor::{ }; pub use self::memory::{MemoryStackAccount, MemoryStackState, MemoryStackSubstate}; pub use self::precompile::{ - IsPrecompileResult, PrecompileFailure, PrecompileFn, PrecompileHandle, PrecompileOutput, - PrecompileSet, + PrecompileFailure, PrecompileFn, PrecompileHandle, PrecompileOutput, PrecompileSet, }; pub use ethereum::Log; diff --git a/src/executor/stack/precompile.rs b/src/executor/stack/precompile.rs index a2d438fcf..62ced53f3 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -104,15 +104,7 @@ pub trait PrecompileSet { /// Check if the given address is a precompile. Should only be called to /// perform the check while not executing the precompile afterward, since /// `execute` already performs a check internally. - fn is_precompile(&self, address: H160, remaining_gas: u64) -> IsPrecompileResult; -} - -pub enum IsPrecompileResult { - Answer { - is_precompile: bool, - extra_cost: u64, - }, - OutOfGas, + fn is_precompile(&self, address: H160) -> bool; } impl PrecompileSet for () { @@ -120,11 +112,8 @@ impl PrecompileSet for () { None } - fn is_precompile(&self, _: H160, _: u64) -> IsPrecompileResult { - IsPrecompileResult::Answer { - is_precompile: false, - extra_cost: 0, - } + fn is_precompile(&self, _: H160) -> bool { + false } } @@ -161,10 +150,7 @@ impl PrecompileSet for BTreeMap { /// Check if the given address is a precompile. Should only be called to /// perform the check while not executing the precompile afterward, since /// `execute` already performs a check internally. - fn is_precompile(&self, address: H160, _: u64) -> IsPrecompileResult { - IsPrecompileResult::Answer { - is_precompile: self.contains_key(&address), - extra_cost: 0, - } + fn is_precompile(&self, address: H160) -> bool { + self.contains_key(&address) } }