fix(l1): close EIP-8025 stateless witness validation gaps (re-enable 9 zkevm tests)

crates/blockchain/blockchain.rs

@@ -1407,41 +1407,17 @@ impl Blockchain {
         }
 
         // - We now need necessary block headers, these go from the first block referenced (via BLOCKHASH or just the first block to execute) up to the parent of the last block to execute.
-        let mut block_headers_bytes = Vec::new();
-
-        let first_blockhash_opcode_number = blockhash_opcode_references.keys().min();
-        let first_needed_block_hash = first_blockhash_opcode_number
-            .and_then(|n| {
-                (*n < first_block_header.number.saturating_sub(1))
-                    .then(|| blockhash_opcode_references.get(n))?
-                    .copied()
-            })
-            .unwrap_or(first_block_header.parent_hash);
-
-        // At the beginning this is the header of the last block to execute.
-        let mut current_header = blocks
+        let walk_start_header = &blocks
             .last()
             .ok_or_else(|| ChainError::WitnessGeneration("Empty batch".to_string()))?
-            .header
-            .clone();
-
-        // Headers from latest - 1 until we reach first block header we need.
-        // We do it this way because we want to fetch headers by hash, not by number
-        while current_header.hash() != first_needed_block_hash {
-            let parent_hash = current_header.parent_hash;
-            let current_number = current_header.number - 1;
-
-            current_header = self
-                .storage
-                .get_block_header_by_hash(parent_hash)?
-                .ok_or_else(|| {
-                    ChainError::WitnessGeneration(format!(
-                        "Failed to get block {current_number} header"
-                    ))
-                })?;
-
-            block_headers_bytes.push(current_header.encode_to_vec());
-        }
+            .header;
+        let block_headers_bytes = build_ascending_ancestor_headers_bytes(
+            walk_start_header,
+            first_block_header.number,
+            first_block_header.parent_hash,
+            &blockhash_opcode_references,
+            &self.storage,
+        )?;
 
         // Get initial state trie root and embed the rest of the trie into it
         let nodes: BTreeMap<H256, Node> = used_trie_nodes
@@ -1646,36 +1622,13 @@ impl Blockchain {
         }
 
         // - We now need necessary block headers, these go from the first block referenced (via BLOCKHASH or just the first block to execute) up to the parent of the last block to execute.
-        let mut block_headers_bytes = Vec::new();
-
-        let first_blockhash_opcode_number = blockhash_opcode_references.keys().min();
-        let first_needed_block_hash = first_blockhash_opcode_number
-            .and_then(|n| {
-                (*n < block.header.number.saturating_sub(1))
-                    .then(|| blockhash_opcode_references.get(n))?
-                    .copied()
-            })
-            .unwrap_or(block.header.parent_hash);
-
-        let mut current_header = block.header.clone();
-
-        // Headers from latest - 1 until we reach first block header we need.
-        // We do it this way because we want to fetch headers by hash, not by number
-        while current_header.hash() != first_needed_block_hash {
-            let parent_hash = current_header.parent_hash;
-            let current_number = current_header.number - 1;
-
-            current_header = self
-                .storage
-                .get_block_header_by_hash(parent_hash)?
-                .ok_or_else(|| {
-                    ChainError::WitnessGeneration(format!(
-                        "Failed to get block {current_number} header"
-                    ))
-                })?;
-
-            block_headers_bytes.push(current_header.encode_to_vec());
-        }
+        let block_headers_bytes = build_ascending_ancestor_headers_bytes(
+            &block.header,
+            block.header.number,
+            block.header.parent_hash,
+            &blockhash_opcode_references,
+            &self.storage,
+        )?;
 
         // Get initial state trie root and embed the rest of the trie into it
         let nodes: BTreeMap<H256, Node> = used_trie_nodes
@@ -3151,6 +3104,46 @@ fn branchify(node: Node) -> Box<BranchNode> {
     }
 }
 
+/// Walks the chain backward from `walk_start_header` via `storage`, stopping
+/// at the oldest BLOCKHASH-referenced ancestor (or the first executed block's
+/// parent), and returns the encoded ancestor headers in ascending block-number
+/// order.
+///
+/// Ref: <https://github.com/ethereum/execution-specs/blob/projects/zkevm/src/ethereum/forks/amsterdam/stateless.py#L171-L191>
+fn build_ascending_ancestor_headers_bytes(
+    walk_start_header: &BlockHeader,
+    first_block_number: u64,
+    first_block_parent_hash: H256,
+    blockhash_opcode_references: &HashMap<u64, H256>,
+    storage: &Store,
+) -> Result<Vec<Vec<u8>>, ChainError> {
+    let first_blockhash_opcode_number = blockhash_opcode_references.keys().min();
+    let first_needed_block_hash = first_blockhash_opcode_number
+        .and_then(|n| {
+            (*n < first_block_number.saturating_sub(1))
+                .then(|| blockhash_opcode_references.get(n))?
+                .copied()
+        })
+        .unwrap_or(first_block_parent_hash);
+
+    let mut block_headers_bytes = Vec::new();
+    let mut current_header = walk_start_header.clone();
+    while current_header.hash() != first_needed_block_hash {
+        let parent_hash = current_header.parent_hash;
+        let current_number = current_header.number - 1;
+        current_header = storage
+            .get_block_header_by_hash(parent_hash)?
+            .ok_or_else(|| {
+                ChainError::WitnessGeneration(format!(
+                    "Failed to get block {current_number} header"
+                ))
+            })?;
+        block_headers_bytes.push(current_header.encode_to_vec());
+    }
+    block_headers_bytes.reverse();
+    Ok(block_headers_bytes)
+}
+
 fn collect_trie(index: u8, mut trie: Trie) -> Result<(Box<BranchNode>, Vec<TrieNode>), TrieError> {
     let root = branchify(
         trie.root_node()?

crates/common/types/block_execution_witness.rs

@@ -151,36 +151,32 @@ impl RpcExecutionWitness {
             ));
         }
 
-        let mut initial_state_root = None;
-
-        for h in &self.headers {
-            let header = BlockHeader::decode(h)?;
-            if header.number == first_block_number - 1 {
-                initial_state_root = Some(header.state_root);
-                break;
-            }
-        }
-
-        let initial_state_root = initial_state_root.ok_or_else(|| {
-            GuestProgramStateError::Custom(format!(
-                "header for block {} not found",
-                first_block_number - 1
-            ))
-        })?;
+        // Local lookup only — malformed entries are still carried into
+        // `block_headers_bytes` and will be rejected by `from_witness`.
+        let initial_state_root = self
+            .headers
+            .iter()
+            .filter_map(|h| BlockHeader::decode(h).ok())
+            .find(|header| header.number == first_block_number - 1)
+            .map(|header| header.state_root)
+            .ok_or_else(|| {
+                GuestProgramStateError::Custom(format!(
+                    "header for block {} not found",
+                    first_block_number - 1
+                ))
+            })?;
 
+        // EIP-8025: drop entries that don't decode. They can't be looked up by
+        // hash anyway; if execution needs them, the trie walk fails there.
+        // Ref: https://github.com/ethereum/execution-specs/blob/projects/zkevm/src/ethereum/forks/amsterdam/witness_state.py#L37-L42
         let nodes: BTreeMap<H256, Node> = self
             .state
             .into_iter()
             .filter_map(|b| {
-                if b == Bytes::from_static(&[0x80]) {
-                    // other implementations of debug_executionWitness allow for a `Null` node,
-                    // which would fail to decode in ours
-                    return None;
-                }
-                let hash = keccak(&b);
-                Some(Node::decode(&b).map(|node| (hash, node)))
+                let node = Node::decode(&b).ok()?;
+                Some((keccak(&b), node))
             })
-            .collect::<Result<_, RLPDecodeError>>()?;
+            .collect();
 
         // get state trie root and embed the rest of the trie into it
         let state_trie_root = if let NodeRef::Node(state_trie_root, _) =
@@ -325,17 +321,20 @@ impl GuestProgramState {
         value: ExecutionWitness,
         crypto: &dyn Crypto,
     ) -> Result<Self, GuestProgramStateError> {
-        let block_headers: BTreeMap<u64, BlockHeader> = value
-            .block_headers_bytes
-            .into_iter()
-            .map(|bytes| BlockHeader::decode(bytes.as_ref()))
-            .collect::<Result<Vec<_>, _>>()
-            .map_err(|e| {
-                GuestProgramStateError::Custom(format!("Failed to decode block headers: {}", e))
-            })?
-            .into_iter()
-            .map(|header| (header.number, header))
-            .collect();
+        // Headers must decode and form a contiguous chain in list order.
+        // Ref: https://github.com/ethereum/execution-specs/blob/projects/zkevm/src/ethereum/forks/amsterdam/stateless.py#L171-L191
+        let mut block_headers: BTreeMap<u64, BlockHeader> = BTreeMap::new();
+        let mut prev_hash: Option<H256> = None;
+        for bytes in &value.block_headers_bytes {
+            let header = BlockHeader::decode(bytes.as_ref())?;
+            if let Some(expected_parent) = prev_hash
+                && header.parent_hash != expected_parent
+            {
+                return Err(GuestProgramStateError::NoncontiguousBlockHeaders);
+            }
+            prev_hash = Some(H256(crypto.keccak256(bytes)));
+            block_headers.insert(header.number, header);
+        }
 
         let parent_number =
             value
@@ -588,28 +587,21 @@ impl GuestProgramState {
         Ok(self.chain_config)
     }
 
-    /// Retrieves the account code for a specific account.
-    /// Returns an Err if the code is not found.
+    /// Retrieves bytecode by code hash. Errors if missing — EIP-8025.
+    ///
+    /// Ref: https://github.com/ethereum/execution-specs/blob/projects/zkevm/src/ethereum/forks/amsterdam/witness_state.py#L204-L212
     pub fn get_account_code(&self, code_hash: H256) -> Result<Code, GuestProgramStateError> {
         if code_hash == *EMPTY_KECCACK_HASH {
             return Ok(Code::default());
         }
-        match self.codes_hashed.get(&code_hash) {
-            Some(code) => Ok(code.clone()),
-            None => {
-                // We do this because what usually happens is that the Witness doesn't have the code we asked for but it is because it isn't relevant for that particular case.
-                // In client implementations there are differences and it's natural for some clients to access more/less information in some edge cases.
-                // Sidenote: logger doesn't work inside SP1, that's why we use println!
-                println!(
-                    "Missing bytecode for hash {} in witness. Defaulting to empty code.", // If there's a state root mismatch and this prints we have to see if it's the cause or not.
-                    hex::encode(code_hash)
-                );
-                Ok(Code::default())
-            }
-        }
+        self.codes_hashed.get(&code_hash).cloned().ok_or_else(|| {
+            GuestProgramStateError::Database(format!(
+                "missing bytecode for hash {code_hash:x} in witness"
+            ))
+        })
     }
 
-    /// Retrieves code metadata (length) for a specific code hash.
+    /// Code length by hash. Errors on miss, like `get_account_code`.
     /// This is an optimized path for EXTCODESIZE opcode.
     pub fn get_code_metadata(
         &self,
@@ -620,19 +612,16 @@ impl GuestProgramState {
         if code_hash == *EMPTY_KECCACK_HASH {
             return Ok(CodeMetadata { length: 0 });
         }
-        match self.codes_hashed.get(&code_hash) {
-            Some(code) => Ok(CodeMetadata {
+        self.codes_hashed
+            .get(&code_hash)
+            .map(|code| CodeMetadata {
                 length: code.bytecode.len() as u64,
-            }),
-            None => {
-                // Same as get_account_code - default to empty for missing bytecode
-                println!(
-                    "Missing bytecode for hash {} in witness. Defaulting to empty code metadata.",
-                    hex::encode(code_hash)
-                );
-                Ok(CodeMetadata { length: 0 })
-            }
-        }
+            })
+            .ok_or_else(|| {
+                GuestProgramStateError::Database(format!(
+                    "missing bytecode for hash {code_hash:x} in witness"
+                ))
+            })
     }
 
     /// When executing multiple blocks in the L2 it happens that the headers in block_headers correspond to the same block headers that we have in the blocks array. The main goal is to hash these only once and set them in both places.

crates/guest-program/src/common/execution.rs

@@ -70,7 +70,10 @@ where
 
     // Validate execution witness' block hashes
     report_cycles("get_first_invalid_block_hash", || {
-        if let Ok(Some(invalid_block_header)) = wrapped_db.get_first_invalid_block_hash() {
+        if let Some(invalid_block_header) = wrapped_db
+            .get_first_invalid_block_hash()
+            .map_err(ExecutionError::GuestProgramState)?
+        {
             return Err(ExecutionError::InvalidBlockHash(invalid_block_header));
         }
         Ok(())

test/tests/blockchain/mod.rs

@@ -1,3 +1,4 @@
 mod batch_tests;
 mod mempool_tests;
 mod smoke_tests;
+mod witness_tests;

test/tests/blockchain/witness_tests.rs

@@ -0,0 +1,79 @@
+use std::{fs::File, io::BufReader, path::PathBuf};
+
+use bytes::Bytes;
+use ethrex_blockchain::{
+    Blockchain,
+    payload::{BuildPayloadArgs, create_payload},
+};
+use ethrex_common::{
+    H160, H256,
+    types::{Block, BlockHeader, DEFAULT_BUILDER_GAS_CEIL, ELASTICITY_MULTIPLIER},
+};
+use ethrex_rlp::decode::RLPDecode;
+use ethrex_storage::{EngineType, Store};
+
+#[tokio::test]
+async fn generated_witness_has_ancestor_headers_in_ascending_order() {
+    let store = test_store().await;
+    let genesis_header = store.get_block_header(0).unwrap().unwrap();
+    let blockchain = Blockchain::default_with_store(store.clone());
+
+    let block_1 = new_block(&store, &genesis_header).await;
+    blockchain.add_block(block_1.clone()).unwrap();
+    let block_2 = new_block(&store, &block_1.header).await;
+    blockchain.add_block(block_2.clone()).unwrap();
+    let block_3 = new_block(&store, &block_2.header).await;
+    blockchain.add_block(block_3.clone()).unwrap();
+
+    let witness = blockchain
+        .generate_witness_for_blocks(&[block_2.clone(), block_3.clone()])
+        .await
+        .unwrap();
+
+    let numbers: Vec<u64> = witness
+        .block_headers_bytes
+        .iter()
+        .map(|b| BlockHeader::decode(b).unwrap().number)
+        .collect();
+
+    assert!(
+        numbers.windows(2).all(|w| w[0] < w[1]),
+        "ancestor headers must be ascending, got {numbers:?}"
+    );
+}
+
+async fn new_block(store: &Store, parent: &BlockHeader) -> Block {
+    let args = BuildPayloadArgs {
+        parent: parent.hash(),
+        timestamp: parent.timestamp + 12,
+        fee_recipient: H160::random(),
+        random: H256::random(),
+        withdrawals: Some(Vec::new()),
+        beacon_root: Some(H256::random()),
+        slot_number: None,
+        version: 1,
+        elasticity_multiplier: ELASTICITY_MULTIPLIER,
+        gas_ceil: DEFAULT_BUILDER_GAS_CEIL,
+    };
+    let blockchain = Blockchain::default_with_store(store.clone());
+    let block = create_payload(&args, store, Bytes::new()).unwrap();
+    blockchain.build_payload(block).unwrap().payload
+}
+
+fn workspace_root() -> PathBuf {
+    PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("..")
+}
+
+async fn test_store() -> Store {
+    let file = File::open(workspace_root().join("fixtures/genesis/execution-api.json"))
+        .expect("Failed to open genesis file");
+    let reader = BufReader::new(file);
+    let genesis = serde_json::from_reader(reader).expect("Failed to deserialize genesis file");
+    let mut store =
+        Store::new("store.db", EngineType::InMemory).expect("Failed to build DB for testing");
+    store
+        .add_initial_state(genesis)
+        .await
+        .expect("Failed to add genesis state");
+    store
+}