Skip to content

Commit

Permalink
Bytecode hash, remove override_spec, (#165)
Browse files Browse the repository at this point in the history
* Cache Bytecode Hash

* cleanup
  • Loading branch information
rakita authored Aug 9, 2022
1 parent 7748718 commit 4f81fcf
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 95 deletions.
4 changes: 0 additions & 4 deletions bins/revm-test/src/bin/analysis.rs

Large diffs are not rendered by default.

18 changes: 5 additions & 13 deletions crates/revm/src/evm_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB,
return (Return::CreateContractWithEF, ret, interp.gas, b);
}

// TODO maybe create some macro to hide this `if`
let mut contract_code_size_limit = 0x6000;
if INSPECT {
contract_code_size_limit = self
.inspector
.override_spec()
.eip170_contract_code_size_limit;
}
// EIP-170: Contract code size limit
if SPEC::enabled(SPURIOUS_DRAGON) && bytes.len() > contract_code_size_limit {
// By default limit is 0x6000 (~25kb)
if SPEC::enabled(SPURIOUS_DRAGON)
&& bytes.len() > self.data.env.cfg.limit_contract_code_size
{
self.data.subroutine.checkpoint_revert(checkpoint);
return (Return::CreateContractLimit, ret, interp.gas, b);
}
Expand All @@ -424,10 +419,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB,
self.data.subroutine.checkpoint_commit();
// Do analasis of bytecode streight away.
let bytecode = Bytecode::new_raw(bytes).to_analysed::<SPEC>();
let code_hash = bytecode.hash();
self.data
.subroutine
.set_code(created_address, bytecode, code_hash);
self.data.subroutine.set_code(created_address, bytecode);
(Return::Continue, ret, interp.gas, b)
}
_ => {
Expand Down
18 changes: 0 additions & 18 deletions crates/revm/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,6 @@ pub trait Inspector<DB: Database> {

/// Called when a contract has been self-destructed.
fn selfdestruct(&mut self) {}

/// Override some of the spec.
fn override_spec(&self) -> &OverrideSpec {
&OVERRIDE_SPEC_DEFAULT
}
}

const OVERRIDE_SPEC_DEFAULT: OverrideSpec = OverrideSpec {
eip170_contract_code_size_limit: usize::MAX,
};
pub struct OverrideSpec {
pub eip170_contract_code_size_limit: usize,
}

impl Default for OverrideSpec {
fn default() -> Self {
OVERRIDE_SPEC_DEFAULT
}
}

#[derive(Clone, Copy)]
Expand Down
64 changes: 51 additions & 13 deletions crates/revm/src/interpreter/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub enum BytecodeState {
pub struct Bytecode {
#[cfg_attr(feature = "with-serde", serde(with = "crate::models::serde_hex_bytes"))]
bytecode: Bytes,
hash: H256,
state: BytecodeState,
}

Expand All @@ -37,6 +38,7 @@ impl Bytecode {
// bytecode with one STOP opcode
Bytecode {
bytecode: vec![0].into(),
hash: KECCAK_EMPTY,
state: BytecodeState::Analysed {
len: 0,
jumptable: ValidJumpAddress::new(Arc::new(Vec::new()), 0),
Expand All @@ -45,8 +47,26 @@ impl Bytecode {
}

pub fn new_raw(bytecode: Bytes) -> Self {
let hash = if bytecode.is_empty() {
KECCAK_EMPTY
} else {
H256::from_slice(Keccak256::digest(&bytecode).as_slice())
};
Self {
bytecode,
hash,
state: BytecodeState::Raw,
}
}

/// Create new raw Bytecode with hash
///
/// # Safety
/// Hash need to be appropriate keccak256 over bytecode.
pub unsafe fn new_raw_with_hash(bytecode: Bytes, hash: H256) -> Self {
Self {
bytecode,
hash,
state: BytecodeState::Raw,
}
}
Expand All @@ -56,9 +76,15 @@ impl Bytecode {
/// # Safety
/// Bytecode need to end with STOP (0x00) opcode as checked bytecode assumes that
/// that it is safe to iterate over bytecode without checking lengths
pub unsafe fn new_checked(bytecode: Bytes, len: usize) -> Self {
pub unsafe fn new_checked(bytecode: Bytes, len: usize, hash: Option<H256>) -> Self {
let hash = match hash {
None if len == 0 => KECCAK_EMPTY,
None => H256::from_slice(Keccak256::digest(&bytecode).as_slice()),
Some(hash) => hash,
};
Self {
bytecode,
hash,
state: BytecodeState::Checked { len },
}
}
Expand All @@ -68,9 +94,20 @@ impl Bytecode {
/// # Safety
/// Same as new_checked, bytecode needs to end with STOP (0x00) opcode as checked bytecode assumes
/// that it is safe to iterate over bytecode without checking length
pub unsafe fn new_analysed(bytecode: Bytes, len: usize, jumptable: ValidJumpAddress) -> Self {
pub unsafe fn new_analysed(
bytecode: Bytes,
len: usize,
jumptable: ValidJumpAddress,
hash: Option<H256>,
) -> Self {
let hash = match hash {
None if len == 0 => KECCAK_EMPTY,
None => H256::from_slice(Keccak256::digest(&bytecode).as_slice()),
Some(hash) => hash,
};
Self {
bytecode,
hash,
state: BytecodeState::Analysed { len, jumptable },
}
}
Expand All @@ -80,16 +117,7 @@ impl Bytecode {
}

pub fn hash(&self) -> H256 {
let to_hash = match self.state {
BytecodeState::Raw => self.bytecode.as_ref(),
BytecodeState::Checked { len } => &self.bytecode[..len],
BytecodeState::Analysed { len, .. } => &self.bytecode[..len],
};
if to_hash.is_empty() {
KECCAK_EMPTY
} else {
H256::from_slice(Keccak256::digest(&to_hash).as_slice())
}
self.hash
}

pub fn is_empty(&self) -> bool {
Expand All @@ -116,6 +144,7 @@ impl Bytecode {
bytecode.resize(len + 33, 0);
Self {
bytecode: bytecode.into(),
hash: self.hash,
state: BytecodeState::Checked { len },
}
}
Expand All @@ -124,6 +153,7 @@ impl Bytecode {
}

pub fn to_analysed<SPEC: Spec>(self) -> Self {
let hash = self.hash;
let (bytecode, len) = match self.state {
BytecodeState::Raw => {
let len = self.bytecode.len();
Expand All @@ -137,16 +167,22 @@ impl Bytecode {

Self {
bytecode,
hash,
state: BytecodeState::Analysed { len, jumptable },
}
}

pub fn lock<SPEC: Spec>(self) -> BytecodeLocked {
let Bytecode { bytecode, state } = self.to_analysed::<SPEC>();
let Bytecode {
bytecode,
hash,
state,
} = self.to_analysed::<SPEC>();
if let BytecodeState::Analysed { len, jumptable } = state {
BytecodeLocked {
bytecode,
len,
hash,
jumptable,
}
} else {
Expand Down Expand Up @@ -232,6 +268,7 @@ impl Bytecode {
pub struct BytecodeLocked {
bytecode: Bytes,
len: usize,
hash: H256,
jumptable: ValidJumpAddress,
}

Expand All @@ -250,6 +287,7 @@ impl BytecodeLocked {
pub fn unlock(self) -> Bytecode {
Bytecode {
bytecode: self.bytecode,
hash: self.hash,
state: BytecodeState::Analysed {
len: self.len,
jumptable: self.jumptable,
Expand Down
2 changes: 1 addition & 1 deletion crates/revm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub type DummyStateDB = InMemoryDB;
pub use db::{Database, DatabaseCommit, InMemoryDB};
pub use evm::{evm_inner, new, EVM};
pub use gas::Gas;
pub use inspector::{Inspector, NoOpInspector, OverrideSpec};
pub use inspector::{Inspector, NoOpInspector};
pub use instructions::{
opcode::{self, spec_opcode_gas, OpCode, OPCODE_JUMPMAP},
Return,
Expand Down
8 changes: 8 additions & 0 deletions crates/revm/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ pub struct CfgEnv {
/// This is is not really needed on mainnet, and defaults to false, but in most cases it is
/// safe to be set to `true`, depending on the chain.
pub perf_all_precompiles_have_balance: bool,
/// Bytecode that is created with CREATE/CREATE2 is by default analysed and jumptable is created.
/// This is very benefitial for testing and speeds up execution of that bytecode when
pub perf_analyse_created_bytecodes: bool,
/// Effects EIP-170: Contract code size limit. Usefull to increase this because of tests.
/// By default it is 0x6000 (~25kb).
pub limit_contract_code_size: usize,
/// A hard memory limit in bytes beyond which [Memory] cannot be resized.
///
/// In cases where the gas limit may be extraordinarily high, it is recommended to set this to
Expand All @@ -232,6 +238,8 @@ impl Default for CfgEnv {
chain_id: 1.into(),
spec_id: SpecId::LATEST,
perf_all_precompiles_have_balance: false,
perf_analyse_created_bytecodes: true,
limit_contract_code_size: 0x6000,
#[cfg(feature = "memory_limit")]
memory_limit: 2u64.pow(32) - 1,
}
Expand Down
6 changes: 3 additions & 3 deletions crates/revm/src/subroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{interpreter::bytecode::Bytecode, models::SelfDestructResult, Return,
use alloc::{vec, vec::Vec};
use core::mem::{self};
use hashbrown::{hash_map::Entry, HashMap as Map};
use primitive_types::{H160, H256, U256};
use primitive_types::{H160, U256};

use crate::{db::Database, AccountInfo, Log};

Expand Down Expand Up @@ -166,10 +166,10 @@ impl SubRoutine {
}

/// use it only if you know that acc is hot
pub fn set_code(&mut self, address: H160, code: Bytecode, code_hash: H256) {
pub fn set_code(&mut self, address: H160, code: Bytecode) {
let acc = self.log_dirty(address, |_| {});
acc.info.code_hash = code.hash();
acc.info.code = Some(code);
acc.info.code_hash = code_hash;
}

pub fn inc_nonce(&mut self, address: H160) -> u64 {
Expand Down
43 changes: 0 additions & 43 deletions crates/revm_precompiles/src/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,46 +24,3 @@ fn identity_run(input: &[u8], gas_limit: u64) -> PrecompileResult {
)?;
Ok(PrecompileOutput::without_logs(gas_used, input.to_vec()))
}
/*
#[cfg(test)]
mod tests {
use evm::Return;
use crate::test_utils::new_context;
use super::*;
#[test]
fn test_identity() {
let input = [0u8, 1, 2, 3];
let expected = input[0..2].to_vec();
let res = Identity::run(&input[0..2], 18, &new_context(), false)
.unwrap()
.output;
assert_eq!(res, expected);
let expected = input.to_vec();
let res = Identity::run(&input, 18, &new_context(), false)
.unwrap()
.output;
assert_eq!(res, expected);
// gas fail
let res = Identity::run(&input[0..2], 17, &new_context(), false);
assert!(matches!(res, Err(Return::OutOfGas)));
// larger input
let input = [
0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
24, 25, 26, 27, 28, 29, 30, 31, 32,
];
let res = Identity::run(&input, 21, &new_context(), false)
.unwrap()
.output;
assert_eq!(res, input.to_vec());
}
}
*/

0 comments on commit 4f81fcf

Please sign in to comment.