From bbf18d7d3db6d65c9d40f60f9f90179753def804 Mon Sep 17 00:00:00 2001 From: lightsing Date: Thu, 6 Jun 2024 15:01:54 +0800 Subject: [PATCH 1/2] many fix and more assert --- crates/interpreter/src/instructions/host.rs | 7 ++- crates/interpreter/src/instructions/macros.rs | 37 +++++++++++ crates/primitives/src/state.rs | 62 ++++++++++++++++--- crates/revm/src/db/in_memory_db.rs | 12 +--- crates/revm/src/journaled_state.rs | 23 +------ 5 files changed, 101 insertions(+), 40 deletions(-) diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 144a9051cf..e0dde0d506 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -164,6 +164,8 @@ pub fn blockhash(interpreter: &mut Interpreter, ho #[cfg(feature = "scroll")] pub fn blockhash(interpreter: &mut Interpreter, host: &mut H) { + use revm_primitives::BLOCK_HASH_HISTORY; + gas!(interpreter, gas::BLOCKHASH); pop_top!(interpreter, number); @@ -172,11 +174,12 @@ pub fn blockhash(interpreter: &mut Interpreter, ho match block_number.checked_sub(*number) { Some(diff) if !diff.is_zero() => { let diff = as_usize_saturated!(diff); + let block_number = as_u64_or_fail!(interpreter, number); - if SPEC::enabled(BERNOULLI) { + if SPEC::enabled(BERNOULLI) && diff <= BLOCK_HASH_HISTORY { let mut hasher = crate::primitives::Keccak256::new(); hasher.update(host.env().cfg.chain_id.to_be_bytes()); - hasher.update(diff.to_be_bytes()); + hasher.update(block_number.to_be_bytes()); *number = U256::from_be_bytes(*hasher.finalize()); return; } diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index abac40bc27..dcde71dc44 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -367,3 +367,40 @@ macro_rules! as_usize_or_fail_ret { } }; } + +/// Converts a `U256` value to a `u64`, failing the instruction if the value is too large. +#[macro_export] +macro_rules! as_u64_or_fail { + ($interp:expr, $v:expr) => { + $crate::as_u64_or_fail_ret!($interp, $v, ()) + }; + ($interp:expr, $v:expr, $reason:expr) => { + $crate::as_u64_or_fail_ret!($interp, $v, $reason, ()) + }; +} + +/// Converts a `U256` value to a `usize` and returns `ret`, +/// failing the instruction if the value is too large. +#[macro_export] +macro_rules! as_u64_or_fail_ret { + ($interp:expr, $v:expr, $ret:expr) => { + $crate::as_u64_or_fail_ret!( + $interp, + $v, + $crate::InstructionResult::InvalidOperandOOG, + $ret + ) + }; + + ($interp:expr, $v:expr, $reason:expr, $ret:expr) => { + match $v.as_limbs() { + x => { + if (x[0] > u64::MAX) | (x[1] != 0) | (x[2] != 0) | (x[3] != 0) { + $interp.instruction_result = $reason; + return $ret; + } + x[0] as usize + } + } + }; +} diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index 2e11278be8..221ea09d28 100644 --- a/crates/primitives/src/state.rs +++ b/crates/primitives/src/state.rs @@ -233,6 +233,7 @@ impl PartialEq for AccountInfo { #[cfg(all(debug_assertions, feature = "scroll"))] if eq { + assert_eq!(self.code_size, other.code_size); assert_eq!(self.keccak_code_hash, other.keccak_code_hash); } eq @@ -281,6 +282,15 @@ impl AccountInfo { /// - nonce is zero pub fn is_empty(&self) -> bool { let code_empty = self.is_empty_code_hash() || self.code_hash == B256::ZERO; + + #[cfg(all(feature = "scroll", debug_assertions))] + if code_empty { + assert_eq!( + self.code_size, 0, + "code size should be zero if code hash is empty" + ); + } + code_empty && self.balance == U256::ZERO && self.nonce == 0 } @@ -312,13 +322,18 @@ impl AccountInfo { /// Returns true if the code hash is the Keccak256 hash of the empty string `""`. #[inline] pub fn is_empty_code_hash(&self) -> bool { - #[cfg(feature = "scroll")] - { - self.code_hash == POSEIDON_EMPTY - } - #[cfg(not(feature = "scroll"))] - { - self.code_hash == KECCAK_EMPTY + cfg_if::cfg_if! { + if #[cfg(feature = "scroll")] { + #[cfg(debug_assertions)] + if self.code_hash == POSEIDON_EMPTY { + assert_eq!(self.code_size, 0); + assert_eq!(self.keccak_code_hash, KECCAK_EMPTY); + } + + self.code_hash == POSEIDON_EMPTY + } else { + self.code_hash = KECCAK_EMPTY + } } } @@ -327,6 +342,39 @@ impl AccountInfo { self.code.take() } + /// Re-hash the code, set to empty if code is None, + /// otherwise update the code hash. + pub fn set_code_rehash_slow(&mut self, code: Option) { + match code { + Some(code) => { + cfg_if::cfg_if! { + if #[cfg(feature = "scroll")] { + self.code_size = code.len(); + self.code_hash = code.poseidon_hash_slow(); + self.keccak_code_hash = code.keccak_hash_slow(); + } else { + self.code_hash = code.keccak_hash_slow(); + } + } + + self.code = Some(code); + } + None => { + cfg_if::cfg_if! { + if #[cfg(feature = "scroll")] { + self.code_size = 0; + self.code_hash = POSEIDON_EMPTY; + self.keccak_code_hash = KECCAK_EMPTY; + } else { + self.code_hash = KECCAK_EMPTY; + } + } + + self.code = None; + } + } + } + pub fn from_balance(balance: U256) -> Self { AccountInfo { balance, diff --git a/crates/revm/src/db/in_memory_db.rs b/crates/revm/src/db/in_memory_db.rs index b3a061d6e3..04cf5887e3 100644 --- a/crates/revm/src/db/in_memory_db.rs +++ b/crates/revm/src/db/in_memory_db.rs @@ -75,6 +75,7 @@ impl CacheDB { } #[cfg(feature = "scroll")] { + account.code_size = code.len(); if account.code_hash == POSEIDON_EMPTY { account.code_hash = code.poseidon_hash_slow(); } @@ -88,16 +89,7 @@ impl CacheDB { } } if account.code_hash == B256::ZERO { - #[cfg(not(feature = "scroll"))] - { - account.code_hash = KECCAK_EMPTY; - } - #[cfg(feature = "scroll")] - { - debug_assert_eq!(account.keccak_code_hash, B256::ZERO); - account.code_hash = POSEIDON_EMPTY; - account.keccak_code_hash = KECCAK_EMPTY; - } + account.set_code_empty(); } } diff --git a/crates/revm/src/journaled_state.rs b/crates/revm/src/journaled_state.rs index 8c112e109e..aed84c24c0 100644 --- a/crates/revm/src/journaled_state.rs +++ b/crates/revm/src/journaled_state.rs @@ -157,17 +157,7 @@ impl JournaledState { .unwrap() .push(JournalEntry::CodeChange { address }); - #[cfg(not(feature = "scroll"))] - { - account.info.code_hash = code.hash_slow(); - } - #[cfg(feature = "scroll")] - { - account.info.code_size = code.len(); - account.info.code_hash = code.poseidon_hash_slow(); - account.info.keccak_code_hash = code.keccak_hash_slow(); - } - account.info.code = Some(code); + account.info.set_code_rehash_slow(Some(code)); } #[inline] @@ -409,16 +399,7 @@ impl JournaledState { } JournalEntry::CodeChange { address } => { let acc = state.get_mut(&address).unwrap(); - #[cfg(not(feature = "scroll"))] - { - acc.info.code_hash = KECCAK_EMPTY; - } - #[cfg(feature = "scroll")] - { - acc.info.code_hash = POSEIDON_EMPTY; - acc.info.keccak_code_hash = KECCAK_EMPTY; - } - acc.info.code = None; + acc.info.set_code_empty(); } } } From c65aee1641c6f5516b2e0538476420592701f3d6 Mon Sep 17 00:00:00 2001 From: lightsing Date: Thu, 6 Jun 2024 15:07:12 +0800 Subject: [PATCH 2/2] fix default --- crates/primitives/src/state.rs | 4 ++-- crates/revm/src/db/in_memory_db.rs | 2 +- crates/revm/src/journaled_state.rs | 14 +++----------- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index 221ea09d28..74089676e6 100644 --- a/crates/primitives/src/state.rs +++ b/crates/primitives/src/state.rs @@ -332,7 +332,7 @@ impl AccountInfo { self.code_hash == POSEIDON_EMPTY } else { - self.code_hash = KECCAK_EMPTY + self.code_hash == KECCAK_EMPTY } } } @@ -353,7 +353,7 @@ impl AccountInfo { self.code_hash = code.poseidon_hash_slow(); self.keccak_code_hash = code.keccak_hash_slow(); } else { - self.code_hash = code.keccak_hash_slow(); + self.code_hash = code.hash_slow(); } } diff --git a/crates/revm/src/db/in_memory_db.rs b/crates/revm/src/db/in_memory_db.rs index 04cf5887e3..4b652e7864 100644 --- a/crates/revm/src/db/in_memory_db.rs +++ b/crates/revm/src/db/in_memory_db.rs @@ -89,7 +89,7 @@ impl CacheDB { } } if account.code_hash == B256::ZERO { - account.set_code_empty(); + account.set_code_rehash_slow(None); } } diff --git a/crates/revm/src/journaled_state.rs b/crates/revm/src/journaled_state.rs index aed84c24c0..852a177900 100644 --- a/crates/revm/src/journaled_state.rs +++ b/crates/revm/src/journaled_state.rs @@ -1,16 +1,13 @@ use crate::interpreter::{InstructionResult, SelfDestructResult}; use crate::primitives::{ db::Database, hash_map::Entry, Account, Address, Bytecode, EVMError, HashMap, HashSet, Log, - SpecId::*, State, StorageSlot, TransientStorage, KECCAK_EMPTY, PRECOMPILE3, U256, + SpecId::*, State, StorageSlot, TransientStorage, PRECOMPILE3, U256, }; use core::mem; use revm_interpreter::primitives::SpecId; use revm_interpreter::{LoadAccountResult, SStoreResult}; use std::vec::Vec; -#[cfg(feature = "scroll")] -use crate::primitives::POSEIDON_EMPTY; - /// JournalState is internal EVM state that is used to contain state and track changes to that state. /// It contains journal of changes that happened to state so that they can be reverted. #[derive(Debug, Clone, Eq, PartialEq)] @@ -399,7 +396,7 @@ impl JournaledState { } JournalEntry::CodeChange { address } => { let acc = state.get_mut(&address).unwrap(); - acc.info.set_code_empty(); + acc.info.set_code_rehash_slow(None); } } } @@ -613,12 +610,7 @@ impl JournaledState { ) -> Result<(&mut Account, bool), EVMError> { let (acc, is_cold) = self.load_account(address, db)?; if acc.info.code.is_none() { - #[cfg(not(feature = "scroll"))] - let is_empty = acc.info.code_hash == KECCAK_EMPTY; - #[cfg(feature = "scroll")] - let is_empty = acc.info.code_hash == POSEIDON_EMPTY; - - if is_empty { + if acc.info.is_empty_code_hash() { let empty = Bytecode::default(); acc.info.code = Some(empty); } else {