fix(engine-api): add engine_appendBatchSignature RPC for sync nodes#93
Merged
chengwenxi merged 2 commits intomainfrom Apr 13, 2026
Merged
fix(engine-api): add engine_appendBatchSignature RPC for sync nodes#93chengwenxi merged 2 commits intomainfrom
chengwenxi merged 2 commits intomainfrom
Conversation
…nodes The consensus layer (morphnode) calls engine_appendBatchSignature on the execution layer when it receives precommit votes that carry a BatchHash. This happens after blocksync completes and the node switches to consensus mode to follow the live chain. Without this method, morph-reth returns "Method not found", which the retryClient treats as retryable (retryableError returns true for everything except DiscontinuousBlockError). The exponential backoff runs for up to 30 minutes per vote, blocking the tendermint consensus goroutine and preventing any new blocks from being imported. For non-sequencer sync nodes, storing BLS batch signatures is unnecessary (only sequencers use them for L1 batch submission via CommitBatch), so a no-op implementation is sufficient. Changes: - Add BatchSignature type to morph-payload-types - Add append_batch_signature default no-op to MorphL2EngineApi trait - Register engine_appendBatchSignature JSON-RPC endpoint in rpc.rs
📝 WalkthroughWalkthroughThis PR extends the Engine API and RPC interface with a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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📝 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 |
![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)
crates/payload/types/src/params.rs (1)
73-87: Add serde round-trip coverage forBatchSignature.This new RPC-facing type should have explicit JSON shape tests to prevent accidental wire-format regressions.
Proposed test additions
#[cfg(test)] mod tests { use super::*; + use alloy_primitives::Address; + #[test] + fn test_batch_signature_serde_roundtrip() { + let sig = BatchSignature { + signer: Address::from([0x11; 20]), + signer_pub_key: Bytes::from(vec![0x01, 0x02, 0x03]), + signature: Bytes::from(vec![0xAA, 0xBB, 0xCC]), + }; + + let json = serde_json::to_string(&sig).expect("serialize"); + let decoded: BatchSignature = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(sig, decoded); + } + + #[test] + fn test_batch_signature_camel_case_fields() { + let json = r#"{ + "signer": "0x1111111111111111111111111111111111111111", + "signerPubKey": "0x010203", + "signature": "0xaabbcc" + }"#; + let parsed: BatchSignature = serde_json::from_str(json).expect("deserialize"); + assert_eq!(parsed.signer, Address::from([0x11; 20])); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/payload/types/src/params.rs` around lines 73 - 87, Add serde round-trip unit tests for the BatchSignature type: create tests that construct a BatchSignature (populating signer, signer_pub_key, and signature with deterministic sample values), serialize it to JSON using serde_json::to_string, assert the JSON shape/fields are as expected, then deserialize back with serde_json::from_str and assert equality with the original via PartialEq. Reference the BatchSignature struct and its fields (signer, signer_pub_key, signature) and add tests alongside other payload types tests so any future wire-format changes are caught.
🤖 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/engine-api/src/rpc.rs`:
- Around line 191-195: The debug message for appendBatchSignature is misleading
because it claims a "no-op for sync node" while the method then calls
self.inner.append_batch_signature(...); update the tracing::debug call (the log
string tied to %batch_hash) to not assert it's a no-op — instead log that the
RPC was received and is being forwarded to self.inner.append_batch_signature (or
that behavior may vary by implementation), so the message accurately reflects
the actual action performed.
---
Nitpick comments:
In `@crates/payload/types/src/params.rs`:
- Around line 73-87: Add serde round-trip unit tests for the BatchSignature
type: create tests that construct a BatchSignature (populating signer,
signer_pub_key, and signature with deterministic sample values), serialize it to
JSON using serde_json::to_string, assert the JSON shape/fields are as expected,
then deserialize back with serde_json::from_str and assert equality with the
original via PartialEq. Reference the BatchSignature struct and its fields
(signer, signer_pub_key, signature) and add tests alongside other payload types
tests so any future wire-format changes are caught.
🪄 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: 52334c55-abda-4903-bfc5-ed6cfa148a59
📒 Files selected for processing (5)
README.mdcrates/engine-api/src/api.rscrates/engine-api/src/rpc.rscrates/payload/types/src/lib.rscrates/payload/types/src/params.rs
Comment on lines
+191
to
+195
| tracing::debug!( | ||
| target: "morph::engine", | ||
| %batch_hash, | ||
| "RPC appendBatchSignature called (no-op for sync node)" | ||
| ); |
There was a problem hiding this comment.
Log message is misleading for non-no-op implementations.
Line 194 says this RPC call is a no-op, but Line 197 forwards to self.inner.append_batch_signature(...), which may do real work in overridden implementations.
Suggested logging tweak
tracing::debug!(
target: "morph::engine",
%batch_hash,
- "RPC appendBatchSignature called (no-op for sync node)"
+ "RPC appendBatchSignature called"
);📝 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
| tracing::debug!( | |
| target: "morph::engine", | |
| %batch_hash, | |
| "RPC appendBatchSignature called (no-op for sync node)" | |
| ); | |
| tracing::debug!( | |
| target: "morph::engine", | |
| %batch_hash, | |
| "RPC appendBatchSignature called" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/engine-api/src/rpc.rs` around lines 191 - 195, The debug message for
appendBatchSignature is misleading because it claims a "no-op for sync node"
while the method then calls self.inner.append_batch_signature(...); update the
tracing::debug call (the log string tied to %batch_hash) to not assert it's a
no-op — instead log that the RPC was received and is being forwarded to
self.inner.append_batch_signature (or that behavior may vary by implementation),
so the message accurately reflects the actual action performed.
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
2 participants
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
engine_appendBatchSignatureno-op stub to the Morph L2 Engine APIProblem
After a sync node completes fast sync (blocksync), Tendermint switches it to consensus mode to follow the live chain. In consensus mode, it receives precommit votes from validators. When a vote carries a
BatchHash, the consensus state machine callsAppendBlsData→engine_appendBatchSignatureon the execution layer.morph-reth doesn't implement this method, returning "Method not found". The morphnode's
retryableError()treats this as retryable, triggering exponential backoff retries for up to 30 minutes per vote. This blocks the consensus goroutine entirely, preventingnewL2Blockfrom being called and halting block import.Root Cause Trace
Fix
For non-sequencer sync nodes, BLS batch signatures are not needed (only sequencers aggregate them for L1 batch submission via
CommitBatch). A no-op implementation that returnsOk(())immediately unblocks the consensus state machine.Changes
crates/payload/types/src/params.rsBatchSignaturetypecrates/payload/types/src/lib.rsBatchSignaturecrates/engine-api/src/api.rsappend_batch_signaturedefault no-op to traitcrates/engine-api/src/rpc.rsengine_appendBatchSignatureJSON-RPC endpointREADME.mdTest plan
cargo test -p morph-engine-api -p morph-payload-types— 60 tests passcargo clippy -p morph-engine-api -- -D warnings— cleanSummary by CodeRabbit
New Features
engine_appendBatchSignatureEngine API method that accepts BLS batch signatures and batch hashes from the consensus layer. Compatible with both sequencer and sync node configurations.Documentation
appendBatchSignaturemethod and its behavior specifications.