diff --git a/.github/workflows/pr-main_l1.yaml b/.github/workflows/pr-main_l1.yaml index f1962402b8..271a444a34 100644 --- a/.github/workflows/pr-main_l1.yaml +++ b/.github/workflows/pr-main_l1.yaml @@ -126,6 +126,24 @@ jobs: run: | make -C tooling/ef_tests/blockchain test + test-stateless-zkevm: + name: Test - Stateless zkEVM + needs: detect-changes + if: ${{ needs.detect-changes.outputs.run_tests == 'true' && github.event_name != 'merge_group' }} + runs-on: ubuntu-22.04 + steps: + - name: Checkout sources + uses: actions/checkout@v4 + + - name: Free Disk Space + uses: ./.github/actions/free-disk + + - name: Setup Rust Environment + uses: ./.github/actions/setup-rust + + - name: Run Blockchain EF tests + run: make -C tooling/ef_tests/blockchain test-stateless-zkevm + docker_build: name: Build Docker runs-on: ubuntu-latest diff --git a/crates/common/types/block_execution_witness.rs b/crates/common/types/block_execution_witness.rs index 71071764fa..86e51a5bb5 100644 --- a/crates/common/types/block_execution_witness.rs +++ b/crates/common/types/block_execution_witness.rs @@ -151,16 +151,27 @@ 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; + // Decode headers up front so we can validate the ordering invariant: + // an RPC witness must provide headers in ascending, contiguous block-number + // order. Reordered or gapped inputs would otherwise be silently normalized + // by the BTreeMap in `GuestProgramState::from_witness` and yield an + // incorrect conclusion about the block's validity. + let decoded_headers: Vec = self + .headers + .iter() + .map(|h| BlockHeader::decode(h)) + .collect::>()?; + for window in decoded_headers.windows(2) { + if window[1].number != window[0].number + 1 { + return Err(GuestProgramStateError::NoncontiguousBlockHeaders); } } + let initial_state_root = decoded_headers + .iter() + .find(|h| h.number == first_block_number - 1) + .map(|h| h.state_root); + let initial_state_root = initial_state_root.ok_or_else(|| { GuestProgramStateError::Custom(format!( "header for block {} not found", @@ -177,10 +188,13 @@ impl RpcExecutionWitness { // which would fail to decode in ours return None; } + // Tolerate extra unused witness entries that don't decode as trie nodes. + // Nodes that are actually needed for state access are looked up by hash + // later; extra junk cannot be referenced. let hash = keccak(&b); - Some(Node::decode(&b).map(|node| (hash, node))) + Node::decode(&b).ok().map(|node| (hash, node)) }) - .collect::>()?; + .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, _) = @@ -589,28 +603,25 @@ impl GuestProgramState { } /// Retrieves the account code for a specific account. - /// Returns an Err if the code is not found. + /// Returns an Err if the code is not found — a missing code hash means + /// the execution witness is invalid and the guest program must not + /// conclude anything about the state. pub fn get_account_code(&self, code_hash: H256) -> Result { 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::Custom(format!( + "missing bytecode for hash {} in execution witness", + hex::encode(code_hash) + )) + }) } /// Retrieves code metadata (length) for a specific code hash. /// This is an optimized path for EXTCODESIZE opcode. + /// Returns an Err if the code is not found — same rationale as + /// [`Self::get_account_code`]. pub fn get_code_metadata( &self, code_hash: H256, @@ -620,19 +631,17 @@ 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.", + }) + .ok_or_else(|| { + GuestProgramStateError::Custom(format!( + "missing bytecode for hash {} in execution witness", hex::encode(code_hash) - ); - Ok(CodeMetadata { length: 0 }) - } - } + )) + }) } /// 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. diff --git a/tooling/ef_tests/blockchain/.fixtures_url_zkevm b/tooling/ef_tests/blockchain/.fixtures_url_zkevm index 7b92a046c9..1c4b11fd86 100644 --- a/tooling/ef_tests/blockchain/.fixtures_url_zkevm +++ b/tooling/ef_tests/blockchain/.fixtures_url_zkevm @@ -1 +1 @@ -https://github.com/ethereum/execution-spec-tests/releases/download/zkevm%40v0.3.0/fixtures_zkevm.tar.gz +https://github.com/ethereum/execution-spec-tests/releases/download/zkevm%40v0.3.3/fixtures_zkevm.tar.gz diff --git a/tooling/ef_tests/blockchain/test_runner.rs b/tooling/ef_tests/blockchain/test_runner.rs index 85fac6dd9b..9740ed2fba 100644 --- a/tooling/ef_tests/blockchain/test_runner.rs +++ b/tooling/ef_tests/blockchain/test_runner.rs @@ -529,6 +529,11 @@ async fn re_run_stateless( /// trie nodes, codes, and ancestor headers needed for that specific block. /// Following the spec, we execute each block /// independently with its own witness. +/// +/// The fixture's `statelessOutputBytes` encodes the expected output as +/// `new_payload_request_root (32 bytes) || valid (1 byte) || padding`. When +/// `valid == 0`, the witness is intentionally invalid and stateless execution +/// is expected to fail (or conclude the block is invalid). #[cfg(feature = "stateless")] async fn run_stateless_from_fixture( test: &TestUnit, @@ -551,6 +556,9 @@ async fn run_stateless_from_fixture( continue; }; + let expect_stateless_valid = + expected_stateless_valid(block_data.stateless_output_bytes.as_deref()); + let block: CoreBlock = block_data.clone().into(); let block_number = block.header.number; @@ -559,9 +567,21 @@ async fn run_stateless_from_fixture( format!("Failed to parse executionWitness for block {block_number}: {e}") })?; - let execution_witness = rpc_witness - .into_execution_witness(*chain_config, block_number) - .map_err(|e| format!("Witness conversion failed for block {block_number}: {e}"))?; + let witness_conversion_result = + rpc_witness.into_execution_witness(*chain_config, block_number); + let execution_witness = match witness_conversion_result { + Ok(w) => w, + Err(e) => { + // An intentionally invalid witness may fail conversion — that's a valid + // "block is invalid" conclusion when the fixture marks this block invalid. + if !expect_stateless_valid { + continue; + } + return Err(format!( + "Witness conversion failed for block {block_number}: {e}" + )); + } + }; let program_input = ProgramInput::new(vec![block], execution_witness); @@ -571,12 +591,39 @@ async fn run_stateless_from_fixture( BackendType::SP1 => Sp1Backend::new().execute(program_input), }; - if let Err(e) = execute_result { - return Err(format!( - "Stateless execution from fixture failed for {test_key} block {block_number}: {e}" - )); + match execute_result { + Ok(()) => { + if !expect_stateless_valid { + return Err(format!( + "Expected stateless execution to fail for {test_key} block {block_number} (fixture valid=0x00) but it succeeded" + )); + } + } + Err(e) => { + if expect_stateless_valid { + return Err(format!( + "Stateless execution from fixture failed for {test_key} block {block_number}: {e}" + )); + } + } } } Ok(()) } + +/// Parse the `valid` flag (byte 32) from a fixture's `statelessOutputBytes`. +/// +/// Layout: `new_payload_request_root (32 bytes) || valid (1 byte) || padding`. +/// If absent or unparseable, default to expecting success (true) to preserve +/// the prior behavior for fixtures without this field. +#[cfg(feature = "stateless")] +fn expected_stateless_valid(hex_str: Option<&str>) -> bool { + let Some(s) = hex_str else { return true }; + let s = s.strip_prefix("0x").unwrap_or(s); + // valid byte is at hex offset 64..66 (bytes[32]) + match s.get(64..66).and_then(|b| u8::from_str_radix(b, 16).ok()) { + Some(0) => false, + _ => true, + } +}