chore(levm): improve storage_original_values hashmap and gen_db field names (#3261)

**Motivation**

We can avoid a double lookup with `storage_original_values` by using a
tuple.

I found gen_db field names to be a bit confusing and not giving useful
information, "immutable_cache" is not really a cache but a store for
initial account states, so i changed it. Same with cache.

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves #111, Resolves #222 -->

Closes #issue_number

Author: edg-l

cmd/ef_tests/state/runner/levm_runner.rs

@@ -330,7 +330,7 @@ pub async fn ensure_post_state(
     fork: &Fork,
     db: &mut GeneralizedDatabase,
 ) -> Result<(), EFTestRunnerError> {
-    let cache = db.cache.clone();
+    let cache = db.current_accounts_state.clone();
     match levm_execution_result {
         Ok(execution_report) => {
             match test.post.vector_post_value(vector, *fork).expect_exception {

cmd/ef_tests/state/runner/revm_runner.rs

@@ -376,7 +376,7 @@ pub async fn ensure_post_state(
         // We only want to compare account updates when no exception is expected.
         None => {
             let mut db = load_initial_state_levm(test).await;
-            db.cache = levm_cache;
+            db.current_accounts_state = levm_cache;
             let levm_account_updates = backends::levm::LEVM::get_state_transitions(&mut db)
                 .map_err(|_| {
                     InternalError::Custom("Error at LEVM::get_state_transitions()".to_owned())

crates/l2/sequencer/block_producer/payload_builder.rs

@@ -276,12 +276,9 @@ fn get_account_diffs_in_tx(
             })?;
             // First we add the account info
             for (address, original_account) in transaction_backup.original_accounts_info.iter() {
-                let new_account =
-                    db.cache
-                        .get(address)
-                        .ok_or(BlockProducerError::FailedToGetDataFrom(
-                            "DB Cache".to_owned(),
-                        ))?;
+                let new_account = db.current_accounts_state.get(address).ok_or(
+                    BlockProducerError::FailedToGetDataFrom("DB Cache".to_owned()),
+                )?;
 
                 let nonce_diff: u16 = (new_account.info.nonce - original_account.info.nonce)
                     .try_into()
@@ -314,12 +311,9 @@ fn get_account_diffs_in_tx(
             for (address, original_storage_slots) in
                 transaction_backup.original_account_storage_slots.iter()
             {
-                let account_info =
-                    db.cache
-                        .get(address)
-                        .ok_or(BlockProducerError::FailedToGetDataFrom(
-                            "DB Cache".to_owned(),
-                        ))?;
+                let account_info = db.current_accounts_state.get(address).ok_or(
+                    BlockProducerError::FailedToGetDataFrom("DB Cache".to_owned()),
+                )?;
 
                 let mut added_storage = BTreeMap::new();
                 for key in original_storage_slots.keys() {

crates/vm/backends/levm/mod.rs

@@ -192,10 +192,10 @@ impl LEVM {
         db: &mut GeneralizedDatabase,
     ) -> Result<Vec<AccountUpdate>, EvmError> {
         let mut account_updates: Vec<AccountUpdate> = vec![];
-        for (address, new_state_account) in db.cache.iter() {
+        for (address, new_state_account) in db.current_accounts_state.iter() {
             // In case the account is not in immutable_cache (rare) we search for it in the actual database.
             let initial_state_account =
-                db.immutable_cache
+                db.initial_accounts_state
                     .get(address)
                     .ok_or(EvmError::Custom(format!(
                         "Failed to get account {address} from immutable cache",
@@ -258,8 +258,8 @@ impl LEVM {
 
             account_updates.push(account_update);
         }
-        db.cache.clear();
-        db.immutable_cache.clear();
+        db.current_accounts_state.clear();
+        db.initial_accounts_state.clear();
         Ok(account_updates)
     }
 
@@ -279,7 +279,7 @@ impl LEVM {
                 .clone(); // Not a big deal cloning here because it's an EOA.
 
             account.info.balance += increment.into();
-            db.cache.insert(address, account);
+            db.current_accounts_state.insert(address, account);
         }
         Ok(())
     }
@@ -435,8 +435,11 @@ pub fn generic_system_contract_levm(
 ) -> Result<ExecutionReport, EvmError> {
     let chain_config = db.store.get_chain_config()?;
     let config = EVMConfig::new_from_chain_config(&chain_config, block_header);
-    let system_account_backup = db.cache.get(&system_address).cloned();
-    let coinbase_backup = db.cache.get(&block_header.coinbase).cloned();
+    let system_account_backup = db.current_accounts_state.get(&system_address).cloned();
+    let coinbase_backup = db
+        .current_accounts_state
+        .get(&block_header.coinbase)
+        .cloned();
     let env = Environment {
         origin: system_address,
         // EIPs 2935, 4788, 7002 and 7251 dictate that the system calls have a gas limit of 30 million and they do not use intrinsic gas.
@@ -466,17 +469,19 @@ pub fn generic_system_contract_levm(
     let report = vm.execute().map_err(EvmError::from)?;
 
     if let Some(system_account) = system_account_backup {
-        db.cache.insert(system_address, system_account);
+        db.current_accounts_state
+            .insert(system_address, system_account);
     } else {
         // If the system account was not in the cache, we need to remove it
-        db.cache.remove(&system_address);
+        db.current_accounts_state.remove(&system_address);
     }
 
     if let Some(coinbase_account) = coinbase_backup {
-        db.cache.insert(block_header.coinbase, coinbase_account);
+        db.current_accounts_state
+            .insert(block_header.coinbase, coinbase_account);
     } else {
         // If the coinbase account was not in the cache, we need to remove it
-        db.cache.remove(&block_header.coinbase);
+        db.current_accounts_state.remove(&block_header.coinbase);
     }
 
     Ok(report)

crates/vm/levm/src/db/gen_db.rs

@@ -21,17 +21,17 @@ use super::cache;
 #[derive(Clone)]
 pub struct GeneralizedDatabase {
     pub store: Arc<dyn Database>,
-    pub cache: CacheDB,
-    pub immutable_cache: HashMap<Address, Account>,
+    pub current_accounts_state: CacheDB,
+    pub initial_accounts_state: HashMap<Address, Account>,
     pub tx_backup: Option<CallFrameBackup>,
 }
 
 impl GeneralizedDatabase {
-    pub fn new(store: Arc<dyn Database>, cache: CacheDB) -> Self {
+    pub fn new(store: Arc<dyn Database>, current_accounts_state: CacheDB) -> Self {
         Self {
             store,
-            cache: cache.clone(),
-            immutable_cache: cache,
+            current_accounts_state: current_accounts_state.clone(),
+            initial_accounts_state: current_accounts_state,
             tx_backup: None,
         }
     }
@@ -40,11 +40,12 @@ impl GeneralizedDatabase {
     /// Gets account, first checking the cache and then the database
     /// (caching in the second case)
     pub fn get_account(&mut self, address: Address) -> Result<&Account, InternalError> {
-        if !cache::account_is_cached(&self.cache, &address) {
+        if !cache::account_is_cached(&self.current_accounts_state, &address) {
             let account = self.get_account_from_database(address)?;
-            cache::insert_account(&mut self.cache, address, account);
+            cache::insert_account(&mut self.current_accounts_state, address, account);
         }
-        cache::get_account(&self.cache, &address).ok_or(InternalError::AccountNotFound)
+        cache::get_account(&self.current_accounts_state, &address)
+            .ok_or(InternalError::AccountNotFound)
     }
 
     /// **Accesses to an account's information.**
@@ -62,25 +63,25 @@ impl GeneralizedDatabase {
         Ok((account, address_was_cold))
     }
 
-    /// Gets account from storage, storing in Immutable Cache for efficiency when getting AccountUpdates.
+    /// Gets account from storage, storing in initial_accounts_state for efficiency when getting AccountUpdates.
     pub fn get_account_from_database(
         &mut self,
         address: Address,
     ) -> Result<Account, InternalError> {
         let account = self.store.get_account(address)?;
-        self.immutable_cache.insert(address, account.clone());
+        self.initial_accounts_state.insert(address, account.clone());
         Ok(account)
     }
 
-    /// Gets storage slot from Database, storing in Immutable Cache for efficiency when getting AccountUpdates.
+    /// Gets storage slot from Database, storing in initial_accounts_state for efficiency when getting AccountUpdates.
     pub fn get_value_from_database(
         &mut self,
         address: Address,
         key: H256,
     ) -> Result<U256, InternalError> {
         let value = self.store.get_storage_value(address, key)?;
-        // Account must already be in immutable_cache
-        match self.immutable_cache.get_mut(&address) {
+        // Account must already be in initial_accounts_state
+        match self.initial_accounts_state.get_mut(&address) {
             Some(account) => {
                 account.storage.insert(key, value);
             }
@@ -133,15 +134,15 @@ impl<'a> VM<'a> {
 
     */
     pub fn get_account_mut(&mut self, address: Address) -> Result<&mut Account, InternalError> {
-        if cache::is_account_cached(&self.db.cache, &address) {
+        if cache::is_account_cached(&self.db.current_accounts_state, &address) {
             self.backup_account_info(address)?;
-            cache::get_account_mut(&mut self.db.cache, &address)
+            cache::get_account_mut(&mut self.db.current_accounts_state, &address)
                 .ok_or(InternalError::AccountNotFound)
         } else {
             let acc = self.db.get_account_from_database(address)?;
-            cache::insert_account(&mut self.db.cache, address, acc);
+            cache::insert_account(&mut self.db.current_accounts_state, address, acc);
             self.backup_account_info(address)?;
-            cache::get_account_mut(&mut self.db.cache, &address)
+            cache::get_account_mut(&mut self.db.current_accounts_state, &address)
                 .ok_or(InternalError::AccountNotFound)
         }
     }
@@ -214,7 +215,7 @@ impl<'a> VM<'a> {
         account: Account,
     ) -> Result<(), InternalError> {
         self.backup_account_info(address)?;
-        let _ = cache::insert_account(&mut self.db.cache, address, account);
+        let _ = cache::insert_account(&mut self.db.current_accounts_state, address, account);
 
         Ok(())
     }
@@ -226,19 +227,12 @@ impl<'a> VM<'a> {
         address: Address,
         key: H256,
     ) -> Result<U256, InternalError> {
-        if let Some(value) = self
-            .storage_original_values
-            .get(&address)
-            .and_then(|account_storage| account_storage.get(&key))
-        {
+        if let Some(value) = self.storage_original_values.get(&(address, key)) {
             return Ok(*value);
         }
 
         let value = self.get_storage_value(address, key)?;
-        self.storage_original_values
-            .entry(address)
-            .or_default()
-            .insert(key, value);
+        self.storage_original_values.insert((address, key), value);
         Ok(value)
     }
 
@@ -270,7 +264,7 @@ impl<'a> VM<'a> {
         address: Address,
         key: H256,
     ) -> Result<U256, InternalError> {
-        if let Some(account) = cache::get_account(&self.db.cache, &address) {
+        if let Some(account) = cache::get_account(&self.db.current_accounts_state, &address) {
             if let Some(value) = account.storage.get(&key) {
                 return Ok(*value);
             }
@@ -333,7 +327,7 @@ impl<'a> VM<'a> {
             .contains_key(&address);
 
         if is_not_backed_up {
-            let account = cache::get_account(&self.db.cache, &address)
+            let account = cache::get_account(&self.db.current_accounts_state, &address)
                 .ok_or(InternalError::AccountNotFound)?;
             let info = account.info.clone();
             let code = account.code.clone();

crates/vm/levm/src/utils.rs

@@ -130,7 +130,9 @@ pub fn restore_cache_state(
     callframe_backup: CallFrameBackup,
 ) -> Result<(), VMError> {
     for (address, account) in callframe_backup.original_accounts_info {
-        if let Some(current_account) = cache::get_account_mut(&mut db.cache, &address) {
+        if let Some(current_account) =
+            cache::get_account_mut(&mut db.current_accounts_state, &address)
+        {
             current_account.info = account.info;
             current_account.code = account.code;
         }
@@ -140,7 +142,7 @@ pub fn restore_cache_state(
         // This call to `get_account_mut` should never return None, because we are looking up accounts
         // that had their storage modified, which means they should be in the cache. That's why
         // we return an internal error in case we haven't found it.
-        let account = cache::get_account_mut(&mut db.cache, &address)
+        let account = cache::get_account_mut(&mut db.current_accounts_state, &address)
             .ok_or(InternalError::AccountNotFound)?;
 
         for (key, value) in storage {

crates/vm/levm/src/vm.rs

@@ -47,7 +47,7 @@ pub struct VM<'a> {
     pub hooks: Vec<Rc<RefCell<dyn Hook>>>,
     pub substate_backups: Vec<Substate>,
     /// Original storage values before the transaction. Used for gas calculations in SSTORE.
-    pub storage_original_values: HashMap<Address, HashMap<H256, U256>>,
+    pub storage_original_values: HashMap<(Address, H256), U256>,
     /// When enabled, it "logs" relevant information during execution
     pub tracer: LevmCallTracer,
     /// Mode for printing some useful stuff, only used in development!