[ilitteri]

Motivation

tests-zkevm@v21.0.6 grows the EIP-8025 conformance set from 182 to 298 cases and adds one spec check: a witness whose account or storage trie has a path longer than the 64-nibble hashed key is rejected (execution-specs #3193). Against the new bundle, five of the new cases validated a block the spec rejects: the two overlength-path cases, and three zero-amount-withdrawal cases whose witness leaves out the recipient's proof.

Description

• .fixtures_url_zkevm moves to tests-zkevm@v21.0.6. The release is filled from tests@v21.0.1, and the two pins have to describe the same chain, so .fixtures_url moves to tests@v21.0.1 with it (a coverage-only follow-up to v21.0.0, 81 more cases, no spec changes) and the hive Amsterdam config follows with that release's eels_commit.
• Secured trie paths are checked after the witness tries are rebuilt, on both the SSZ path the guest takes and the RPC path the node takes: a leaf has to end exactly at the key length, an extension has to stop short of it, and a branch cannot sit on it. Inline nodes are decoded and walked like the spec does; hash references are left alone. Without this a 67-nibble leaf still hashes to a root the header can carry, so the block validated. Unit tests cover the leaf, branch, short-leaf and storage-trie shapes and a correct trie.
• A zero-amount withdrawal now reads its recipient. process_withdrawals skipped withdrawals of zero, so a witness that left out the recipient's proof was never asked for it, while the spec's create_ether reads the account. The read does not mark the account modified, so state and the block access list are unchanged. The other 21 unchanged-read shapes in the same test already passed.
• docs/eip-8025.md records the new pins and baseline, and the zkEVM bench README and the Makefile comment name the new release.

Some test ids changed in this release (storage-delete siblings, missing-code and header-size cases). Nothing in the repository referenced them: EXTRA_SKIPS is empty and the one conformance workload in the zkEVM bench manifest kept its id.

The guest-crate equivalence run (host_fixtures.rs, which CI does not run) matches the expected output bytes on all 33830 stateless inputs of the new bundle.

Checklist

• Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.

---

The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Four
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList, then #3652 in tests-zkevm@v21.0.1, which removed
public_keys outright without bumping the revision. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.

---

ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rs → unsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.

---

Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
   reviewers, and add environment: to the two jobs that sign.
2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

[github-actions]

🤖 Kimi Code Review

I'll review this PR focusing on the code changes in the execution witness validation and withdrawal processing. The documentation and configuration updates appear straightforward version bumps.

crates/common/types/block_execution_witness.rs

Critical: Potential panic in validate_secured_trie_paths (Line 541)

let partial = leaf.partial.len() - usize::from(leaf.partial.is_leaf());

This will panic on underflow if leaf.partial.is_leaf() is true and leaf.partial.len() is 0. While len() == 0 && is_leaf() may be invalid RLP encoding, the function receives witness data from the network and must not panic on malicious input. Use saturating_sub or check explicitly:

let partial = leaf.partial.len().saturating_sub(usize::from(leaf.partial.is_leaf()));

Critical: Missing length check in validate_secured_child_paths (Line 570-573)

NodeRef::Hash(hash @ NodeHash::Inline(_)) if hash.is_valid() => {
    validate_secured_trie_paths(&Node::decode(hash.as_ref())?, remaining_nibbles)
}

hash.as_ref() returns &[u8], but Node::decode expects valid RLP. If hash.as_ref() has length < 32 but contains invalid RLP, this returns an error (good). However, there's no explicit length check that the inline hash is actually a valid encoded node. The spec's _validate_secured_trie_paths decodes inline nodes too, but verify this matches expected behavior for embedded nodes under 32 bytes.

Medium: Inconsistent error propagation in validate_secured_child_paths

The NodeRef::Hash(NodeHash::Inline(_)) case propagates RLPDecodeError through ?, but GuestProgramStateError has no From<RLPDecodeError> impl shown. Verify this compiles—if RLPDecodeError converts via the DB variant or similar, that's fragile. Consider adding an explicit variant or mapping:

NodeRef::Hash(hash @ NodeHash::Inline(_)) if hash.is_valid() => {
    let node = Node::decode(hash.as_ref())
        .map_err(|e| GuestProgramStateError::RebuildTrie(format!("invalid inline node: {e}")))?;
    validate_secured_trie_paths(&node, remaining_nibbles)
}

Medium: remaining_nibbles underflow risk in recursive calls

In Node::Branch handling (line 555-561):

for child in &branch.choices {
    validate_secured_child_paths(child, remaining_nibbles - 1)?;
}

If remaining_nibbles == 0, the function returns an error before this loop (line 556-559). Good. But in Node::Extension (line 545-551), remaining_nibbles - extension.prefix.len() could underflow if extension.prefix.len() > remaining_nibbles. The check extension.prefix.len() >= remaining_nibbles catches >=, but what about the case where prefix length exceeds remaining nibbles due to a malformed extension? The >= check prevents underflow, but verify this is the intended spec behavior (should it be > vs >=?).

Actually, re-reading: >= means remaining_nibbles - extension.prefix.len() would be 0 or negative. Since >= rejects it, underflow is prevented. This is correct.

Minor: Missing test for inline hash node validation

The test rejects_a_leaf_past_the_key_length uses trie_over which builds a standard trie. No test exercises the NodeRef::Hash(NodeHash::Inline(_)) path in validate_secured_child_paths. Add a test with a trie containing an embedded (<32 byte) node.

crates/vm/backends/levm/mod.rs

Medium: Error message inconsistency (Line 3804-3812)

db.get_account(address).map_err(|_| {
    EvmError::DB(format!("Withdrawal account {address} could not be read"))
})?;

vs. existing:

let account = db
    .get_account_mut(address)
    .map_err(|_| EvmError::DB(format!("Withdrawal account {address} not found")))?;

The zero-amount path says "could not be read" while the non-zero path says "not found". Both use map_err(|_| ...) discarding the original error. Consider consistent messaging and whether the original error should be preserved for debugging:

.map_err(|e| EvmError::DB(format!("Withdrawal account {address}: {e}")))?

Low: continue after read makes control flow slightly subtle

The pattern:

if withdrawal.amount == 0 {
    db.get_account(address)?; // read for witness validation
    continue;
}
let increment = ...;
// ... modify account

This is correct per the spec comment, but the witness validation side effect is implicit. Consider a more explicit structure or comment explaining that the read is required for stateless witness validation even when no state change occurs.

General Observations

1. Spec compliance: The changes align with the documented spec fixes in docs/eip-8025.md—secured trie path validation and zero-withdrawal account reads. Good.

2. Test coverage: The new secured_trie_path_tests module is comprehensive for the leaf/branch/extension cases it covers. Missing: inline hash nodes, deeply nested invalid paths, and concurrent validation of multiple storage tries.

3. Version consistency: The fixture URL updates (v21.0.0 → v21.0.1, v21.0.1 → v21.0.6) are consistent across .fixtures_url, .fixtures_url_zkevm, amsterdam.yaml, and documentation. Good.

Summary

Severity	Location	Issue
Critical	block_execution_witness.rs:541	Potential panic on usize underflow in leaf.partial.len() - ...
Medium	block_execution_witness.rs:570-573	Verify RLPDecodeError conversion to GuestProgramStateError is intentional
Medium	levm/mod.rs:3804-3812	Inconsistent error messages; consider preserving original DB error
Low	block_execution_witness.rs	Missing test for NodeHash::Inline validation path

The critical underflow is the main blocker. The rest are polish items.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

[github-actions]

🤖 Claude Code Review

Review of PR 7438: zkEVM fixtures bump to v21.0.6, secured-trie path check, zero-amount withdrawal read

I read validate_secured_trie_paths and process_withdrawals. I did not build the code or run the conformance suite. The checkout is a single squashed commit, so I had no diff to read and looked at the touched code directly.

The change looks correct and consistent with the spec behaviour it describes. I have no blocking findings.

Secured-trie path validation (crates/common/types/block_execution_witness.rs:527-588)

• The recursion tracks remaining_nibbles correctly:
  • A leaf must end exactly at 64.
  • An extension must stop short of 64.
  • A branch cannot sit at 64.
• The subtractions can't underflow:
  • The extension subtraction is guarded by the >= check just before it.
  • The branch subtraction is guarded by the == 0 check.
• It is called from both the SSZ guest path (lines 239 and 268) and the RPC path (lines 377 and 396). It runs after the tries are rebuilt, so a malformed trie can't validate just because its root hash matches.
• Inline nodes (under 32 bytes) are decoded and walked, which matches the spec. Hash references are skipped, as the spec does.
• Recursion depth is bounded by the 64-nibble budget, so a malicious witness can't cause unbounded recursion.
• Line 543, leaf.partial.len() - usize::from(leaf.partial.is_leaf()), assumes a leaf-flagged partial always has at least one nibble. That is true for decoded nodes. Using saturating_sub would be a cheap safeguard against a panic in guest code, but it isn't required.
• Minor: Node::decode(hash.as_ref())? at line 584 returns a TrieError or RLPDecodeError through #[from]. That is fine, but the error carries no trie-path context.

Zero-amount withdrawal (crates/vm/backends/levm/mod.rs:3806-3816)

• The read-without-modify matches the spec's create_ether. Using get_account rather than get_account_mut avoids marking the account as modified, so no spurious state transition is produced.
• BAL (EIP-7928): this branch records nothing in the BAL recorder. Please confirm the spec expects a zero-amount withdrawal recipient to be absent from the BAL, or to appear only as a read. Before this change the withdrawal was skipped entirely, so BAL behaviour is unchanged, but a quick check against the Amsterdam fixtures is worthwhile.
• The error message "Withdrawal account {address} could not be read" discards the underlying DB error (map_err(|_| ...)). That matches the existing style on line 3820, but including the cause would make debugging witness failures easier.

Fixture and config bump

• The pin and hive config changes are mechanical. I couldn't check them without a diff, so confirm the checksums or refs match tests-zkevm@v21.0.6 and tests@v21.0.1.

Tests

• The new conformance cases cover the happy and rejection paths.
• A small unit test for validate_secured_trie_paths would also be useful, since it protects a guest-side consensus check. It should cover a 65-nibble leaf, an extension ending at 64, and an inline-node case.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

Looks good overall. The two behavioral changes are narrowly scoped, align with Ethereum stateless-validation semantics, and I don’t see an obvious consensus or memory-safety regression.

• crates/common/types/block_execution_witness.rs:524 adds secured-trie path validation at the right boundary: after reconstructing embedded roots, before walking accounts/storage. The checks for leaf exactness, extension < remaining, and branch remaining > 0 match the 64-nibble invariant for hashed account/storage keys.
• crates/common/types/block_execution_witness.rs:576 correctly descends into inline child nodes via NodeHash::Inline(_) and otherwise leaves hashed references alone, which is the right trust boundary for partial witnesses.
• crates/common/types/block_execution_witness.rs:1344 adds good negative tests for overlong/short state and storage trie keys. That gives decent coverage for the new consensus-facing validation.
• crates/vm/backends/levm/mod.rs:3808 also looks correct: zero-amount withdrawals are now forced to read the recipient account, which matches the stateless-witness requirement that even unchanged reads must be proven. That is a subtle but important fix.

Minor note only:

• crates/vm/backends/levm/mod.rs:3812 drops the underlying DB error in the zero-withdrawal path (map_err(|_| ...)). That’s not a correctness bug, but preserving |e| would make witness/debug failures easier to diagnose and would be more consistent with nearby code such as crates/vm/backends/levm/mod.rs:1494.

No security or performance concerns stood out from this diff beyond the intended extra trie traversal / account read.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

Lines of code report

Total lines added: 127
Total lines removed: 0
Total lines changed: 127

Detailed view

+-------------------------------------------------------+-------+------+
| File                                                  | Lines | Diff |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/block_execution_witness.rs | 1057  | +123 |
+-------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                 | 4205  | +4   |
+-------------------------------------------------------+-------+------+