From 49f6fef97c50c6c4bb5b6e1e6aa79541e0e17ddd Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Thu, 12 Sep 2024 12:56:30 +0200 Subject: [PATCH 1/2] Redactore IsPrecompile --- gasometer/src/lib.rs | 22 +++++++++++----------- runtime/src/handler.rs | 2 +- src/executor/stack/executor.rs | 25 +++++-------------------- src/executor/stack/precompile.rs | 8 +------- 4 files changed, 18 insertions(+), 39 deletions(-) diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 12bf6f13..4f46b4b4 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 c21975e5..d73f44a3 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 a1106d1e..962002ef 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -1336,30 +1336,20 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler } } - fn is_cold(&mut self, address: H160, maybe_index: Option) -> Result { - Ok(match maybe_index { + fn is_cold(&mut self, address: H160, maybe_index: Option) -> bool { + 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), + IsPrecompileResult::Answer { is_precompile } => is_precompile, }; !is_precompile && self.state.is_cold(address) } Some(index) => self.state.is_storage_cold(address, index), - }) + } } fn gas_left(&self) -> U256 { @@ -1610,15 +1600,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/precompile.rs b/src/executor/stack/precompile.rs index a2d438fc..6ec273a9 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -108,11 +108,7 @@ pub trait PrecompileSet { } pub enum IsPrecompileResult { - Answer { - is_precompile: bool, - extra_cost: u64, - }, - OutOfGas, + Answer { is_precompile: bool }, } impl PrecompileSet for () { @@ -123,7 +119,6 @@ impl PrecompileSet for () { fn is_precompile(&self, _: H160, _: u64) -> IsPrecompileResult { IsPrecompileResult::Answer { is_precompile: false, - extra_cost: 0, } } } @@ -164,7 +159,6 @@ impl PrecompileSet for BTreeMap { fn is_precompile(&self, address: H160, _: u64) -> IsPrecompileResult { IsPrecompileResult::Answer { is_precompile: self.contains_key(&address), - extra_cost: 0, } } } From a10a583d352d816363b19ec3ef84f322ee2efaa9 Mon Sep 17 00:00:00 2001 From: Evgeny Ukhanov Date: Mon, 23 Sep 2024 12:42:50 +0200 Subject: [PATCH 2/2] Remove type: IsPrecompileResult --- src/executor/stack/executor.rs | 13 ++----------- src/executor/stack/mod.rs | 3 +-- src/executor/stack/precompile.rs | 18 +++++------------- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 962002ef..597e09ae 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}; @@ -1338,16 +1338,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler fn is_cold(&mut self, address: H160, maybe_index: Option) -> bool { match maybe_index { - None => { - let is_precompile = match self - .precompile_set - .is_precompile(address, self.state.metadata().gasometer.gas()) - { - IsPrecompileResult::Answer { is_precompile } => is_precompile, - }; - - !is_precompile && self.state.is_cold(address) - } + None => !self.precompile_set.is_precompile(address) && self.state.is_cold(address), Some(index) => self.state.is_storage_cold(address, index), } } diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index 2c9fca67..9908f24e 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 6ec273a9..62ced53f 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -104,11 +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 }, + fn is_precompile(&self, address: H160) -> bool; } impl PrecompileSet for () { @@ -116,10 +112,8 @@ impl PrecompileSet for () { None } - fn is_precompile(&self, _: H160, _: u64) -> IsPrecompileResult { - IsPrecompileResult::Answer { - is_precompile: false, - } + fn is_precompile(&self, _: H160) -> bool { + false } } @@ -156,9 +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), - } + fn is_precompile(&self, address: H160) -> bool { + self.contains_key(&address) } }