From 2c4cffc3464e6ac06f3485a642c5c32a17982e54 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Sun, 31 Jul 2022 02:23:38 +0200 Subject: [PATCH 1/9] build: use new revm with analysis cache --- Cargo.lock | 4 ---- Cargo.toml | 2 ++ anvil/src/eth/backend/db.rs | 6 +++--- anvil/src/eth/backend/mem/in_memory_db.rs | 18 ++++++++++++------ anvil/src/eth/backend/mem/mod.rs | 5 ++--- evm/src/coverage/visitor.rs | 2 +- evm/src/executor/backend/fuzz.rs | 9 ++++----- evm/src/executor/backend/in_memory_db.rs | 8 +++----- evm/src/executor/backend/mod.rs | 11 +++++------ evm/src/executor/fork/backend.rs | 13 ++++++++----- evm/src/executor/fork/cache.rs | 2 +- evm/src/executor/fork/database.rs | 9 ++++----- evm/src/executor/inspector/cheatcodes/env.rs | 4 ++-- evm/src/executor/inspector/cheatcodes/mod.rs | 2 +- evm/src/executor/inspector/coverage.rs | 6 ++++-- evm/src/executor/inspector/debugger.rs | 16 +++++++++------- evm/src/executor/inspector/tracer.rs | 2 +- evm/src/executor/mod.rs | 9 +++++++-- evm/src/fuzz/strategies/state.rs | 4 ++-- evm/src/utils.rs | 5 ++--- 20 files changed, 73 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 188f12c85947..e825bbb72cbf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4113,8 +4113,6 @@ dependencies = [ [[package]] name = "revm" version = "1.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455988f0a9920c91f192268ee31d79fcb980006058514d1e6b543af3d71670f3" dependencies = [ "arrayref", "auto_impl 1.0.1", @@ -4132,8 +4130,6 @@ dependencies = [ [[package]] name = "revm_precompiles" version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af88e7e9feb30cc4ed64645f09b966e84a1f6be56551ce5f1691105def45705d" dependencies = [ "bytes", "k256", diff --git a/Cargo.toml b/Cargo.toml index 6b6fab33cb86..fe177fe017dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,3 +62,5 @@ debug = 0 #ethers-signers = { path = "../ethers-rs/ethers-signers" } #ethers-etherscan = { path = "../ethers-rs/ethers-etherscan" } #ethers-solc = { path = "../ethers-rs/ethers-solc" } +[patch.crates-io] +revm = { path = "../../bluealloy/revm/crates/revm" } diff --git a/anvil/src/eth/backend/db.rs b/anvil/src/eth/backend/db.rs index c95f0e228b4c..0cf1eb5d8f04 100644 --- a/anvil/src/eth/backend/db.rs +++ b/anvil/src/eth/backend/db.rs @@ -9,7 +9,7 @@ use ethers::{ use forge::revm::KECCAK_EMPTY; use foundry_evm::{ executor::DatabaseRef, - revm::{db::CacheDB, Database, DatabaseCommit, InMemoryDB}, + revm::{db::CacheDB, Bytecode, Database, DatabaseCommit, InMemoryDB}, HashMap, }; @@ -43,7 +43,7 @@ pub trait Db: DatabaseRef + Database + DatabaseCommit + Send + Sync { H256::from_slice(&keccak256(code.as_ref())[..]) }; info.code_hash = code_hash; - info.code = Some(code.to_vec().into()); + info.code = Some(Bytecode::new_raw(code.0)); self.insert_account(address, info); } @@ -123,7 +123,7 @@ impl DatabaseRef for StateDb { self.0.basic(address) } - fn code_by_hash(&self, code_hash: H256) -> bytes::Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { self.0.code_by_hash(code_hash) } diff --git a/anvil/src/eth/backend/mem/in_memory_db.rs b/anvil/src/eth/backend/mem/in_memory_db.rs index e267b5d91d7e..c3bb080f65ce 100644 --- a/anvil/src/eth/backend/mem/in_memory_db.rs +++ b/anvil/src/eth/backend/mem/in_memory_db.rs @@ -11,7 +11,7 @@ use tracing::{trace, warn}; // reexport for convenience pub use foundry_evm::executor::{backend::MemDb, DatabaseRef}; -use forge::revm::KECCAK_EMPTY; +use forge::revm::{Bytecode, KECCAK_EMPTY}; impl Db for MemDb { fn insert_account(&mut self, address: Address, account: AccountInfo) { @@ -38,6 +38,8 @@ impl Db for MemDb { .info .code .unwrap_or_else(|| self.inner.code_by_hash(v.info.code_hash)) + .bytes() + .clone() .into(), storage: v.storage.into_iter().collect(), }, @@ -57,7 +59,11 @@ impl Db for MemDb { AccountInfo { balance: account.balance, code_hash: KECCAK_EMPTY, // will be set automatically - code: if account.code.0.is_empty() { None } else { Some(account.code.0) }, + code: if account.code.0.is_empty() { + None + } else { + Some(Bytecode::new_raw(account.code.0)) + }, // use max nonce in case account is imported multiple times with difference // nonces to prevent collisions nonce: std::cmp::max( @@ -110,7 +116,7 @@ mod tests { Address, }; use bytes::Bytes; - use forge::revm::KECCAK_EMPTY; + use forge::revm::{Bytecode, KECCAK_EMPTY}; use foundry_evm::{ executor::{backend::MemDb, DatabaseRef}, HashMap, @@ -126,7 +132,7 @@ mod tests { let mut dump_db = MemDb::default(); - let contract_code: Bytes = Bytes::from("fake contract code"); + let contract_code: Bytecode = Bytecode::new_raw(Bytes::from("fake contract code")); dump_db.insert_account( test_addr, @@ -163,7 +169,7 @@ mod tests { let test_addr2: Address = Address::from_str("0x70997970c51812dc3a010c7d01b50e0d17dc79c8").unwrap(); - let contract_code: Bytes = Bytes::from("fake contract code"); + let contract_code: Bytecode = Bytecode::new_raw(Bytes::from("fake contract code")); let mut db = MemDb::default(); @@ -199,7 +205,7 @@ mod tests { test_addr, SerializableAccountRecord { balance: 100100.into(), - code: contract_code.clone().into(), + code: contract_code.bytes().clone().into(), nonce: 100, storage: new_storage, }, diff --git a/anvil/src/eth/backend/mem/mod.rs b/anvil/src/eth/backend/mem/mod.rs index 0c1c0eaabf8d..6484536ffb6c 100644 --- a/anvil/src/eth/backend/mem/mod.rs +++ b/anvil/src/eth/backend/mem/mod.rs @@ -1,5 +1,4 @@ //! In memory blockchain backend - use crate::{ eth::{ backend::{ @@ -1096,9 +1095,9 @@ impl Backend { trace!(target: "backend", "get code for {:?}", address); let account = db.basic(address); let code = if let Some(code) = account.code { - code.into() + code.bytes().clone().into() } else { - db.code_by_hash(account.code_hash).into() + db.code_by_hash(account.code_hash).bytes().clone().into() }; Ok(code) }) diff --git a/evm/src/coverage/visitor.rs b/evm/src/coverage/visitor.rs index 87ad7567d8ca..6fb7c345368d 100644 --- a/evm/src/coverage/visitor.rs +++ b/evm/src/coverage/visitor.rs @@ -481,7 +481,7 @@ impl Visitor { // We found a push, so we do some PC -> IC translation accounting, but we also check if // this push is coupled with the JUMPI we are interested in. - if opcode_infos[op as usize].is_push { + if opcode_infos[op as usize].is_push() { let element = if let Some(element) = source_map.get(pc - cumulative_push_size) { element } else { diff --git a/evm/src/executor/backend/fuzz.rs b/evm/src/executor/backend/fuzz.rs index 84b2209080e8..8bc38ac23ec7 100644 --- a/evm/src/executor/backend/fuzz.rs +++ b/evm/src/executor/backend/fuzz.rs @@ -5,12 +5,11 @@ use crate::{ }, Address, }; -use bytes::Bytes; use ethers::prelude::{H160, H256, U256}; use hashbrown::HashMap as Map; use revm::{ - db::DatabaseRef, Account, AccountInfo, Database, Env, Inspector, Log, Return, SubRoutine, - TransactOut, + db::DatabaseRef, Account, AccountInfo, Bytecode, Database, Env, Inspector, Log, Return, + SubRoutine, TransactOut, }; use std::borrow::Cow; use tracing::trace; @@ -142,7 +141,7 @@ impl<'a> DatabaseRef for FuzzBackendWrapper<'a> { DatabaseRef::basic(self.backend.as_ref(), address) } - fn code_by_hash(&self, code_hash: H256) -> Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { DatabaseRef::code_by_hash(self.backend.as_ref(), code_hash) } @@ -159,7 +158,7 @@ impl<'a> Database for FuzzBackendWrapper<'a> { fn basic(&mut self, address: H160) -> AccountInfo { DatabaseRef::basic(self, address) } - fn code_by_hash(&mut self, code_hash: H256) -> Bytes { + fn code_by_hash(&mut self, code_hash: H256) -> Bytecode { DatabaseRef::code_by_hash(self, code_hash) } fn storage(&mut self, address: H160, index: U256) -> U256 { diff --git a/evm/src/executor/backend/in_memory_db.rs b/evm/src/executor/backend/in_memory_db.rs index be30bfca1600..2bf9406b0b53 100644 --- a/evm/src/executor/backend/in_memory_db.rs +++ b/evm/src/executor/backend/in_memory_db.rs @@ -1,9 +1,7 @@ //! The in memory DB - -use bytes::Bytes; use ethers::prelude::{H160, H256, U256}; use hashbrown::HashMap as Map; -use revm::{db::DatabaseRef, Account, AccountInfo, Database, DatabaseCommit, InMemoryDB}; +use revm::{db::DatabaseRef, Account, AccountInfo, Bytecode, Database, DatabaseCommit, InMemoryDB}; use crate::executor::snapshot::Snapshots; @@ -27,7 +25,7 @@ impl DatabaseRef for MemDb { DatabaseRef::basic(&self.inner, address) } - fn code_by_hash(&self, code_hash: H256) -> Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { DatabaseRef::code_by_hash(&self.inner, code_hash) } @@ -45,7 +43,7 @@ impl Database for MemDb { Database::basic(&mut self.inner, address) } - fn code_by_hash(&mut self, code_hash: H256) -> Bytes { + fn code_by_hash(&mut self, code_hash: H256) -> Bytecode { Database::code_by_hash(&mut self.inner, code_hash) } diff --git a/evm/src/executor/backend/mod.rs b/evm/src/executor/backend/mod.rs index 8167f75e29cc..dfde9592c493 100644 --- a/evm/src/executor/backend/mod.rs +++ b/evm/src/executor/backend/mod.rs @@ -2,7 +2,6 @@ use crate::executor::{ fork::{CreateFork, ForkId, MultiFork, SharedBackend}, snapshot::Snapshots, }; -use bytes::Bytes; use ethers::{ prelude::{H160, H256, U256}, types::Address, @@ -10,8 +9,8 @@ use ethers::{ use hashbrown::HashMap as Map; use revm::{ db::{CacheDB, DatabaseRef}, - Account, AccountInfo, Database, DatabaseCommit, Env, InMemoryDB, Inspector, Log, Return, - SubRoutine, TransactOut, TransactTo, KECCAK_EMPTY, + Account, AccountInfo, Bytecode, Database, DatabaseCommit, Env, InMemoryDB, Inspector, Log, + Return, SubRoutine, TransactOut, TransactTo, KECCAK_EMPTY, }; use std::collections::{HashMap, HashSet}; use tracing::{trace, warn}; @@ -702,7 +701,7 @@ impl DatabaseRef for Backend { } } - fn code_by_hash(&self, code_hash: H256) -> Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { if let Some(db) = self.active_fork_db() { db.code_by_hash(code_hash) } else { @@ -736,7 +735,7 @@ impl<'a> DatabaseRef for &'a mut Backend { } } - fn code_by_hash(&self, code_hash: H256) -> Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { if let Some(db) = self.active_fork_db() { DatabaseRef::code_by_hash(db, code_hash) } else { @@ -780,7 +779,7 @@ impl Database for Backend { } } - fn code_by_hash(&mut self, code_hash: H256) -> Bytes { + fn code_by_hash(&mut self, code_hash: H256) -> Bytecode { if let Some(db) = self.active_fork_db_mut() { db.code_by_hash(code_hash) } else { diff --git a/evm/src/executor/fork/backend.rs b/evm/src/executor/fork/backend.rs index a515bfb1ca66..8e81596dc8d3 100644 --- a/evm/src/executor/fork/backend.rs +++ b/evm/src/executor/fork/backend.rs @@ -1,6 +1,4 @@ //! Smart caching and deduplication of requests when using a forking provider -use revm::{db::DatabaseRef, AccountInfo, KECCAK_EMPTY}; - use crate::executor::fork::{cache::FlushJsonBlockCacheDB, BlockchainDb}; use ethers::{ core::abi::ethereum_types::BigEndianHash, @@ -14,6 +12,7 @@ use futures::{ task::{Context, Poll}, Future, FutureExt, }; +use revm::{db::DatabaseRef, AccountInfo, Bytecode, KECCAK_EMPTY}; use std::{ collections::{hash_map::Entry, HashMap, VecDeque}, pin::Pin, @@ -280,8 +279,12 @@ where }; // update the cache - let acc = - AccountInfo { nonce: nonce.as_u64(), balance, code, code_hash }; + let acc = AccountInfo { + nonce: nonce.as_u64(), + balance, + code: code.map(|bytes| Bytecode::new_raw(bytes)), + code_hash, + }; pin.db.accounts().write().insert(addr, acc.clone()); // notify all listeners @@ -495,7 +498,7 @@ impl DatabaseRef for SharedBackend { }) } - fn code_by_hash(&self, _address: H256) -> bytes::Bytes { + fn code_by_hash(&self, _address: H256) -> Bytecode { panic!("Should not be called. Code is already loaded.") } diff --git a/evm/src/executor/fork/cache.rs b/evm/src/executor/fork/cache.rs index afb196f7a719..d2f9162f9382 100644 --- a/evm/src/executor/fork/cache.rs +++ b/evm/src/executor/fork/cache.rs @@ -202,7 +202,7 @@ impl MemDb { .code .as_ref() .filter(|code| !code.is_empty()) - .map(|code| H256::from_slice(&keccak256(code))) + .map(|code| H256::from_slice(&keccak256(code.bytes()))) { acc.info.code_hash = code_hash; } else if acc.info.code_hash.is_zero() { diff --git a/evm/src/executor/fork/database.rs b/evm/src/executor/fork/database.rs index bc40de245a4d..36ec20374980 100644 --- a/evm/src/executor/fork/database.rs +++ b/evm/src/executor/fork/database.rs @@ -7,11 +7,10 @@ use crate::{ }, revm::db::CacheDB, }; -use bytes::Bytes; use ethers::prelude::{Address, H256, U256}; use hashbrown::HashMap as Map; use parking_lot::Mutex; -use revm::{db::DatabaseRef, Account, AccountInfo, Database, DatabaseCommit}; +use revm::{db::DatabaseRef, Account, AccountInfo, Bytecode, Database, DatabaseCommit}; use std::{collections::BTreeMap, sync::Arc}; use tracing::{trace, warn}; @@ -145,7 +144,7 @@ impl Database for ForkedDatabase { self.cache_db.basic(address) } - fn code_by_hash(&mut self, code_hash: H256) -> bytes::Bytes { + fn code_by_hash(&mut self, code_hash: H256) -> Bytecode { self.cache_db.code_by_hash(code_hash) } @@ -163,7 +162,7 @@ impl DatabaseRef for ForkedDatabase { self.cache_db.basic(address) } - fn code_by_hash(&self, code_hash: H256) -> bytes::Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { self.cache_db.code_by_hash(code_hash) } @@ -212,7 +211,7 @@ impl DatabaseRef for ForkDbSnapshot { } } - fn code_by_hash(&self, code_hash: H256) -> Bytes { + fn code_by_hash(&self, code_hash: H256) -> Bytecode { self.local.code_by_hash(code_hash) } diff --git a/evm/src/executor/inspector/cheatcodes/env.rs b/evm/src/executor/inspector/cheatcodes/env.rs index a9f23e6db13e..f24dc996ef6e 100644 --- a/evm/src/executor/inspector/cheatcodes/env.rs +++ b/evm/src/executor/inspector/cheatcodes/env.rs @@ -8,7 +8,7 @@ use ethers::{ types::{Address, H256, U256}, utils::keccak256, }; -use revm::{Database, EVMData}; +use revm::{Bytecode, Database, EVMData}; #[derive(Clone, Debug, Default)] pub struct Broadcast { @@ -175,7 +175,7 @@ pub fn apply( // TODO: Does this increase gas usage? data.subroutine.load_account(inner.0, data.db); - data.subroutine.set_code(inner.0, code.0, hash); + data.subroutine.set_code(inner.0, Bytecode::new_raw(code.0), hash); Ok(Bytes::new()) } HEVMCalls::Deal(inner) => { diff --git a/evm/src/executor/inspector/cheatcodes/mod.rs b/evm/src/executor/inspector/cheatcodes/mod.rs index 3200f591b6d9..202d1888e42a 100644 --- a/evm/src/executor/inspector/cheatcodes/mod.rs +++ b/evm/src/executor/inspector/cheatcodes/mod.rs @@ -184,7 +184,7 @@ where fn step(&mut self, interpreter: &mut Interpreter, _: &mut EVMData<'_, DB>, _: bool) -> Return { // Record writes and reads if `record` has been called if let Some(storage_accesses) = &mut self.accesses { - match interpreter.contract.code[interpreter.program_counter()] { + match interpreter.contract.bytecode.bytecode()[interpreter.program_counter()] { opcode::SLOAD => { let key = try_or_continue!(interpreter.stack().peek(0)); storage_accesses diff --git a/evm/src/executor/inspector/coverage.rs b/evm/src/executor/inspector/coverage.rs index c0baa046321d..a0d4b40f5bc0 100644 --- a/evm/src/executor/inspector/coverage.rs +++ b/evm/src/executor/inspector/coverage.rs @@ -59,8 +59,10 @@ where // map for a given address does not exist, *but* we need to account for the fact that the // code given by the interpreter may either be the contract init code, or the runtime code. if let Some(context) = self.context.last() { - self.ic_map - .insert(*context, build_ic_map(data.env.cfg.spec_id, &interp.contract().code)); + self.ic_map.insert( + *context, + build_ic_map(data.env.cfg.spec_id, &interp.contract().bytecode.bytecode()), + ); } Return::Continue } diff --git a/evm/src/executor/inspector/debugger.rs b/evm/src/executor/inspector/debugger.rs index 5de2682765f4..f68a8755a21b 100644 --- a/evm/src/executor/inspector/debugger.rs +++ b/evm/src/executor/inspector/debugger.rs @@ -103,8 +103,10 @@ where // TODO: This is rebuilt for all contracts every time. We should only run this if the IC // map for a given address does not exist, *but* we need to account for the fact that the // code given by the interpreter may either be the contract init code, or the runtime code. - self.ic_map - .insert(self.context, build_ic_map(data.env.cfg.spec_id, &interp.contract().code)); + self.ic_map.insert( + self.context, + build_ic_map(data.env.cfg.spec_id, &interp.contract().bytecode.bytecode()), + ); self.previous_gas_block = interp.contract.first_gas_block(); Return::Continue } @@ -116,20 +118,20 @@ where _is_static: bool, ) -> Return { let pc = interpreter.program_counter(); - let op = interpreter.contract.code[pc]; + let op = interpreter.contract.bytecode.bytecode()[pc]; // Get opcode information let opcode_infos = spec_opcode_gas(data.env.cfg.spec_id); let opcode_info = &opcode_infos[op as usize]; // Extract the push bytes - let push_size = if opcode_info.is_push { (op - opcode::PUSH1 + 1) as usize } else { 0 }; + let push_size = if opcode_info.is_push() { (op - opcode::PUSH1 + 1) as usize } else { 0 }; let push_bytes = match push_size { 0 => None, n => { let start = pc + 1; let end = start + n; - Some(interpreter.contract.code[start..end].to_vec()) + Some(interpreter.contract.bytecode.bytecode()[start..end].to_vec()) } }; @@ -139,11 +141,11 @@ where .spend() .saturating_sub(self.previous_gas_block) .saturating_add(self.current_gas_block); - if opcode_info.is_gas_block_end { + if opcode_info.is_gas_block_end() { self.previous_gas_block = interpreter.contract.gas_block(pc); self.current_gas_block = 0; } else { - self.current_gas_block += opcode_info.gas; + self.current_gas_block += opcode_info.get_gas() as u64; } self.arena.arena[self.head].steps.push(DebugStep { diff --git a/evm/src/executor/inspector/tracer.rs b/evm/src/executor/inspector/tracer.rs index 66e5b29d6369..64856759a97a 100644 --- a/evm/src/executor/inspector/tracer.rs +++ b/evm/src/executor/inspector/tracer.rs @@ -151,7 +151,7 @@ where .info .code .as_ref() - .map_or(vec![], |code| code.to_vec()), + .map_or(vec![], |code| code.bytes().to_vec()), None => vec![], }; self.fill_trace( diff --git a/evm/src/executor/mod.rs b/evm/src/executor/mod.rs index 67c7ea94bcd7..24e3d746ca08 100644 --- a/evm/src/executor/mod.rs +++ b/evm/src/executor/mod.rs @@ -14,7 +14,7 @@ use ethers::{ use foundry_utils::IntoFunction; use hashbrown::HashMap; use revm::{ - db::DatabaseCommit, return_ok, Account, BlockEnv, CreateScheme, Return, TransactOut, + db::DatabaseCommit, return_ok, Account, BlockEnv, Bytecode, CreateScheme, Return, TransactOut, TransactTo, TxEnv, EVM, }; /// Reexport commonly used revm types @@ -83,7 +83,12 @@ impl Executor { // does not fail backend.insert_account_info( CHEATCODE_ADDRESS, - revm::AccountInfo { code: Some(Bytes::from_static(&[1])), ..Default::default() }, + revm::AccountInfo { + // Safety: The bytecode is a singular STOP opcode (0x00) and has a length of 1, + // which respects the invariants outlined in REVM. + code: Some(unsafe { Bytecode::new_checked(vec![0u8].into(), 1) }), + ..Default::default() + }, ); Executor { backend, env, inspector_config, gas_limit } diff --git a/evm/src/fuzz/strategies/state.rs b/evm/src/fuzz/strategies/state.rs index fd6163502996..edf943b50382 100644 --- a/evm/src/fuzz/strategies/state.rs +++ b/evm/src/fuzz/strategies/state.rs @@ -96,7 +96,7 @@ pub fn collect_state_from_call( // Insert push bytes if let Some(code) = &account.info.code { - for push_byte in collect_push_bytes(code.clone()) { + for push_byte in collect_push_bytes(code.bytes().clone()) { state.insert(push_byte); } } @@ -134,7 +134,7 @@ fn collect_push_bytes(code: Bytes) -> Vec<[u8; 32]> { let mut i = 0; while i < code.len().min(PUSH_BYTE_ANALYSIS_LIMIT) { let op = code[i]; - if opcode_infos[op as usize].is_push { + if opcode_infos[op as usize].is_push() { let push_size = (op - opcode::PUSH1 + 1) as usize; let push_start = i + 1; let push_end = push_start + push_size; diff --git a/evm/src/utils.rs b/evm/src/utils.rs index bb7a13c19676..edcb1b77c618 100644 --- a/evm/src/utils.rs +++ b/evm/src/utils.rs @@ -1,4 +1,3 @@ -use bytes::Bytes; use ethers::prelude::{H256, U256}; use revm::{opcode, spec_opcode_gas, SpecId}; use std::collections::BTreeMap; @@ -30,7 +29,7 @@ pub fn h256_to_u256_le(storage: H256) -> U256 { /// Builds the instruction counter map for the given bytecode. // TODO: Some of the same logic is performed in REVM, but then later discarded. We should // investigate if we can reuse it -pub fn build_ic_map(spec: SpecId, code: &Bytes) -> BTreeMap { +pub fn build_ic_map(spec: SpecId, code: &[u8]) -> BTreeMap { let opcode_infos = spec_opcode_gas(spec); let mut ic_map: BTreeMap = BTreeMap::new(); @@ -39,7 +38,7 @@ pub fn build_ic_map(spec: SpecId, code: &Bytes) -> BTreeMap { while i < code.len() { let op = code[i]; ic_map.insert(i, i - cumulative_push_size); - if opcode_infos[op as usize].is_push { + if opcode_infos[op as usize].is_push() { // Skip the push bytes. // // For more context on the math, see: https://github.com/bluealloy/revm/blob/007b8807b5ad7705d3cacce4d92b89d880a83301/crates/revm/src/interpreter/contract.rs#L114-L115 From 352617968a417b09b4c5ac747cfaf3fb541f0447 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 17:58:32 +0200 Subject: [PATCH 2/9] refactor: use checked bytecode See https://github.com/bluealloy/revm/issues/121#issuecomment-1201102905 --- anvil/src/eth/backend/db.rs | 2 +- anvil/src/eth/backend/mem/in_memory_db.rs | 2 +- evm/src/executor/fork/backend.rs | 2 +- evm/src/executor/inspector/cheatcodes/env.rs | 2 +- evm/src/executor/inspector/coverage.rs | 2 +- evm/src/executor/inspector/debugger.rs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/anvil/src/eth/backend/db.rs b/anvil/src/eth/backend/db.rs index 0cf1eb5d8f04..5079bfdb724d 100644 --- a/anvil/src/eth/backend/db.rs +++ b/anvil/src/eth/backend/db.rs @@ -43,7 +43,7 @@ pub trait Db: DatabaseRef + Database + DatabaseCommit + Send + Sync { H256::from_slice(&keccak256(code.as_ref())[..]) }; info.code_hash = code_hash; - info.code = Some(Bytecode::new_raw(code.0)); + info.code = Some(Bytecode::new_raw(code.0).to_checked()); self.insert_account(address, info); } diff --git a/anvil/src/eth/backend/mem/in_memory_db.rs b/anvil/src/eth/backend/mem/in_memory_db.rs index c3bb080f65ce..58bcdf6a03b7 100644 --- a/anvil/src/eth/backend/mem/in_memory_db.rs +++ b/anvil/src/eth/backend/mem/in_memory_db.rs @@ -62,7 +62,7 @@ impl Db for MemDb { code: if account.code.0.is_empty() { None } else { - Some(Bytecode::new_raw(account.code.0)) + Some(Bytecode::new_raw(account.code.0).to_checked()) }, // use max nonce in case account is imported multiple times with difference // nonces to prevent collisions diff --git a/evm/src/executor/fork/backend.rs b/evm/src/executor/fork/backend.rs index 8e81596dc8d3..00adc9bd9ed7 100644 --- a/evm/src/executor/fork/backend.rs +++ b/evm/src/executor/fork/backend.rs @@ -282,7 +282,7 @@ where let acc = AccountInfo { nonce: nonce.as_u64(), balance, - code: code.map(|bytes| Bytecode::new_raw(bytes)), + code: code.map(|bytes| Bytecode::new_raw(bytes).to_checked()), code_hash, }; pin.db.accounts().write().insert(addr, acc.clone()); diff --git a/evm/src/executor/inspector/cheatcodes/env.rs b/evm/src/executor/inspector/cheatcodes/env.rs index f24dc996ef6e..4a9246a25896 100644 --- a/evm/src/executor/inspector/cheatcodes/env.rs +++ b/evm/src/executor/inspector/cheatcodes/env.rs @@ -175,7 +175,7 @@ pub fn apply( // TODO: Does this increase gas usage? data.subroutine.load_account(inner.0, data.db); - data.subroutine.set_code(inner.0, Bytecode::new_raw(code.0), hash); + data.subroutine.set_code(inner.0, Bytecode::new_raw(code.0).to_checked(), hash); Ok(Bytes::new()) } HEVMCalls::Deal(inner) => { diff --git a/evm/src/executor/inspector/coverage.rs b/evm/src/executor/inspector/coverage.rs index a0d4b40f5bc0..487c6de4c848 100644 --- a/evm/src/executor/inspector/coverage.rs +++ b/evm/src/executor/inspector/coverage.rs @@ -61,7 +61,7 @@ where if let Some(context) = self.context.last() { self.ic_map.insert( *context, - build_ic_map(data.env.cfg.spec_id, &interp.contract().bytecode.bytecode()), + build_ic_map(data.env.cfg.spec_id, interp.contract().bytecode.bytecode()), ); } Return::Continue diff --git a/evm/src/executor/inspector/debugger.rs b/evm/src/executor/inspector/debugger.rs index f68a8755a21b..3e59d9834d8e 100644 --- a/evm/src/executor/inspector/debugger.rs +++ b/evm/src/executor/inspector/debugger.rs @@ -105,7 +105,7 @@ where // code given by the interpreter may either be the contract init code, or the runtime code. self.ic_map.insert( self.context, - build_ic_map(data.env.cfg.spec_id, &interp.contract().bytecode.bytecode()), + build_ic_map(data.env.cfg.spec_id, interp.contract().bytecode.bytecode()), ); self.previous_gas_block = interp.contract.first_gas_block(); Return::Continue From 311736d9fe262927db9a07e3a754dffa8aa6ee1f Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 18:42:03 +0200 Subject: [PATCH 3/9] build: use git revm --- Cargo.lock | 23 ++++++++++++++++++++++- Cargo.toml | 2 -- evm/Cargo.toml | 2 +- ui/Cargo.toml | 2 +- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e825bbb72cbf..bd60643dd4f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4112,7 +4112,8 @@ dependencies = [ [[package]] name = "revm" -version = "1.7.0" +version = "1.8.0" +source = "git+https://github.com/bluealloy/revm#76a07d649ef84980a7bb8e5d58a1df393684d817" dependencies = [ "arrayref", "auto_impl 1.0.1", @@ -4130,12 +4131,14 @@ dependencies = [ [[package]] name = "revm_precompiles" version = "1.1.0" +source = "git+https://github.com/bluealloy/revm#76a07d649ef84980a7bb8e5d58a1df393684d817" dependencies = [ "bytes", "k256", "num 0.4.0", "primitive-types", "ripemd", + "secp256k1", "sha2 0.10.2", "sha3", "substrate-bn", @@ -4365,6 +4368,24 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secp256k1" +version = "0.23.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ece73253dd9e1fb540ff324eae554113a31c25fb598d22fd13b088a6a03f90d" +dependencies = [ + "secp256k1-sys", +] + +[[package]] +name = "secp256k1-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7058dc8eaf3f2810d7828680320acda0b25a288f6d288e19278e249bbf74226b" +dependencies = [ + "cc", +] + [[package]] name = "security-framework" version = "2.6.1" diff --git a/Cargo.toml b/Cargo.toml index fe177fe017dd..6b6fab33cb86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,5 +62,3 @@ debug = 0 #ethers-signers = { path = "../ethers-rs/ethers-signers" } #ethers-etherscan = { path = "../ethers-rs/ethers-etherscan" } #ethers-solc = { path = "../ethers-rs/ethers-solc" } -[patch.crates-io] -revm = { path = "../../bluealloy/revm/crates/revm" } diff --git a/evm/Cargo.toml b/evm/Cargo.toml index d192e7a2e131..3b1a42146b46 100644 --- a/evm/Cargo.toml +++ b/evm/Cargo.toml @@ -36,7 +36,7 @@ once_cell = "1.13" # EVM bytes = "1.1.0" hashbrown = { version = "0.12", features = ["serde"] } -revm = { version = "1.7", default-features = false, features = ["std", "k256", "with-serde", "memory_limit"] } +revm = { git = "https://github.com/bluealloy/revm", default-features = false, features = ["std", "k256", "with-serde", "memory_limit"] } # Fuzzer proptest = "1.0.0" diff --git a/ui/Cargo.toml b/ui/Cargo.toml index cf8f47b469a5..6e6f6b103b48 100644 --- a/ui/Cargo.toml +++ b/ui/Cargo.toml @@ -14,4 +14,4 @@ eyre = "0.6.5" hex = "0.4.3" ethers = { git = "https://github.com/gakonst/ethers-rs" } forge = { path = "../forge" } -revm = { version = "1.7", default-features = false, features = ["std", "k256", "with-serde"] } +revm = { git = "https://github.com/bluealloy/revm", features = ["std", "k256", "with-serde"] } From efc8838ac93296ff032ef271ea97fcaa29d42185 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 19:13:35 +0200 Subject: [PATCH 4/9] build: use revm 1.8 --- Cargo.lock | 6 ++++-- evm/Cargo.toml | 2 +- ui/Cargo.toml | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd60643dd4f0..0ad0ff4bb421 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4113,7 +4113,8 @@ dependencies = [ [[package]] name = "revm" version = "1.8.0" -source = "git+https://github.com/bluealloy/revm#76a07d649ef84980a7bb8e5d58a1df393684d817" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f0033a9176ff78b9d6791f5d2af3c74455c0ace2303a81ad8400a65de64b640" dependencies = [ "arrayref", "auto_impl 1.0.1", @@ -4131,7 +4132,8 @@ dependencies = [ [[package]] name = "revm_precompiles" version = "1.1.0" -source = "git+https://github.com/bluealloy/revm#76a07d649ef84980a7bb8e5d58a1df393684d817" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af88e7e9feb30cc4ed64645f09b966e84a1f6be56551ce5f1691105def45705d" dependencies = [ "bytes", "k256", diff --git a/evm/Cargo.toml b/evm/Cargo.toml index 3b1a42146b46..22ab5542bf38 100644 --- a/evm/Cargo.toml +++ b/evm/Cargo.toml @@ -36,7 +36,7 @@ once_cell = "1.13" # EVM bytes = "1.1.0" hashbrown = { version = "0.12", features = ["serde"] } -revm = { git = "https://github.com/bluealloy/revm", default-features = false, features = ["std", "k256", "with-serde", "memory_limit"] } +revm = { version = "1.8", default-features = false, features = ["std", "k256", "with-serde", "memory_limit"] } # Fuzzer proptest = "1.0.0" diff --git a/ui/Cargo.toml b/ui/Cargo.toml index 6e6f6b103b48..20ab3f77f5bf 100644 --- a/ui/Cargo.toml +++ b/ui/Cargo.toml @@ -14,4 +14,4 @@ eyre = "0.6.5" hex = "0.4.3" ethers = { git = "https://github.com/gakonst/ethers-rs" } forge = { path = "../forge" } -revm = { git = "https://github.com/bluealloy/revm", features = ["std", "k256", "with-serde"] } +revm = { version = "1.8", features = ["std", "k256", "with-serde"] } From 9cce7aa3bd865ff291a22ee4e774496ba09aa44c Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 19:14:52 +0200 Subject: [PATCH 5/9] test: fix test --- anvil/src/eth/backend/mem/in_memory_db.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/anvil/src/eth/backend/mem/in_memory_db.rs b/anvil/src/eth/backend/mem/in_memory_db.rs index 58bcdf6a03b7..b2c0a144b70e 100644 --- a/anvil/src/eth/backend/mem/in_memory_db.rs +++ b/anvil/src/eth/backend/mem/in_memory_db.rs @@ -132,7 +132,8 @@ mod tests { let mut dump_db = MemDb::default(); - let contract_code: Bytecode = Bytecode::new_raw(Bytes::from("fake contract code")); + let contract_code: Bytecode = + Bytecode::new_raw(Bytes::from("fake contract code")).to_checked(); dump_db.insert_account( test_addr, @@ -169,7 +170,8 @@ mod tests { let test_addr2: Address = Address::from_str("0x70997970c51812dc3a010c7d01b50e0d17dc79c8").unwrap(); - let contract_code: Bytecode = Bytecode::new_raw(Bytes::from("fake contract code")); + let contract_code: Bytecode = + Bytecode::new_raw(Bytes::from("fake contract code")).to_checked(); let mut db = MemDb::default(); From b2e8eb23428d69343493e55cac8d6ff99b392a86 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 20:11:56 +0200 Subject: [PATCH 6/9] fix: correct bytecode getters/setters Whenever we output the bytecode of an account, whether to a file or in a response, we need to return the *original* bytecode using `Bytecode::len`, otherwise the bytecode will differ depending on whether the bytecode has been checked, analyzed or not. --- anvil/src/eth/backend/mem/in_memory_db.rs | 15 +++++++-------- anvil/src/eth/backend/mem/mod.rs | 6 +++--- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/anvil/src/eth/backend/mem/in_memory_db.rs b/anvil/src/eth/backend/mem/in_memory_db.rs index b2c0a144b70e..1e872dd2e824 100644 --- a/anvil/src/eth/backend/mem/in_memory_db.rs +++ b/anvil/src/eth/backend/mem/in_memory_db.rs @@ -29,18 +29,17 @@ impl Db for MemDb { .clone() .into_iter() .map(|(k, v)| { + let code = v + .info + .code + .unwrap_or_else(|| self.inner.code_by_hash(v.info.code_hash)) + .to_checked(); ( k, SerializableAccountRecord { nonce: v.info.nonce, balance: v.info.balance, - code: v - .info - .code - .unwrap_or_else(|| self.inner.code_by_hash(v.info.code_hash)) - .bytes() - .clone() - .into(), + code: code.bytes()[..code.len()].to_vec().into(), storage: v.storage.into_iter().collect(), }, ) @@ -207,7 +206,7 @@ mod tests { test_addr, SerializableAccountRecord { balance: 100100.into(), - code: contract_code.bytes().clone().into(), + code: contract_code.bytes()[..contract_code.len()].to_vec().into(), nonce: 100, storage: new_storage, }, diff --git a/anvil/src/eth/backend/mem/mod.rs b/anvil/src/eth/backend/mem/mod.rs index 6484536ffb6c..a04efb128d8c 100644 --- a/anvil/src/eth/backend/mem/mod.rs +++ b/anvil/src/eth/backend/mem/mod.rs @@ -1095,11 +1095,11 @@ impl Backend { trace!(target: "backend", "get code for {:?}", address); let account = db.basic(address); let code = if let Some(code) = account.code { - code.bytes().clone().into() + code } else { - db.code_by_hash(account.code_hash).bytes().clone().into() + db.code_by_hash(account.code_hash) }; - Ok(code) + Ok(code.bytes()[..code.len()].to_vec().into()) }) } From 10ccd106dc9176f6fdce3941b77a5c4af5e9ad05 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 20:20:38 +0200 Subject: [PATCH 7/9] refactor: use `Bytecode::hash` --- evm/src/executor/fork/cache.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/evm/src/executor/fork/cache.rs b/evm/src/executor/fork/cache.rs index d2f9162f9382..d277de2ee0b1 100644 --- a/evm/src/executor/fork/cache.rs +++ b/evm/src/executor/fork/cache.rs @@ -1,8 +1,5 @@ //! Cache related abstraction -use ethers::{ - types::{Address, H256, U256}, - utils::keccak256, -}; +use ethers::types::{Address, H256, U256}; use parking_lot::RwLock; use revm::{Account, AccountInfo, DatabaseCommit, Filth, KECCAK_EMPTY}; use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; @@ -197,12 +194,8 @@ impl MemDb { storage.remove(&add); } else { // insert account - if let Some(code_hash) = acc - .info - .code - .as_ref() - .filter(|code| !code.is_empty()) - .map(|code| H256::from_slice(&keccak256(code.bytes()))) + if let Some(code_hash) = + acc.info.code.as_ref().filter(|code| !code.is_empty()).map(|code| code.hash()) { acc.info.code_hash = code_hash; } else if acc.info.code_hash.is_zero() { From 84be4826c22d48dff59a965ad627a96f1f572286 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 20:21:01 +0200 Subject: [PATCH 8/9] fix: get original account code for traces --- evm/src/executor/inspector/tracer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/src/executor/inspector/tracer.rs b/evm/src/executor/inspector/tracer.rs index 64856759a97a..ce69854e8d3d 100644 --- a/evm/src/executor/inspector/tracer.rs +++ b/evm/src/executor/inspector/tracer.rs @@ -151,7 +151,7 @@ where .info .code .as_ref() - .map_or(vec![], |code| code.bytes().to_vec()), + .map_or(vec![], |code| code.bytes()[..code.len()].to_vec()), None => vec![], }; self.fill_trace( From 0ace51924bb4208d689ec620bb19627c9e0dc35b Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 1 Aug 2022 20:21:17 +0200 Subject: [PATCH 9/9] refactor: remove unsafe code --- evm/src/executor/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/evm/src/executor/mod.rs b/evm/src/executor/mod.rs index 24e3d746ca08..cd0b9e576fd0 100644 --- a/evm/src/executor/mod.rs +++ b/evm/src/executor/mod.rs @@ -84,9 +84,7 @@ impl Executor { backend.insert_account_info( CHEATCODE_ADDRESS, revm::AccountInfo { - // Safety: The bytecode is a singular STOP opcode (0x00) and has a length of 1, - // which respects the invariants outlined in REVM. - code: Some(unsafe { Bytecode::new_checked(vec![0u8].into(), 1) }), + code: Some(Bytecode::new_raw(vec![0u8].into()).to_checked()), ..Default::default() }, );