chore(primitives): rename State/Storage to EvmState/EvmStorage (#1459)

* fix(Interpreter): wrong block number used

* chore(primitives): rename State to PlainState

* Restructure StorageSlot and rename PlainState to EvmState

* doc

crates/primitives/src/result.rs

@@ -1,4 +1,4 @@
-use crate::{Address, Bytes, Log, State, U256};
+use crate::{Address, Bytes, EvmState, Log, U256};
 use core::fmt;
 use std::{boxed::Box, string::String, vec::Vec};
 
@@ -14,7 +14,7 @@ pub struct ResultAndState {
     /// Status of execution
     pub result: ExecutionResult,
     /// State that got updated
-    pub state: State,
+    pub state: EvmState,
 }
 
 /// Result of a transaction execution.

crates/primitives/src/state.rs

@@ -3,21 +3,21 @@ use bitflags::bitflags;
 use core::hash::{Hash, Hasher};
 
 /// EVM State is a mapping from addresses to accounts.
-pub type State = HashMap<Address, Account>;
+pub type EvmState = HashMap<Address, Account>;
 
 /// Structure used for EIP-1153 transient storage.
 pub type TransientStorage = HashMap<(Address, U256), U256>;
 
-/// An account's Storage is a mapping from 256-bit integer keys to [StorageSlot]s.
-pub type Storage = HashMap<U256, StorageSlot>;
+/// An account's Storage is a mapping from 256-bit integer keys to [EvmStorageSlot]s.
+pub type EvmStorage = HashMap<U256, EvmStorageSlot>;
 
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Account {
     /// Balance, nonce, and code.
     pub info: AccountInfo,
     /// Storage cache
-    pub storage: Storage,
+    pub storage: EvmStorage,
     /// Account status flags.
     pub status: AccountStatus,
 }
@@ -119,8 +119,8 @@ impl Account {
 
     /// Returns an iterator over the storage slots that have been changed.
     ///
-    /// See also [StorageSlot::is_changed]
-    pub fn changed_storage_slots(&self) -> impl Iterator<Item = (&U256, &StorageSlot)> {
+    /// See also [EvmStorageSlot::is_changed]
+    pub fn changed_storage_slots(&self) -> impl Iterator<Item = (&U256, &EvmStorageSlot)> {
         self.storage.iter().filter(|(_, slot)| slot.is_changed())
     }
 }
@@ -138,42 +138,37 @@ impl From<AccountInfo> for Account {
 /// This type keeps track of the current value of a storage slot.
 #[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-pub struct StorageSlot {
-    /// The value of the storage slot before it was changed.
-    ///
-    /// When the slot is first loaded, this is the original value.
-    ///
-    /// If the slot was not changed, this is equal to the present value.
-    pub previous_or_original_value: U256,
-    /// When loaded with sload present value is set to original value
+pub struct EvmStorageSlot {
+    /// Original value of the storage slot.
+    pub original_value: U256,
+    /// Present value of the storage slot.
     pub present_value: U256,
 }
 
-impl StorageSlot {
-    /// Creates a new _unchanged_ `StorageSlot` for the given value.
+impl EvmStorageSlot {
+    /// Creates a new _unchanged_ `EvmStorageSlot` for the given value.
     pub fn new(original: U256) -> Self {
         Self {
-            previous_or_original_value: original,
+            original_value: original,
             present_value: original,
         }
     }
 
-    /// Creates a new _changed_ `StorageSlot`.
-    pub fn new_changed(previous_or_original_value: U256, present_value: U256) -> Self {
+    /// Creates a new _changed_ `EvmStorageSlot`.
+    pub fn new_changed(original_value: U256, present_value: U256) -> Self {
         Self {
-            previous_or_original_value,
+            original_value,
             present_value,
         }
     }
-
     /// Returns true if the present value differs from the original value
     pub fn is_changed(&self) -> bool {
-        self.previous_or_original_value != self.present_value
+        self.original_value != self.present_value
     }
 
     /// Returns the original value of the storage slot.
     pub fn original_value(&self) -> U256 {
-        self.previous_or_original_value
+        self.original_value
     }
 
     /// Returns the current value of the storage slot.

crates/revm/src/db/states.rs

@@ -18,7 +18,7 @@ pub use bundle_state::{BundleBuilder, BundleState, OriginalValuesKnown};
 pub use cache::CacheState;
 pub use cache_account::CacheAccount;
 pub use changes::{PlainStateReverts, PlainStorageChangeset, PlainStorageRevert, StateChangeset};
-pub use plain_account::{PlainAccount, StorageWithOriginalValues};
+pub use plain_account::{PlainAccount, StorageSlot, StorageWithOriginalValues};
 pub use reverts::{AccountRevert, RevertToSlot};
 pub use state::{DBBox, State, StateDBBox};
 pub use state_builder::StateBuilder;

crates/revm/src/db/states/bundle_account.rs

@@ -1,8 +1,8 @@
 use super::{
-    reverts::AccountInfoRevert, AccountRevert, AccountStatus, RevertToSlot,
+    reverts::AccountInfoRevert, AccountRevert, AccountStatus, RevertToSlot, StorageSlot,
     StorageWithOriginalValues, TransitionAccount,
 };
-use revm_interpreter::primitives::{AccountInfo, StorageSlot, U256};
+use revm_interpreter::primitives::{AccountInfo, U256};
 use revm_precompile::HashMap;
 
 /// Account information focused on creating of database changesets

crates/revm/src/db/states/bundle_state.rs

@@ -1,12 +1,13 @@
 use super::{
     changes::{PlainStorageChangeset, StateChangeset},
     reverts::{AccountInfoRevert, Reverts},
-    AccountRevert, AccountStatus, BundleAccount, PlainStateReverts, RevertToSlot, TransitionState,
+    AccountRevert, AccountStatus, BundleAccount, PlainStateReverts, RevertToSlot, StorageSlot,
+    TransitionState,
 };
 use core::{mem, ops::RangeInclusive};
 use revm_interpreter::primitives::{
     hash_map::{self, Entry},
-    AccountInfo, Address, Bytecode, HashMap, HashSet, StorageSlot, B256, KECCAK_EMPTY, U256,
+    AccountInfo, Address, Bytecode, HashMap, HashSet, B256, KECCAK_EMPTY, U256,
 };
 use std::{
     collections::{BTreeMap, BTreeSet},

crates/revm/src/db/states/cache.rs

@@ -2,7 +2,7 @@ use super::{
     plain_account::PlainStorage, transition_account::TransitionAccount, CacheAccount, PlainAccount,
 };
 use revm_interpreter::primitives::{
-    Account, AccountInfo, Address, Bytecode, HashMap, State as EVMState, B256,
+    Account, AccountInfo, Address, Bytecode, EvmState, HashMap, B256,
 };
 use std::vec::Vec;
 
@@ -88,7 +88,7 @@ impl CacheState {
     }
 
     /// Apply output of revm execution and create account transitions that are used to build BundleState.
-    pub fn apply_evm_state(&mut self, evm_state: EVMState) -> Vec<(Address, TransitionAccount)> {
+    pub fn apply_evm_state(&mut self, evm_state: EvmState) -> Vec<(Address, TransitionAccount)> {
         let mut transitions = Vec::with_capacity(evm_state.len());
         for (address, account) in evm_state {
             if let Some(transition) = self.apply_account_state(address, account) {
@@ -121,6 +121,17 @@ impl CacheState {
             return this_account.selfdestruct();
         }
 
+        let is_created = account.is_created();
+        let is_empty = account.is_empty();
+
+        // transform evm storage to storage with previous value.
+        let changed_storage = account
+            .storage
+            .into_iter()
+            .filter(|(_, slot)| slot.is_changed())
+            .map(|(key, slot)| (key, slot.into()))
+            .collect();
+
         // Note: it can happen that created contract get selfdestructed in same block
         // that is why is_created is checked after selfdestructed
         //
@@ -129,25 +140,25 @@ impl CacheState {
         // Note: It is possibility to create KECCAK_EMPTY contract with some storage
         // by just setting storage inside CRATE constructor. Overlap of those contracts
         // is not possible because CREATE2 is introduced later.
-        if account.is_created() {
-            return Some(this_account.newly_created(account.info, account.storage));
+        if is_created {
+            return Some(this_account.newly_created(account.info, changed_storage));
         }
 
         // Account is touched, but not selfdestructed or newly created.
         // Account can be touched and not changed.
         // And when empty account is touched it needs to be removed from database.
         // EIP-161 state clear
-        if account.is_empty() {
+        if is_empty {
             if self.has_state_clear {
                 // touch empty account.
                 this_account.touch_empty_eip161()
             } else {
                 // if account is empty and state clear is not enabled we should save
                 // empty account.
-                this_account.touch_create_pre_eip161(account.storage)
+                this_account.touch_create_pre_eip161(changed_storage)
             }
         } else {
-            Some(this_account.change(account.info, account.storage))
+            Some(this_account.change(account.info, changed_storage))
         }
     }
 }

crates/revm/src/db/states/plain_account.rs

@@ -1,4 +1,4 @@
-use revm_interpreter::primitives::{AccountInfo, HashMap, StorageSlot, U256};
+use crate::primitives::{AccountInfo, EvmStorageSlot, HashMap, U256};
 
 // TODO rename this to BundleAccount. As for the block level we have original state.
 #[derive(Clone, Debug, Default, PartialEq, Eq)]
@@ -20,9 +20,62 @@ impl PlainAccount {
     }
 }
 
+/// This type keeps track of the current value of a storage slot.
+#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct StorageSlot {
+    /// The value of the storage slot before it was changed.
+    ///
+    /// When the slot is first loaded, this is the original value.
+    ///
+    /// If the slot was not changed, this is equal to the present value.
+    pub previous_or_original_value: U256,
+    /// When loaded with sload present value is set to original value
+    pub present_value: U256,
+}
+
+impl From<EvmStorageSlot> for StorageSlot {
+    fn from(value: EvmStorageSlot) -> Self {
+        Self::new_changed(value.original_value, value.present_value)
+    }
+}
+
+impl StorageSlot {
+    /// Creates a new _unchanged_ `StorageSlot` for the given value.
+    pub fn new(original: U256) -> Self {
+        Self {
+            previous_or_original_value: original,
+            present_value: original,
+        }
+    }
+
+    /// Creates a new _changed_ `StorageSlot`.
+    pub fn new_changed(previous_or_original_value: U256, present_value: U256) -> Self {
+        Self {
+            previous_or_original_value,
+            present_value,
+        }
+    }
+
+    /// Returns true if the present value differs from the original value
+    pub fn is_changed(&self) -> bool {
+        self.previous_or_original_value != self.present_value
+    }
+
+    /// Returns the original value of the storage slot.
+    pub fn original_value(&self) -> U256 {
+        self.previous_or_original_value
+    }
+
+    /// Returns the current value of the storage slot.
+    pub fn present_value(&self) -> U256 {
+        self.present_value
+    }
+}
+
 /// This storage represent values that are before block changed.
 ///
-/// Note: Storage that we get EVM contains original values before t
+/// Note: Storage that we get EVM contains original values before block changed.
 pub type StorageWithOriginalValues = HashMap<U256, StorageSlot>;
 
 /// Simple plain storage that does not have previous value.

crates/revm/src/db/states/state.rs

@@ -303,10 +303,10 @@ impl<DB: Database> DatabaseCommit for State<DB> {
 mod tests {
     use super::*;
     use crate::db::{
-        states::reverts::AccountInfoRevert, AccountRevert, AccountStatus, BundleAccount,
-        RevertToSlot,
+        states::{reverts::AccountInfoRevert, StorageSlot},
+        AccountRevert, AccountStatus, BundleAccount, RevertToSlot,
     };
-    use revm_interpreter::primitives::{keccak256, StorageSlot};
+    use revm_interpreter::primitives::keccak256;
 
     #[test]
     fn block_hash_cache() {

crates/revm/src/journaled_state.rs

@@ -1,7 +1,7 @@
 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,
+    db::Database, hash_map::Entry, Account, Address, Bytecode, EVMError, EvmState, EvmStorageSlot,
+    HashMap, HashSet, Log, SpecId::*, TransientStorage, KECCAK_EMPTY, PRECOMPILE3, U256,
 };
 use core::mem;
 use revm_interpreter::primitives::SpecId;
@@ -14,7 +14,7 @@ use std::vec::Vec;
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct JournaledState {
     /// Current state.
-    pub state: State,
+    pub state: EvmState,
     /// [EIP-1153](https://eips.ethereum.org/EIPS/eip-1153) transient storage that is discarded after every transactions
     pub transient_storage: TransientStorage,
     /// logs
@@ -62,7 +62,7 @@ impl JournaledState {
 
     /// Return reference to state.
     #[inline]
-    pub fn state(&mut self) -> &mut State {
+    pub fn state(&mut self) -> &mut EvmState {
         &mut self.state
     }
 
@@ -101,7 +101,7 @@ impl JournaledState {
     ///
     /// This resets the [JournaledState] to its initial state in [Self::new]
     #[inline]
-    pub fn finalize(&mut self) -> (State, Vec<Log>) {
+    pub fn finalize(&mut self) -> (EvmState, Vec<Log>) {
         let Self {
             state,
             transient_storage,
@@ -273,7 +273,7 @@ impl JournaledState {
         // Set all storages to default value. They need to be present to act as accessed slots in access list.
         // it shouldn't be possible for them to have different values then zero as code is not existing for this account,
         // but because tests can change that assumption we are doing it.
-        let empty = StorageSlot::default();
+        let empty = EvmStorageSlot::default();
         account
             .storage
             .iter_mut()
@@ -314,7 +314,7 @@ impl JournaledState {
     /// Revert all changes that happened in given journal entries.
     #[inline]
     fn journal_revert(
-        state: &mut State,
+        state: &mut EvmState,
         transient_storage: &mut TransientStorage,
         journal_entries: Vec<JournalEntry>,
         is_spurious_dragon_enabled: bool,
@@ -542,7 +542,7 @@ impl JournaledState {
         for slot in slots {
             if let Entry::Vacant(entry) = account.storage.entry(*slot) {
                 let storage = db.storage(address, *slot).map_err(EVMError::Database)?;
-                entry.insert(StorageSlot::new(storage));
+                entry.insert(EvmStorageSlot::new(storage));
             }
         }
         Ok(account)
@@ -660,7 +660,7 @@ impl JournaledState {
                         had_value: None,
                     });
 
-                vac.insert(StorageSlot::new(value));
+                vac.insert(EvmStorageSlot::new(value));
 
                 (value, true)
             }
@@ -692,7 +692,7 @@ impl JournaledState {
         // new value is same as present, we don't need to do anything
         if present == new {
             return Ok(SStoreResult {
-                original_value: slot.previous_or_original_value,
+                original_value: slot.original_value(),
                 present_value: present,
                 new_value: new,
                 is_cold,
@@ -710,7 +710,7 @@ impl JournaledState {
         // insert value into present state.
         slot.present_value = new;
         Ok(SStoreResult {
-            original_value: slot.previous_or_original_value,
+            original_value: slot.original_value(),
             present_value: present,
             new_value: new,
             is_cold,