test(l1): fix and document tests

crates/common/trie/trie.rs

@@ -261,6 +261,11 @@ impl Trie {
         all_nodes: &BTreeMap<H256, Vec<u8>>,
         root_hash: H256,
     ) -> Result<NodeRef, TrieError> {
+        // If the root hash is of the empty trie then we can get away by setting the NodeRef to default
+        if root_hash == *EMPTY_TRIE_HASH {
+            return Ok(NodeRef::default());
+        }
+
         let root_rlp = all_nodes
             .get(&root_hash)
             .ok_or(TrieError::InconsistentTree)?;

tooling/ef_tests/blockchain/tests/all.rs

@@ -1,63 +1,66 @@
 use ef_tests_blockchain::test_runner::parse_and_execute;
+use ethrex_prover_lib::backend::Backend;
 use std::path::Path;
 
+// Enable only one of `sp1` or `stateless` at a time.
+#[cfg(all(feature = "sp1", feature = "stateless"))]
+compile_error!("Only one of `sp1` and `stateless` can be enabled at a time.");
+
 const TEST_FOLDER: &str = "vectors/";
 
-#[cfg(not(any(feature = "sp1", feature = "stateless")))]
-const SKIPPED_TESTS: &[&str] = &[
-    "system_contract_deployment",
-    "HighGasPriceParis", // Skipped because it sets a gas price higher than u64::MAX, which most clients don't implement and is a virtually impossible scenario
-    "dynamicAccountOverwriteEmpty_Paris", // Skipped because the scenario described is virtually impossible
-    "create2collisionStorageParis", // Skipped because it's not worth implementing since the scenario of the test is virtually impossible. See https://github.com/lambdaclass/ethrex/issues/1555
-    "RevertInCreateInInitCreate2Paris", // Skipped because it's not worth implementing since the scenario of the test is virtually impossible. See https://github.com/lambdaclass/ethrex/issues/1555
-    "test_tx_gas_larger_than_block_gas_limit",
-    "createBlobhashTx",
-    "RevertInCreateInInit_Paris",
+// Base skips shared by all runs.
+const SKIPPED_BASE: &[&str] = &[
+    // These tests contain accounts without nonce or code but have storage, which is a virtually impossible scenario. That's why we fail, but that's okay.
+    // When creating an account we don't check the storage root but just if it has nonce or code.
+    // Fix is on its way on https://github.com/lambdaclass/ethrex/pull/4813
     "InitCollisionParis",
-    "ValueOverflowParis",
-];
-// We are skipping test_tx_gas_larger_than_block_gas_limit[fork_Osaka-blockchain_test-exceed_block_gas_limit_True] because of an
-// inconsistency on the expected exception. Exception returned is InvalidBlock(GasUsedMismatch(0x06000000,0x05000000)) while
-// exception expected GAS_ALLOWANCE_EXCEEDED. The thing is gas allowance exception is supposed to be thrown on any transaction
-// execution in case the transaction's gas limit is larger than the block's, which is not the case of this test.
-// This test has a block with "gasLimit": "0x055d4a80", "gasUsed": "0x05000000" and six transactions with "gasLimit": "0x01000000",
-// Apparently each transaction consumes up to its gas limit, which together is larger than the block's. Then when executing validate_gas_used
-// after the block's execution, it throws InvalidBlock(GasUsedMismatch(0x06000000,0x05000000)) on comparing the receipt's cumulative gas used agains the block's gas limit.
-#[cfg(any(feature = "sp1", feature = "stateless"))]
-const SKIPPED_TESTS: &[&str] = &[
-    // We skip most of these for the same reason we skip them in normal runs; since we need to do a normal run before running with the stateless backend
-    "system_contract_deployment",
-    "test_tx_gas_larger_than_block_gas_limit",
-    "HighGasPriceParis",
-    "dynamicAccountOverwriteEmpty_Paris",
-    "create2collisionStorageParis",
     "RevertInCreateInInitCreate2Paris",
-    "createBlobhashTx",
+    "create2collisionStorageParis",
+    "dynamicAccountOverwriteEmpty_Paris",
     "RevertInCreateInInit_Paris",
-    "InitCollisionParis",
+    // Skip because they take too long to run, but they pass
+    "static_Call50000_sha256",
+    "CALLBlake2f_MaxRounds",
+    "loopMul",
+    // It checks that withdrawal requests and consolidation requests accounts have code, which is always the case, except for this test.
+    // The fix is just an if, it's on its way on https://github.com/lambdaclass/ethrex/pull/4731
+    "system_contract_deployment",
+    // Gas price higher than u64::MAX; impractical scenario. Fix is on its way on https://github.com/lambdaclass/ethrex/pull/4823
+    "HighGasPriceParis",
+    // Skip because it tries to deserialize number > U256::MAX
     "ValueOverflowParis",
-    // We skip these two tests because they fail with stateless backend specifically. See https://github.com/lambdaclass/ethrex/issues/4502
-    "test_large_amount",
-    "test_multiple_withdrawals_same_address",
+    // Skip because it's a "Create" Blob Transaction, which doesn't actually exist. It never reaches the EVM because we can't even parse it as an actual Transaction.
+    "createBlobhashTx",
 ];
 
+// Extra skips added only for prover backends.
+#[cfg(feature = "sp1")]
+const EXTRA_SKIPS: &[&str] = &[
+    // I believe these tests fail because of how much stress they put into the zkVM, they probably cause an OOM though this should be checked
+    "static_Call50000",
+    "Return50000",
+    "static_Call1MB1024Calldepth",
+];
+#[cfg(not(feature = "sp1"))]
+const EXTRA_SKIPS: &[&str] = &[];
+
+// Select backend
+#[cfg(feature = "stateless")]
+const BACKEND: Option<Backend> = Some(Backend::Exec);
+#[cfg(feature = "sp1")]
+const BACKEND: Option<Backend> = Some(Backend::SP1);
 #[cfg(not(any(feature = "sp1", feature = "stateless")))]
-fn blockchain_runner(path: &Path) -> datatest_stable::Result<()> {
-    parse_and_execute(path, Some(SKIPPED_TESTS), None)
-}
+const BACKEND: Option<Backend> = None;
 
-// If `sp1` or `stateless` is enabled
-#[cfg(any(feature = "sp1", feature = "stateless"))]
 fn blockchain_runner(path: &Path) -> datatest_stable::Result<()> {
-    #[cfg(feature = "stateless")]
-    let backend = Some(ethrex_prover_lib::backend::Backend::Exec);
-    #[cfg(feature = "sp1")]
-    let backend = Some(ethrex_prover_lib::backend::Backend::SP1);
+    // Compose the final skip list
+    let skips: Vec<&'static str> = SKIPPED_BASE
+        .iter()
+        .copied()
+        .chain(EXTRA_SKIPS.iter().copied())
+        .collect();
 
-    parse_and_execute(path, Some(SKIPPED_TESTS), backend)
+    parse_and_execute(path, Some(&skips), BACKEND)
 }
 
 datatest_stable::harness!(blockchain_runner, TEST_FOLDER, r".*");
-
-#[cfg(any(all(feature = "sp1", feature = "stateless"),))]
-compile_error!("Only one of `sp1` and `stateless` can be enabled at a time.");

tooling/ef_tests/state/parser.rs

@@ -20,7 +20,7 @@ pub enum EFTestParseError {
     FailedToParseTestFile(String),
 }
 
-const IGNORED_TESTS: [&str; 11] = [
+const IGNORED_TESTS: &[&str] = &[
     "static_Call50000_sha256.json", // Skip because it takes longer to run than some tests, but not a huge deal.
     "CALLBlake2f_MaxRounds.json",   // Skip because it takes extremely long to run, but passes.
     "ValueOverflow.json",           // Skip because it tries to deserialize number > U256::MAX

tooling/ef_tests/state_v2/src/modules/parser.rs

@@ -23,21 +23,27 @@ pub struct RunnerOptions {
 }
 
 //TODO: Use this constant, improve it.
-const IGNORED_TESTS: [&str; 14] = [
-    "static_Call50000_sha256.json", // Skip because it takes longer to run than some tests, but not a huge deal.
-    "CALLBlake2f_MaxRounds.json",   // Skip because it takes extremely long to run, but passes.
-    "ValueOverflow.json",           // Skip because it tries to deserialize number > U256::MAX
-    "ValueOverflowParis.json",      // Skip because it tries to deserialize number > U256::MAX
-    "loopMul.json",                 // Skip because it takes too long to run
+const IGNORED_TESTS: &[&str] = &[
+    // These tests contain accounts without nonce or code but have storage, which is a virtually impossible scenario. That's why we fail, but that's okay.
+    // When creating an account we don't check the storage root but just if it has nonce or code, and that's the right check for real case scenarios.
     "dynamicAccountOverwriteEmpty_Paris.json",
     "RevertInCreateInInitCreate2Paris.json",
     "RevertInCreateInInit_Paris.json",
     "create2collisionStorageParis.json",
     "InitCollisionParis.json",
     "InitCollision.json",
-    "contract_create.json", // Skip for now as it requires special transaction type handling
-    "HighGasPrice.json", // Skip until we fix gas price unit (u64 -> U256) https://github.com/lambdaclass/ethrex/issues/3629
-    "HighGasPriceParis.json", // Skip until we fix gas price unit (u64 -> U256) https://github.com/lambdaclass/ethrex/issues/3629
+    // Gas price higher than u64::MAX; impractical scenario. We don't use 256 bits for gas price for performance reasons, however, it's debatable. See https://github.com/lambdaclass/ethrex/issues/3629
+    "HighGasPrice.json",
+    "HighGasPriceParis.json",
+    // Skip because they take too long to run, but they pass
+    "static_Call50000_sha256.json",
+    "CALLBlake2f_MaxRounds.json",
+    "loopMul.json",
+    // Skip because it tries to deserialize number > U256::MAX
+    "ValueOverflow.json",
+    "ValueOverflowParis.json",
+    // Skip for now as it requires special transaction type handling in test runner, we should improve that.
+    "contract_create.json",
 ];
 
 /// Parse a `.json` file of tests into a Vec<Test>.