feat(levm): running all ef tests with revm and fixing problems (#1275)

**Motivation**

<!-- Why does this pull request exist? What are its goals? -->
- We want to run all eftests with revm and make them pass so it can be a
reliable source for comparing execution with levm.

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
- Runs all tests with revm -> Done
- Fix execution of revm -> Most of Cancun tests have been fixed, 8 of
them don't pass but it is good enough

- Access lists are now deserialized, in .json files there are 1 access
list per `data` element. So we use `data` index for accessing them.

- I didn't make serious error handling for running with revm because it
is not a core functionality, doing that will take a little bit more work
but maybe it is not necessary.
<!-- Link to issues: Resolves #111, Resolves #222 -->

Closes #issue_number

---------

Co-authored-by: ilitteri <[email protected]>
Co-authored-by: Ivan Litteri <[email protected]>

cmd/ef_tests/levm/deserialize.rs

@@ -1,6 +1,6 @@
-use crate::types::{EFTest, EFTests};
+use crate::types::{EFTest, EFTestAccessListItem, EFTests};
 use bytes::Bytes;
-use ethrex_core::U256;
+use ethrex_core::{H256, U256};
 use serde::Deserialize;
 use std::{collections::HashMap, str::FromStr};
 
@@ -65,6 +65,50 @@ where
     )
 }
 
+pub fn deserialize_h256_vec_optional_safe<'de, D>(
+    deserializer: D,
+) -> Result<Option<Vec<H256>>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    let s = Option::<Vec<String>>::deserialize(deserializer)?;
+    match s {
+        Some(s) => {
+            let mut ret = Vec::new();
+            for s in s {
+                ret.push(H256::from_str(s.trim_start_matches("0x")).map_err(|err| {
+                    serde::de::Error::custom(format!(
+                        "error parsing H256 when deserializing H256 vec optional: {err}"
+                    ))
+                })?);
+            }
+            Ok(Some(ret))
+        }
+        None => Ok(None),
+    }
+}
+
+pub fn deserialize_access_lists<'de, D>(
+    deserializer: D,
+) -> Result<Option<Vec<Vec<EFTestAccessListItem>>>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    let access_lists: Option<Vec<Option<Vec<EFTestAccessListItem>>>> =
+        Option::<Vec<Option<Vec<EFTestAccessListItem>>>>::deserialize(deserializer)?;
+
+    let mut final_access_lists: Vec<Vec<EFTestAccessListItem>> = Vec::new();
+
+    if let Some(access_lists) = access_lists {
+        for access_list in access_lists {
+            // Treat `null` as an empty vector
+            final_access_lists.push(access_list.unwrap_or_default());
+        }
+    }
+
+    Ok(Some(final_access_lists))
+}
+
 pub fn deserialize_u256_optional_safe<'de, D>(deserializer: D) -> Result<Option<U256>, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -164,6 +208,20 @@ impl<'de> Deserialize<'de> for EFTests {
                             sender: raw_tx.sender,
                             to: raw_tx.to.clone(),
                             value: *value,
+                            blob_versioned_hashes: raw_tx
+                                .blob_versioned_hashes
+                                .clone()
+                                .unwrap_or_default(),
+                            max_fee_per_blob_gas: raw_tx.max_fee_per_blob_gas,
+                            max_priority_fee_per_gas: raw_tx.max_priority_fee_per_gas,
+                            max_fee_per_gas: raw_tx.max_fee_per_gas,
+                            access_list: raw_tx
+                                .access_lists
+                                .clone()
+                                .unwrap_or_default()
+                                .get(data_id)
+                                .cloned()
+                                .unwrap_or_default(),
                         };
                         transactions.insert((data_id, gas_limit_id, value_id), tx);
                     }

cmd/ef_tests/levm/runner/levm_runner.rs

@@ -78,7 +78,7 @@ pub fn prepare_vm_for_tx(vector: &TestVector, test: &EFTest) -> Result<VM, EFTes
             origin: test.transactions.get(vector).unwrap().sender,
             consumed_gas: U256::default(),
             refunded_gas: U256::default(),
-            gas_limit: test.env.current_gas_limit,
+            gas_limit: test.env.current_gas_limit, //this should be tx gas limit
             block_number: test.env.current_number,
             coinbase: test.env.current_coinbase,
             timestamp: test.env.current_timestamp,

cmd/ef_tests/levm/runner/mod.rs

@@ -88,6 +88,45 @@ fn run_with_levm(
     Ok(())
 }
 
+/// ### Runs all tests with REVM
+/// **Note:** This is not used in the current implementation because we only run with REVM the tests that failed with LEVM so that execution time is minimized.
+fn _run_with_revm(
+    reports: &mut Vec<EFTestReport>,
+    ef_tests: &[EFTest],
+) -> Result<(), EFTestRunnerError> {
+    let revm_run_time = std::time::Instant::now();
+    let mut revm_run_spinner = Spinner::new(
+        Dots,
+        "Running all tests with REVM...".to_owned(),
+        Color::Cyan,
+    );
+    for (idx, test) in ef_tests.iter().enumerate() {
+        let total_tests = ef_tests.len();
+        revm_run_spinner.update_text(format!(
+            "{} {}/{total_tests} - {}",
+            "Running all tests with REVM".bold(),
+            idx + 1,
+            format_duration_as_mm_ss(revm_run_time.elapsed())
+        ));
+        let ef_test_report = match revm_runner::_run_ef_test_revm(test) {
+            Ok(ef_test_report) => ef_test_report,
+            Err(EFTestRunnerError::Internal(err)) => return Err(EFTestRunnerError::Internal(err)),
+            non_internal_errors => {
+                return Err(EFTestRunnerError::Internal(InternalError::FirstRunInternal(format!(
+                    "Non-internal error raised when executing revm. This should not happen: {non_internal_errors:?}",
+                ))))
+            }
+        };
+        reports.push(ef_test_report);
+        revm_run_spinner.update_text(report::progress(reports, revm_run_time.elapsed()));
+    }
+    revm_run_spinner.success(&format!(
+        "Ran all tests with REVM in {}",
+        format_duration_as_mm_ss(revm_run_time.elapsed())
+    ));
+    Ok(())
+}
+
 fn re_run_with_revm(
     reports: &mut [EFTestReport],
     ef_tests: &[EFTest],

cmd/ef_tests/levm/runner/revm_runner.rs

@@ -1,10 +1,14 @@
 use crate::{
     report::{AccountUpdatesReport, EFTestReport, TestReRunReport, TestVector},
-    runner::{levm_runner, EFTestRunnerError, InternalError},
-    types::EFTest,
+    runner::{
+        levm_runner::{self, post_state_root},
+        EFTestRunnerError, InternalError,
+    },
+    types::{EFTest, EFTestTransaction},
     utils::load_initial_state,
 };
-use ethrex_core::{types::TxKind, Address, H256};
+use bytes::Bytes;
+use ethrex_core::{types::TxKind, Address, H256, U256};
 use ethrex_levm::{
     errors::{TransactionReport, TxResult},
     Account, StorageSlot,
@@ -15,8 +19,8 @@ use revm::{
     db::State,
     inspectors::TracerEip3155 as RevmTracerEip3155,
     primitives::{
-        BlobExcessGasAndPrice, BlockEnv as RevmBlockEnv, EVMError as REVMError,
-        ExecutionResult as RevmExecutionResult, TxEnv as RevmTxEnv, TxKind as RevmTxKind,
+        AccessListItem, BlobExcessGasAndPrice, BlockEnv as RevmBlockEnv, EVMError as REVMError,
+        ExecutionResult as RevmExecutionResult, TxEnv as RevmTxEnv, TxKind as RevmTxKind, B256,
     },
     Evm as Revm,
 };
@@ -86,6 +90,19 @@ pub fn re_run_failed_ef_test_tx(
     Ok(())
 }
 
+// If gas price is not provided, calculate it with current base fee and priority fee
+pub fn effective_gas_price(test: &EFTest, tx: &&EFTestTransaction) -> U256 {
+    match tx.gas_price {
+        None => {
+            let current_base_fee = test.env.current_base_fee.unwrap();
+            let priority_fee = tx.max_priority_fee_per_gas.unwrap();
+            let max_fee_per_gas = tx.max_fee_per_gas.unwrap();
+            std::cmp::min(max_fee_per_gas, current_base_fee + priority_fee)
+        }
+        Some(price) => price,
+    }
+}
+
 pub fn prepare_revm_for_tx<'state>(
     initial_state: &'state mut EvmState,
     vector: &TestVector,
@@ -102,12 +119,10 @@ pub fn prepare_revm_for_tx<'state>(
         basefee: RevmU256::from_limbs(test.env.current_base_fee.unwrap_or_default().0),
         difficulty: RevmU256::from_limbs(test.env.current_difficulty.0),
         prevrandao: test.env.current_random.map(|v| v.0.into()),
-        blob_excess_gas_and_price: Some(BlobExcessGasAndPrice::new(
-            test.env
-                .current_excess_blob_gas
-                .unwrap_or_default()
-                .as_u64(),
-        )),
+        blob_excess_gas_and_price: test
+            .env
+            .current_excess_blob_gas
+            .map(|gas| BlobExcessGasAndPrice::new(gas.as_u64())),
     };
     let tx = &test
         .transactions
@@ -116,29 +131,52 @@ pub fn prepare_revm_for_tx<'state>(
             "Vector {vector:?} not found in test {}",
             test.name
         )))?;
+
+    let revm_access_list: Vec<AccessListItem> = tx
+        .access_list
+        .iter()
+        .map(|eftest_access_list_item| AccessListItem {
+            address: RevmAddress(eftest_access_list_item.address.0.into()),
+            storage_keys: eftest_access_list_item
+                .storage_keys
+                .iter()
+                .map(|key| B256::from(key.0))
+                .collect(),
+        })
+        .collect();
+
     let tx_env = RevmTxEnv {
         caller: tx.sender.0.into(),
         gas_limit: tx.gas_limit.as_u64(),
-        gas_price: RevmU256::from_limbs(tx.gas_price.unwrap_or_default().0),
+        gas_price: RevmU256::from_limbs(effective_gas_price(test, tx).0),
         transact_to: match tx.to {
             TxKind::Call(to) => RevmTxKind::Call(to.0.into()),
             TxKind::Create => RevmTxKind::Create,
         },
         value: RevmU256::from_limbs(tx.value.0),
         data: tx.data.to_vec().into(),
         nonce: Some(tx.nonce.as_u64()),
-        chain_id: None,
-        access_list: Vec::default(),
-        gas_priority_fee: None,
-        blob_hashes: Vec::default(),
-        max_fee_per_blob_gas: None,
+        chain_id: Some(chain_spec.chain_id), //TODO: See what to do with this... ChainId test fails IDK why.
+        access_list: revm_access_list,
+        gas_priority_fee: tx
+            .max_priority_fee_per_gas
+            .map(|fee| RevmU256::from_limbs(fee.0)),
+        blob_hashes: tx
+            .blob_versioned_hashes
+            .iter()
+            .map(|h256| B256::from(h256.0))
+            .collect::<Vec<B256>>(),
+        max_fee_per_blob_gas: tx
+            .max_fee_per_blob_gas
+            .map(|fee| RevmU256::from_limbs(fee.0)),
         authorization_list: None,
     };
+
     let evm_builder = Revm::builder()
         .with_block_env(block_env)
         .with_tx_env(tx_env)
         .modify_cfg_env(|cfg| cfg.chain_id = chain_spec.chain_id)
-        .with_spec_id(SpecId::CANCUN)
+        .with_spec_id(SpecId::CANCUN) //TODO: In the future replace cancun for the actual spec id
         .with_external_context(
             RevmTracerEip3155::new(Box::new(std::io::stderr())).without_summary(),
         );
@@ -322,3 +360,133 @@ pub fn compare_levm_revm_account_updates(
             .collect::<HashSet<Address>>(),
     }
 }
+
+pub fn _run_ef_test_revm(test: &EFTest) -> Result<EFTestReport, EFTestRunnerError> {
+    dbg!(&test.name);
+    let mut ef_test_report = EFTestReport::new(
+        test.name.clone(),
+        test._info.generated_test_hash,
+        test.fork(),
+    );
+    for (vector, _tx) in test.transactions.iter() {
+        match _run_ef_test_tx_revm(vector, test) {
+            Ok(_) => continue,
+            Err(EFTestRunnerError::VMInitializationFailed(reason)) => {
+                ef_test_report.register_vm_initialization_failure(reason, *vector);
+            }
+            Err(EFTestRunnerError::FailedToEnsurePreState(reason)) => {
+                ef_test_report.register_pre_state_validation_failure(reason, *vector);
+            }
+            Err(EFTestRunnerError::ExecutionFailedUnexpectedly(error)) => {
+                ef_test_report.register_unexpected_execution_failure(error, *vector);
+            }
+            Err(EFTestRunnerError::FailedToEnsurePostState(transaction_report, reason)) => {
+                ef_test_report.register_post_state_validation_failure(
+                    transaction_report,
+                    reason,
+                    *vector,
+                );
+            }
+            Err(EFTestRunnerError::VMExecutionMismatch(_)) => {
+                return Err(EFTestRunnerError::Internal(InternalError::FirstRunInternal(
+                    "VM execution mismatch errors should only happen when COMPARING LEVM AND REVM. This failed during revm's execution."
+                        .to_owned(),
+                )));
+            }
+            Err(EFTestRunnerError::Internal(reason)) => {
+                return Err(EFTestRunnerError::Internal(reason));
+            }
+        }
+    }
+    Ok(ef_test_report)
+}
+
+pub fn _run_ef_test_tx_revm(vector: &TestVector, test: &EFTest) -> Result<(), EFTestRunnerError> {
+    // dbg!(vector);
+    let (mut state, _block_hash) = load_initial_state(test);
+    let mut revm = prepare_revm_for_tx(&mut state, vector, test)?;
+    let revm_execution_result = revm.transact_commit();
+    drop(revm); // Need to drop the state mutable reference.
+
+    _ensure_post_state_revm(revm_execution_result, vector, test, &mut state)?;
+
+    Ok(())
+}
+
+pub fn _ensure_post_state_revm(
+    revm_execution_result: Result<RevmExecutionResult, REVMError<StoreError>>,
+    vector: &TestVector,
+    test: &EFTest,
+    revm_state: &mut EvmState,
+) -> Result<(), EFTestRunnerError> {
+    match revm_execution_result {
+        Ok(_execution_result) => {
+            match test.post.vector_post_value(vector).expect_exception {
+                // Execution result was successful but an exception was expected.
+                Some(expected_exception) => {
+                    let error_reason = format!("Expected exception: {expected_exception}");
+                    println!("Expected exception: {expected_exception}");
+                    return Err(EFTestRunnerError::FailedToEnsurePostState(
+                        TransactionReport {
+                            result: TxResult::Success,
+                            gas_used: 42,
+                            gas_refunded: 42,
+                            logs: vec![],
+                            output: Bytes::new(),
+                            new_state: HashMap::new(),
+                            created_address: None,
+                        },
+                        //TODO: This is not a TransactionReport because it is REVM
+                        error_reason,
+                    ));
+                }
+                // Execution result was successful and no exception was expected.
+                None => {
+                    let revm_account_updates = ethrex_vm::get_state_transitions(revm_state);
+                    let pos_state_root = post_state_root(&revm_account_updates, test);
+                    let expected_post_state_root_hash = test.post.vector_post_value(vector).hash;
+                    if expected_post_state_root_hash != pos_state_root {
+                        println!(
+                            "Post-state root mismatch: expected {expected_post_state_root_hash:#x}, got {pos_state_root:#x}",
+                        );
+                        let error_reason = format!(
+                            "Post-state root mismatch: expected {expected_post_state_root_hash:#x}, got {pos_state_root:#x}",
+                        );
+                        return Err(EFTestRunnerError::FailedToEnsurePostState(
+                            TransactionReport {
+                                result: TxResult::Success,
+                                gas_used: 42,
+                                gas_refunded: 42,
+                                logs: vec![],
+                                output: Bytes::new(),
+                                new_state: HashMap::new(),
+                                created_address: None,
+                            },
+                            //TODO: This is not a TransactionReport because it is REVM
+                            error_reason,
+                        ));
+                    }
+                }
+            }
+        }
+        Err(err) => {
+            match test.post.vector_post_value(vector).expect_exception {
+                // Execution result was unsuccessful and an exception was expected.
+                // TODO: See if we want to map revm exceptions to expected exceptions, probably not.
+                Some(_expected_exception) => {}
+                // Execution result was unsuccessful but no exception was expected.
+                None => {
+                    println!(
+                        "Unexpected exception. Name: {}, vector: {:?}, error: {:?}",
+                        &test.name, vector, err
+                    );
+                    return Err(EFTestRunnerError::ExecutionFailedUnexpectedly(
+                        ethrex_levm::errors::VMError::AddressAlreadyOccupied,
+                        //TODO: Use another kind of error for this.
+                    ));
+                }
+            }
+        }
+    };
+    Ok(())
+}