many fix and more assert (#7)

* many fix and more assert * fix default
scroll-tech · Jun 6, 2024 · 074cba2 · 074cba2
1 parent 8a28fe3
commit 074cba2
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 50 deletions.
diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs
@@ -164,6 +164,8 @@ pub fn blockhash<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, ho
 
 #[cfg(feature = "scroll")]
 pub fn blockhash<H: Host + ?Sized, SPEC: Spec>(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<H: Host + ?Sized, SPEC: Spec>(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
@@ -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
@@ -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<Bytecode>) {
+        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.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
@@ -75,6 +75,7 @@ impl<ExtDB> CacheDB<ExtDB> {
                 }
                 #[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<ExtDB> CacheDB<ExtDB> {
             }
         }
         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_rehash_slow(None);
         }
     }
 

diff --git 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)]
@@ -157,17 +154,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 +396,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_rehash_slow(None);
                 }
             }
         }
@@ -632,12 +610,7 @@ impl JournaledState {
     ) -> Result<(&mut Account, bool), EVMError<DB::Error>> {
         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 {