Skip to content

feat(evm): bump reth to v2.4.0 and support revmc JIT execution#222

Open
davidtaikocha wants to merge 25 commits into
mainfrom
feat/reth-v2.4.0-jit
Open

feat(evm): bump reth to v2.4.0 and support revmc JIT execution#222
davidtaikocha wants to merge 25 commits into
mainfrom
feat/reth-v2.4.0-jit

Conversation

@davidtaikocha

@davidtaikocha davidtaikocha commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Why

  • Bump the pinned Reth stack from rev 27bfddea (pre-v2.3.0) to v2.4.0-era f2eecc6 (the v2.4.0 tag plus nine upstream commits, matching the OP monorepo's pin — see the reth-optimism-trie bullet), crossing two release boundaries (alloy 2.1.1, alloy-evm 0.37, revm 41, published reth crates 0.5.x, Rust 1.95).
  • Support opt-in revmc JIT execution using reth v2.4.0's native JIT surfaces, superseding the vendored-revmc approach from feat(evm): support revmc JIT execution #218.
  • Keep proving-critical execution structurally isolated from JIT: consensus zk-gas metering, RPC execution, and traced/inspected execution always run the interpreter.

How

Dependency bump

  • All reth git deps move to the v2.4.0 tag commit; reth-codecs/reth-primitives-traits to 0.5.x (0.5.2 in the lockfile); revm-database-interface to 41.0.0; toolchain and workspace MSRV to 1.95.
  • The reth umbrella crate is consumed with default-features = false minus its jit default (which would drag LLVM into every build through reth-node-ethereum); reth-revm/portable is re-applied explicitly.
  • reth-optimism-trie is consumed from the OP monorepo as a git dependency (rev = the head of chore: bump reth to f2eecc6 (post-v2.4.0) ethereum-optimism/optimism#21766, OP's post-v2.4.0 reth bump). The crate's reth dependencies resolve inside OP's workspace, so Alethia's reth pin must reference the identical paradigmxyz/reth commit — hence f2eecc6 rather than the tag. ⚠️ #21766 is still open: bump the dependency to its merge commit once it lands (and re-sync the reth pin if OP's rev changes). Upstream moved live collection into an engine service; the Taiko proof-history sidecar drives collection itself, so the live.rs collector is now first-party code at crates/node/src/proof_history/live.rs (ported to the crate's public storage API, with direct unit tests). An earlier revision of this branch vendored the crate to decouple the pins; the git dependency deliberately re-couples them in exchange for dropping the vendored source.
  • Notable API migrations: ConsensusError::Other now carries Arc<dyn Error>; BlockExecutor::commit_transaction is infallible (the zk-gas block budget is pre-checked in execute_transaction_without_commit, preserving truncation semantics); state hooks are delivered through the State database; BlockExecutorFactory grew Executor/TxExecutionResult GATs; BuildArguments::state_root_handle; EthBuiltPayload over RecoveredBlock; BAL (Amsterdam) parameters are threaded through and ignored (Taiko schedules no Amsterdam fork).

revmc JIT

  • revmc is pinned to 520462a4 — the revision reth f2eecc6 locks. Relative to reth v2.4.0's original lock (7e3536d, which already had revmc#391 non-blocking runtime controls) it adds revmc#395 (dynamic-gas failure ordering), revmc#394 (stack sync for diverging builtins), and revmc#400 (LOG memory operands in gas analysis) — the latter two fix compiled execution diverging from the interpreter.
  • TaikoEvmFactory owns the shared backend, wraps TaikoEvm in revmc::revm_evm::JitEvm, and implements reth_evm::JitBackend, so the upstream reth_jit RPC action (enable/disable/pause/unpause/clear) works against the Taiko EVM config. CLI flags reuse upstream JitArgs via reth's NodeConfig (--jit, --jit.hot-threshold, …) — no custom CLI surface.
  • Compiled code can only run when all four gates pass:
    1. build: jit cargo feature (binary default; --no-default-features builds without the LLVM toolchain),
    2. runtime: --jit or reth_jit (default off),
    3. local opt-in: with_jit_support() — called by reth's engine tree for canonical execution and by the Taiko payload builder; RPC call/trace/simulation/estimation/pending-block execution keeps the base config and stays interpreter-only,
    4. fork allowlist: spec_supports_jit is an exhaustive match — Unzen is interpreter-only (zk-gas metering needs per-opcode interpreter hooks; schedule_for() re-checked as a belt-and-suspenders guard), and any new fork fails compilation until explicitly classified.
  • Inspected execution always selects the disabled backend (compiled code cannot deliver per-step inspector callbacks). TaikoEvmWrapper::transact_raw drives the JIT dispatcher with TaikoEvmHandler directly so anchor/fee-share semantics are preserved.
  • Out-of-process compilation: the binary serves as the revmc compile helper when re-executed (maybe_run_jit_helper).
  • Proof-history reads state and never executes the EVM, so it is structurally unaffected.
  • CI/Docker install LLVM 22 (.github/scripts/install_llvm.sh); a check-no-jit job proves the interpreter-only build stays LLVM-free, and an arm64 job guards the Docker base image's LLVM availability.

Tests

  • JIT-vs-interpreter differential tests (jit_execution_matches_interpreter_execution, jit_matches_interpreter_on_dynamic_gas_failure_order) and gating tests (jit_requires_local_support_and_falls_back_for_unzen, config isolation in crates/block/src/config.rs) run real blocking in-process compilation.

Relation to #218

Supersedes it: reth v2.4.0 ships the JIT runtime surfaces #218 had to vendor (vendor/revmc, ~100 files) and backport. The fork allowlist, inspected-execution rule, and handler-driven execution design carry over.

Tests

  • just fmt / just fmt-check — passed.
  • just clippy — passed (single pass, missing-docs gates across the whole workspace).
  • cargo nextest run --workspace --all-features247 passed, 0 skipped (macOS, LLVM 22.1.8; JIT tests exercise real compilation). The former vendored crate's ~470 tests left with it: its test suite is byte-identical to upstream's and runs in OP's CI.
  • cargo check --locked -p alethia-reth-bin --no-default-features — passed without LLVM installed.

🤖 Generated with Claude Code

davidtaikocha and others added 8 commits July 14, 2026 22:05
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n 1.95

- reth git pin -> 943af245 (tag v2.4.0); published reth crates -> 0.5
- alloy 2.1.1 / alloy-evm 0.37 / alloy-primitives 1.6 / revm 41
- reth default features minus jit (keeps LLVM out of non-jit builds)
- vendor reth-optimism-trie from OP develop 9b802fdb (OP still pins reth
  v2.3.0; published-crate split 0.4/0.5 prevents a git dep) — see
  vendor/reth-optimism-trie/TAIKO-PROVENANCE.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- PayloadTypes: block_to_payload bal param + From<EthBuiltPayload> for
  TaikoExecutionData (BAL discarded; Taiko schedules no Amsterdam fork)
- zk-gas metered loop mirrors revm 41 run_plain (step Err protocol,
  GasTable threading); EvmFactory::Error bound -> DBErrorMarker
- ConsensusError::Other now carries Arc<dyn Error>: add
  TaikoConsensusMessage + other_consensus_error helper
- FullConsensus::validate_block_post_execution BAL-hash param

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- block: infallible commit_transaction (zk-gas budget pre-checked in
  execute_transaction_without_commit), state hooks now delivered via the
  State database, BlockExecutorFactory Executor GAT + TxExecutionResult,
  GasOutput accessors, mark_invalid by-value, BuildPendingEnv overrides param
- payload: BuildArguments state_root_handle, DB-level state hooks,
  EthBuiltPayload over RecoveredBlock
- rpc: BalProvider bounds on the Taiko engine API, tree-validator
  StateTrieOverlayManager param, proof-window accessor
- node: adapt proof-history to vendored trie develop API (opt_block shim,
  pruner builder, ComputedTrieData); retain LiveTrieCollector in vendor
  (upstream moved live collection into its engine service)
- cli: NodeConfig jit field passthrough, TracingGuards

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All-features workspace now compiles and the full suite passes
(706 tests, including the vendored reth-optimism-trie suite).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire upstream revmc (pinned at 56ed0063, reth v2.4.0's locked revision
fast-forwarded to include the revmc#395 dynamic-gas failure-order fix)
into the Taiko EVM, using reth v2.4.0's native JIT surfaces.

Proving isolation — compiled code can only run when every gate passes:
- build: workspace jit features (bin default; --no-default-features
  builds LLVM-free)
- runtime: --jit / upstream reth_jit RPC (via ConfigureEvm::jit_backend)
- local opt-in: with_jit_support() from reth's engine tree and the Taiko
  payload builder; RPC execution keeps the base config (interpreter)
- fork allowlist: spec_supports_jit is an exhaustive match — Unzen stays
  interpreter-only (zk-gas needs per-opcode hooks), new forks fail
  compilation until classified; schedule_for() re-checked as guard
Inspected execution always selects the disabled backend, and
TaikoEvmWrapper::transact_raw drives JitEvm with TaikoEvmHandler so
anchor/fee-share semantics are preserved.

Adds JIT-vs-interpreter differential tests and config isolation tests
(714 tests pass with the jit feature); ports LLVM 22 CI/Docker infra.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g design note

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two small, behavior-preserving cleanups over the v2.4.0 migration:

- block/config: route the jit-gated `with_jit_support_enabled` and
  `jit_backend` through the crate's own `self.evm_factory()` accessor
  instead of reaching through `self.executor_factory.evm_factory()`,
  matching the adjacent `self.chain_spec()` usage.
- evm/zk_gas/tests: extract the duplicated blocking in-process JIT
  backend construction into a single `blocking_jit_backend()` helper
  shared by both jit tests.

Verified: cargo clippy (--all-features, CI lint gate) + nextest for
alethia-reth-{block,evm} — 72 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@davidtaikocha davidtaikocha changed the title feat: bump reth to v2.4.0 and support revmc JIT execution feat(evm): bump reth to v2.4.0 and support revmc JIT execution Jul 15, 2026
@davidtaikocha
davidtaikocha marked this pull request as ready for review July 15, 2026 01:31
davidtaikocha and others added 2 commits July 15, 2026 10:33
The reth v2.4.0 bump silently changed the committed zk gas of an
OOG-terminating step. Pre-bump revm (interpreter 35) ran halt_oog()
(spend-all) inside `step` itself, so the metered loop's gas delta
charged the full forfeited remaining gas — the reference behavior
taiko-geth's zk mirror follows (June 2026 cross-client audit). revm 41
returns a bare Err from `step` and leaves halt materialization to the
caller, so the rewritten loop measured a wrapped/untouched `remaining`
and the charge collapsed to zero. run_metered_plain now materializes
the exceptional halt before charging, restoring the audited semantics;
these totals feed header.difficulty on Unzen. revm's stock inspected
loop already halts before step_end, so ZkGasInspector measures the
same values without further changes.

Also:
- pin committed zk gas for OOG-terminating (forfeited gas x
  multiplier) and stack-underflow-terminating (static gas x
  multiplier; known taiko-geth divergence tracked geth-side) steps on
  both metering paths
- forward log/log_full/frame_start/frame_end/selfdestruct from
  ZkGasInspector to the wrapped inner inspector (previously dropped)
- guard the inspected-execution JIT invariant: debug_assert the
  disabled backend in transact_raw and downgrade the backend when
  set_inspector_enabled switches an instance to inspect mode
- unit-test commit_would_exceed_block_limit against commit_transaction
  at the block-limit boundary and on u64 overflow

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nits

- ship upstream's LICENSE-MIT inside vendor/reth-optimism-trie (copied
  from rust/op-reth/LICENSE-MIT at the vendored commit) and note it in
  TAIKO-PROVENANCE.md
- install_llvm_ubuntu.sh hard-fails when a required binary is missing
  instead of warning and surfacing later as an obscure link error
- align the remaining actions/checkout@v4 in ci.yml with the other jobs
- name-discard the builder's EIP-7928 block access list with a
  debug_assert tripwire (Taiko schedules no Amsterdam fork)
- refresh the init_tracing doc for the TracingGuards return type

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@davidtaikocha
davidtaikocha force-pushed the feat/reth-v2.4.0-jit branch from db8cb24 to 5aa1746 Compare July 15, 2026 02:33
davidtaikocha and others added 3 commits July 15, 2026 12:03
reth's re-execute command parses JitArgs but never consumes them
(v2.4.0), and the components closure it accepts only receives the chain
spec — so `alethia-reth re-execute --jit` silently ran the interpreter
on a permanently disabled backend, and non-jit builds accepted the flag
without the "built without the `jit` feature" error the node command
raises.

The node executor builder's backend construction is now a shared
`evm_config_from_jit_args` helper covering both cfg branches; the
ReExecute arm builds its EVM config from the command's own JitArgs
through it. Unit tests pin the enabled-flag wiring on jit builds and
the hard error on non-jit builds.

Also excludes the vendored reth-optimism-trie manifest from `just
fmt`'s cargo-sort pass, which was flattening upstream's dependency
grouping on every run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The justfile's clippy recipe already exempts the vendored
reth-optimism-trie crate from the missing-docs gates, but AGENTS.md's
Documentation Policy listed only tests and examples as exclusions, so
enforcement and the written policy disagreed. Records the vendor
exemption in the policy with the same rationale (keep vendored sources
close to upstream; `-D warnings` still applies).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consume reth-optimism-trie straight from the Optimism monorepo instead of
the vendored copy. The vendor existed because OP pinned reth v2.3.0
(published crates 0.4.x) while Alethia pins v2.4.0 (0.5.x), a split cargo
cannot unify; ethereum-optimism/optimism#21766 moves OP to f2eecc6
(v2.4.0 + 9 commits), so the git dependency is possible again provided
both workspaces reference the identical paradigmxyz/reth commit. The reth
pin therefore moves from the v2.4.0 tag to f2eecc6 to match.

The dependency rev is the head of the still-open #21766; bump it to the
merge commit once that PR lands.

The Taiko-retained live collector (upstream deleted the module in favor
of its engine service) moves in-tree as first-party code at
crates/node/src/proof_history/live.rs, ported to the crate's public API
and given direct unit tests (execute/store happy path, window and
state-root rejections, unwind and reorg-replacement round-trips). The
vendored crate's own test suite was byte-identical to upstream's and now
runs in upstream CI instead.

With no vendored source left, the vendor carve-outs are reverted: single
clippy pass with the missing-docs gates, cargo sort --workspace, and the
AGENTS.md vendor/** exclusion and CLAUDE.md tree entry are removed.

Verified with just fmt, just clippy, and just test (247/247), plus a
default-features check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
davidtaikocha and others added 3 commits July 15, 2026 14:54
Picks up two compiled-code correctness fixes over 56ed0063: revmc#394
(sync stack for diverging builtins) and revmc#400 (account LOG memory
operands in the gas analysis), both fixing compiled execution that
diverges from the interpreter — consensus-relevant with JIT enabled for
canonical execution and payload building. Also includes revmc#403
(native-only LLVM target init). A clean three-commit fast-forward; the
pin now matches the revmc revision reth f2eecc6 locks.

Verified with just fmt, just clippy, and just test (247/247); the
JIT/interpreter differential tests exercise real in-process compilation
against the new revision.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ture

- Replace the Taiko-local TaikoConsensusMessage newtype + other_consensus_error
  helper with reth's built-in ConsensusError::msg (identical Other(MessageError)
  wrapping); 11 call sites across consensus validation/{mod,anchor}.rs.
- Fold the repeated BlockchainProvider::new(factory) construction into the
  genesis_fixture test helper in proof_history/live.rs (5 collector tests).

Behavior-preserving; no consensus-critical path (zk-gas, JIT gating, block
executor) touched. Verified with just fmt-check, just clippy (--all-features,
missing-docs gate), and just test (--all-features): 248 passed, 0 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@YoGhurt111 YoGhurt111 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two proof-history issues in the current head.

Comment thread crates/node/src/proof_history/storage_init.rs
Comment thread crates/node/src/proof_history/live.rs Outdated
davidtaikocha and others added 5 commits July 16, 2026 19:41
ethereum-optimism/optimism#21766 landed as 4f21ce6b, so the dependency
moves off the PR-head rev it was pinned to while that PR was in flight.

OP's reth pin is unchanged at f2eecc6 across the merge, so the rev
lockstep between reth-optimism-trie and the reth stack still holds and
the graph keeps a single reth copy. The crate's only changes since the
old rev are internal cleanups (store.rs, store_v2/{backfill,write}.rs);
no first-party call sites are affected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y reorgs

Two proof-history availability fixes, both crashing the node via the
critical sidecar task:

- A database initialized under the previous reth-optimism-trie pin has no
  LatestBlock row when no block was stored after initialization (the old
  store wrote only EarliestBlock and fell back to it as latest). The
  strict reads now make such a database look uninitialized while its
  completed anchor turns re-initialization into a no-op, so startup fails
  permanently. Repair it at sidecar startup by re-committing the completed
  anchor, which writes EarliestBlock and the missing LatestBlock in one
  transaction after verifying the anchor matches the stored earliest block.

- A reorg whose common ancestor is exactly the retained earliest block
  passed the sidecar guard but was refused by replace_updates
  (ReorgBaseOutOfWindow), turning an ordinary reorg right after
  initialization into a crash. The sidecar now routes that boundary through
  unwind-and-reprocess, and the collector rebuilds the window atomically
  (unwind to the anchor, then append) when the common ancestor is the
  hash-matching anchor, failing closed with OutOfOrder when the new chain
  does not descend from it.

Regression tests cover the legacy MDBX layout migration (including
mismatching-anchor and no-op cases) and boundary reorgs at the earliest
window block.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main's release-please commit (#212) bumped the workspace version 1.2.0 ->
1.3.0 in Cargo.toml without regenerating Cargo.lock. CI checks out the
merge of this branch into main, which merges textually clean but leaves
Cargo.toml at 1.3.0 and Cargo.lock at 1.2.0, so the check-no-jit job's
`cargo check --locked` refused with "cannot update the lock file".

This branch is the first to pass --locked in CI, so it is the first to
surface the drift; the branch head alone always passed. The delta is only
the 12 alethia-reth-* version strings — no dependency resolution changed.

Verified: cargo check --locked -p alethia-reth-bin --no-default-features.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
revmc's runtime coerces `enabled = true` and `jit_hot_threshold = 0`
whenever `blocking` is set (revmc-runtime `new_inner`, pinned 520462a4),
so forwarding `--jit.blocking` straight through meant the flag alone
turned on JIT for canonical execution and payload building — gate 3
(`with_jit_support()`) is already satisfied there — and compiled every
contract synchronously on first touch. The "experimental revmc JIT
backend" warning is gated on our own `enabled`, so it stayed silent.

Upstream reth does not have this hole: `build_evm_config` short-circuits
to a disabled factory before `blocking` can reach the runtime. Gating
`blocking` on `enabled` closes it while still building the backend from
the operator's tuning, so a later `reth_jit` RPC enable starts with the
configured thresholds rather than revmc's defaults (which is what an
upstream-style early return would have cost).

Gates 3 and 4 were unaffected — Unzen and RPC stayed interpreter-only —
so this was an opt-in bypass on a hidden flag, not a fork or metering
bypass. The new test fails without the fix (backend reports enabled).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
davidtaikocha and others added 4 commits July 17, 2026 12:55
The EIP-7928 justification rested on a `debug_assert` that could not
fire — and not because asserts compile out in release. The builder never
calls `with_bal_builder{,_if}` on the `State`, so `bal_builder` is always
`None` and `take_built_alloy_bal()` unconditionally yields `None`
regardless of fork schedule. The assert restated the builder's own DB
construction, not the "Taiko schedules no Amsterdam fork" assumption the
comment claimed it guarded.

That assumption is operator config, not structure: `TaikoChainSpec::from`
delegates to `ChainSpec::from(genesis)`, which maps `amsterdamTime` to
`EthereumHardfork::Amsterdam`. Had a genesis activated it, the builder
would emit no BAL, the assembler would seal `block_access_list_hash:
None`, and reth's post-execution validation would skip the check (it is
guarded on `let Some(hash)`) — silently producing non-compliant blocks.

Guard the real assumption instead: refuse to build when Amsterdam is
active at the payload timestamp. The retained discard is now an honest
backstop rather than a tautology, and its comment says so.

Tests cover both directions of the activation boundary and pin that the
shipped mainnet spec never activates Amsterdam, so the guard cannot
reject ordinary builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
27170a2 made payload building fail closed when a genesis config
schedules amsterdamTime, but import stayed fail-open: the same
misconfigured node would refuse to build yet follow a chain it cannot
validate, because the executor never computes a block access list and
reth's post-execution validation skips the EIP-7928 comparison whenever
the computed hash is absent.

Reject Amsterdam-active timestamps in
TaikoBeaconConsensus::validate_header — the chokepoint both the engine
newPayload flow and staged sync route through — and restore upstream
EthBeaconConsensus parity by rejecting stray
block_access_list_hash/slot_number header fields, which the v2.4.0
migration's custom validator had silently dropped. No Taiko network
schedules Amsterdam: Unzen, the latest fork, activates Osaka-level
rules only, and a new test pins that for every shipped spec.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
transact_raw propagated handler errors with ? before
journal.finalize(), while revm's ExecuteEvm::transact finalizes
unconditionally ("if the transaction fails, the journal is considered
broken") — the shape main inherited by delegating to inner.transact, so
the v2.4.0 migration silently changed error-path behavior.

The residue cannot corrupt later transactions today: TaikoEvmHandler
inherits revm's default run/inspect_run, whose catch_error discards the
failed transaction's journal entries, leaving only cold, untouched
cache entries holding original DB values — semantically inert for
execution and filtered out by State::commit on the way to any bundle
state. But payload building (legacy mode) and derived-block execution
skip invalid transactions and keep executing on the same EVM, so the
next successful transaction's returned state was padded with the failed
transaction's loaded accounts, and the safety argument rested on revm
internals (discard_tx retaining reverted entries) rather than on our
own code.

Finalize before propagating the error, restoring the upstream contract
and pre-2.4.0 parity. The regression test asserts the observable that
actually changes — the journal state map is empty after an errored
transact_raw — and fails without the fix; an invalid-then-valid
result-level test would pass either way.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@johntaiko johntaiko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Large, well-structured migration to reth v2.4.0 (f2eecc6) plus opt-in revmc JIT. The execution model is carefully gated (Cargo feature → --jit/reth_jit → local with_jit_support → fork allowlist excluding Unzen), and Taiko semantics are preserved by driving JitEvm through TaikoEvmHandler rather than revmc’s MainnetHandler entry points. zk-gas metering is updated for revm 41’s step/GasTable control flow and alloy-evm 0.37’s infallible commit_transaction, with a solid pre-check + test suite. Dominant residual risks are operational (open OP monorepo pin, LLVM install side effects) and a known gap in the ported proof-history live collector (parent hash not verified against the stored window). No consensus-breaking EVM/JIT bug stood out in review; the gates and differential tests substantially reduce JIT divergence risk.

Issue counts by severity

  • bugs: 0
  • suggestions: 3
  • nits: 2

Issues outside the diff

These findings reference lines that are not present in the diff and could not be posted as inline comments:

  • [suggestion] Cargo.toml:176 -- reth-optimism-trie is pinned to rev 4f21ce6… described as the head of ethereum-optimism/optimism#21766. The PR description notes that PR is still open. Until it merges (or is rebased), force-pushes on that branch can rewrite history, and Alethia’s reth pin must stay byte-identical to OP’s pin or Cargo will dual-resolve incompatible reth graphs. This is a supply-chain / maintenance footgun for a critical dependency.
    • Suggestion: Prefer a merge commit (or tagged release) once #21766 lands; until then document the pin in release notes and re-check the rev on every CI run / before cutting a release. Consider a temporary cargo tree -i reth check in CI that fails if two reth revs appear.

let block_ref =
BlockWithParent::new(block.parent_hash(), NumHash::new(block.number(), block.hash()));

// TODO: should we check block hash here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] execute_and_store_block_updates only checks that the parent height falls inside the proof window; the TODO acknowledges that the stored block hash at parent_block_number is not compared to block.parent_hash(). On a reorg race (canonical parent hash H2 at height N while the window still holds H1 at N), the L2 state is loaded by parent hash but the proof overlay is keyed by height only, so trie collection can mix wrong-fork overlay with correct L2 parent state. State-root mismatch usually fails closed, but the gap can still produce hard-to-debug storage errors or, in pathological cases, incorrect stored diffs if the root still matches.

Suggestion: After reading the proof window, resolve the stored block at parent_block_number (or the window’s latest when contiguous) and require its hash equals block.parent_hash() (or the window’s recorded hash for that height). Reject with a dedicated “parent hash mismatch / out of order” error, mirroring the boundary check already done in unwind_and_store_block_updates.

fn commit_current_transaction_zk_gas(&mut self)
where
Evm: TaikoZkGasEvm,
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] commit_current_transaction_zk_gas uses unreachable! when the budget is exceeded. That is intentional given alloy-evm 0.37’s infallible commit_transaction and the pre-check in execute_transaction_without_commit, but any future caller that builds an EthTxResult outside that path (or a trait default that commits without re-checking) will panic the node on Unzen rather than surface a recoverable truncation error.

Suggestion: Keep the pre-check as the primary gate, but consider debug_assert! + a defensive reset_current_transaction_zk_gas / soft zk_gas_exhausted path in release builds if the trait ever allows returning an error again; at minimum, document in the BlockExecutor impl that commit_transaction must only be fed results from execute_transaction_without_commit on this type.

echo "Error: $bin-$version not found after install" >&2
exit 1
fi
ln -fs "$(command -v "$bin-$version")" "/usr/bin/$bin"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The installer force-symlinks clang, lld, ld.lld, llvm-config, and FileCheck into /usr/bin/, overwriting any existing system tools. Fine for disposable CI/Docker images, but harmful if someone runs .github/scripts/install_llvm.sh ubuntu on a shared developer machine.

Suggestion: Symlink into a dedicated prefix (e.g. /usr/lib/llvm-22/bin already on PATH, or $HOME/.local/bin) for non-CI use, or document prominently that the script is CI/Docker-only and mutates /usr/bin.

let window = provider_ro.get_proof_window()?;
let (earliest, latest) = (window.earliest.number, window.latest.number);

let parent_block_number = block.number() - 1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] let parent_block_number = block.number() - 1 wraps on block 0. Genesis is unlikely on this path and currently fails closed via MissingParentBlock, but the wrap is easy to misread and could interact oddly if earliest is ever non-zero.

Suggestion: Use block.number().checked_sub(1).ok_or(...) (or reject number == 0 explicitly) so genesis/underflow is an explicit error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants