refactor(levm): improve and simplify some db functions (#2651)

**Motivation**

<!-- Why does this pull request exist? What are its goals? -->
- Try to remove `account_exists` if possible because it adds complexity
and unnecessary checks to the DB.
- Try to finally remove `get_account_no_push_cache`, which is related to
the previous thing too.

**Description**

- We now ignore a specific test because [EIP-7702 spec has
changed](ethereum/EIPs#9710) and we no longer
need to check if the account exists in the trie.
- Remove `Option` from `specific_tests`
- Remove `get_account_no_push_cache` and the usage of `account_exists`
in LEVM. This method is not deleted from the Database because it's used
in `get_state_transitions`, and even here it could be removed but I
think it is better to keep it in this PR and maybe decide later what to
do with this function. (If we remove it it wouldn't make a difference to
the state though).
- We were able to remove a SpuriousDragon check because we don't support
pre-merge forks now


Note: `account_exists` hasn't been completely removed from `Database`
because we use it in `get_state_transitions` but that is going to change
soon and we'll be able to remove it.
<!-- Link to issues: Resolves #111, Resolves #222 -->

Closes #issue_number

Author: JereSalo

cmd/ef_tests/state/parser.rs

@@ -32,6 +32,11 @@ const IGNORED_TESTS: [&str; 11] = [
     "InitCollision.json",              // Skip because it fails on REVM
 ];
 
+// One .json can have multiple tests, sometimes we want to skip one of those.
+pub const SPECIFIC_IGNORED_TESTS: [&str; 1] = [
+    "test_set_code_to_non_empty_storage[fork_Prague-state_test-zero_nonce]", // Skip because EIP-7702 has changed. See https://github.com/ethereum/EIPs/pull/9710
+];
+
 pub fn parse_ef_tests(opts: &EFTestRunnerOptions) -> Result<Vec<EFTest>, EFTestParseError> {
     let parsing_time = std::time::Instant::now();
     let cargo_manifest_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));

cmd/ef_tests/state/runner/mod.rs

@@ -1,6 +1,7 @@
 use std::collections::HashMap;
 
 use crate::{
+    parser::SPECIFIC_IGNORED_TESTS,
     report::{self, format_duration_as_mm_ss, EFTestReport, TestReRunReport},
     types::EFTest,
 };
@@ -61,8 +62,8 @@ pub struct EFTestRunnerOptions {
     #[arg(short, long, value_name = "TESTS", use_value_delimiter = true)]
     pub tests: Vec<String>,
     /// For running tests with a specific name
-    #[arg(value_name = "SPECIFIC_TESTS", use_value_delimiter = true)]
-    pub specific_tests: Option<Vec<String>>,
+    #[arg(long, value_name = "SPECIFIC_TESTS", use_value_delimiter = true)]
+    pub specific_tests: Vec<String>,
     /// For running tests only with LEVM without the REVM re-run.
     #[arg(short, long, value_name = "SUMMARY", default_value = "false")]
     pub summary: bool,
@@ -106,9 +107,17 @@ async fn run_with_levm(
     println!("{}", report::progress(reports, levm_run_time.elapsed()));
 
     for test in ef_tests.iter() {
-        if opts.specific_tests.is_some()
-            && !opts.specific_tests.clone().unwrap().contains(&test.name)
-        {
+        let is_not_specific = !opts.specific_tests.is_empty()
+            && !opts
+                .specific_tests
+                .iter()
+                .any(|name| test.name.contains(name));
+        let is_ignored = SPECIFIC_IGNORED_TESTS
+            .iter()
+            .any(|skip| test.name.contains(skip));
+
+        // Skip tests that are not specific (if specific tests were indicated) and ignored ones.
+        if is_not_specific || is_ignored {
             continue;
         }
         if opts.verbose {

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

@@ -42,14 +42,6 @@ impl GeneralizedDatabase {
         }
     }
 
-    /// Gets account without pushing it to the cache
-    pub fn get_account_no_push_cache(&self, address: Address) -> Result<Account, DatabaseError> {
-        match cache::get_account(&self.cache, &address) {
-            Some(acc) => Ok(acc.clone()),
-            None => self.store.get_account(address),
-        }
-    }
-
     /// **Accesses to an account's information.**
     ///
     /// Accessed accounts are stored in the `touched_accounts` set.

crates/vm/levm/src/gas_cost.rs

@@ -832,7 +832,6 @@ pub fn call(
     current_memory_size: usize,
     address_was_cold: bool,
     address_is_empty: bool,
-    address_exists: bool,
     value_to_transfer: U256,
     gas_from_stack: U256,
     gas_left: u64,
@@ -858,10 +857,8 @@ pub fn call(
     } else {
         0
     };
-    // https://eips.ethereum.org/EIPS/eip-161 change the definition of empty address
-    let value_to_empty_account = if (!address_exists && fork < Fork::SpuriousDragon)
-        || address_is_empty && !value_to_transfer.is_zero() && fork >= Fork::SpuriousDragon
-    {
+
+    let value_to_empty_account = if address_is_empty && !value_to_transfer.is_zero() {
         CALL_TO_EMPTY_ACCOUNT
     } else {
         0

crates/vm/levm/src/opcode_handlers/system.rs

@@ -71,7 +71,7 @@ impl<'a> VM<'a> {
             calculate_memory_size(return_data_start_offset, return_data_size)?;
         let new_memory_size = new_memory_size_for_args.max(new_memory_size_for_return_data);
 
-        let (account_info, address_was_cold) =
+        let (account, address_was_cold) =
             self.db.access_account(&mut self.accrued_substate, callee)?;
 
         let (is_delegation, eip7702_gas_consumed, code_address, bytecode) =
@@ -89,8 +89,7 @@ impl<'a> VM<'a> {
             new_memory_size,
             current_memory_size,
             address_was_cold,
-            account_info.is_empty(),
-            account_exists(self.db, callee),
+            account.is_empty(),
             value_to_transfer,
             gas,
             gas_left,
@@ -589,15 +588,10 @@ impl<'a> VM<'a> {
             self.db.access_account(&mut self.accrued_substate, to)?;
         let balance_to_transfer = current_account.info.balance;
 
-        let account_is_empty = if self.env.config.fork >= Fork::SpuriousDragon {
-            target_account.is_empty()
-        } else {
-            !account_exists(self.db, target_address)
-        };
         self.current_call_frame_mut()?
             .increase_consumed_gas(gas_cost::selfdestruct(
                 target_account_is_cold,
-                account_is_empty,
+                target_account.is_empty(),
                 balance_to_transfer,
                 fork,
             )?)?;
@@ -632,11 +626,6 @@ impl<'a> VM<'a> {
 
             self.accrued_substate.selfdestruct_set.insert(to);
         }
-        if account_exists(self.db, target_address) && target_account.is_empty() {
-            self.accrued_substate
-                .touched_accounts
-                .insert(target_address);
-        }
 
         Ok(OpcodeResult::Halt)
     }

crates/vm/levm/src/utils.rs

@@ -1,9 +1,6 @@
 use crate::{
     constants::*,
-    db::{
-        cache::{self},
-        gen_db::GeneralizedDatabase,
-    },
+    db::gen_db::GeneralizedDatabase,
     errors::{InternalError, OutOfGasError, TxValidationError, VMError},
     gas_cost::{
         self, fake_exponential, ACCESS_LIST_ADDRESS_COST, ACCESS_LIST_STORAGE_KEY_COST,
@@ -331,12 +328,12 @@ pub fn eip7702_recover_address(
 /// The idea of this function comes from ethereum/execution-specs:
 /// https://github.com/ethereum/execution-specs/blob/951fc43a709b493f27418a8e57d2d6f3608cef84/src/ethereum/prague/vm/eoa_delegation.py#L115
 pub fn eip7702_get_code(
-    db: &GeneralizedDatabase,
+    db: &mut GeneralizedDatabase,
     accrued_substate: &mut Substate,
     address: Address,
 ) -> Result<(bool, u64, Address, Bytes), VMError> {
     // Address is the delgated address
-    let account = db.get_account_no_push_cache(address)?;
+    let account = db.get_account(address)?;
     let bytecode = account.code.clone();
 
     // If the Address doesn't have a delegation code
@@ -358,19 +355,11 @@ pub fn eip7702_get_code(
         COLD_ADDRESS_ACCESS_COST
     };
 
-    let authorized_bytecode = db.get_account_no_push_cache(auth_address)?.code;
+    let authorized_bytecode = db.get_account(auth_address)?.code;
 
     Ok((true, access_cost, auth_address, authorized_bytecode))
 }
 
-/// Checks if a given account exists in the database or cache
-pub fn account_exists(db: &mut GeneralizedDatabase, address: Address) -> bool {
-    match cache::get_account(&db.cache, &address) {
-        Some(_) => true,
-        None => db.store.account_exists(address),
-    }
-}
-
 impl<'a> VM<'a> {
     /// Sets the account code as the EIP7702 determines.
     pub fn eip7702_set_access_code(&mut self) -> Result<(), VMError> {
@@ -400,10 +389,9 @@ impl<'a> VM<'a> {
             };
 
             // 4. Add authority to accessed_addresses (as defined in EIP-2929).
-            self.accrued_substate
-                .touched_accounts
-                .insert(authority_address);
-            let authority_account = self.db.get_account_no_push_cache(authority_address)?;
+            let (authority_account, _address_was_cold) = self
+                .db
+                .access_account(&mut self.accrued_substate, authority_address)?;
 
             // 5. Verify the code of authority is either empty or already delegated.
             let empty_or_delegated =
@@ -420,9 +408,7 @@ impl<'a> VM<'a> {
             }
 
             // 7. Add PER_EMPTY_ACCOUNT_COST - PER_AUTH_BASE_COST gas to the global refund counter if authority exists in the trie.
-            if cache::is_account_cached(&self.db.cache, &authority_address)
-                || account_exists(self.db, authority_address)
-            {
+            if !authority_account.is_empty() {
                 let refunded_gas_if_exists = PER_EMPTY_ACCOUNT_COST - PER_AUTH_BASE_COST;
                 refunded_gas = refunded_gas
                     .checked_add(refunded_gas_if_exists)