From 18c5efb6395b8d324617832a6a114019db735a3d Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 4 Jun 2024 12:37:15 +0200 Subject: [PATCH 1/7] perf: avoid cloning precompiles twice --- .../revm/src/context/context_precompiles.rs | 34 ++++++++++++------- .../revm/src/handler/mainnet/pre_execution.rs | 6 ++-- crates/revm/src/optimism/handler_register.rs | 6 ++-- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/crates/revm/src/context/context_precompiles.rs b/crates/revm/src/context/context_precompiles.rs index a6fdc5e67e..6f18a9a37f 100644 --- a/crates/revm/src/context/context_precompiles.rs +++ b/crates/revm/src/context/context_precompiles.rs @@ -4,7 +4,7 @@ use crate::{ }; use core::ops::{Deref, DerefMut}; use dyn_clone::DynClone; -use revm_precompile::Precompiles; +use revm_precompile::{PrecompileSpecId, PrecompileWithAddress, Precompiles}; use std::{boxed::Box, sync::Arc}; use super::InnerEvmContext; @@ -36,22 +36,32 @@ pub struct ContextPrecompiles { inner: HashMap>, } +impl Extend<(Address, ContextPrecompile)> for ContextPrecompiles { + fn extend)>>(&mut self, iter: T) { + self.inner.extend(iter.into_iter().map(Into::into)) + } +} + +impl Extend for ContextPrecompiles { + fn extend>(&mut self, iter: T) { + self.inner.extend(iter.into_iter().map(|precompile| { + let (address, precompile) = precompile.into(); + (address, precompile.into()) + })); + } +} + impl ContextPrecompiles { - /// Returns precompiles addresses. + /// Creates a new precompiles at the given spec ID. #[inline] - pub fn addresses(&self) -> impl Iterator { - self.inner.keys() + pub fn new(spec_id: PrecompileSpecId) -> Self { + Precompiles::new(spec_id).into() } - /// Extends the precompiles with the given precompiles. - /// - /// Other precompiles with overwrite existing precompiles. + /// Returns precompiles addresses. #[inline] - pub fn extend( - &mut self, - other: impl IntoIterator)>>, - ) { - self.inner.extend(other.into_iter().map(Into::into)); + pub fn addresses(&self) -> impl Iterator { + self.inner.keys() } /// Call precompile and executes it. Returns the result of the precompile execution. diff --git a/crates/revm/src/handler/mainnet/pre_execution.rs b/crates/revm/src/handler/mainnet/pre_execution.rs index c0f4fe7bde..d3b9759dcf 100644 --- a/crates/revm/src/handler/mainnet/pre_execution.rs +++ b/crates/revm/src/handler/mainnet/pre_execution.rs @@ -3,7 +3,7 @@ //! They handle initial setup of the EVM, call loop and the final return of the EVM use crate::{ - precompile::{PrecompileSpecId, Precompiles}, + precompile::PrecompileSpecId, primitives::{ db::Database, Account, EVMError, Env, Spec, @@ -16,9 +16,7 @@ use crate::{ /// Main precompile load #[inline] pub fn load_precompiles() -> ContextPrecompiles { - Precompiles::new(PrecompileSpecId::from_spec_id(SPEC::SPEC_ID)) - .clone() - .into() + ContextPrecompiles::new(PrecompileSpecId::from_spec_id(SPEC::SPEC_ID)) } /// Main load handle diff --git a/crates/revm/src/optimism/handler_register.rs b/crates/revm/src/optimism/handler_register.rs index d4996cca21..899ba41279 100644 --- a/crates/revm/src/optimism/handler_register.rs +++ b/crates/revm/src/optimism/handler_register.rs @@ -14,7 +14,7 @@ use crate::{ Context, ContextPrecompiles, FrameResult, }; use core::ops::Mul; -use revm_precompile::{secp256r1, PrecompileSpecId, Precompiles}; +use revm_precompile::{secp256r1, PrecompileSpecId}; use std::string::ToString; use std::sync::Arc; @@ -143,7 +143,7 @@ pub fn last_frame_return( /// Load precompiles for Optimism chain. #[inline] pub fn load_precompiles() -> ContextPrecompiles { - let mut precompiles = Precompiles::new(PrecompileSpecId::from_spec_id(SPEC::SPEC_ID)).clone(); + let mut precompiles = ContextPrecompiles::new(PrecompileSpecId::from_spec_id(SPEC::SPEC_ID)); if SPEC::enabled(SpecId::FJORD) { precompiles.extend([ @@ -152,7 +152,7 @@ pub fn load_precompiles() -> ContextPrecompiles Date: Tue, 4 Jun 2024 13:52:19 +0200 Subject: [PATCH 2/7] perf: cow it up --- crates/precompile/src/lib.rs | 4 +- crates/primitives/src/precompile.rs | 20 +- .../revm/src/context/context_precompiles.rs | 209 +++++++++++++----- crates/revm/src/context/evm_context.rs | 4 +- 4 files changed, 173 insertions(+), 64 deletions(-) diff --git a/crates/precompile/src/lib.rs b/crates/precompile/src/lib.rs index 6f85a874d0..3620c2260d 100644 --- a/crates/precompile/src/lib.rs +++ b/crates/precompile/src/lib.rs @@ -168,13 +168,13 @@ impl Precompiles { /// Returns an iterator over the precompiles addresses. #[inline] - pub fn addresses(&self) -> impl Iterator { + pub fn addresses(&self) -> impl ExactSizeIterator { self.inner.keys() } /// Consumes the type and returns all precompile addresses. #[inline] - pub fn into_addresses(self) -> impl Iterator { + pub fn into_addresses(self) -> impl ExactSizeIterator { self.inner.into_keys() } diff --git a/crates/primitives/src/precompile.rs b/crates/primitives/src/precompile.rs index 275244f849..aa12bec74a 100644 --- a/crates/primitives/src/precompile.rs +++ b/crates/primitives/src/precompile.rs @@ -110,11 +110,25 @@ impl Precompile { /// Call the precompile with the given input and gas limit and return the result. pub fn call(&mut self, bytes: &Bytes, gas_price: u64, env: &Env) -> PrecompileResult { - match self { + match *self { + Precompile::Standard(p) => p(bytes, gas_price), + Precompile::Env(p) => p(bytes, gas_price, env), + Precompile::Stateful(ref p) => p.call(bytes, gas_price, env), + Precompile::StatefulMut(ref mut p) => p.call_mut(bytes, gas_price, env), + } + } + + /// Call the precompile with the given input and gas limit and return the result. + /// + /// # Panics + /// + /// Panics if the precompile is a mutable stateful precompile. + pub fn call_ref(&self, bytes: &Bytes, gas_price: u64, env: &Env) -> PrecompileResult { + match *self { Precompile::Standard(p) => p(bytes, gas_price), Precompile::Env(p) => p(bytes, gas_price, env), - Precompile::Stateful(p) => p.call(bytes, gas_price, env), - Precompile::StatefulMut(p) => p.call_mut(bytes, gas_price, env), + Precompile::Stateful(ref p) => p.call(bytes, gas_price, env), + Precompile::StatefulMut(_) => panic!("call_ref on mutable stateful precompile"), } } } diff --git a/crates/revm/src/context/context_precompiles.rs b/crates/revm/src/context/context_precompiles.rs index 6f18a9a37f..2418e0debe 100644 --- a/crates/revm/src/context/context_precompiles.rs +++ b/crates/revm/src/context/context_precompiles.rs @@ -1,15 +1,13 @@ +use super::InnerEvmContext; use crate::{ precompile::{Precompile, PrecompileResult}, primitives::{db::Database, Address, Bytes, HashMap}, }; -use core::ops::{Deref, DerefMut}; use dyn_clone::DynClone; use revm_precompile::{PrecompileSpecId, PrecompileWithAddress, Precompiles}; use std::{boxed::Box, sync::Arc}; -use super::InnerEvmContext; - -/// Precompile and its handlers. +/// A single precompile handler. pub enum ContextPrecompile { /// Ordinary precompiles Ordinary(Precompile), @@ -24,64 +22,167 @@ pub enum ContextPrecompile { impl Clone for ContextPrecompile { fn clone(&self) -> Self { match self { - Self::Ordinary(arg0) => Self::Ordinary(arg0.clone()), - Self::ContextStateful(arg0) => Self::ContextStateful(arg0.clone()), - Self::ContextStatefulMut(arg0) => Self::ContextStatefulMut(arg0.clone()), + Self::Ordinary(p) => Self::Ordinary(p.clone()), + Self::ContextStateful(p) => Self::ContextStateful(p.clone()), + Self::ContextStatefulMut(p) => Self::ContextStatefulMut(p.clone()), } } } -#[derive(Clone)] -pub struct ContextPrecompiles { - inner: HashMap>, +enum PrecompilesCow { + /// Default precompiles, returned by `Precompiles::new`. Used to fast-path the default case. + Default(&'static Precompiles), + Owned(HashMap>), } -impl Extend<(Address, ContextPrecompile)> for ContextPrecompiles { - fn extend)>>(&mut self, iter: T) { - self.inner.extend(iter.into_iter().map(Into::into)) +impl Clone for PrecompilesCow { + fn clone(&self) -> Self { + match *self { + PrecompilesCow::Default(p) => PrecompilesCow::Default(p), + PrecompilesCow::Owned(ref inner) => PrecompilesCow::Owned(inner.clone()), + } } } -impl Extend for ContextPrecompiles { - fn extend>(&mut self, iter: T) { - self.inner.extend(iter.into_iter().map(|precompile| { - let (address, precompile) = precompile.into(); - (address, precompile.into()) - })); +/// Precompiles context. +pub struct ContextPrecompiles { + inner: PrecompilesCow, +} + +impl Clone for ContextPrecompiles { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } } } impl ContextPrecompiles { - /// Creates a new precompiles at the given spec ID. + /// Creates a new precompiles context at the given spec ID. + /// + /// This is a cheap operation that does not allocate by reusing the global precompiles. #[inline] pub fn new(spec_id: PrecompileSpecId) -> Self { - Precompiles::new(spec_id).into() + Self::from_static_precompiles(Precompiles::new(spec_id)) + } + + /// Creates a new precompiles context from the given static precompiles. + /// + /// NOTE: The internal precompiles must not be `StatefulMut` or `call` will panic. + /// This is done because the default precompiles are not stateful. + #[inline] + pub fn from_static_precompiles(precompiles: &'static Precompiles) -> Self { + Self { + inner: PrecompilesCow::Default(precompiles), + } + } + + /// Creates a new precompiles context from the given precompiles. + #[inline] + pub fn from_precompiles(precompiles: HashMap>) -> Self { + Self { + inner: PrecompilesCow::Owned(precompiles), + } } /// Returns precompiles addresses. #[inline] - pub fn addresses(&self) -> impl Iterator { - self.inner.keys() + pub fn addresses(&self) -> impl ExactSizeIterator { + // SAFETY: `keys` does not touch values. + unsafe { self.fake_as_owned() }.keys() + } + + /// Returns `true` if the precompiles contains the given address. + #[inline] + pub fn contains(&self, address: &Address) -> bool { + // SAFETY: `contains_key` does not touch values. + unsafe { self.fake_as_owned() }.contains_key(address) + } + + /// View the internal precompiles map as the `Owned` variant. + /// + /// # Safety + /// + /// The resulting value type is wrong if this is `Default`, + /// but this works for methods that operate only on keys because both maps' internal value types + /// have the same size. + #[inline] + unsafe fn fake_as_owned(&self) -> &HashMap> { + const _: () = assert!( + core::mem::size_of::>() + == core::mem::size_of::() + ); + match self.inner { + PrecompilesCow::Default(inner) => unsafe { core::mem::transmute(inner) }, + PrecompilesCow::Owned(ref inner) => inner, + } } /// Call precompile and executes it. Returns the result of the precompile execution. - /// None if the precompile does not exist. + /// + /// Returns `None` if the precompile does not exist. #[inline] pub fn call( &mut self, - addess: Address, + address: &Address, bytes: &Bytes, gas_price: u64, evmctx: &mut InnerEvmContext, ) -> Option { - let precompile = self.inner.get_mut(&addess)?; + Some(match self.inner { + PrecompilesCow::Default(p) => p.get(address)?.call_ref(bytes, gas_price, &evmctx.env), + PrecompilesCow::Owned(ref mut owned) => match owned.get_mut(address)? { + ContextPrecompile::Ordinary(p) => p.call(bytes, gas_price, &evmctx.env), + ContextPrecompile::ContextStateful(p) => p.call(bytes, gas_price, evmctx), + ContextPrecompile::ContextStatefulMut(p) => p.call_mut(bytes, gas_price, evmctx), + }, + }) + } - match precompile { - ContextPrecompile::Ordinary(p) => Some(p.call(bytes, gas_price, &evmctx.env)), - ContextPrecompile::ContextStatefulMut(p) => Some(p.call_mut(bytes, gas_price, evmctx)), - ContextPrecompile::ContextStateful(p) => Some(p.call(bytes, gas_price, evmctx)), + /// Returns a mutable reference to the precompiles map. + /// + /// Clones the precompiles map if it is shared. + #[inline] + pub fn to_mut(&mut self) -> &mut HashMap> { + match self.inner { + PrecompilesCow::Default(_) => self.to_mut_slow(), + PrecompilesCow::Owned(ref mut inner) => inner, } } + + #[allow(clippy::wrong_self_convention)] + #[cold] + fn to_mut_slow(&mut self) -> &mut HashMap> { + let PrecompilesCow::Default(precompiles) = self.inner else { + unreachable!() + }; + self.inner = PrecompilesCow::Owned( + precompiles + .inner + .iter() + .map(|(k, v)| (*k, v.clone().into())) + .collect(), + ); + match self.inner { + PrecompilesCow::Default(_) => unreachable!(), + PrecompilesCow::Owned(ref mut inner) => inner, + } + } +} + +impl Extend<(Address, ContextPrecompile)> for ContextPrecompiles { + fn extend)>>(&mut self, iter: T) { + self.to_mut().extend(iter.into_iter().map(Into::into)) + } +} + +impl Extend for ContextPrecompiles { + fn extend>(&mut self, iter: T) { + self.to_mut().extend(iter.into_iter().map(|precompile| { + let (address, precompile) = precompile.into(); + (address, precompile.into()) + })); + } } impl Default for ContextPrecompiles { @@ -92,17 +193,9 @@ impl Default for ContextPrecompiles { } } -impl Deref for ContextPrecompiles { - type Target = HashMap>; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl DerefMut for ContextPrecompiles { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner +impl Default for PrecompilesCow { + fn default() -> Self { + Self::Owned(Default::default()) } } @@ -142,22 +235,24 @@ impl From for ContextPrecompile { } } -impl From for ContextPrecompiles { - fn from(p: Precompiles) -> Self { - ContextPrecompiles { - inner: p.inner.into_iter().map(|(k, v)| (k, v.into())).collect(), - } - } -} +#[cfg(test)] +mod tests { + use super::*; + use crate::db::EmptyDB; -impl From<&Precompiles> for ContextPrecompiles { - fn from(p: &Precompiles) -> Self { - ContextPrecompiles { - inner: p - .inner - .iter() - .map(|(&k, v)| (k, v.clone().into())) - .collect(), - } + #[test] + fn test_precompiles_context() { + let custom_address = Address::with_last_byte(0xff); + + let mut precompiles = ContextPrecompiles::::new(PrecompileSpecId::HOMESTEAD); + assert_eq!(precompiles.addresses().count(), 4); + assert!(matches!(precompiles.inner, PrecompilesCow::Default(_))); + assert!(!precompiles.contains(&custom_address)); + + let precompile = Precompile::Standard(|_, _| panic!()); + precompiles.extend([(custom_address, precompile.into())]); + assert_eq!(precompiles.addresses().count(), 5); + assert!(matches!(precompiles.inner, PrecompilesCow::Owned(_))); + assert!(precompiles.contains(&custom_address)); } } diff --git a/crates/revm/src/context/evm_context.rs b/crates/revm/src/context/evm_context.rs index 0cb969f6f6..f313abd751 100644 --- a/crates/revm/src/context/evm_context.rs +++ b/crates/revm/src/context/evm_context.rs @@ -105,7 +105,7 @@ impl EvmContext { #[inline] fn call_precompile( &mut self, - address: Address, + address: &Address, input_data: &Bytes, gas: Gas, ) -> Result, EVMError> { @@ -199,7 +199,7 @@ impl EvmContext { _ => {} }; - if let Some(result) = self.call_precompile(inputs.bytecode_address, &inputs.input, gas)? { + if let Some(result) = self.call_precompile(&inputs.bytecode_address, &inputs.input, gas)? { if matches!(result.result, return_ok!()) { self.journaled_state.checkpoint_commit(); } else { From 0d7d432c9d590573fcf764cd9050e7ad96114e6e Mon Sep 17 00:00:00 2001 From: rakita Date: Sat, 15 Jun 2024 18:43:35 +0200 Subject: [PATCH 3/7] pedantic changes, unsafe removed, nit renames --- crates/precompile/src/lib.rs | 34 +++++++- crates/primitives/src/precompile.rs | 4 +- .../revm/src/context/context_precompiles.rs | 85 ++++++++----------- crates/revm/src/context/evm_context.rs | 7 +- 4 files changed, 73 insertions(+), 57 deletions(-) diff --git a/crates/precompile/src/lib.rs b/crates/precompile/src/lib.rs index 3620c2260d..aec1abb0b7 100644 --- a/crates/precompile/src/lib.rs +++ b/crates/precompile/src/lib.rs @@ -26,6 +26,7 @@ use core::hash::Hash; use once_cell::race::OnceBox; #[doc(hidden)] pub use revm_primitives as primitives; +use revm_primitives::HashSet; pub use revm_primitives::{ precompile::{PrecompileError as Error, *}, Address, Bytes, HashMap, Log, B256, @@ -39,7 +40,9 @@ pub fn calc_linear_cost_u32(len: usize, base: u64, word: u64) -> u64 { #[derive(Clone, Default, Debug)] pub struct Precompiles { /// Precompiles. - pub inner: HashMap, + inner: HashMap, + /// Addresses of precompile. + addresses: HashSet
, } impl Precompiles { @@ -71,6 +74,11 @@ impl Precompiles { }) } + /// Returns inner HashMap of precompiles. + pub fn inner(&self) -> &HashMap { + &self.inner + } + /// Returns precompiles for Byzantium spec. pub fn byzantium() -> &'static Self { static INSTANCE: OnceBox = OnceBox::new(); @@ -206,11 +214,19 @@ impl Precompiles { self.inner.len() } + /// Returns the precompiles addresses as a set. + pub fn addresses_set(&self) -> &HashSet
{ + &self.addresses + } + /// Extends the precompiles with the given precompiles. /// /// Other precompiles with overwrite existing precompiles. + #[inline] pub fn extend(&mut self, other: impl IntoIterator) { - self.inner.extend(other.into_iter().map(Into::into)); + let items = other.into_iter().collect::>(); + self.addresses.extend(items.iter().map(|p| *p.address())); + self.inner.extend(items.into_iter().map(Into::into)); } } @@ -229,6 +245,20 @@ impl From for (Address, Precompile) { } } +impl PrecompileWithAddress { + /// Returns reference of address. + #[inline] + pub fn address(&self) -> &Address { + &self.0 + } + + /// Returns reference of precompile. + #[inline] + pub fn precompile(&self) -> &Precompile { + &self.1 + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd)] pub enum PrecompileSpecId { HOMESTEAD, diff --git a/crates/primitives/src/precompile.rs b/crates/primitives/src/precompile.rs index aa12bec74a..6f01c855fb 100644 --- a/crates/primitives/src/precompile.rs +++ b/crates/primitives/src/precompile.rs @@ -128,7 +128,9 @@ impl Precompile { Precompile::Standard(p) => p(bytes, gas_price), Precompile::Env(p) => p(bytes, gas_price, env), Precompile::Stateful(ref p) => p.call(bytes, gas_price, env), - Precompile::StatefulMut(_) => panic!("call_ref on mutable stateful precompile"), + Precompile::StatefulMut(_) => Err(PrecompileErrors::Fatal { + msg: "call_ref on mutable stateful precompile".to_string(), + }), } } } diff --git a/crates/revm/src/context/context_precompiles.rs b/crates/revm/src/context/context_precompiles.rs index 2418e0debe..6f80295eb4 100644 --- a/crates/revm/src/context/context_precompiles.rs +++ b/crates/revm/src/context/context_precompiles.rs @@ -1,7 +1,7 @@ use super::InnerEvmContext; use crate::{ precompile::{Precompile, PrecompileResult}, - primitives::{db::Database, Address, Bytes, HashMap}, + primitives::{db::Database, Address, Bytes, HashMap, HashSet}, }; use dyn_clone::DynClone; use revm_precompile::{PrecompileSpecId, PrecompileWithAddress, Precompiles}; @@ -31,32 +31,25 @@ impl Clone for ContextPrecompile { enum PrecompilesCow { /// Default precompiles, returned by `Precompiles::new`. Used to fast-path the default case. - Default(&'static Precompiles), + StaticRef(&'static Precompiles), Owned(HashMap>), } impl Clone for PrecompilesCow { fn clone(&self) -> Self { match *self { - PrecompilesCow::Default(p) => PrecompilesCow::Default(p), + PrecompilesCow::StaticRef(p) => PrecompilesCow::StaticRef(p), PrecompilesCow::Owned(ref inner) => PrecompilesCow::Owned(inner.clone()), } } } /// Precompiles context. +#[derive(Clone)] pub struct ContextPrecompiles { inner: PrecompilesCow, } -impl Clone for ContextPrecompiles { - fn clone(&self) -> Self { - Self { - inner: self.inner.clone(), - } - } -} - impl ContextPrecompiles { /// Creates a new precompiles context at the given spec ID. /// @@ -73,7 +66,7 @@ impl ContextPrecompiles { #[inline] pub fn from_static_precompiles(precompiles: &'static Precompiles) -> Self { Self { - inner: PrecompilesCow::Default(precompiles), + inner: PrecompilesCow::StaticRef(precompiles), } } @@ -85,36 +78,29 @@ impl ContextPrecompiles { } } + /// Returns precompiles addresses as a HashSet. + pub fn addresses_set(&self) -> HashSet
{ + match self.inner { + PrecompilesCow::StaticRef(inner) => inner.addresses_set().clone(), + PrecompilesCow::Owned(ref inner) => inner.keys().cloned().collect(), + } + } + /// Returns precompiles addresses. #[inline] - pub fn addresses(&self) -> impl ExactSizeIterator { - // SAFETY: `keys` does not touch values. - unsafe { self.fake_as_owned() }.keys() + pub fn addresses<'a>(&'a self) -> Box + 'a> { + match self.inner { + PrecompilesCow::StaticRef(inner) => Box::new(inner.addresses()), + PrecompilesCow::Owned(ref inner) => Box::new(inner.keys()), + } } /// Returns `true` if the precompiles contains the given address. #[inline] pub fn contains(&self, address: &Address) -> bool { - // SAFETY: `contains_key` does not touch values. - unsafe { self.fake_as_owned() }.contains_key(address) - } - - /// View the internal precompiles map as the `Owned` variant. - /// - /// # Safety - /// - /// The resulting value type is wrong if this is `Default`, - /// but this works for methods that operate only on keys because both maps' internal value types - /// have the same size. - #[inline] - unsafe fn fake_as_owned(&self) -> &HashMap> { - const _: () = assert!( - core::mem::size_of::>() - == core::mem::size_of::() - ); match self.inner { - PrecompilesCow::Default(inner) => unsafe { core::mem::transmute(inner) }, - PrecompilesCow::Owned(ref inner) => inner, + PrecompilesCow::StaticRef(inner) => inner.contains(address), + PrecompilesCow::Owned(ref inner) => inner.contains_key(address), } } @@ -130,7 +116,7 @@ impl ContextPrecompiles { evmctx: &mut InnerEvmContext, ) -> Option { Some(match self.inner { - PrecompilesCow::Default(p) => p.get(address)?.call_ref(bytes, gas_price, &evmctx.env), + PrecompilesCow::StaticRef(p) => p.get(address)?.call_ref(bytes, gas_price, &evmctx.env), PrecompilesCow::Owned(ref mut owned) => match owned.get_mut(address)? { ContextPrecompile::Ordinary(p) => p.call(bytes, gas_price, &evmctx.env), ContextPrecompile::ContextStateful(p) => p.call(bytes, gas_price, evmctx), @@ -144,29 +130,28 @@ impl ContextPrecompiles { /// Clones the precompiles map if it is shared. #[inline] pub fn to_mut(&mut self) -> &mut HashMap> { - match self.inner { - PrecompilesCow::Default(_) => self.to_mut_slow(), - PrecompilesCow::Owned(ref mut inner) => inner, - } - } + self.mutate_into_owned(); - #[allow(clippy::wrong_self_convention)] - #[cold] - fn to_mut_slow(&mut self) -> &mut HashMap> { - let PrecompilesCow::Default(precompiles) = self.inner else { + let PrecompilesCow::Owned(inner) = &mut self.inner else { unreachable!() }; + inner + } + + /// Mutates Self into Owned variant, or do nothing if it is already Owned. + /// Mutation will clone all precompiles. + #[inline] + pub fn mutate_into_owned(&mut self) { + let PrecompilesCow::StaticRef(precompiles) = self.inner else { + return; + }; self.inner = PrecompilesCow::Owned( precompiles - .inner + .inner() .iter() .map(|(k, v)| (*k, v.clone().into())) .collect(), ); - match self.inner { - PrecompilesCow::Default(_) => unreachable!(), - PrecompilesCow::Owned(ref mut inner) => inner, - } } } @@ -246,7 +231,7 @@ mod tests { let mut precompiles = ContextPrecompiles::::new(PrecompileSpecId::HOMESTEAD); assert_eq!(precompiles.addresses().count(), 4); - assert!(matches!(precompiles.inner, PrecompilesCow::Default(_))); + assert!(matches!(precompiles.inner, PrecompilesCow::StaticRef(_))); assert!(!precompiles.contains(&custom_address)); let precompile = Precompile::Standard(|_, _| panic!()); diff --git a/crates/revm/src/context/evm_context.rs b/crates/revm/src/context/evm_context.rs index f313abd751..1ff5f2cafe 100644 --- a/crates/revm/src/context/evm_context.rs +++ b/crates/revm/src/context/evm_context.rs @@ -7,7 +7,7 @@ use crate::{ interpreter::{ return_ok, CallInputs, Contract, Gas, InstructionResult, Interpreter, InterpreterResult, }, - primitives::{Address, Bytes, EVMError, Env, HashSet, U256}, + primitives::{Address, Bytes, EVMError, Env, U256}, ContextPrecompiles, FrameOrResult, CALL_STACK_LIMIT, }; use core::{ @@ -96,8 +96,7 @@ impl EvmContext { #[inline] pub fn set_precompiles(&mut self, precompiles: ContextPrecompiles) { // set warm loaded addresses. - self.journaled_state.warm_preloaded_addresses = - precompiles.addresses().copied().collect::>(); + self.journaled_state.warm_preloaded_addresses = precompiles.addresses_set(); self.precompiles = precompiles; } @@ -232,7 +231,7 @@ pub(crate) mod test_utils { use crate::{ db::{CacheDB, EmptyDB}, journaled_state::JournaledState, - primitives::{address, SpecId, B256}, + primitives::{address, HashSet, SpecId, B256}, }; /// Mock caller address. From 566f3e3d9dacaae44d1bcf1a9ae7cf6b1e601d87 Mon Sep 17 00:00:00 2001 From: rakita Date: Sat, 15 Jun 2024 18:50:55 +0200 Subject: [PATCH 4/7] into string --- crates/precompile/src/bls12_381/utils.rs | 1 - crates/precompile/src/lib.rs | 3 +-- crates/primitives/src/precompile.rs | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/precompile/src/bls12_381/utils.rs b/crates/precompile/src/bls12_381/utils.rs index ba18475c67..cd3f80ab16 100644 --- a/crates/precompile/src/bls12_381/utils.rs +++ b/crates/precompile/src/bls12_381/utils.rs @@ -1,5 +1,4 @@ use core::cmp::Ordering; - use blst::{ blst_bendian_from_fp, blst_fp, blst_fp_from_bendian, blst_scalar, blst_scalar_from_bendian, }; diff --git a/crates/precompile/src/lib.rs b/crates/precompile/src/lib.rs index aec1abb0b7..05b4a9f757 100644 --- a/crates/precompile/src/lib.rs +++ b/crates/precompile/src/lib.rs @@ -26,10 +26,9 @@ use core::hash::Hash; use once_cell::race::OnceBox; #[doc(hidden)] pub use revm_primitives as primitives; -use revm_primitives::HashSet; pub use revm_primitives::{ precompile::{PrecompileError as Error, *}, - Address, Bytes, HashMap, Log, B256, + Address, Bytes, HashMap, HashSet, Log, B256, }; use std::boxed::Box; diff --git a/crates/primitives/src/precompile.rs b/crates/primitives/src/precompile.rs index 6f01c855fb..a5525b25cc 100644 --- a/crates/primitives/src/precompile.rs +++ b/crates/primitives/src/precompile.rs @@ -129,7 +129,7 @@ impl Precompile { Precompile::Env(p) => p(bytes, gas_price, env), Precompile::Stateful(ref p) => p.call(bytes, gas_price, env), Precompile::StatefulMut(_) => Err(PrecompileErrors::Fatal { - msg: "call_ref on mutable stateful precompile".to_string(), + msg: "call_ref on mutable stateful precompile".into(), }), } } From b5b9e5c7fadbed88ae62e445d0b204c075e074cc Mon Sep 17 00:00:00 2001 From: rakita Date: Sat, 15 Jun 2024 20:24:05 +0200 Subject: [PATCH 5/7] nits --- crates/precompile/src/bls12_381/utils.rs | 2 +- crates/precompile/src/lib.rs | 2 +- crates/revm/src/context/context_precompiles.rs | 11 +++++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/precompile/src/bls12_381/utils.rs b/crates/precompile/src/bls12_381/utils.rs index cd3f80ab16..a2ed3cd885 100644 --- a/crates/precompile/src/bls12_381/utils.rs +++ b/crates/precompile/src/bls12_381/utils.rs @@ -1,7 +1,7 @@ -use core::cmp::Ordering; use blst::{ blst_bendian_from_fp, blst_fp, blst_fp_from_bendian, blst_scalar, blst_scalar_from_bendian, }; +use core::cmp::Ordering; use revm_primitives::PrecompileError; /// Number of bits used in the BLS12-381 curve finite field elements. diff --git a/crates/precompile/src/lib.rs b/crates/precompile/src/lib.rs index 05b4a9f757..e930f2cdfc 100644 --- a/crates/precompile/src/lib.rs +++ b/crates/precompile/src/lib.rs @@ -30,7 +30,7 @@ pub use revm_primitives::{ precompile::{PrecompileError as Error, *}, Address, Bytes, HashMap, HashSet, Log, B256, }; -use std::boxed::Box; +use std::{boxed::Box, vec::Vec}; pub fn calc_linear_cost_u32(len: usize, base: u64, word: u64) -> u64 { (len as u64 + 32 - 1) / 32 * word + base diff --git a/crates/revm/src/context/context_precompiles.rs b/crates/revm/src/context/context_precompiles.rs index 6f80295eb4..7edf684386 100644 --- a/crates/revm/src/context/context_precompiles.rs +++ b/crates/revm/src/context/context_precompiles.rs @@ -45,11 +45,18 @@ impl Clone for PrecompilesCow { } /// Precompiles context. -#[derive(Clone)] pub struct ContextPrecompiles { inner: PrecompilesCow, } +impl Clone for ContextPrecompiles { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + impl ContextPrecompiles { /// Creates a new precompiles context at the given spec ID. /// @@ -141,7 +148,7 @@ impl ContextPrecompiles { /// Mutates Self into Owned variant, or do nothing if it is already Owned. /// Mutation will clone all precompiles. #[inline] - pub fn mutate_into_owned(&mut self) { + fn mutate_into_owned(&mut self) { let PrecompilesCow::StaticRef(precompiles) = self.inner else { return; }; From fa38da59a94ce43a8b508e0342a860a3ed81fa39 Mon Sep 17 00:00:00 2001 From: rakita Date: Sun, 16 Jun 2024 02:27:27 +0200 Subject: [PATCH 6/7] Make to_mut call cold fn on owned mutation --- crates/revm/src/context/context_precompiles.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/revm/src/context/context_precompiles.rs b/crates/revm/src/context/context_precompiles.rs index 7edf684386..a47cef7132 100644 --- a/crates/revm/src/context/context_precompiles.rs +++ b/crates/revm/src/context/context_precompiles.rs @@ -137,17 +137,19 @@ impl ContextPrecompiles { /// Clones the precompiles map if it is shared. #[inline] pub fn to_mut(&mut self) -> &mut HashMap> { - self.mutate_into_owned(); + if let PrecompilesCow::StaticRef(_) = self.inner { + self.mutate_into_owned(); + } let PrecompilesCow::Owned(inner) = &mut self.inner else { - unreachable!() + unreachable!("self is mutated to Owned.") }; inner } /// Mutates Self into Owned variant, or do nothing if it is already Owned. /// Mutation will clone all precompiles. - #[inline] + #[cold] fn mutate_into_owned(&mut self) { let PrecompilesCow::StaticRef(precompiles) = self.inner else { return; From dc9b75e984e4ff13062343131f26310df5b304ec Mon Sep 17 00:00:00 2001 From: rakita Date: Mon, 17 Jun 2024 03:28:09 +0200 Subject: [PATCH 7/7] doc update --- crates/primitives/src/precompile.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/primitives/src/precompile.rs b/crates/primitives/src/precompile.rs index a5525b25cc..58f7cfe953 100644 --- a/crates/primitives/src/precompile.rs +++ b/crates/primitives/src/precompile.rs @@ -120,9 +120,7 @@ impl Precompile { /// Call the precompile with the given input and gas limit and return the result. /// - /// # Panics - /// - /// Panics if the precompile is a mutable stateful precompile. + /// Returns an error if the precompile is mutable. pub fn call_ref(&self, bytes: &Bytes, gas_price: u64, env: &Env) -> PrecompileResult { match *self { Precompile::Standard(p) => p(bytes, gas_price),