fix: check each RPC transaction hash against its own encoding in verify_block_integrity, read gas_used from the executor

[flyq]

Summary

PR 3/6 of the #170 split, stacked on #195. Two rewrites of security-critical derivations, separated from the cleanup so each gets argued on its own. The first turned out to be a fix rather than a perf change: the check it replaces was a tautology.

vincent's review of #170 on the second one: "This is the one I would most like to see argued explicitly… If they can disagree, this is a consensus divergence. Please state why they are equivalent and add an assertion test pinning it."

1. verify_block_integrity checks the claimed transaction hash against the envelope's own encoding

Before: tx.inner.clone().into_inner().trie_hash() == *hash() per transaction, then a second clone inside ordered_trie_root_with_encoder for the transactions root.

That hash check never checked anything. Encodable2718::trie_hash() defaults to keccak256(encoded_2718()), but every variant in play overrides it: Signed<T>::trie_hash returns *self.hash() (alloy-consensus 1.1.2 src/signed.rs:515), Sealed<T>::trie_hash returns self.hash() (alloy-eips 1.1.2 src/eip2718.rs:327), and OpTxEnvelope's TransactionEnvelope derive forwards per variant. The deserializer seeds that cached hash from the RPC hash field through new_unchecked (src/signed.rs:583), so the old predicate compared the provider's claim with itself. A provider returning the right transaction bytes under a wrong hash passed verification, and the transactions-root check cannot catch that because the root is computed from the bytes.

After: each envelope is encoded once via encoded_2718(); keccak256 of those bytes is checked against the claimed hash, and the same bytes feed the ordered trie (crates/stateless-common/src/rpc_client.rs:1578). The two clones per transaction are gone; the keccak is new work and the first real check of the field.

Pinned: verify_block_integrity_rejects_a_forged_transaction_hash (rpc_client.rs:2997) forges the hash of the first (deposit, Sealed) and last (Signed) transaction of a mainnet fixture block through a JSON round trip and asserts the block is rejected. Under the old predicate the forged block verifies — checked by reverting the comparison locally and watching the test fail.

2. gas_used comes from BlockExecutionResult::gas_used

Before: execution_result.receipts.last().map(|r| r.cumulative_gas_used()).unwrap_or(0).
After: execution_result.gas_used (crates/stateless-core/src/executor.rs:497).

Equivalence, against the pinned mega-evm (v1.7.0, 30ce038, crates/mega-evm/src/block/executor.rs, MegaBlockExecutor::finish):

let gas_used = self.receipts.last().map(|r| r.cumulative_gas_used()).unwrap_or_default();

Character for character the expression the header check read before, and finish() computes it after commit_system_call_outcomes, so MegaETH's system transactions are accounted identically on both sides. There is only one derivation.

Pinned where the contract lives. verify_replay_outputs (executor.rs:622) fails any block whose replayed gas_used differs from the header's claim, in release, on every real block, and validate_block_mainnet_fixtures runs it over every paired mainnet fixture; a mega-evm that starts accounting gas outside the receipt chain fails there. An earlier revision of this PR also carried a debug_assert_eq! at the derivation site and a third full-fixture replay test asserting the same field; both were dropped as lower-altitude restatements of that check (the assertion could only ever fire on a correct upstream redefinition, which the header check is what would then adjudicate).

Testing

cargo fmt --all --check, cargo clippy --workspace --all-targets --all-features (0 warnings), cargo sort --check, full workspace suite 481 passed / 0 failed (new: verify_block_integrity_rejects_a_forged_transaction_hash), cargo test -p stateless-core --no-default-features --lib --no-run clean.

The positive path is covered by the validator's mock-RPC integration test, which fetches every fixture block through the verifying path.

[mega-maxwell]

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

❓ 1 open question(s)

Last reviewed: 015aa8af..c0ac4b31 · updated 2026-09-08T07:14:05+00:00

New this round: 0 finding(s), 1 question(s) · Resolved this round: 0 · Open questions: 1

Open questions awaiting an answer:

• ❓ The PR description's "Pinned two ways" section still promises a debug_assert_eq! at the gas_used derivation site and a dedicated replayed_gas_used_matches_the_mainnet_header fixture test, but both were removed in the current head — the surviving pin is verify_replay_outputs (`output.gas_used (asked here)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.5%. Comparing base (7ca2253) to head (c0ac4b3).

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e62e3e14ae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[mega-maxwell]

❓ Review complete — 1 open question(s)

Reviewed 015aa8af..c0ac4b31.

Incremental review of the verify_block_integrity single-encode rewrite (with a new forged-hash regression test) and the gas_used sourcing change in replay_block.

Open questions — answer them in a reply on this PR. Each one is marked answered here once a later review round confirms the answer, so this list stays current:

❓ **Open question · Low confidence**

• The PR description's "Pinned two ways" section still promises a debug_assert_eq! at the gas_used derivation site and a dedicated replayed_gas_used_matches_the_mainnet_header fixture test, but both were removed in the current head — the surviving pin is verify_replay_outputs (output.gas_used == header.gas_used) exercised by validate_block_mainnet_fixtures. Was dropping the receipt-chain-equivalence pin intentional, or should either the description or the pin be restored?
• Why it matters: The removed debug_assert_eq! was the only check that would catch a mega-evm change where execution_result.gas_used stops equaling receipts.last().cumulative_gas_used() while still matching the header — a signal of upstream derivation drift, even when it does not surface as a consensus failure. If the description reflects the intended safety net, either it or the code should be aligned before landing.
• How to verify: Look at crates/stateless-core/src/executor.rs:497 — only let gas_used = execution_result.gas_used; remains — and search the repo for replayed_gas_used_matches_the_mainnet_header (no hits outside .pr-review/). Confirm whether the author still considers verify_replay_outputs the single pin, and update the PR description accordingly, or re-add the internal debug_assert_eq! pin.