feat: implement eth_config RPC with morph extension#72
Conversation
…patibility Add a Morph-specific eth_config RPC handler that extends the standard EIP-7910 response with morph extension fields (useZktrie, jadeForkTime). morphnode calls eth_config at startup to determine the trie type and Jade fork timing. Without this implementation, morphnode cannot connect to morph-reth and fails to start.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Morph Changes
Sequence DiagramsequenceDiagram
participant Client as RPC Client
participant Handler as MorphEthConfigHandler
participant Provider as Chain Provider
participant ChainSpec as Chain Spec
participant EVM as EVM (per-fork)
Client->>Handler: eth_config()
Handler->>Provider: fetch latest header / block timestamp(s)
Provider-->>Handler: header(s)
Handler->>ChainSpec: query fork activation timestamps & Jade metadata
ChainSpec-->>Handler: fork activation data
Handler->>EVM: instantiate per-fork EVM context (current/next/last)
EVM-->>Handler: precompile registry & config
Handler->>Handler: assemble MorphForkConfig entries (blob schedule, fork-id, precompiles, extension)
Handler-->>Client: MorphEthConfig response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/rpc/src/eth_config.rs (1)
291-335: Add one fork-resolution test case (not just serialization).Current tests validate JSON shape, but they don’t cover
current/next/lastselection behavior. A focused test for “latest timestamp before first timestamp fork” would lock in the expected semantics and prevent regressions inconfig_impl.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/rpc/src/eth_config.rs`:
- Around line 65-66: The doc comment for the precompiles map is reversed: update
the comment on the precompiles field (type BTreeMap<String, Address>) to state
"name -> address" (since the map key is the precompile name and the value is the
Address) and make the same correction in the other occurrences noted (around
lines 272-285) so documentation matches the builder and type (reference symbols:
precompiles, BTreeMap<String, Address>).
- Around line 200-206: The current selection logic for
current_fork_idx/current_fork_timestamp uses .or_else(||
fork_timestamps.len().checked_sub(1)) which incorrectly falls back to the last
scheduled fork when latest.timestamp() is earlier than the first fork; remove
that fallback so a position of 0 yields None instead of wrapping to the last
index (i.e., drop the .or_else fallback and let .and_then(...).ok_or_else(...)
trigger the error), and make the same change for the analogous selection at the
block handling lines 217–219 (referencing variables/current logic using
fork_timestamps, position(|ts| &latest.timestamp() < ts), and the
current_fork_idx/current_fork_timestamp assignment).
🪄 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: 742aa3e0-7cb6-41e8-a9bd-5bef961ef8b9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
crates/node/src/add_ons.rscrates/rpc/Cargo.tomlcrates/rpc/src/eth_config.rscrates/rpc/src/lib.rs
- Fix precompiles field doc comment: "address -> name" → "name -> address" to match the actual BTreeMap<String, Address> type. - Replace position/checked_sub/or_else chain with rfind/find for current fork selection. The old logic incorrectly fell back to the last fork when latest timestamp was before the first timestamp fork.
chengwenxi
left a comment
There was a problem hiding this comment.
LGTM. EIP-7910 eth_config implementation is well-structured. The fork-selection fix (rfind) in commit 49c6522 is correct. MorphExtension with jadeForkTime/useZktrie is cleanly integrated. Minor suggestions for follow-up:
- Add a unit test for
config_implfork-selection logic (current/next/last partitioning, error path) system_contractsbeing always empty is fine for now but worth documenting if morphnode ever expects L2-specific contracts there
Summary
eth_configRPC method (EIP-7910) with Morph-specific extension fields (useZktrie,jadeForkTime) that morphnode requires at startupMorphEthConfigHandlerinmorph-rpccrate that follows the upstreamEthConfigHandlerlogic but produces a response with themorphextension object on each fork configmerge_configuredin the node add-onsThis is a prerequisite for morphnode to connect to morph-reth. morphnode calls
eth_configat startup to determine:current.morph.useZktrie)morph.jadeForkTimefrom current/next/last)Without this implementation, morphnode fails to start when connected to morph-reth.
Test plan
MorphExtension,MorphForkConfig(field names, hex encoding, omitempty)cargo check -p morph-rpc -p morph-nodepassescargo test -p morph-rpc -p morph-nodepasses (all 17 tests)curl -X POST -d '{"jsonrpc":"2.0","method":"eth_config","params":[],"id":1}' http://localhost:8545returns response withmorphextensionSummary by CodeRabbit
New Features
Chores