feat(rollup-relayer): support Validium #1693
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes introduce a "validium mode" to the rollup system, affecting batch proposal, relayer logic, configuration, and ABI handling. This includes new ABI definitions for Validium, configuration flags, mode-aware logic in relayer and batch proposer components, and updated utility functions for batch encoding and metadata extraction in validium mode. Changes
Sequence Diagram(s)sequenceDiagram
participant Config
participant App
participant BatchProposer
participant Relayer
participant Utils
participant ValidiumABI
Config->>App: Provide ValidiumMode flag
App->>BatchProposer: Initialize with ValidiumMode
BatchProposer->>Utils: CalculateBatchMetrics(..., validiumMode)
Relayer->>Utils: GetBatchMetadata(..., validiumMode)
Relayer->>Relayer: If ValidiumMode, use validium ABI for calldata packing
Relayer->>ValidiumABI: Pack calldata for commitBatch/finalizeBundle (validium mode)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
common/version/version.go(1 hunks)rollup/abi/validium_abi.go(1 hunks)rollup/cmd/rollup_relayer/app/app.go(1 hunks)rollup/conf/config.json(2 hunks)rollup/internal/config/relayer.go(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(9 hunks)rollup/internal/controller/watcher/batch_proposer.go(7 hunks)rollup/internal/orm/batch.go(1 hunks)rollup/internal/utils/utils.go(4 hunks)
🧰 Additional context used
🧠 Learnings (6)
rollup/conf/config.json (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1645
File: rollup/proposer-tool-config.json:34-40
Timestamp: 2025-04-15T08:52:44.176Z
Learning: In configuration files like `rollup/proposer-tool-config.json`, placeholders such as `<mainnet read db config>` are intentionally left as-is to be replaced by users with their own configuration values when deploying the tool.
rollup/cmd/rollup_relayer/app/app.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/controller/watcher/batch_proposer.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/orm/batch.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/utils/utils.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/controller/relayer/l2_relayer.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
🧬 Code Graph Analysis (3)
rollup/cmd/rollup_relayer/app/app.go (2)
rollup/internal/config/l2.go (1)
L2Config(10-29)rollup/internal/config/relayer.go (1)
RelayerConfig(55-82)
rollup/internal/orm/batch.go (1)
rollup/internal/utils/utils.go (1)
GetBatchMetadata(148-207)
rollup/internal/utils/utils.go (2)
rollup/internal/orm/batch.go (2)
Batch(22-72)Batch(80-82)coordinator/internal/types/auth.go (1)
Hash(134-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
🔇 Additional comments (7)
common/version/version.go (1)
8-8: LGTM: Version bump is correctly implemented.The version increment from v4.5.27 to v4.5.28 properly reflects the addition of new Validium mode functionality.
rollup/conf/config.json (1)
39-39: LGTM: Well-structured configuration addition.The
validium_modefield is appropriately placed within the relayer configuration and uses a safe default value offalsefor backward compatibility.rollup/internal/config/relayer.go (1)
56-57: LGTM: Proper struct field implementation.The
ValidiumModefield is correctly added with appropriate JSON tagging and a clear descriptive comment. The placement at the beginning of the struct is logical for a fundamental operational mode setting.rollup/cmd/rollup_relayer/app/app.go (1)
111-111: Confirmed:SetValidiumModeExists onBatchProposerThe
SetValidiumMode(validiumMode bool)method is defined in
rollup/internal/controller/watcher/batch_proposer.go:134.
The integration inrollup/cmd/rollup_relayer/app/app.go:111is correct and can be approved.rollup/internal/orm/batch.go (1)
288-288: LGTM: Correct function signature alignment.The addition of
metrics.ValidiumModeparameter correctly aligns with the updatedGetBatchMetadatafunction signature that now accepts avalidiumModeboolean parameter.rollup/internal/controller/watcher/batch_proposer.go (1)
35-35: LGTM! Validium mode integration is clean and consistent.The addition of the
validiumModefield with proper initialization and the consistent propagation to allCalculateBatchMetricscalls is well implemented.Also applies to: 67-68, 133-137, 181-181, 297-297, 322-322, 332-332
rollup/internal/controller/relayer/l2_relayer.go (1)
1017-1019: Confirm EndBlockHash usage as validium commitmentWe weren’t able to locate any in-repo documentation or examples that define how the commitment should be derived for validium mode. The code currently does:
lastChunk := lastBatch.Chunks[len(lastBatch.Chunks)-1] commitment := common.HexToHash(lastChunk.EndBlockHash)Please verify against the official validium protocol specification that using the last chunk’s
EndBlockHashhere is the correct source for the commitment.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1693 +/- ##
===========================================
+ Coverage 39.86% 39.90% +0.03%
===========================================
Files 236 233 -3
Lines 18808 18373 -435
===========================================
- Hits 7498 7331 -167
+ Misses 10575 10333 -242
+ Partials 735 709 -26
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:
|
Co-authored-by: Péter Garamvölgyi <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
984-1027: Consider the validium commitment strategy and batch processing differences.The function correctly implements validium-specific logic:
- Uses last block hash as commitment (acknowledged as temporary in TODO)
- Calculates metrics without processing individual L2 blocks (appropriate for validium)
- Validates batch consistency and codec versions
However, consider if the temporary commitment strategy meets security requirements for your use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
rollup/abi/validium_abi.go(1 hunks)rollup/cmd/rollup_relayer/app/app.go(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(9 hunks)rollup/internal/controller/watcher/batch_proposer.go(7 hunks)rollup/internal/controller/watcher/batch_proposer_test.go(4 hunks)rollup/internal/controller/watcher/bundle_proposer_test.go(1 hunks)rollup/internal/controller/watcher/proposer_tool.go(1 hunks)rollup/internal/utils/utils.go(4 hunks)rollup/tests/rollup_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- rollup/cmd/rollup_relayer/app/app.go
- rollup/internal/controller/watcher/batch_proposer.go
- rollup/internal/utils/utils.go
- rollup/abi/validium_abi.go
🧰 Additional context used
🧠 Learnings (5)
rollup/internal/controller/watcher/proposer_tool.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/tests/rollup_test.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/controller/watcher/bundle_proposer_test.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/controller/watcher/batch_proposer_test.go (1)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
rollup/internal/controller/relayer/l2_relayer.go (2)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1693
File: rollup/abi/validium_abi.go:13-15
Timestamp: 2025-07-07T12:02:52.191Z
Learning: In the Scroll codebase, ABI initialization in files like bridge_abi.go and validium_abi.go follows a consistent pattern of ignoring errors from GetAbi() in init functions to maintain code style consistency across the project.
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
🔇 Additional comments (10)
rollup/internal/controller/watcher/proposer_tool.go (1)
128-128: LGTM: Constructor update is consistent with validium mode support.The addition of the
false /* rollup mode */parameter aligns with the BatchProposer constructor signature update to support both rollup and validium modes.rollup/internal/controller/watcher/bundle_proposer_test.go (1)
106-106: LGTM: Test constructor update is correct.The test correctly specifies rollup mode with the
false /* rollup mode */parameter, which is appropriate for general batch proposer testing.rollup/tests/rollup_test.go (1)
131-131: LGTM: Test constructor update maintains existing behavior.The explicit
false /* rollup mode */parameter ensures the test continues to use rollup mode as expected.rollup/internal/controller/watcher/batch_proposer_test.go (1)
103-103: LGTM: Consistent test constructor updates across all test functions.All test functions correctly add the
false /* rollup mode */parameter to maintain existing rollup mode behavior in tests.Also applies to: 181-181, 249-249, 338-338
rollup/internal/controller/relayer/l2_relayer.go (6)
82-82: LGTM: ValidiumABI field addition and initialization.The validiumABI field is properly added and initialized from the bridgeAbi package, following the same pattern as the existing l1RollupABI.
Also applies to: 176-176
279-296: LGTM: Genesis batch commitment logic branches correctly for validium mode.The conditional logic properly handles both rollup and validium modes:
- Validium mode: Only passes batchHeader to importGenesisBatch
- Rollup mode: Passes both batchHeader and stateRoot
- Logging appropriately identifies the mode being used
Also applies to: 303-303, 328-328
484-496: LGTM: Batch processing correctly branches based on validium mode.The ProcessPendingBatches function properly calls different payload construction methods based on the validium mode flag, with appropriate error handling for each path.
716-726: LGTM: Bundle finalization correctly branches based on validium mode.The finalizeBundle function appropriately calls different payload construction methods for validium vs rollup modes with proper error handling.
1045-1045: LGTM: Added logging for rollup mode finalization.The logging addition provides useful visibility into the rollup mode finalization process, mirroring the logging added for validium mode.
1061-1078: Validium finalizeBundle always requires aggProofI compared both implementations and confirmed:
- In rollup mode (
constructFinalizeBundlePayloadCodecV7), anilaggProof falls back tofinalizeBundlePostEuclidV2NoProof.- In validium mode (
constructFinalizeBundlePayloadValidium), anilaggProof returns an error (lines 1064–1066).- There are no test-bypass flags or special branches for validium mode.
This behavior is intentional: validium finalization cannot proceed without a proof. If you need proof-less finalization in test environments, add a
nil-proof branch similar to the rollup version.
colinlyguo
force-pushed
the
validium-devnet-wip
branch
from
a398f64 to
7dfb8b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
1000-1027: LGTM: Correct validium batch payload construction with follow-up needed.The method correctly constructs the validium batch payload with proper metrics calculation and ABI packing. However, the TODO comment on lines 1013-1013 indicates the commitment calculation is temporary and may need future enhancement.
Would you like me to help track this TODO item by opening a new issue to address the commitment calculation enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rollup/cmd/rollup_relayer/app/app.go(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rollup/cmd/rollup_relayer/app/app.go
🧰 Additional context used
🧠 Learnings (1)
rollup/internal/controller/relayer/l2_relayer.go (2)
Learnt from: colinlyguo
PR: scroll-tech/scroll#1693
File: rollup/abi/validium_abi.go:13-15
Timestamp: 2025-07-07T12:02:52.214Z
Learning: In the Scroll codebase, ABI initialization in files like bridge_abi.go and validium_abi.go follows a consistent pattern of ignoring errors from GetAbi() in init functions to maintain code style consistency across the project.
Learnt from: colinlyguo
PR: scroll-tech/scroll#1530
File: rollup/internal/controller/watcher/batch_proposer.go:291-294
Timestamp: 2024-10-20T16:13:20.397Z
Learning: In `batch_proposer.go`, it's acceptable to call `utils.CalculateBatchMetrics` multiple times within the loop because the batch's chunks are increasing in the loop, and each calculation reflects the updated batch state.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
🔇 Additional comments (9)
rollup/internal/controller/relayer/l2_relayer.go (9)
82-82: LGTM: Clean addition of validium ABI field.The new
validiumABIfield follows the same pattern as the existingl1RollupABIfield and properly supports the validium mode functionality.
176-176: LGTM: Proper ABI initialization.The initialization follows the established pattern and correctly assigns the ValidiumABI from the bridge ABI package.
244-248: LGTM: Proper batch creation updates for validium support.The addition of the
Blocksfield and the inclusion ofValidiumModeinBatchMetricscorrectly propagates the validium mode information through the batch creation process.
280-297: LGTM: Well-structured mode-aware genesis batch commitment.The branching logic correctly handles the different requirements for validium and rollup modes, with appropriate error handling and logging for both cases.
557-562: LGTM: Correct batch limit enforcement for validium mode.The method properly enforces single batch submission for validium mode while preserving existing behavior for rollup mode, addressing the architectural requirements.
341-342: LGTM: Proper batch processing logic for validium mode.The integration of
getEffectiveBatchLimitsand the mode-aware payload construction correctly implements the validium requirements while maintaining existing rollup functionality.Also applies to: 488-504
732-742: LGTM: Consistent finalize bundle payload construction.The mode-aware branching for finalize bundle payload construction follows the established pattern and correctly handles both validium and rollup modes.
1061-1079: LGTM: Robust validium finalize bundle implementation.The method correctly handles the nil proof case and properly constructs the validium finalize bundle payload. The previous nil pointer issue has been addressed.
304-304: LGTM: Enhanced logging for better observability.The logging updates provide valuable debugging information and help distinguish between validium and rollup mode operations, improving system observability.
Also applies to: 329-329, 1045-1046, 1062-1062
5 participants
Purpose or design rationale of this PR
Support Validium in rollup-relayer.
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
Chores