Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bytecode hash, remove override_spec, #165

Merged
merged 2 commits into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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());
}
}
*/