refactor(deps): use [patch] to separate upstream reth from fork overlay#49
Closed
chengwenxi wants to merge 1 commit intomainfrom
Closed
refactor(deps): use [patch] to separate upstream reth from fork overlay#49chengwenxi wants to merge 1 commit intomainfrom
chengwenxi wants to merge 1 commit intomainfrom
Conversation
Declare all reth crates in [workspace.dependencies] against the canonical upstream paradigmxyz/reth tag v1.10.0, then redirect the entire source to the morph-l2/reth fork via a single [patch."https://github.com/paradigmxyz/reth"] block. The fork (rev 1dd7227) is based on v1.10.0 and adds only 3 file changes: - reth-engine-primitives: adds StateRootValidator trait - reth-engine-tree: makes state root validation pluggable - reth-stages: downgrades state root error to warning during unwind This makes the dependency intent explicit: workspace.dependencies describes what we depend on (upstream v1.10.0), while [patch] describes the minimal override needed. Upgrading reth now requires updating the tag in [workspace.dependencies] and rebasing the fork branch, rather than editing 52 individual crate URLs. No changes to validator.rs or any morph-reth source code.
📝 WalkthroughWalkthroughReth dependencies across multiple crates are redirected from morph-l2/reth to paradigmxyz/reth (v1.10.0), with a unified patch block routing all paradigmxyz/reth crates back to the morph-l2/reth git source. Default-features adjustments applied to several crates. Lines: +115/-52. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Cargo.toml (1)
57-112: Add a lightweight consistency check for the tworeth-*lists.The manifest is correct now, but the crate set is duplicated in
[workspace.dependencies]and[patch."https://github.com/paradigmxyz/reth"]. A tiny guard in CI would catch the next missed entry before the workspace accidentally mixes upstream and forked sources.Expected result:
missing patch entries: []. Anextra patch-only entriesvalue likereth-stagesis fine if intentional.#!/bin/bash set -euo pipefail python - <<'PY' from pathlib import Path import re text = Path("Cargo.toml").read_text() section = None workspace_reth = set() patch_reth = set() for line in text.splitlines(): header = re.match(r'\[(.+)\]\s*$', line.strip()) if header: section = header.group(1) continue dep = re.match(r'\s*(reth-[A-Za-z0-9-]+)\s*=', line) if not dep: continue name = dep.group(1) if section == "workspace.dependencies": workspace_reth.add(name) elif section == 'patch."https://github.com/paradigmxyz/reth"': patch_reth.add(name) missing = sorted(workspace_reth - patch_reth) extra = sorted(patch_reth - workspace_reth) print("workspace.dependencies reth crates:", len(workspace_reth)) print('patch."https://github.com/paradigmxyz/reth" crates:', len(patch_reth)) print("missing patch entries:", missing) print("extra patch-only entries:", extra) if missing: raise SystemExit(1) PYAlso applies to: 193-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cargo.toml` around lines 57 - 112, The workspace lists all reth-* crates in [workspace.dependencies] but the same set must appear under patch."https://github.com/paradigmxyz/reth"; add a CI gate that parses Cargo.toml and fails when any crate present in workspace.dependencies (matching reth-*) is missing from patch."https://github.com/paradigmxyz/reth" (or vice versa if you want to detect extras). Implement the check as a small script (shell/python) run in CI that extracts reth-* keys from the two sections and exits non‑zero if workspace_reth - patch_reth is non-empty, returning the missing names so the PR author can add the missing patch entries or intentionally document extras like reth-stages. Ensure the job runs early in the pipeline and references the two manifest sections exactly: [workspace.dependencies] and patch."https://github.com/paradigmxyz/reth".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Cargo.toml`:
- Around line 185-188: Update the Cargo.toml comment block that currently only
mentions the StateRootValidator hook to also document the behavioral divergence
in reth-stages: explicitly state that the morph-l2/reth fork modifies
reth-stages to downgrade unwind state-root failures to warnings; update the
descriptive text alongside the existing reference to StateRootValidator so
readers know both the hook addition and the reth-stages behavioral delta.
---
Nitpick comments:
In `@Cargo.toml`:
- Around line 57-112: The workspace lists all reth-* crates in
[workspace.dependencies] but the same set must appear under
patch."https://github.com/paradigmxyz/reth"; add a CI gate that parses
Cargo.toml and fails when any crate present in workspace.dependencies (matching
reth-*) is missing from patch."https://github.com/paradigmxyz/reth" (or vice
versa if you want to detect extras). Implement the check as a small script
(shell/python) run in CI that extracts reth-* keys from the two sections and
exits non‑zero if workspace_reth - patch_reth is non-empty, returning the
missing names so the PR author can add the missing patch entries or
intentionally document extras like reth-stages. Ensure the job runs early in the
pipeline and references the two manifest sections exactly:
[workspace.dependencies] and patch."https://github.com/paradigmxyz/reth".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5dabd3f-4188-4d8d-9fde-3db613b4da8e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
Cargo.toml
Comment on lines
+185
to
+188
| # Redirect all paradigmxyz/reth v1.10.0 crates to the morph-l2/reth fork. | ||
| # The fork is based on upstream v1.10.0 and adds only the StateRootValidator | ||
| # hook (3 files changed) needed for pre-Jade ZK-trie / post-Jade MPT transition. | ||
| # See: https://github.com/morph-l2/reth/tree/panos/v1.10.0-state-root-hook |
There was a problem hiding this comment.
Document the reth-stages divergence here too.
This comment currently reads as if the fork only adds the StateRootValidator hook, but the fork also changes reth-stages to downgrade unwind state-root failures to warnings. Since this block is the upgrade breadcrumb, please call out that behavioral delta explicitly.
📝 Suggested comment update
# Redirect all paradigmxyz/reth v1.10.0 crates to the morph-l2/reth fork.
-# The fork is based on upstream v1.10.0 and adds only the StateRootValidator
-# hook (3 files changed) needed for pre-Jade ZK-trie / post-Jade MPT transition.
+# The fork is based on upstream v1.10.0 and contains only 3 local changes:
+# - reth-engine-primitives: adds the StateRootValidator trait
+# - reth-engine-tree: makes state root validation pluggable
+# - reth-stages: downgrades unwind state-root failures to warnings
+# These changes are needed for the pre-Jade ZK-trie / post-Jade MPT transition.
# See: https://github.com/morph-l2/reth/tree/panos/v1.10.0-state-root-hook📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Redirect all paradigmxyz/reth v1.10.0 crates to the morph-l2/reth fork. | |
| # The fork is based on upstream v1.10.0 and adds only the StateRootValidator | |
| # hook (3 files changed) needed for pre-Jade ZK-trie / post-Jade MPT transition. | |
| # See: https://github.com/morph-l2/reth/tree/panos/v1.10.0-state-root-hook | |
| # Redirect all paradigmxyz/reth v1.10.0 crates to the morph-l2/reth fork. | |
| # The fork is based on upstream v1.10.0 and contains only 3 local changes: | |
| # - reth-engine-primitives: adds the StateRootValidator trait | |
| # - reth-engine-tree: makes state root validation pluggable | |
| # - reth-stages: downgrades unwind state-root failures to warnings | |
| # These changes are needed for the pre-Jade ZK-trie / post-Jade MPT transition. | |
| # See: https://github.com/morph-l2/reth/tree/panos/v1.10.0-state-root-hook |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Cargo.toml` around lines 185 - 188, Update the Cargo.toml comment block that
currently only mentions the StateRootValidator hook to also document the
behavioral divergence in reth-stages: explicitly state that the morph-l2/reth
fork modifies reth-stages to downgrade unwind state-root failures to warnings;
update the descriptive text alongside the existing reference to
StateRootValidator so readers know both the hook addition and the reth-stages
behavioral delta.
Contributor
Author
|
关闭：Cargo 的 [patch] 无法部分替换 monorepo crate，全量 patch 比直接用 fork URL 更冗余，没有实际收益。保持 PR #47 的方案。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
paradigmxyz/rethtag v1.10.0[patch."https://github.com/paradigmxyz/reth"]to redirect the entire source to themorph-l2/rethfork (rev1dd7227)validator.rs, etc.)Motivation
PR #47 switched all 52 reth crate URLs directly to
morph-l2/reth, making it hard to see what is actually modified. The fork only changes 3 files:reth-engine-primitiveslib.rsStateRootValidatortraitreth-engine-treepayload_validator.rsreth-stagesmerkle.rsWith this refactor,
[workspace.dependencies]expresses what we depend on (upstream v1.10.0), while[patch]expresses what we override (the fork).Why [patch] lists all crates, not just the 3 modified ones
The ideal would be to patch only the 3 modified crates. However, Cargo's
[patch]mechanism cannot partially substitute crates from a git monorepo.When
reth-engine-treeis patched from the fork, Cargo must resolve all of its transitive dependencies from the same git source. This pullsreth-payload-primitives,reth-primitives-traits, etc. from the fork. Meanwhile, unpatched crates likereth-ethereum-engine-primitivesstill pull those same crates from upstream — resulting in two incompatible instances ofPayloadTypesand a compile error:Cargo requires that all crates from the same git source resolve to the same revision. The full-list
[patch]block is the minimal correct solution: it ensures every reth crate resolves consistently from the fork, with zero source-code duplication in this repo.Upgrade path
To upgrade reth:
tagin[workspace.dependencies]revin[patch]Test plan
cargo checkpassescargo test --workspacepasses