Skip to content

refactor: migrate engine validation to reth main hooks#140

Merged
panos-xyz merged 5 commits into
mainfrom
codex/reth-main-upgrade
Jul 16, 2026
Merged

refactor: migrate engine validation to reth main hooks#140
panos-xyz merged 5 commits into
mainfrom
codex/reth-main-upgrade

Conversation

@panos-xyz

@panos-xyz panos-xyz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Upgrade reth from v2.2.0 to the official v2.4.0 release, align the workspace with the corresponding revm 41 / Alloy 2.1 APIs and Rust 1.95, and remove morph-engine-tree-ext, replacing the vendored payload validator with official StateRootStrategy + a thin MorphTreeEngineValidator wrapper.
  • Preserve Morph consensus rules: pre-Jade trusts header ZK-trie root while still persisting real MPT trie updates; post-Jade uses upstream strict state-root validation; parent-aware L1 message index and withdraw-trie-root CL cross-checks remain.
  • Clear withdraw-trie-root expectations on the L1-index early-reject path in validate_payload so failed payloads do not leave stale cache entries until LRU eviction.

Test plan

  • cargo check --workspace
  • cargo clippy --all --all-targets -- -D warnings
  • cargo nextest run --workspace (598 passed)
  • cargo nextest run -p morph-node --features test-utils -E 'binary(it)' (97 passed)
  • cargo test -p morph-node --lib validator::tests
  • Full Hoodi / mainnet historical replay against morph-geth canonical chain before merge

Summary by CodeRabbit

  • New Features

    • Added Ethereum RPC support for pool transaction submission and required subscription wiring.
    • Improved Ethereum simulate flow by explicitly exposing state-root decision behavior.
    • Updated payload building to follow the latest state-root and gas-limit attribute workflow.
  • Bug Fixes

    • Strengthened Morph block validation around Jade-era state-root and withdraw-trie-root handling, including safer expectation clearing.
    • Improved EVM instruction failure propagation and stack-underflow handling.
    • Preserved correct EIP-7702 Morph refund behavior.
  • Compatibility

    • Bumped Rust toolchain and refreshed core dependency versions.
  • Tests / Chores

    • Updated and adjusted hardfork/validation tests and related assertions.

panos-xyz and others added 2 commits July 13, 2026 21:48
validate_payload already registered an expectation in convert_payload_to_block;
clearing it on the early L1-index reject path matches the inner-failure cleanup
and avoids a stale cache entry until LRU eviction.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 480ee030-386e-4e69-9081-f5eab7bc9b00

📥 Commits

Reviewing files that changed from the base of the PR and between f1c0cac and 8e3b97e.

📒 Files selected for processing (1)
  • crates/node/src/validator.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/node/src/validator.rs

📝 Walkthrough

Walkthrough

The workspace now targets Rust 1.95 and newer Reth, Revm, Alloy, and Vergen APIs. Morph engine-tree validation is implemented locally, payload building uses state-root handles, custom EVM instructions return execution results, and RPC and consensus integrations are updated.

Changes

Workspace and API upgrade

Layer / File(s) Summary
Workspace and dependency migration
Cargo.toml, crates/*/Cargo.toml, crates/node/build.rs, deny.toml
Rust, workspace membership, Reth/Revm/Alloy/Vergen versions, build metadata APIs, dependencies, and advisory ignores are updated.
Consensus and factory compatibility
crates/consensus/src/validation.rs, crates/evm/src/evm.rs
Consensus post-execution validation and EVM database error bounds match newer interfaces.

Tree validator migration

Layer / File(s) Summary
Morph tree validator integration
crates/node/src/validator.rs
Morph validation wraps BasicEngineValidator, enforces strict state-root validation, checks Jade-gated L1 queue state, and validates withdraw-trie-root expectations.
Validator coverage
crates/node/src/validator.rs, crates/node/tests/it/engine.rs
Unit and integration coverage verifies sorted-state withdraw-root handling, expectation clearing, timestamp validation, and block-input imports without a registered withdraw-root expectation.

Payload and state-root flow

Layer / File(s) Summary
Payload builder state-root wiring
crates/payload/builder/src/builder.rs, crates/engine-api/src/builder.rs
Payload construction uses state_root_handle, updates state-hook management, and emits revised executed-block data.
Payload contracts and builders
crates/payload/types/*, crates/node/src/node.rs, crates/node/src/test_utils.rs, crates/node/tests/it/helpers.rs
Payload attributes initialize target_gas_limit, payload conversion accepts the new argument, and test builders use the updated request shape.

EVM execution compatibility

Layer / File(s) Summary
Block execution and environment updates
crates/evm/src/block/mod.rs, crates/evm/src/context.rs
Executor-owned state hooks are removed and block overrides are forwarded into pending environment construction.
Revm instruction and precompile APIs
crates/revm/src/evm.rs, crates/revm/src/precompiles.rs
Morph opcode handlers return InstructionExecResult, and precompile warm addresses use AddressSet.

RPC and hardfork updates

Layer / File(s) Summary
Hardfork semantics tests
crates/chainspec/src/hardfork.rs
EIP-7702 gas and refund assertions use updated GasParams accessors.
Ethereum RPC traits
crates/rpc/src/eth/*
Morph RPC support adds state-root simulation delegation, pool transaction forwarding, and subscription trait implementation.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested labels: enhancement

Suggested reviewers: anylots

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving engine validation onto Reth main hooks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/reth-main-upgrade

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Format the L1-index cleanup path, upgrade crossbeam-epoch to 0.9.20, and
drop advisory ignores that no longer match after the reth main bump.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/node/src/validator.rs`:
- Around line 225-240: Update the L1 message validation loop in the block
validation method around expected so the first transaction’s queue_index must
equal parent.next_l1_msg_index before advancing expected; continue requiring
each subsequent queue index to match the current expected value, then increment
it with checked_add. Add a regression case covering a first queue index that
differs from the parent index and ensure validation rejects it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 498f00c1-6109-49e9-859e-f7eb7c9ddf3c

📥 Commits

Reviewing files that changed from the base of the PR and between 43c3e59 and 204c2ce.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (28)
  • Cargo.toml
  • crates/chainspec/src/hardfork.rs
  • crates/consensus/src/validation.rs
  • crates/engine-api/src/builder.rs
  • crates/engine-tree-ext/Cargo.toml
  • crates/engine-tree-ext/src/gate.rs
  • crates/engine-tree-ext/src/lib.rs
  • crates/engine-tree-ext/src/payload_validator.rs
  • crates/engine-tree-ext/src/trie_updates.rs
  • crates/engine-tree-ext/tests/jade_boundary.rs
  • crates/evm/Cargo.toml
  • crates/evm/src/block/mod.rs
  • crates/evm/src/context.rs
  • crates/evm/src/evm.rs
  • crates/node/Cargo.toml
  • crates/node/build.rs
  • crates/node/src/node.rs
  • crates/node/src/test_utils.rs
  • crates/node/src/validator.rs
  • crates/node/tests/it/engine.rs
  • crates/node/tests/it/helpers.rs
  • crates/payload/builder/src/builder.rs
  • crates/payload/types/src/attributes.rs
  • crates/payload/types/src/lib.rs
  • crates/revm/src/evm.rs
  • crates/revm/src/precompiles.rs
  • crates/rpc/src/eth/call.rs
  • crates/rpc/src/eth/mod.rs
💤 Files with no reviewable changes (6)
  • crates/engine-tree-ext/src/lib.rs
  • crates/engine-tree-ext/src/gate.rs
  • crates/engine-tree-ext/tests/jade_boundary.rs
  • crates/engine-tree-ext/Cargo.toml
  • crates/engine-tree-ext/src/trie_updates.rs
  • crates/engine-tree-ext/src/payload_validator.rs

Comment thread crates/node/src/validator.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Cargo.toml (1)

102-161: 🩺 Stability & Availability | 🔵 Trivial

Ensure thorough historical replay testing for the revm/reth upgrade.

The upgraded reth v2.4.0 and underlying revm dependencies introduce changes to core execution APIs. As highlighted by the provided context snippets, downstream paths—such as ExecutionResult log handling in receipt building, EvmFactory type bounds, and RPC gas allowance error mapping—rely closely on the specific semantics of these upgraded traits.

As per the PR objectives, full mainnet historical replay remains pending. It is critical to complete this replay to verify that the new dependency APIs do not introduce subtle consensus or runtime deviations before release.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Cargo.toml` around lines 102 - 161, Complete full mainnet historical replay
testing for the reth v2.4.0/revm upgrade, covering receipt construction through
ExecutionResult log handling, EvmFactory trait-bound paths, and RPC
gas-allowance error mapping. Validate replayed results against expected
consensus behavior and resolve any compatibility or runtime deviations before
release.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Cargo.toml`:
- Around line 102-161: Complete full mainnet historical replay testing for the
reth v2.4.0/revm upgrade, covering receipt construction through ExecutionResult
log handling, EvmFactory trait-bound paths, and RPC gas-allowance error mapping.
Validate replayed results against expected consensus behavior and resolve any
compatibility or runtime deviations before release.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b6edf28-c7f4-42a4-8a6f-8f2033f2191a

📥 Commits

Reviewing files that changed from the base of the PR and between 204c2ce and f1c0cac.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Cargo.toml
  • crates/node/src/validator.rs
  • deny.toml
💤 Files with no reviewable changes (1)
  • deny.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/node/src/validator.rs

@panos-xyz
panos-xyz requested review from anylots and chengwenxi July 16, 2026 03:39
@anylots

anylots commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This change upgrades reth from v2.2.0 to the official v2.4.0 release, aligns the workspace with revm 41 / Alloy 2.1 / Rust 1.95 APIs, and removes the entire morph-engine-tree-ext crate (~2700 lines of vendored code), replacing it with the upstream StateRootStrategy trait + BasicEngineValidator. Only a thin MorphTreeEngineValidator wrapper and MorphStateRootStrategy implementation remain to preserve Morph consensus rules. The architecture shifts from "fork-and-vendored" to "hook-and-extend" — elegant design, massive deletion (-4022/+1523), consensus semantics fully preserved. cargo check passes.

1. Issues Found

Severity File:Line Issue
WARNING crates/node/src/validator.rs:451 PreJadeStateRootJob::finish computes MPT trie updates but discards _mpt_root without any observable diagnostic signal
WARNING deny.toml Removed 5 RUSTSEC ignore entries (RUSTSEC-2026-0097/0098/0099/0118/0119); need to confirm v2.4.0 dependency tree has actually fixed these vulnerabilities
SUGGESTION crates/node/src/validator.rs:182 MorphTreeEngineValidator::new marked as const fn but contains Arc<MorphChainSpec> and BasicEngineValidator<...> which are not const-constructible on stable Rust 1.95

Detailed Findings

1. PreJadeStateRootJob discards MPT root without diagnostic signal

  • File: crates/node/src/validator.rs
  • Confidence: 88%
  • Problem: PreJadeStateRootJob::finish() calls provider.state_root_with_updates(hashed_state.get().as_ref().clone())? yielding (mpt_root, trie_updates), but discards _mpt_root and returns block.header().state_root() as the outcome's state root. The design intent is correct — pre-Jade headers store ZK-trie roots that cannot be compared to MPT roots. However, completely discarding the computed value means: if the MPT computation itself is erroneous (DB corruption, code bug), the pre-Jade path produces no observable signal. The error is silently swallowed until the first post-Jade strict check exposes it, making root-cause analysis difficult at that point.
  • Suggestion: Add a tracing::debug! log recording the _mpt_root vs header root difference. This doesn't affect production behavior but provides a diagnostic signal for root-cause analysis:
    let (mpt_root, trie_updates) = provider.state_root_with_updates(...)?;
    tracing::debug!(
        target: "morph::state_root",
        header_root = %block.header().state_root(),
        mpt_root = %mpt_root,
        "pre-Jade: trusting header ZK-trie root; MPT root computed for persistence only"
    );
    Ok(StateRootJobOutcome::new(block.header().state_root(), Arc::new(trie_updates)))

2. deny.toml RUSTSEC ignore removal needs verification

  • File: deny.toml
  • Confidence: 85%
  • Problem: Removed ignore entries for RUSTSEC-2026-0097 (rand unsound), RUSTSEC-2026-0098/0099 (rustls-webpki URI/wildcard name constraints), RUSTSEC-2026-0118/0119 (hickory-proto NSEC3/O(n²)). If v2.4.0's dependency tree has upgraded the affected crates (rand → 0.9.x, rustls-webpki → 0.102.x, hickory → 0.26.x), the removal is correct. However, if these were removed simply because reth v2.4.0's Cargo.lock no longer directly pulls these versions while morph's own indirect dependencies might still introduce them, cargo deny check will re-report these advisories.
  • Suggestion: Run cargo deny check advisories to confirm no new advisories are reported. If any appear, re-add the appropriate ignore entries with updated justification comments.

3. const fn annotation may be ineffective

  • File: crates/node/src/validator.rs
  • Confidence: 78%
  • Problem: MorphTreeEngineValidator::new is marked as const fn, but its fields include Arc<MorphChainSpec> and BasicEngineValidator<...>. Arc::new is not const on stable Rust, so this const fn annotation cannot actually be invoked in a const context — it's merely an "optimistic annotation."
  • Suggestion: If const-context invocation is not needed, remove the const fn annotation to avoid misleading readers. If the intent is to enable it once Rust supports const Arc::new, keep it but add a comment explaining the current limitation.

2. Incomplete Fix:

Finding — the early-reject cleanup fix is incomplete (Low severity, but contradicts stated goal)

Commit 204c2ce's stated third goal is "clear withdraw-trie-root expectations on the early-reject path so failed payloads do not leave stale cache entries." This was applied to validate_payload but not to validate_block:

// validator.rs:332 — validate_block
fn validate_block(&mut self, block, ctx) -> ... {
self.validate_next_l1_msg_index_for_block(&block, &ctx)?; // early return, NO clear
let output = self.inner.validate_block(block, ctx)?; // early return, NO clear
self.validate_withdraw_trie_root(output)
}

This path is reachable with a registered expectation. When a newPayload arrives with a missing parent, convert_payload_to_block registers the expectation (mod.rs:2035, buffered branch), the block is buffered, and on reattach it is consumed via validate_block (not validate_payload). If validate_block then fails — either the L1-index check (now that the parent is present) or upstream state-root validation — the expectation leaks until LRU eviction.

  • Impact: memory hygiene only. The LRU is bounded at 4096, and entries are keyed by and correct for the exact block hash, so a leaked entry causes no correctness issue even on re-import. This is the same low severity as the bug 204c2ce fixed — which is exactly why the fix should be symmetric.
  • Suggested fix: mirror the validate_payload cleanup in validate_block

@panos-xyz

Copy link
Copy Markdown
Contributor Author

@anylots Thanks for the detailed review. I checked each item against the current v2.4.0 dependency tree and Engine Tree flow:

  1. Pre-Jade MPT root diagnostic — no code change. Discarding the computed MPT root is intentional because the pre-Jade header contains a ZK-trie root, so the two values are not comparable. state_root_with_updates errors are already propagated by ?; logging two deliberately different hashes for every replayed block would add noise without detecting corruption.

  2. RustSec ignores — verified, no code change. The resolved versions are rand 0.8.6/0.9.4/0.10.2, rustls-webpki 0.103.13, and hickory-proto 0.26.1, all outside the affected ranges for RUSTSEC-2026-0097/0098/0099/0118/0119. The latest cargo-deny job also completed with advisories ok: https://github.com/morph-l2/morph-reth/actions/runs/29468642118/job/87527122470

  3. const fn new — no code change. The constructor only moves an already-created Arc; it does not call Arc::new in const evaluation. This is valid on the pinned Rust toolchain and is consistent with the existing MorphNode::new(Arc<MorphChainSpec>) constructor.

  4. validate_block cleanup — fixed in 8e3b97e. The buffered reattach path now clears the registered withdraw-root expectation when either the parent-aware L1-index precheck or upstream inner.validate_block fails. Payload and block validation now share clear_withdraw_trie_root_expectation_on_error, with regression coverage for error cleanup and success preservation.

Validation after the fix:

  • cargo test -p morph-node --lib validator::tests: 18 passed
  • cargo clippy -p morph-node --lib -- -D warnings: passed
  • cargo nextest run --workspace: 600 passed

Regarding the separate CodeRabbit historical-replay note: agreed; full Hoodi/mainnet replay remains a pre-merge validation item in the PR test plan rather than a source change.

@panos-xyz
panos-xyz merged commit a4eea59 into main Jul 16, 2026
14 checks passed
@panos-xyz
panos-xyz deleted the codex/reth-main-upgrade branch July 16, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants