feat: add morph engine api #12
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as RPC Client
participant RPC as MorphL2EngineRpcHandler
participant API as MorphL2EngineApi Impl
participant Exec as Execution Layer
Client->>RPC: assembleL2Block(params)
RPC->>API: assemble_l2_block(params)
API->>Exec: build/assemble L2 block (internal)
Exec-->>API: ExecutableL2Data
API-->>RPC: EngineApiResult<ExecutableL2Data>
RPC->>RPC: into_rpc_result()
RPC-->>Client: RpcResult<ExecutableL2Data>
Client->>RPC: validateL2Block(data)
RPC->>API: validate_l2_block(data)
API->>Exec: validate block
Exec-->>API: GenericResponse
API-->>RPC: EngineApiResult<GenericResponse>
RPC-->>Client: RpcResult<GenericResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
🤖 Fix all issues with AI agents
In `@crates/engine-api/src/validator.rs`:
- Line 10: Remove the unused import MorphHardforks from the top-level use
statement; keep MorphChainSpec only, since is_mpt_fork_active_at_timestamp
operates on MorphChainSpec without requiring the MorphHardforks trait in
scope—update the use line to import only MorphChainSpec and run cargo check to
confirm no further references to MorphHardforks remain.
🧹 Nitpick comments (2)
crates/engine-api/src/lib.rs (1)
7-7: Documentation inconsistency: "Emerald hardfork" vs "MPT fork"The documentation references "Emerald hardfork support" but
validator.rsand other code consistently refer to "MPT fork". Consider aligning the terminology for clarity.📝 Suggested fix
-//! - [`MorphValidationContext`]: Validation context with Emerald hardfork support +//! - [`MorphValidationContext`]: Validation context with MPT fork supportcrates/engine-api/src/error.rs (1)
100-115: Consider expanding test coverage for remaining error variants.Currently only
DiscontinuousBlockNumberandInvalidTransactionare tested. Other variants likeWrongParentHash,WithdrawTrieRootMismatch, andDatabasealso have specific error codes that could benefit from verification.
| //! because state root verification happens in the ZK proof instead (using ZK-trie). | ||
| //! After MPT fork, state root validation is performed in the node (using MPT). | ||
|
|
||
| use morph_chainspec::{MorphChainSpec, MorphHardforks}; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat crates/engine-api/src/validator.rsRepository: morph-l2/morph-reth
Length of output: 4102
🏁 Script executed:
rg "MorphHardforks" crates/engine-api/src/validator.rsRepository: morph-l2/morph-reth
Length of output: 117
🏁 Script executed:
rg "MorphHardforks" crates/engine-api/src/ -g "*.rs"Repository: morph-l2/morph-reth
Length of output: 152
Remove unused import: MorphHardforks
The MorphHardforks trait is imported but never used in this file. The is_mpt_fork_active_at_timestamp method works on MorphChainSpec without requiring the trait to be in scope. Remove it from line 10:
-use morph_chainspec::{MorphChainSpec, MorphHardforks};
+use morph_chainspec::MorphChainSpec;📝 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.
| use morph_chainspec::{MorphChainSpec, MorphHardforks}; | |
| use morph_chainspec::MorphChainSpec; |
🤖 Prompt for AI Agents
In `@crates/engine-api/src/validator.rs` at line 10, Remove the unused import
MorphHardforks from the top-level use statement; keep MorphChainSpec only, since
is_mpt_fork_active_at_timestamp operates on MorphChainSpec without requiring the
MorphHardforks trait in scope—update the use line to import only MorphChainSpec
and run cargo check to confirm no further references to MorphHardforks remain.
There was a problem hiding this comment.
The is_mpt_fork_active_at_timestamp function is MorphHardforks's trait function, so we need this trait import
There was a problem hiding this comment.
@panos-xyz, you're absolutely right! I apologize for the incorrect suggestion. In Rust, trait methods require the trait to be in scope for them to be callable on implementing types. Even though MorphHardforks isn't directly referenced in the code, it must be imported so that is_mpt_fork_active_at_timestamp() can be called on MorphChainSpec instances. Thank you for the clarification!
🐰📚
✏️ Learnings added
Learnt from: panos-xyz
Repo: morph-l2/morph-reth PR: 12
File: crates/engine-api/src/validator.rs:10-10
Timestamp: 2026-01-23T10:00:04.805Z
Learning: In Rust code within the morph-reth repository, trait imports are necessary even when not directly referenced if trait methods are being called on implementing types. For example, `MorphHardforks` must be imported to call `is_mpt_fork_active_at_timestamp()` on `MorphChainSpec` instances.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Add MorphL2EngineApi trait for block building and validation - Add MorphL2EngineRpcServer for JSON-RPC implementation - Add MorphValidationContext for state root validation - Refactor hardfork code into separate modules (block/curie.rs, block/receipt.rs) - Fix state root validation logic for MPT fork
panos-xyz
force-pushed
the
feat/engine-api
branch
from
1c606ca to
caf7715
Compare
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.