feat(rollup-relayer): support codecv8 #1681
Conversation
WalkthroughThis change updates dependency versions for several modules, increments the main version tag, adds support for codec version 8 in batch processing logic, and introduces a new configuration field in the proposer tool genesis JSON. Method signatures and logic in the relayer are adjusted to handle codec version 8 alongside version 7. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layer2Relayer
participant DB
User->>Layer2Relayer: ProcessPendingBatches()
Layer2Relayer->>Layer2Relayer: Determine codec version (V7 or V8)
alt Codec V7 or V8
Layer2Relayer->>Layer2Relayer: constructCommitBatchPayloadCodecV7()
Layer2Relayer->>Layer2Relayer: contextIDFromBatches(codecVersion, batches)
else Other versions
Layer2Relayer->>Layer2Relayer: Handle accordingly
end
Layer2Relayer->>DB: SendTransaction(contextID)
DB-->>Layer2Relayer: Transaction result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
469-475: Method name & log message now misleading for V8 supportThe switch correctly re-uses
constructCommitBatchPayloadCodecV7for both V7 and V8, but
- the helper’s name still ends in “CodecV7”;
- the error log hard-codes “…payload for V7”.
This will confuse future maintainers when troubleshooting V8 flows. Rename the helper (e.g.
constructCommitBatchPayloadCodecV7V8) or introduce a thin wrapper for V8, and update the log message to reflect the actual codecVersion.
🧹 Nitpick comments (1)
rollup/proposer-tool-config.json (1)
4-8: Validate increased chunk proposer limits
Raisingmax_block_num_per_chunkto1000andmax_l2_gas_per_chunkto200000000000can stress memory, database, and network layers. Confirm that the infrastructure and upstream components can handle these new limits without performance regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
bridge-history-api/go.sumis excluded by!**/*.sumcoordinator/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumrollup/go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
bridge-history-api/go.mod(1 hunks)common/version/version.go(1 hunks)coordinator/go.mod(1 hunks)coordinator/internal/logic/provertask/batch_prover_task.go(1 hunks)coordinator/internal/types/prover.go(1 hunks)rollup/docker-compose-proposer-tool.yml(1 hunks)rollup/go.mod(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(4 hunks)rollup/internal/controller/watcher/proposer_tool.go(1 hunks)rollup/proposer-tool-config.json(1 hunks)rollup/proposer-tool-genesis.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
rollup/internal/orm/batch.go (2)
Batch(22-72)Batch(80-82)
🔇 Additional comments (10)
common/version/version.go (1)
8-8: Version tag bumped correctly.The patch version was incremented to
v4.5.22in line with the updatedda-codecandgo-ethereumdependencies across modules.rollup/internal/controller/watcher/proposer_tool.go (1)
1-162: No logic changes detected.This PR only reorders the import of
scroll-tech/database/migrate; functionality remains unchanged.rollup/go.mod (1)
14-15: Ensure consistency of dependency versions across modules.Please verify that the updated
da-codecandgo-ethereumcommit hashes here match exactly incoordinator/go.modandbridge-history-api/go.modto avoid build mismatches.Run:
#!/bin/bash rg "github.com/scroll-tech/da-codec v0.1.3-0.20250616065246-17bf3e4990cd" -l rg "github.com/scroll-tech/go-ethereum v1.10.14-0.20250611141528-cf3d22ef8707" -lcoordinator/go.mod (1)
12-13: Align dependency versions with other modules.The
da-codecandgo-ethereumversions were bumped here; confirm they are identical to those inrollup/go.modandbridge-history-api/go.modto maintain cross-module compatibility.bridge-history-api/go.mod (2)
13-14: Verify updated require directives.The
da-codecandgo-ethereumversions have been upgraded; ensure these commits align with those in therollupandcoordinatormodules.
21-21: Confirm hotfix coverage in replace directive.The replace directive for
go-ethereumpoints to commit20250616074821-cba908db071dwith a note about a header hash hotfix. Please verify that this commit indeed contains the intended fix.coordinator/internal/types/prover.go (1)
3-7: No functional changes detected
The blank line added between the"fmt"and"scroll-tech/common/types/message"imports is purely stylistic and does not affect behavior.rollup/docker-compose-proposer-tool.yml (1)
30-32: Verify updated--start-l2-blockvalue
Ensure that the new block number"14907015"correctly aligns with the intended activation point for CODEC V8 support and that no downstream tooling assumes the old start height.rollup/proposer-tool-genesis.json (1)
8-11: EnsurefeynmanTimefield is utilized
The new"feynmanTime": 0field has been added to the genesis config. Verify that the proposer‐tool code actually reads and applies this parameter, otherwise it may be silently ignored.coordinator/internal/logic/provertask/batch_prover_task.go (1)
267-272: Extend batch codec support to V8
Incorporatingencoding.CodecV8alongside existing versions is correct. Please ensure you have added or updated unit/integration tests to cover the new V8 decoding path and validate backward compatibility.
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
30153c1 to
377e8bb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1681 +/- ##
===========================================
- Coverage 40.16% 40.07% -0.10%
===========================================
Files 232 232
Lines 18447 18440 -7
===========================================
- Hits 7409 7389 -20
- Misses 10315 10331 +16
+ Partials 723 720 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| github.com/stretchr/testify v1.9.0 | ||
| github.com/urfave/cli/v2 v2.25.7 | ||
| golang.org/x/sync v0.11.0 | ||
| gorm.io/gorm v1.25.7-0.20240204074919-46816ad31dde | ||
| ) | ||
|
|
||
| replace github.com/scroll-tech/go-ethereum => github.com/scroll-tech/go-ethereum v1.10.14-0.20250305084331-57148478e950 // It's a hotfix for the header hash incompatibility issue, pls change this with caution |
There was a problem hiding this comment.
is this version update intended?
There was a problem hiding this comment.
Yes. It's based on this pr: scroll-tech/go-ethereum#1133 (the latest commit is merging develop branch) because it's nice to include recent fixes like scroll-tech/go-ethereum#1191 and scroll-tech/go-ethereum#1195, especially scroll-tech/go-ethereum#1191.
rollup/proposer-tool-config.json
Outdated
| @@ -2,8 +2,8 @@ | |||
| "l2_config": { | |||
| "endpoint": "https://rpc.scroll.io", | |||
| "chunk_proposer_config": { | |||
| "max_block_num_per_chunk": 100, | |||
| "max_l2_gas_per_chunk": 20000000, | |||
| "max_block_num_per_chunk": 1000, | |||
There was a problem hiding this comment.
Why these updates to the config?
There was a problem hiding this comment.
This is changed to test the previous experimental dynamic encoding of codecv8, to remove other limits, so setting large values in them.
There was a problem hiding this comment.
Reverted in commit d274123.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
468-476: Minor log message mismatch after Codec V8 support
The error log inside the new switch block is hard-coded to “for V7”, which is misleading whencodecVersionis V8.- log.Error("failed to construct constructCommitBatchPayloadCodecV7 payload for V7", "codecVersion", codecVersion, ... + log.Error("failed to construct commitBatch payload", "codecVersion", codecVersion, ...Purely cosmetic, but worth adjusting to avoid confusion when debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rollup/internal/controller/relayer/l2_relayer.go(4 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
rollup/internal/controller/relayer/l2_relayer.go
525-525: undefined: dbBatchWithChunksAndParent
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: test
- GitHub Check: check
- GitHub Check: tests
🔇 Additional comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
534-537: Future-proof parsing LGTM
The generalizedbatchHashesFromContextIDcorrectly handles anyv<N>-…prefix and gracefully falls back. Nice improvement over the old fixed-version check.
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
e5a75c7 to
d274123
Compare
Purpose or design rationale of this PR
Enable rollup-relayer to support codecv8 and update bridge-history's da-codec commit for batch hash validation compatibility.
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Improvements
Chores