From 94c9036f1e674e49b614e2cef700d099c0bfd0ad Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 13 Apr 2023 10:36:14 +0200 Subject: [PATCH 01/17] Add support for external cost recording --- Cargo.toml | 1 + src/executor/stack/executor.rs | 21 +++++++++++++++++++-- src/executor/stack/precompile.rs | 4 ++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bc87d4a57..b3671ccab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ with-serde = [ "evm-core/with-serde", "ethereum/with-serde", ] +with-substrate = [] tracing = [ "environmental", "evm-gasometer/tracing", diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 05d41c3ee..c0d90bcb5 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -3,7 +3,7 @@ use crate::executor::stack::precompile::{ IsPrecompileResult, PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileSet, }; use crate::executor::stack::tagged_runtime::{RuntimeKind, TaggedRuntime}; -use crate::gasometer::{self, Gasometer, StorageTarget}; +use crate::gasometer::{self, Gasometer, GasCost, StorageTarget}; use crate::maybe_borrowed::MaybeBorrowed; use crate::{ Capture, Config, Context, CreateScheme, ExitError, ExitReason, Handler, Opcode, Runtime, Stack, @@ -229,6 +229,15 @@ pub trait StackState<'config>: Backend { fn code_hash(&self, address: H160) -> H256 { H256::from_slice(Keccak256::digest(&self.code(address)).as_slice()) } + + fn record_external_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { + Ok(()) + } + + #[cfg(feature = "with-substrate")] + fn record_external_cost(&mut self, _ref_time: u64, _proof_size: u64) -> Result<(), ExitError> { + Ok(()) + } } /// Stack-based executor. @@ -1261,8 +1270,9 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler )?; let gasometer = &mut self.state.metadata_mut().gasometer; - gasometer.record_dynamic_cost(gas_cost, memory_cost)?; + self.state.record_external_opcode_cost(opcode, gas_cost, target)?; + match target { StorageTarget::Address(address) => { self.state.metadata_mut().access_address(address) @@ -1377,6 +1387,13 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr .record_cost(cost) } + #[cfg(feature = "with-substrate")] + fn record_external_cost(&mut self, ref_time: u64, proof_size: u64) -> Result<(), ExitError> { + self.executor + .state + .record_external_cost(ref_time, proof_size) + } + /// Retreive the remaining gas. fn remaining_gas(&self) -> u64 { self.executor.state.metadata().gasometer.gas() diff --git a/src/executor/stack/precompile.rs b/src/executor/stack/precompile.rs index bca915f12..936999ca6 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -50,6 +50,10 @@ pub trait PrecompileHandle { /// Record cost to the Runtime gasometer. fn record_cost(&mut self, cost: u64) -> Result<(), ExitError>; + #[cfg(feature = "with-substrate")] + /// Record Substrate specific cost. + fn record_external_cost(&mut self, ref_time: u64, proof_size: u64) -> Result<(), ExitError>; + /// Retreive the remaining gas. fn remaining_gas(&self) -> u64; From 932044e2d58e33b05cb94e02155c71247bd8e954 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Fri, 14 Apr 2023 09:40:55 +0200 Subject: [PATCH 02/17] Add refund methods --- src/executor/stack/executor.rs | 18 ++++++++++++++++-- src/executor/stack/precompile.rs | 6 +++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index c0d90bcb5..6293f83d4 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -235,7 +235,12 @@ pub trait StackState<'config>: Backend { } #[cfg(feature = "with-substrate")] - fn record_external_cost(&mut self, _ref_time: u64, _proof_size: u64) -> Result<(), ExitError> { + fn record_external_cost(&mut self, _ref_time: Option, _proof_size: Option) -> Result<(), ExitError> { + Ok(()) + } + + #[cfg(feature = "with-substrate")] + fn refund_external_cost(&mut self, _ref_time: Option, _proof_size: Option) -> Result<(), ExitError> { Ok(()) } } @@ -1388,12 +1393,21 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr } #[cfg(feature = "with-substrate")] - fn record_external_cost(&mut self, ref_time: u64, proof_size: u64) -> Result<(), ExitError> { + /// Record Substrate specific cost. + fn record_external_cost(&mut self, ref_time: Option, proof_size: Option) -> Result<(), ExitError> { self.executor .state .record_external_cost(ref_time, proof_size) } + #[cfg(feature = "with-substrate")] + /// Refund Substrate specific cost. + fn refund_external_cost(&mut self, ref_time: Option, proof_size: Option) { + self.executor + .state + .refund_external_cost(ref_time, proof_size) + } + /// Retreive the remaining gas. fn remaining_gas(&self) -> u64 { self.executor.state.metadata().gasometer.gas() diff --git a/src/executor/stack/precompile.rs b/src/executor/stack/precompile.rs index 936999ca6..2e2fdbcda 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -52,7 +52,11 @@ pub trait PrecompileHandle { #[cfg(feature = "with-substrate")] /// Record Substrate specific cost. - fn record_external_cost(&mut self, ref_time: u64, proof_size: u64) -> Result<(), ExitError>; + fn record_external_cost(&mut self, ref_time: Option, proof_size: Option) -> Result<(), ExitError>; + + #[cfg(feature = "with-substrate")] + /// Refund Substrate specific cost. + fn refund_external_cost(&mut self, ref_time: Option, proof_size: Option); /// Retreive the remaining gas. fn remaining_gas(&self) -> u64; From 44b5296430f4072a2df12671c5302600891e4f8d Mon Sep 17 00:00:00 2001 From: tgmichel Date: Mon, 17 Apr 2023 14:41:11 +0200 Subject: [PATCH 03/17] Modify `refund_external_cost` signature --- src/executor/stack/executor.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 6293f83d4..0b48ee921 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -240,9 +240,7 @@ pub trait StackState<'config>: Backend { } #[cfg(feature = "with-substrate")] - fn refund_external_cost(&mut self, _ref_time: Option, _proof_size: Option) -> Result<(), ExitError> { - Ok(()) - } + fn refund_external_cost(&mut self, _ref_time: Option, _proof_size: Option) {} } /// Stack-based executor. @@ -1405,7 +1403,7 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr fn refund_external_cost(&mut self, ref_time: Option, proof_size: Option) { self.executor .state - .refund_external_cost(ref_time, proof_size) + .refund_external_cost(ref_time, proof_size); } /// Retreive the remaining gas. From 70cc19744e8d0be7d45a1099c2d878a562c5188d Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 20 Apr 2023 13:56:36 +0200 Subject: [PATCH 04/17] Add `record_external_static_opcode_cost` method --- src/executor/stack/executor.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 0b48ee921..7a58064f7 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -230,7 +230,11 @@ pub trait StackState<'config>: Backend { H256::from_slice(Keccak256::digest(&self.code(address)).as_slice()) } - fn record_external_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { + fn record_external_static_opcode_cost(&mut self, _opcode: Opcode) -> Result<(), ExitError> { + Ok(()) + } + + fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { Ok(()) } @@ -1261,6 +1265,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler if let Some(cost) = gasometer::static_opcode_cost(opcode) { self.state.metadata_mut().gasometer.record_cost(cost)?; + self.state.record_external_static_opcode_cost(opcode)?; } else { let is_static = self.state.metadata().is_static; let (gas_cost, target, memory_cost) = gasometer::dynamic_opcode_cost( @@ -1274,7 +1279,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler let gasometer = &mut self.state.metadata_mut().gasometer; gasometer.record_dynamic_cost(gas_cost, memory_cost)?; - self.state.record_external_opcode_cost(opcode, gas_cost, target)?; + self.state.record_external_dynamic_opcode_cost(opcode, gas_cost, target)?; match target { StorageTarget::Address(address) => { From 9e7568e75f16883da940bc84ca3cf592974b6c78 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 27 Apr 2023 12:01:13 +0200 Subject: [PATCH 05/17] Handle code size hash oog --- runtime/src/eval/system.rs | 16 ++++++++++++---- runtime/src/handler.rs | 4 ++-- src/executor/stack/executor.rs | 26 ++++++++++++++++---------- src/executor/stack/memory.rs | 5 +++-- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 574355437..27664ae00 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -90,16 +90,24 @@ pub fn base_fee(runtime: &mut Runtime, handler: &H) -> Control { Control::Continue } -pub fn extcodesize(runtime: &mut Runtime, handler: &H) -> Control { +pub fn extcodesize(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - push_u256!(runtime, handler.code_size(address.into())); + let code_size = match handler.code_size(address.into()) { + Ok(v) => v, + Err(e) => return Control::Exit(e.into()), + }; + push_u256!(runtime, code_size); Control::Continue } -pub fn extcodehash(runtime: &mut Runtime, handler: &H) -> Control { +pub fn extcodehash(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - push!(runtime, handler.code_hash(address.into())); + let code_hash = match handler.code_hash(address.into()) { + Ok(v) => v, + Err(e) => return Control::Exit(e.into()), + }; + push!(runtime, code_hash); Control::Continue } diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index e7e25182d..1760a7a6c 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -28,9 +28,9 @@ pub trait Handler { /// Get balance of address. fn balance(&self, address: H160) -> U256; /// Get code size of address. - fn code_size(&self, address: H160) -> U256; + fn code_size(&mut self, address: H160) -> Result; /// Get code hash of address. - fn code_hash(&self, address: H160) -> H256; + fn code_hash(&mut self, address: H160) -> Result; /// Get code of address. fn code(&self, address: H160) -> Vec; /// Get storage value of address at index. diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 7a58064f7..e0d19a178 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -209,7 +209,7 @@ pub trait StackState<'config>: Backend { fn reset_storage(&mut self, address: H160); fn log(&mut self, address: H160, topics: Vec, data: Vec); fn set_deleted(&mut self, address: H160); - fn set_code(&mut self, address: H160, code: Vec); + fn set_code(&mut self, address: H160, code: Vec) -> Result<(), ExitError>; fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError>; fn reset_balance(&mut self, address: H160); fn touch(&mut self, address: H160); @@ -218,16 +218,16 @@ pub trait StackState<'config>: Backend { /// Provide a default implementation by fetching the code, but /// can be customized to use a more performant approach that don't need to /// fetch the code. - fn code_size(&self, address: H160) -> U256 { - U256::from(self.code(address).len()) + fn code_size(&mut self, address: H160) -> Result { + Ok(U256::from(self.code(address).len())) } /// Fetch the code hash of an address. /// Provide a default implementation by fetching the code, but /// can be customized to use a more performant approach that don't need to /// fetch the code. - fn code_hash(&self, address: H160) -> H256 { - H256::from_slice(Keccak256::digest(&self.code(address)).as_slice()) + fn code_hash(&mut self, address: H160) -> Result { + Ok(H256::from_slice(Keccak256::digest(&self.code(address)).as_slice())) } fn record_external_static_opcode_cost(&mut self, _opcode: Opcode) -> Result<(), ExitError> { @@ -742,7 +742,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.enter_substate(gas_limit, false); { - if self.code_size(address) != U256::zero() { + let code_size = match self.code_size(address) { + Ok(v) => v, + Err(e) => return Capture::Exit((e.into(), None, Vec::new())), + }; + if code_size != U256::zero() { let _ = self.exit_substate(StackExitKind::Failed); return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); } @@ -960,7 +964,9 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> { Ok(()) => { let exit_result = self.exit_substate(StackExitKind::Succeeded); - self.state.set_code(address, out); + if let Err(e) = self.state.set_code(address, out) { + return (e.into(), None, Vec::new()); + } if let Err(e) = exit_result { return (e.into(), None, Vec::new()); } @@ -1033,13 +1039,13 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler self.state.basic(address).balance } - fn code_size(&self, address: H160) -> U256 { + fn code_size(&mut self, address: H160) -> Result { self.state.code_size(address) } - fn code_hash(&self, address: H160) -> H256 { + fn code_hash(&mut self, address: H160) -> Result { if !self.exists(address) { - return H256::default(); + return Ok(H256::default()); } self.state.code_hash(address) diff --git a/src/executor/stack/memory.rs b/src/executor/stack/memory.rs index 0ec07eabf..bfd7da72f 100644 --- a/src/executor/stack/memory.rs +++ b/src/executor/stack/memory.rs @@ -530,8 +530,9 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.set_deleted(address) } - fn set_code(&mut self, address: H160, code: Vec) { - self.substate.set_code(address, code, self.backend) + fn set_code(&mut self, address: H160, code: Vec) -> Result<(), ExitError> { + self.substate.set_code(address, code, self.backend); + Ok(()) } fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError> { From 6f46413a7f9f28c97a4ee632bcd037d3e2948276 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 27 Apr 2023 12:02:22 +0200 Subject: [PATCH 06/17] Temp remove static external cost accounting --- src/executor/stack/executor.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index e0d19a178..c7cc35784 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -230,10 +230,6 @@ pub trait StackState<'config>: Backend { Ok(H256::from_slice(Keccak256::digest(&self.code(address)).as_slice())) } - fn record_external_static_opcode_cost(&mut self, _opcode: Opcode) -> Result<(), ExitError> { - Ok(()) - } - fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { Ok(()) } @@ -1271,7 +1267,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler if let Some(cost) = gasometer::static_opcode_cost(opcode) { self.state.metadata_mut().gasometer.record_cost(cost)?; - self.state.record_external_static_opcode_cost(opcode)?; } else { let is_static = self.state.metadata().is_static; let (gas_cost, target, memory_cost) = gasometer::dynamic_opcode_cost( From ba318eef38662584d5f66bbe242bf54faef48de0 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 11 May 2023 16:24:30 +0200 Subject: [PATCH 07/17] `call` fn call must be able to oog --- core/src/stack.rs | 4 ++-- gasometer/src/lib.rs | 10 ++++----- runtime/src/eval/system.rs | 9 +++++++-- runtime/src/handler.rs | 4 ++-- src/backend/memory.rs | 7 ++++--- src/backend/mod.rs | 6 +++--- src/executor/stack/executor.rs | 33 ++++++++++++++++++++---------- src/executor/stack/memory.rs | 37 +++++++++++++++++----------------- 8 files changed, 64 insertions(+), 46 deletions(-) diff --git a/core/src/stack.rs b/core/src/stack.rs index 893af517c..ed72631c2 100644 --- a/core/src/stack.rs +++ b/core/src/stack.rs @@ -32,8 +32,8 @@ impl Stack { #[inline] /// Whether the stack is empty. - pub fn is_empty(&self) -> bool { - self.data.is_empty() + pub fn is_empty(&self) -> Result { + Ok(self.data.is_empty()) } #[inline] diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 6518b66ba..c9c4157ae 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -507,7 +507,7 @@ pub fn dynamic_opcode_cost( value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target), + target_exists: handler.exists(target)?, } } Opcode::STATICCALL => { @@ -516,7 +516,7 @@ pub fn dynamic_opcode_cost( GasCost::StaticCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target), + target_exists: handler.exists(target)?, } } Opcode::SHA3 => GasCost::Sha3 { @@ -550,7 +550,7 @@ pub fn dynamic_opcode_cost( GasCost::DelegateCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target), + target_exists: handler.exists(target)?, } } Opcode::DELEGATECALL => GasCost::Invalid(opcode), @@ -603,7 +603,7 @@ pub fn dynamic_opcode_cost( GasCost::Suicide { value: handler.balance(address), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target), + target_exists: handler.exists(target)?, already_removed: handler.deleted(address), } } @@ -617,7 +617,7 @@ pub fn dynamic_opcode_cost( value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target), + target_exists: handler.exists(target)?, } } diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 27664ae00..a4a8b02fe 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -112,7 +112,7 @@ pub fn extcodehash(runtime: &mut Runtime, handler: &mut H) -> Contro Control::Continue } -pub fn extcodecopy(runtime: &mut Runtime, handler: &H) -> Control { +pub fn extcodecopy(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); pop_u256!(runtime, memory_offset, code_offset, len); @@ -120,11 +120,16 @@ pub fn extcodecopy(runtime: &mut Runtime, handler: &H) -> Control .machine .memory_mut() .resize_offset(memory_offset, len)); + + let code = match handler.code(address.into()) { + Ok(code) => code, + Err(e) => return Control::Exit(e.into()), + }; match runtime.machine.memory_mut().copy_large( memory_offset, code_offset, len, - &handler.code(address.into()), + &code, ) { Ok(()) => (), Err(e) => return Control::Exit(e.into()), diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 1760a7a6c..552bc96f7 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -32,7 +32,7 @@ pub trait Handler { /// Get code hash of address. fn code_hash(&mut self, address: H160) -> Result; /// Get code of address. - fn code(&self, address: H160) -> Vec; + fn code(&mut self, address: H160) -> Result, ExitError>; /// Get storage value of address at index. fn storage(&self, address: H160, index: H256) -> H256; /// Get original storage value of address at index. @@ -62,7 +62,7 @@ pub trait Handler { fn chain_id(&self) -> U256; /// Check whether an address exists. - fn exists(&self, address: H160) -> bool; + fn exists(&mut self, address: H160) -> Result; /// Check whether an address has already been deleted. fn deleted(&self, address: H160) -> bool; /// Checks if the address or (address, index) pair has been previously accessed diff --git a/src/backend/memory.rs b/src/backend/memory.rs index 7382a5e22..1b3d35292 100644 --- a/src/backend/memory.rs +++ b/src/backend/memory.rs @@ -2,6 +2,7 @@ use super::{Apply, ApplyBackend, Backend, Basic, Log}; use alloc::collections::BTreeMap; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; +use crate::ExitError; /// Vicinity value of a memory backend. #[derive(Clone, Debug, Eq, PartialEq)] @@ -135,11 +136,11 @@ impl<'vicinity> Backend for MemoryBackend<'vicinity> { .unwrap_or_default() } - fn code(&self, address: H160) -> Vec { - self.state + fn code(&mut self, address: H160) -> Result, ExitError> { + Ok(self.state .get(&address) .map(|v| v.code.clone()) - .unwrap_or_default() + .unwrap_or_default()) } fn storage(&self, address: H160, index: H256) -> H256 { diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 1a503cbcc..9b3d5a8d6 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -5,7 +5,7 @@ mod memory; pub use self::memory::{MemoryAccount, MemoryBackend, MemoryVicinity}; - +use crate::ExitError; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; @@ -50,7 +50,7 @@ pub enum Apply { } /// EVM backend. -#[auto_impl::auto_impl(&, Arc, Box)] +//#[auto_impl::auto_impl(&, Arc, Box)] pub trait Backend { /// Gas price. Unused for London. fn gas_price(&self) -> U256; @@ -78,7 +78,7 @@ pub trait Backend { /// Get basic account information. fn basic(&self, address: H160) -> Basic; /// Get account code. - fn code(&self, address: H160) -> Vec; + fn code(&mut self, address: H160) -> Result, ExitError>; /// Get storage value of address at index. fn storage(&self, address: H160, index: H256) -> H256; /// Get original storage value of address at index, if available. diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index c7cc35784..0b3441e73 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -199,7 +199,7 @@ pub trait StackState<'config>: Backend { fn exit_revert(&mut self) -> Result<(), ExitError>; fn exit_discard(&mut self) -> Result<(), ExitError>; - fn is_empty(&self, address: H160) -> bool; + fn is_empty(&mut self, address: H160) -> Result; fn deleted(&self, address: H160) -> bool; fn is_cold(&self, address: H160) -> bool; fn is_storage_cold(&self, address: H160, key: H256) -> bool; @@ -219,7 +219,7 @@ pub trait StackState<'config>: Backend { /// can be customized to use a more performant approach that don't need to /// fetch the code. fn code_size(&mut self, address: H160) -> Result { - Ok(U256::from(self.code(address).len())) + Ok(U256::from(self.code(address)?.len())) } /// Fetch the code hash of an address. @@ -227,7 +227,7 @@ pub trait StackState<'config>: Backend { /// can be customized to use a more performant approach that don't need to /// fetch the code. fn code_hash(&mut self, address: H160) -> Result { - Ok(H256::from_slice(Keccak256::digest(&self.code(address)).as_slice())) + Ok(H256::from_slice(Keccak256::digest(&self.code(address)?).as_slice())) } fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { @@ -848,11 +848,17 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } } - let code = self.code(code_address); - self.enter_substate(gas_limit, is_static); self.state.touch(context.address); + let code = match self.code(code_address) { + Ok(code) => code, + Err(e) => { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitReason::Error(e), Vec::new())); + } + }; + if let Some(depth) = self.state.metadata().depth { if depth > self.config.call_stack_limit { let _ = self.exit_substate(StackExitKind::Reverted); @@ -1040,14 +1046,14 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler } fn code_hash(&mut self, address: H160) -> Result { - if !self.exists(address) { + if !self.exists(address)? { return Ok(H256::default()); } self.state.code_hash(address) } - fn code(&self, address: H160) -> Vec { + fn code(&mut self, address: H160) -> Result, ExitError> { self.state.code(address) } @@ -1061,11 +1067,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler .unwrap_or_default() } - fn exists(&self, address: H160) -> bool { + fn exists(&mut self, address: H160) -> Result { if self.config.empty_considered_exists { - self.state.exists(address) + Ok(self.state.exists(address)) } else { - self.state.exists(address) && !self.state.is_empty(address) + Ok(self.state.exists(address) && !self.state.is_empty(address)?) } } @@ -1329,11 +1335,16 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr Err(err) => return (ExitReason::Error(err), Vec::new()), }; + let target_exists = match self.executor.exists(code_address) { + Ok(x) => x, + Err(err) => return (ExitReason::Error(err), Vec::new()), + }; + let gas_cost = crate::gasometer::GasCost::Call { value: transfer.clone().map(|x| x.value).unwrap_or_else(U256::zero), gas: U256::from(gas_limit.unwrap_or(u64::MAX)), target_is_cold, - target_exists: self.executor.exists(code_address), + target_exists, }; // We record the length of the input. diff --git a/src/executor/stack/memory.rs b/src/executor/stack/memory.rs index bfd7da72f..9e3aa9fd7 100644 --- a/src/executor/stack/memory.rs +++ b/src/executor/stack/memory.rs @@ -192,26 +192,26 @@ impl<'config> MemoryStackSubstate<'config> { self.known_account(address).and_then(|acc| acc.code.clone()) } - pub fn known_empty(&self, address: H160) -> Option { + pub fn known_empty(&self, address: H160) -> Result, ExitError> { if let Some(account) = self.known_account(address) { if account.basic.balance != U256::zero() { - return Some(false); + return Ok(Some(false)); } if account.basic.nonce != U256::zero() { - return Some(false); + return Ok(Some(false)); } if let Some(code) = &account.code { - return Some( + return Ok(Some( account.basic.balance == U256::zero() && account.basic.nonce == U256::zero() && code.is_empty(), - ); + )); } } - None + Ok(None) } pub fn known_storage(&self, address: H160, key: H256) -> Option { @@ -393,9 +393,9 @@ impl<'config> MemoryStackSubstate<'config> { } } -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct MemoryStackState<'backend, 'config, B> { - backend: &'backend B, + backend: &'backend mut B, substate: MemoryStackSubstate<'config>, } @@ -442,10 +442,11 @@ impl<'backend, 'config, B: Backend> Backend for MemoryStackState<'backend, 'conf .unwrap_or_else(|| self.backend.basic(address)) } - fn code(&self, address: H160) -> Vec { - self.substate - .known_code(address) - .unwrap_or_else(|| self.backend.code(address)) + fn code(&mut self, address: H160) -> Result, ExitError> { + if let Some(code) = self.substate.known_code(address) { + return Ok(code); + } + self.backend.code(address) } fn storage(&self, address: H160, key: H256) -> H256 { @@ -488,14 +489,14 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.exit_discard() } - fn is_empty(&self, address: H160) -> bool { - if let Some(known_empty) = self.substate.known_empty(address) { - return known_empty; + fn is_empty(&mut self, address: H160) -> Result { + if let Some(known_empty) = self.substate.known_empty(address)? { + return Ok(known_empty); } - self.backend.basic(address).balance == U256::zero() + Ok(self.backend.basic(address).balance == U256::zero() && self.backend.basic(address).nonce == U256::zero() - && self.backend.code(address).len() == 0 + && self.backend.code(address)?.len() == 0) } fn deleted(&self, address: H160) -> bool { @@ -549,7 +550,7 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba } impl<'backend, 'config, B: Backend> MemoryStackState<'backend, 'config, B> { - pub fn new(metadata: StackSubstateMetadata<'config>, backend: &'backend B) -> Self { + pub fn new(metadata: StackSubstateMetadata<'config>, backend: &'backend mut B) -> Self { Self { backend, substate: MemoryStackSubstate::new(metadata), From a9bf6092a2cf15231c4be8254e20c8bc28be0ef6 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Tue, 30 May 2023 15:19:06 +0200 Subject: [PATCH 08/17] Prevent exiting substate on top level create function --- src/executor/stack/executor.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 57d81cf1c..bf33573d6 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -447,7 +447,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> if let Some(limit) = self.config.max_initcode_size { if init_code.len() > limit { self.state.metadata_mut().gasometer.fail(); - let _ = self.exit_substate(StackExitKind::Failed); return emit_exit!(ExitError::CreateContractLimit.into(), Vec::new()); } } From c3db4777854b8b5a7cc6f54b78de988c0c387b4f Mon Sep 17 00:00:00 2001 From: tgmichel Date: Tue, 30 May 2023 15:37:35 +0200 Subject: [PATCH 09/17] `transact_create2` --- src/executor/stack/executor.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index bf33573d6..67c03089c 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -487,7 +487,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> if let Some(limit) = self.config.max_initcode_size { if init_code.len() > limit { self.state.metadata_mut().gasometer.fail(); - let _ = self.exit_substate(StackExitKind::Failed); return emit_exit!(ExitError::CreateContractLimit.into(), Vec::new()); } } From 1b902f6185289cf666cf29439886795d35db2ddd Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 8 Jun 2023 09:36:14 +0200 Subject: [PATCH 10/17] ci --- .github/workflows/rust.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index a3afec529..7303dbe91 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -24,8 +24,10 @@ jobs: - uses: actions/checkout@v2 - name: Build run: cargo build --verbose - - name: Build for feature + - name: Build for feature (tracing) run: cargo build --features tracing --verbose + - name: Build for feature (with-substrate) + run: cargo build --features with-substrate --verbose - name: Run tests run: cargo test --verbose jsontests: From 269f83fb91aa47bac41f7dcd33874806fbe8dd27 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 8 Jun 2023 09:47:31 +0200 Subject: [PATCH 11/17] `record_external_dynamic_opcode_cost` only `with-substrate` --- src/executor/stack/executor.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 67c03089c..c37f17813 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -3,7 +3,7 @@ use crate::executor::stack::precompile::{ IsPrecompileResult, PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileSet, }; use crate::executor::stack::tagged_runtime::{RuntimeKind, TaggedRuntime}; -use crate::gasometer::{self, Gasometer, GasCost, StorageTarget}; +use crate::gasometer::{self, Gasometer, StorageTarget}; use crate::maybe_borrowed::MaybeBorrowed; use crate::{ Capture, Config, Context, CreateScheme, ExitError, ExitReason, Handler, Opcode, Runtime, Stack, @@ -230,7 +230,8 @@ pub trait StackState<'config>: Backend { Ok(H256::from_slice(Keccak256::digest(&self.code(address)?).as_slice())) } - fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: GasCost, _target: StorageTarget) -> Result<(), ExitError> { + #[cfg(feature = "with-substrate")] + fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: crate::gasometer::GasCost, _target: StorageTarget) -> Result<(), ExitError> { Ok(()) } @@ -1299,6 +1300,8 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler let gasometer = &mut self.state.metadata_mut().gasometer; gasometer.record_dynamic_cost(gas_cost, memory_cost)?; + + #[cfg(feature = "with-substrate")] self.state.record_external_dynamic_opcode_cost(opcode, gas_cost, target)?; match target { From 5b2c10c32135f592a752addaf9036a052fac2d5b Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 8 Jun 2023 10:18:08 +0200 Subject: [PATCH 12/17] fmt --- runtime/src/eval/system.rs | 11 +++++------ src/backend/memory.rs | 5 +++-- src/executor/stack/executor.rs | 28 ++++++++++++++++++++++------ src/executor/stack/precompile.rs | 6 +++++- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 0706c7193..2e6c5db48 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -125,12 +125,11 @@ pub fn extcodecopy(runtime: &mut Runtime, handler: &mut H) -> Contro Ok(code) => code, Err(e) => return Control::Exit(e.into()), }; - match runtime.machine.memory_mut().copy_large( - memory_offset, - code_offset, - len, - &code, - ) { + match runtime + .machine + .memory_mut() + .copy_large(memory_offset, code_offset, len, &code) + { Ok(()) => (), Err(e) => return Control::Exit(e.into()), }; diff --git a/src/backend/memory.rs b/src/backend/memory.rs index 520d5c397..3dc5b06d3 100644 --- a/src/backend/memory.rs +++ b/src/backend/memory.rs @@ -1,8 +1,8 @@ use super::{Apply, ApplyBackend, Backend, Basic, Log}; +use crate::ExitError; use alloc::collections::BTreeMap; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; -use crate::ExitError; /// Vicinity value of a memory backend. #[derive(Clone, Debug, Eq, PartialEq)] @@ -145,7 +145,8 @@ impl<'vicinity> Backend for MemoryBackend<'vicinity> { } fn code(&mut self, address: H160) -> Result, ExitError> { - Ok(self.state + Ok(self + .state .get(&address) .map(|v| v.code.clone()) .unwrap_or_default()) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index c37f17813..3950364f3 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -227,16 +227,27 @@ pub trait StackState<'config>: Backend { /// can be customized to use a more performant approach that don't need to /// fetch the code. fn code_hash(&mut self, address: H160) -> Result { - Ok(H256::from_slice(Keccak256::digest(&self.code(address)?).as_slice())) + Ok(H256::from_slice( + Keccak256::digest(&self.code(address)?).as_slice(), + )) } - + #[cfg(feature = "with-substrate")] - fn record_external_dynamic_opcode_cost(&mut self, _opcode: Opcode, _gas_cost: crate::gasometer::GasCost, _target: StorageTarget) -> Result<(), ExitError> { + fn record_external_dynamic_opcode_cost( + &mut self, + _opcode: Opcode, + _gas_cost: crate::gasometer::GasCost, + _target: StorageTarget, + ) -> Result<(), ExitError> { Ok(()) } #[cfg(feature = "with-substrate")] - fn record_external_cost(&mut self, _ref_time: Option, _proof_size: Option) -> Result<(), ExitError> { + fn record_external_cost( + &mut self, + _ref_time: Option, + _proof_size: Option, + ) -> Result<(), ExitError> { Ok(()) } @@ -1302,7 +1313,8 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler gasometer.record_dynamic_cost(gas_cost, memory_cost)?; #[cfg(feature = "with-substrate")] - self.state.record_external_dynamic_opcode_cost(opcode, gas_cost, target)?; + self.state + .record_external_dynamic_opcode_cost(opcode, gas_cost, target)?; match target { StorageTarget::Address(address) => { @@ -1425,7 +1437,11 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr #[cfg(feature = "with-substrate")] /// Record Substrate specific cost. - fn record_external_cost(&mut self, ref_time: Option, proof_size: Option) -> Result<(), ExitError> { + fn record_external_cost( + &mut self, + ref_time: Option, + proof_size: Option, + ) -> Result<(), ExitError> { self.executor .state .record_external_cost(ref_time, proof_size) diff --git a/src/executor/stack/precompile.rs b/src/executor/stack/precompile.rs index 6ab3a9a8f..2baf30e6d 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -52,7 +52,11 @@ pub trait PrecompileHandle { #[cfg(feature = "with-substrate")] /// Record Substrate specific cost. - fn record_external_cost(&mut self, ref_time: Option, proof_size: Option) -> Result<(), ExitError>; + fn record_external_cost( + &mut self, + ref_time: Option, + proof_size: Option, + ) -> Result<(), ExitError>; #[cfg(feature = "with-substrate")] /// Refund Substrate specific cost. From f9b8dd9dd30e26f36557b13c624855f31fc352cf Mon Sep 17 00:00:00 2001 From: tgmichel Date: Thu, 8 Jun 2023 11:01:35 +0200 Subject: [PATCH 13/17] clippy --- core/src/stack.rs | 4 ++-- src/executor/stack/executor.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/stack.rs b/core/src/stack.rs index ed72631c2..893af517c 100644 --- a/core/src/stack.rs +++ b/core/src/stack.rs @@ -32,8 +32,8 @@ impl Stack { #[inline] /// Whether the stack is empty. - pub fn is_empty(&self) -> Result { - Ok(self.data.is_empty()) + pub fn is_empty(&self) -> bool { + self.data.is_empty() } #[inline] diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 3950364f3..b40fadb6c 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -228,7 +228,7 @@ pub trait StackState<'config>: Backend { /// fetch the code. fn code_hash(&mut self, address: H160) -> Result { Ok(H256::from_slice( - Keccak256::digest(&self.code(address)?).as_slice(), + Keccak256::digest(self.code(address)?).as_slice(), )) } From 1fe19d13c8115d35062b0ca368b01eb443103d2c Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 14 Jun 2023 14:07:11 +0200 Subject: [PATCH 14/17] `ExternalOperation` handling --- Cargo.toml | 5 +- core/src/external.rs | 13 +++++ core/src/lib.rs | 2 + gasometer/Cargo.toml | 1 + gasometer/src/lib.rs | 30 +++++++++-- runtime/Cargo.toml | 1 + runtime/src/eval/system.rs | 33 +++++++----- runtime/src/handler.rs | 11 ++-- src/backend/memory.rs | 8 ++- src/backend/mod.rs | 13 ++++- src/executor/stack/executor.rs | 91 +++++++++++++++++++++------------- src/executor/stack/memory.rs | 29 ++++++----- 12 files changed, 159 insertions(+), 78 deletions(-) create mode 100644 core/src/external.rs diff --git a/Cargo.toml b/Cargo.toml index 8a091bdac..8f6b9572f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,7 +64,10 @@ with-serde = [ "evm-core/with-serde", "ethereum/with-serde", ] -with-substrate = [] +with-substrate = [ + "evm-gasometer/with-substrate", + "evm-runtime/with-substrate" +] tracing = [ "environmental", "evm-gasometer/tracing", diff --git a/core/src/external.rs b/core/src/external.rs new file mode 100644 index 000000000..85a98a6c5 --- /dev/null +++ b/core/src/external.rs @@ -0,0 +1,13 @@ +use primitive_types::H160; + +/// Operations for recording external costs +pub enum ExternalOperation { + /// Reading basic account from storage. Fixed size. + AccountBasicRead, + /// Reading address code from storage. Dynamic size. + AddressCodeRead(H160), + /// Basic check for account emptiness. Fixed size. + IsEmpty, + /// Writing to storage. Fixed size. + Write, +} diff --git a/core/src/lib.rs b/core/src/lib.rs index 0dd187c54..20e8fc053 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -8,6 +8,7 @@ extern crate alloc; mod error; mod eval; +mod external; mod memory; mod opcode; mod stack; @@ -15,6 +16,7 @@ mod utils; mod valids; pub use crate::error::{Capture, ExitError, ExitFatal, ExitReason, ExitRevert, ExitSucceed, Trap}; +pub use crate::external::ExternalOperation; pub use crate::memory::Memory; pub use crate::opcode::Opcode; pub use crate::stack::Stack; diff --git a/gasometer/Cargo.toml b/gasometer/Cargo.toml index 4bf14bcf5..42a09ba17 100644 --- a/gasometer/Cargo.toml +++ b/gasometer/Cargo.toml @@ -26,3 +26,4 @@ std = [ tracing = [ "environmental", ] +with-substrate = [] diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 25d56b922..ebc28818e 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -510,7 +510,11 @@ pub fn dynamic_opcode_cost( value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target)?, + target_exists: { + #[cfg(feature = "with-substrate")] + handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; + handler.exists(target) + }, } } Opcode::STATICCALL => { @@ -519,7 +523,11 @@ pub fn dynamic_opcode_cost( GasCost::StaticCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target)?, + target_exists: { + #[cfg(feature = "with-substrate")] + handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; + handler.exists(target) + }, } } Opcode::SHA3 => GasCost::Sha3 { @@ -553,7 +561,11 @@ pub fn dynamic_opcode_cost( GasCost::DelegateCall { gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target)?, + target_exists: { + #[cfg(feature = "with-substrate")] + handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; + handler.exists(target) + }, } } Opcode::DELEGATECALL => GasCost::Invalid(opcode), @@ -606,7 +618,11 @@ pub fn dynamic_opcode_cost( GasCost::Suicide { value: handler.balance(address), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target)?, + target_exists: { + #[cfg(feature = "with-substrate")] + handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; + handler.exists(target) + }, already_removed: handler.deleted(address), } } @@ -620,7 +636,11 @@ pub fn dynamic_opcode_cost( value: U256::from_big_endian(&stack.peek(2)?[..]), gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, - target_exists: handler.exists(target)?, + target_exists: { + #[cfg(feature = "with-substrate")] + handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; + handler.exists(target) + }, } } diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 867264196..90e447ffb 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -27,3 +27,4 @@ std = [ tracing = [ "environmental", ] +with-substrate = [] diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 2e6c5db48..30a97db9d 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -92,10 +92,13 @@ pub fn base_fee(runtime: &mut Runtime, handler: &H) -> Control { pub fn extcodesize(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - let code_size = match handler.code_size(address.into()) { - Ok(v) => v, - Err(e) => return Control::Exit(e.into()), - }; + #[cfg(feature = "with-substrate")] + if let Err(e) = + handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) + { + return Control::Exit(e.into()); + } + let code_size = handler.code_size(address.into()); push_u256!(runtime, code_size); Control::Continue @@ -103,10 +106,13 @@ pub fn extcodesize(runtime: &mut Runtime, handler: &mut H) -> Contro pub fn extcodehash(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - let code_hash = match handler.code_hash(address.into()) { - Ok(v) => v, - Err(e) => return Control::Exit(e.into()), - }; + #[cfg(feature = "with-substrate")] + if let Err(e) = + handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) + { + return Control::Exit(e.into()); + } + let code_hash = handler.code_hash(address.into()); push!(runtime, code_hash); Control::Continue @@ -121,10 +127,13 @@ pub fn extcodecopy(runtime: &mut Runtime, handler: &mut H) -> Contro .memory_mut() .resize_offset(memory_offset, len)); - let code = match handler.code(address.into()) { - Ok(code) => code, - Err(e) => return Control::Exit(e.into()), - }; + #[cfg(feature = "with-substrate")] + if let Err(e) = + handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) + { + return Control::Exit(e.into()); + } + let code = handler.code(address.into()); match runtime .machine .memory_mut() diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index 92476b767..bc10e9a87 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -28,11 +28,11 @@ pub trait Handler { /// Get balance of address. fn balance(&self, address: H160) -> U256; /// Get code size of address. - fn code_size(&mut self, address: H160) -> Result; + fn code_size(&self, address: H160) -> U256; /// Get code hash of address. - fn code_hash(&mut self, address: H160) -> Result; + fn code_hash(&self, address: H160) -> H256; /// Get code of address. - fn code(&mut self, address: H160) -> Result, ExitError>; + fn code(&self, address: H160) -> Vec; /// Get storage value of address at index. fn storage(&self, address: H160, index: H256) -> H256; /// Get original storage value of address at index. @@ -64,7 +64,7 @@ pub trait Handler { fn chain_id(&self) -> U256; /// Check whether an address exists. - fn exists(&mut self, address: H160) -> Result; + fn exists(&self, address: H160) -> bool; /// Check whether an address has already been deleted. fn deleted(&self, address: H160) -> bool; /// Checks if the address or (address, index) pair has been previously accessed @@ -120,4 +120,7 @@ pub trait Handler { fn other(&mut self, opcode: Opcode, _stack: &mut Machine) -> Result<(), ExitError> { Err(ExitError::InvalidCode(opcode)) } + + #[cfg(feature = "with-substrate")] + fn record_external_operation(&mut self, op: crate::ExternalOperation) -> Result<(), ExitError>; } diff --git a/src/backend/memory.rs b/src/backend/memory.rs index 3dc5b06d3..37e7718b1 100644 --- a/src/backend/memory.rs +++ b/src/backend/memory.rs @@ -1,5 +1,4 @@ use super::{Apply, ApplyBackend, Backend, Basic, Log}; -use crate::ExitError; use alloc::collections::BTreeMap; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; @@ -144,12 +143,11 @@ impl<'vicinity> Backend for MemoryBackend<'vicinity> { .unwrap_or_default() } - fn code(&mut self, address: H160) -> Result, ExitError> { - Ok(self - .state + fn code(&self, address: H160) -> Vec { + self.state .get(&address) .map(|v| v.code.clone()) - .unwrap_or_default()) + .unwrap_or_default() } fn storage(&self, address: H160, index: H256) -> H256 { diff --git a/src/backend/mod.rs b/src/backend/mod.rs index ea42ab8b7..014cedc2b 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -5,10 +5,11 @@ mod memory; pub use self::memory::{MemoryAccount, MemoryBackend, MemoryVicinity}; -use crate::ExitError; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; +#[cfg(feature = "with-substrate")] +use crate::ExitError; /// Basic account information. #[derive(Clone, Eq, PartialEq, Debug, Default)] #[cfg_attr( @@ -80,11 +81,19 @@ pub trait Backend { /// Get basic account information. fn basic(&self, address: H160) -> Basic; /// Get account code. - fn code(&mut self, address: H160) -> Result, ExitError>; + fn code(&self, address: H160) -> Vec; /// Get storage value of address at index. fn storage(&self, address: H160, index: H256) -> H256; /// Get original storage value of address at index, if available. fn original_storage(&self, address: H160, index: H256) -> Option; + + #[cfg(feature = "with-substrate")] + fn record_external_operation( + &mut self, + _op: crate::ExternalOperation, + ) -> Result<(), ExitError> { + Ok(()) + } } /// EVM backend that can apply changes. diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index b40fadb6c..7a877d532 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -199,7 +199,7 @@ pub trait StackState<'config>: Backend { fn exit_revert(&mut self) -> Result<(), ExitError>; fn exit_discard(&mut self) -> Result<(), ExitError>; - fn is_empty(&mut self, address: H160) -> Result; + fn is_empty(&self, address: H160) -> bool; fn deleted(&self, address: H160) -> bool; fn is_cold(&self, address: H160) -> bool; fn is_storage_cold(&self, address: H160, key: H256) -> bool; @@ -209,7 +209,7 @@ pub trait StackState<'config>: Backend { fn reset_storage(&mut self, address: H160); fn log(&mut self, address: H160, topics: Vec, data: Vec); fn set_deleted(&mut self, address: H160); - fn set_code(&mut self, address: H160, code: Vec) -> Result<(), ExitError>; + fn set_code(&mut self, address: H160, code: Vec); fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError>; fn reset_balance(&mut self, address: H160); fn touch(&mut self, address: H160); @@ -218,18 +218,16 @@ pub trait StackState<'config>: Backend { /// Provide a default implementation by fetching the code, but /// can be customized to use a more performant approach that don't need to /// fetch the code. - fn code_size(&mut self, address: H160) -> Result { - Ok(U256::from(self.code(address)?.len())) + fn code_size(&self, address: H160) -> U256 { + U256::from(self.code(address).len()) } /// Fetch the code hash of an address. /// Provide a default implementation by fetching the code, but /// can be customized to use a more performant approach that don't need to /// fetch the code. - fn code_hash(&mut self, address: H160) -> Result { - Ok(H256::from_slice( - Keccak256::digest(self.code(address)?).as_slice(), - )) + fn code_hash(&self, address: H160) -> H256 { + H256::from_slice(Keccak256::digest(self.code(address)).as_slice()) } #[cfg(feature = "with-substrate")] @@ -589,7 +587,10 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.initialize_with_access_list(access_list); } - + #[cfg(feature = "with-substrate")] + if let Err(e) = self.record_external_operation(crate::ExternalOperation::AccountBasicRead) { + return (e.into(), Vec::new()); + } if let Err(e) = self.state.inc_nonce(caller) { return (e.into(), Vec::new()); } @@ -726,6 +727,10 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> return Capture::Exit((ExitError::OutOfFund.into(), None, Vec::new())); } + #[cfg(feature = "with-substrate")] + if let Err(e) = self.record_external_operation(crate::ExternalOperation::AccountBasicRead) { + return Capture::Exit((ExitReason::Error(e), None, Vec::new())); + } if let Err(e) = self.state.inc_nonce(caller) { return Capture::Exit((e.into(), None, Vec::new())); } @@ -751,10 +756,14 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.enter_substate(gas_limit, false); { - let code_size = match self.code_size(address) { - Ok(v) => v, - Err(e) => return Capture::Exit((e.into(), None, Vec::new())), - }; + #[cfg(feature = "with-substrate")] + if let Err(e) = + self.record_external_operation(crate::ExternalOperation::AddressCodeRead(address)) + { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitReason::Error(e), None, Vec::new())); + } + let code_size = self.code_size(address); if code_size != U256::zero() { let _ = self.exit_substate(StackExitKind::Failed); return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new())); @@ -787,6 +796,13 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } if self.config.create_increase_nonce { + #[cfg(feature = "with-substrate")] + if let Err(e) = + self.record_external_operation(crate::ExternalOperation::AccountBasicRead) + { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitReason::Error(e), None, Vec::new())); + } if let Err(e) = self.state.inc_nonce(address) { return Capture::Exit((e.into(), None, Vec::new())); } @@ -867,14 +883,14 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.enter_substate(gas_limit, is_static); self.state.touch(context.address); - let code = match self.code(code_address) { - Ok(code) => code, - Err(e) => { - let _ = self.exit_substate(StackExitKind::Failed); - return Capture::Exit((ExitReason::Error(e), Vec::new())); - } - }; - + #[cfg(feature = "with-substrate")] + if let Err(e) = + self.record_external_operation(crate::ExternalOperation::AddressCodeRead(code_address)) + { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitReason::Error(e), Vec::new())); + } + let code = self.code(code_address); if let Some(depth) = self.state.metadata().depth { if depth > self.config.call_stack_limit { let _ = self.exit_substate(StackExitKind::Reverted); @@ -988,9 +1004,14 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> { Ok(()) => { let exit_result = self.exit_substate(StackExitKind::Succeeded); - if let Err(e) = self.state.set_code(address, out) { + + #[cfg(feature = "with-substrate")] + if let Err(e) = + self.record_external_operation(crate::ExternalOperation::Write) + { return (e.into(), None, Vec::new()); } + self.state.set_code(address, out); if let Err(e) = exit_result { return (e.into(), None, Vec::new()); } @@ -1063,19 +1084,19 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler self.state.basic(address).balance } - fn code_size(&mut self, address: H160) -> Result { + fn code_size(&self, address: H160) -> U256 { self.state.code_size(address) } - fn code_hash(&mut self, address: H160) -> Result { - if !self.exists(address)? { - return Ok(H256::default()); + fn code_hash(&self, address: H160) -> H256 { + if !self.exists(address) { + return H256::default(); } self.state.code_hash(address) } - fn code(&mut self, address: H160) -> Result, ExitError> { + fn code(&self, address: H160) -> Vec { self.state.code(address) } @@ -1089,11 +1110,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler .unwrap_or_default() } - fn exists(&mut self, address: H160) -> Result { + fn exists(&self, address: H160) -> bool { if self.config.empty_considered_exists { - Ok(self.state.exists(address)) + self.state.exists(address) } else { - Ok(self.state.exists(address) && !self.state.is_empty(address)?) + self.state.exists(address) && !self.state.is_empty(address) } } @@ -1329,6 +1350,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler Ok(()) } + + #[cfg(feature = "with-substrate")] + fn record_external_operation(&mut self, op: crate::ExternalOperation) -> Result<(), ExitError> { + self.state.record_external_operation(op) + } } struct StackExecutorHandle<'inner, 'config, 'precompiles, S, P> { @@ -1363,10 +1389,7 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr Err(err) => return (ExitReason::Error(err), Vec::new()), }; - let target_exists = match self.executor.exists(code_address) { - Ok(x) => x, - Err(err) => return (ExitReason::Error(err), Vec::new()), - }; + let target_exists = self.executor.exists(code_address); let gas_cost = crate::gasometer::GasCost::Call { value: transfer.clone().map(|x| x.value).unwrap_or_else(U256::zero), diff --git a/src/executor/stack/memory.rs b/src/executor/stack/memory.rs index 6eb9d4c8f..d9658f033 100644 --- a/src/executor/stack/memory.rs +++ b/src/executor/stack/memory.rs @@ -192,26 +192,26 @@ impl<'config> MemoryStackSubstate<'config> { self.known_account(address).and_then(|acc| acc.code.clone()) } - pub fn known_empty(&self, address: H160) -> Result, ExitError> { + pub fn known_empty(&self, address: H160) -> Option { if let Some(account) = self.known_account(address) { if account.basic.balance != U256::zero() { - return Ok(Some(false)); + return Some(false); } if account.basic.nonce != U256::zero() { - return Ok(Some(false)); + return Some(false); } if let Some(code) = &account.code { - return Ok(Some( + return Some( account.basic.balance == U256::zero() && account.basic.nonce == U256::zero() && code.is_empty(), - )); + ); } } - Ok(None) + None } pub fn known_storage(&self, address: H160, key: H256) -> Option { @@ -450,9 +450,9 @@ impl<'backend, 'config, B: Backend> Backend for MemoryStackState<'backend, 'conf .unwrap_or_else(|| self.backend.basic(address)) } - fn code(&mut self, address: H160) -> Result, ExitError> { + fn code(&self, address: H160) -> Vec { if let Some(code) = self.substate.known_code(address) { - return Ok(code); + return code; } self.backend.code(address) } @@ -497,14 +497,14 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.exit_discard() } - fn is_empty(&mut self, address: H160) -> Result { - if let Some(known_empty) = self.substate.known_empty(address)? { - return Ok(known_empty); + fn is_empty(&self, address: H160) -> bool { + if let Some(known_empty) = self.substate.known_empty(address) { + return known_empty; } - Ok(self.backend.basic(address).balance == U256::zero() + self.backend.basic(address).balance == U256::zero() && self.backend.basic(address).nonce == U256::zero() - && self.backend.code(address)?.len() == 0) + && self.backend.code(address).len() == 0 } fn deleted(&self, address: H160) -> bool { @@ -539,9 +539,8 @@ impl<'backend, 'config, B: Backend> StackState<'config> for MemoryStackState<'ba self.substate.set_deleted(address) } - fn set_code(&mut self, address: H160, code: Vec) -> Result<(), ExitError> { + fn set_code(&mut self, address: H160, code: Vec) { self.substate.set_code(address, code, self.backend); - Ok(()) } fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError> { From 6171ea31581de7ebb14eea4388d8fdbabf563398 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 21 Jun 2023 09:46:15 +0200 Subject: [PATCH 15/17] remove `with-substrate` feature --- .github/workflows/rust.yml | 2 -- Cargo.toml | 4 ---- gasometer/Cargo.toml | 1 - gasometer/src/lib.rs | 5 ----- runtime/Cargo.toml | 1 - runtime/src/eval/system.rs | 3 --- runtime/src/handler.rs | 2 +- src/backend/mod.rs | 3 --- src/executor/stack/executor.rs | 13 ------------- src/executor/stack/precompile.rs | 2 -- 10 files changed, 1 insertion(+), 35 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7303dbe91..5256c1077 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -26,8 +26,6 @@ jobs: run: cargo build --verbose - name: Build for feature (tracing) run: cargo build --features tracing --verbose - - name: Build for feature (with-substrate) - run: cargo build --features with-substrate --verbose - name: Run tests run: cargo test --verbose jsontests: diff --git a/Cargo.toml b/Cargo.toml index 8f6b9572f..94ae9d9db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,10 +64,6 @@ with-serde = [ "evm-core/with-serde", "ethereum/with-serde", ] -with-substrate = [ - "evm-gasometer/with-substrate", - "evm-runtime/with-substrate" -] tracing = [ "environmental", "evm-gasometer/tracing", diff --git a/gasometer/Cargo.toml b/gasometer/Cargo.toml index 42a09ba17..4bf14bcf5 100644 --- a/gasometer/Cargo.toml +++ b/gasometer/Cargo.toml @@ -26,4 +26,3 @@ std = [ tracing = [ "environmental", ] -with-substrate = [] diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index ebc28818e..fe57722ad 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -511,7 +511,6 @@ pub fn dynamic_opcode_cost( gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, target_exists: { - #[cfg(feature = "with-substrate")] handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) }, @@ -524,7 +523,6 @@ pub fn dynamic_opcode_cost( gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, target_exists: { - #[cfg(feature = "with-substrate")] handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) }, @@ -562,7 +560,6 @@ pub fn dynamic_opcode_cost( gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, target_exists: { - #[cfg(feature = "with-substrate")] handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) }, @@ -619,7 +616,6 @@ pub fn dynamic_opcode_cost( value: handler.balance(address), target_is_cold: handler.is_cold(target, None)?, target_exists: { - #[cfg(feature = "with-substrate")] handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) }, @@ -637,7 +633,6 @@ pub fn dynamic_opcode_cost( gas: U256::from_big_endian(&stack.peek(0)?[..]), target_is_cold: handler.is_cold(target, None)?, target_exists: { - #[cfg(feature = "with-substrate")] handler.record_external_operation(evm_core::ExternalOperation::IsEmpty)?; handler.exists(target) }, diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 90e447ffb..867264196 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -27,4 +27,3 @@ std = [ tracing = [ "environmental", ] -with-substrate = [] diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index 30a97db9d..adf6bffd1 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -92,7 +92,6 @@ pub fn base_fee(runtime: &mut Runtime, handler: &H) -> Control { pub fn extcodesize(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - #[cfg(feature = "with-substrate")] if let Err(e) = handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) { @@ -106,7 +105,6 @@ pub fn extcodesize(runtime: &mut Runtime, handler: &mut H) -> Contro pub fn extcodehash(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, address); - #[cfg(feature = "with-substrate")] if let Err(e) = handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) { @@ -127,7 +125,6 @@ pub fn extcodecopy(runtime: &mut Runtime, handler: &mut H) -> Contro .memory_mut() .resize_offset(memory_offset, len)); - #[cfg(feature = "with-substrate")] if let Err(e) = handler.record_external_operation(crate::ExternalOperation::AddressCodeRead(address.into())) { diff --git a/runtime/src/handler.rs b/runtime/src/handler.rs index bc10e9a87..10eb976c7 100644 --- a/runtime/src/handler.rs +++ b/runtime/src/handler.rs @@ -121,6 +121,6 @@ pub trait Handler { Err(ExitError::InvalidCode(opcode)) } - #[cfg(feature = "with-substrate")] + /// Records some associated `ExternalOperation`. fn record_external_operation(&mut self, op: crate::ExternalOperation) -> Result<(), ExitError>; } diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 014cedc2b..82d00e475 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -7,8 +7,6 @@ mod memory; pub use self::memory::{MemoryAccount, MemoryBackend, MemoryVicinity}; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; - -#[cfg(feature = "with-substrate")] use crate::ExitError; /// Basic account information. #[derive(Clone, Eq, PartialEq, Debug, Default)] @@ -87,7 +85,6 @@ pub trait Backend { /// Get original storage value of address at index, if available. fn original_storage(&self, address: H160, index: H256) -> Option; - #[cfg(feature = "with-substrate")] fn record_external_operation( &mut self, _op: crate::ExternalOperation, diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index 7a877d532..d20fac77a 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -230,7 +230,6 @@ pub trait StackState<'config>: Backend { H256::from_slice(Keccak256::digest(self.code(address)).as_slice()) } - #[cfg(feature = "with-substrate")] fn record_external_dynamic_opcode_cost( &mut self, _opcode: Opcode, @@ -240,7 +239,6 @@ pub trait StackState<'config>: Backend { Ok(()) } - #[cfg(feature = "with-substrate")] fn record_external_cost( &mut self, _ref_time: Option, @@ -249,7 +247,6 @@ pub trait StackState<'config>: Backend { Ok(()) } - #[cfg(feature = "with-substrate")] fn refund_external_cost(&mut self, _ref_time: Option, _proof_size: Option) {} } @@ -587,7 +584,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.initialize_with_access_list(access_list); } - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::AccountBasicRead) { return (e.into(), Vec::new()); } @@ -727,7 +723,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> return Capture::Exit((ExitError::OutOfFund.into(), None, Vec::new())); } - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::AccountBasicRead) { return Capture::Exit((ExitReason::Error(e), None, Vec::new())); } @@ -756,7 +751,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.enter_substate(gas_limit, false); { - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::AddressCodeRead(address)) { @@ -796,7 +790,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } if self.config.create_increase_nonce { - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::AccountBasicRead) { @@ -883,7 +876,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> self.enter_substate(gas_limit, is_static); self.state.touch(context.address); - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::AddressCodeRead(code_address)) { @@ -1005,7 +997,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Ok(()) => { let exit_result = self.exit_substate(StackExitKind::Succeeded); - #[cfg(feature = "with-substrate")] if let Err(e) = self.record_external_operation(crate::ExternalOperation::Write) { @@ -1333,7 +1324,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler let gasometer = &mut self.state.metadata_mut().gasometer; gasometer.record_dynamic_cost(gas_cost, memory_cost)?; - #[cfg(feature = "with-substrate")] self.state .record_external_dynamic_opcode_cost(opcode, gas_cost, target)?; @@ -1351,7 +1341,6 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler Ok(()) } - #[cfg(feature = "with-substrate")] fn record_external_operation(&mut self, op: crate::ExternalOperation) -> Result<(), ExitError> { self.state.record_external_operation(op) } @@ -1458,7 +1447,6 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr .record_cost(cost) } - #[cfg(feature = "with-substrate")] /// Record Substrate specific cost. fn record_external_cost( &mut self, @@ -1470,7 +1458,6 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr .record_external_cost(ref_time, proof_size) } - #[cfg(feature = "with-substrate")] /// Refund Substrate specific cost. fn refund_external_cost(&mut self, ref_time: Option, proof_size: Option) { self.executor diff --git a/src/executor/stack/precompile.rs b/src/executor/stack/precompile.rs index 2baf30e6d..556df901a 100644 --- a/src/executor/stack/precompile.rs +++ b/src/executor/stack/precompile.rs @@ -50,7 +50,6 @@ pub trait PrecompileHandle { /// Record cost to the Runtime gasometer. fn record_cost(&mut self, cost: u64) -> Result<(), ExitError>; - #[cfg(feature = "with-substrate")] /// Record Substrate specific cost. fn record_external_cost( &mut self, @@ -58,7 +57,6 @@ pub trait PrecompileHandle { proof_size: Option, ) -> Result<(), ExitError>; - #[cfg(feature = "with-substrate")] /// Refund Substrate specific cost. fn refund_external_cost(&mut self, ref_time: Option, proof_size: Option); From 1e497c5e7dd0d41f1a6ccdd99d541872e26b8b88 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 21 Jun 2023 09:51:31 +0200 Subject: [PATCH 16/17] add missing `AccountBasicRead` to `call_inner` --- src/executor/stack/executor.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/executor/stack/executor.rs b/src/executor/stack/executor.rs index d20fac77a..856e6fe2b 100644 --- a/src/executor/stack/executor.rs +++ b/src/executor/stack/executor.rs @@ -891,6 +891,12 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> } if let Some(transfer) = transfer { + if let Err(e) = + self.record_external_operation(crate::ExternalOperation::AccountBasicRead) + { + let _ = self.exit_substate(StackExitKind::Failed); + return Capture::Exit((ExitReason::Error(e), Vec::new())); + } match self.state.transfer(transfer) { Ok(()) => (), Err(e) => { From 53315e22da8494dbcb6b54a18e09775e956b7c73 Mon Sep 17 00:00:00 2001 From: tgmichel Date: Wed, 21 Jun 2023 11:34:49 +0200 Subject: [PATCH 17/17] fmt --- src/backend/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 82d00e475..2309c85e6 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -5,9 +5,9 @@ mod memory; pub use self::memory::{MemoryAccount, MemoryBackend, MemoryVicinity}; +use crate::ExitError; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; -use crate::ExitError; /// Basic account information. #[derive(Clone, Eq, PartialEq, Debug, Default)] #[cfg_attr(