-
Notifications
You must be signed in to change notification settings - Fork 70
support derive commitBatchWithProof event #880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds handling for a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@node/derivation/derivation.go`:
- Line 474: The error message in the return inside commitBatchWithProof is
inconsistent: change the string "commitBatchWithProof submitBatches Unpack
error:%v" to a consistent prefix such as "commitBatchWithProof Unpack error:%v"
so the log reflects the correct function context; update the return in
commitBatchWithProof that currently wraps err to use the revised message.
- Around line 471-493: The error string for the ABI unpack failure in the branch
that detects commitBatchWithProof is incorrect; update the fmt.Errorf call
inside the commitBatchWithProof branch (where you call
rollupABI.Methods["commitBatchWithProof"].Inputs.Unpack) to return
"commitBatchWithProof Unpack error:%v" instead of "commitBatchWithProof
submitBatches Unpack error:%v" so the message matches the method name and other
branches.
🧹 Nitpick comments (1)
node/derivation/derivation.go (1)
471-493: Consider extracting the duplicated batch-unpacking logic into a helper.Lines 471–493 are an exact copy of lines 448–470 (the
commitBatchrollupABI branch). All four branches inUnPackDatashare the same pattern: unpack → type-assert → buildRPCRollupBatch. A small helper (e.g.,unpackBatchArgs(method *abi.Method, data []byte) (geth.RPCRollupBatch, error)) parameterized by the ABI method would eliminate this repetition and reduce the risk of future copy-paste drift.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit