Skip to content

Conversation

@Thegaram
Copy link
Contributor

@Thegaram Thegaram commented Nov 28, 2025

Purpose or design rationale of this PR

Introduce CodecV10 for GalileoV2.

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:

  • feat: A new feature

Breaking change label

Does this PR have the breaking-change label?

  • No, this PR is not a breaking change
  • Yes

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for GalileoV2 chain configuration with updated codec version handling
    • Enhanced compression capabilities for the new codec version
  • Tests

    • Expanded test coverage for codec version selection and chain configuration scenarios

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 28, 2025

Walkthrough

Introduces CodecV10 codec version with GalileoV2 hardfork support. Adds a new DACodecV10 type embedding DACodecV9, updates codec selection logic in CodecFromConfig to prioritize GalileoV2 detection, and extends version-detection functions to distinguish GalileoV2 activation from earlier Galileo versions.

Changes

Cohort / File(s) Summary
Codec V10 Implementation
encoding/codecv10.go
New public type DACodecV10 with embedded DACodecV9 and constructor NewDACodecV10() establishing version-specific codec lineage with forced CodecV10 version.
Version Detection & Selection
encoding/da.go, encoding/interfaces.go
GetHardforkName and GetCodecVersion now distinguish GalileoV2; CodecFromConfig prioritizes GalileoV2 detection returning DACodecV10, with Galileo fallback to V9; CompressScrollBatchBytes signature extended with blockNumber, blockTimestamp, and chainCfg parameters for context-aware compression; GetChunkEnableCompression and GetBatchEnableCompression expanded to include CodecV10 case handling.
Codec Version Enum & Factory
encoding/interfaces.go
Added CodecV10 constant to public codec version enum; extended CodecFromVersion switch with CodecV10 case returning NewDACodecV10().
Test Coverage
encoding/interfaces_test.go
TestCodecFromVersion updated to map CodecV10 to DACodecV10 implementation; new test case "GalileoV2 active" in TestCodecFromConfig verifies GalileoV2 activation yields DACodecV10.
Dependency Update
go.mod
Updated go-ethereum dependency to newer pseudo-version.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Signature change in CompressScrollBatchBytes: Verify all callers are updated with new parameters and that context-aware compression logic is correct.
  • GalileoV2 hardfork detection logic: Confirm chainCfg.IsGalileoV2(startBlockTimestamp) and IsGalileo(startBlockTimestamp) branching is correct and doesn't conflict with existing version logic.
  • Codec embedding chain: Validate that DACodecV10 → DACodecV9 → DACodecV8 → DACodecV7 initialization properly sets forcedVersion to CodecV10.
  • Test coverage: Check that both "GalileoV2 active" and "Galileo active" test cases provide sufficient differentiation and that other codec version tests remain unaffected.

Possibly related PRs

Suggested reviewers

  • jonastheis
  • colinlyguo

Poem

🐰 A shiny V10 codec hops into place,
With GalileoV2's new hardfork embrace,
The chain configuration now knows the way,
To select the right codec each passing day!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The pull request description follows the template structure with the purpose/rationale section completed and the correct 'feat' type checkbox selected; however, the rationale provided ('Introduce CodecV10 for GalileoV2') is minimal and lacks sufficient detail on what, why, and how. Expand the purpose section to explain what CodecV10 does, why GalileoV2 requires it, and how the implementation extends the existing codec chain to support the new hardfork.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'feat: add codecv10 for GalileoV2' clearly summarizes the main change—introducing a new CodecV10 component for the GalileoV2 hardfork, which is reflected throughout all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-add-codecv10-for-galileov2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a92e85 and dececf9.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • encoding/codecv10.go (1 hunks)
  • encoding/da.go (4 hunks)
  • encoding/interfaces.go (2 hunks)
  • encoding/interfaces_test.go (2 hunks)
  • go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/interfaces.go:95-108
Timestamp: 2024-10-17T04:13:14.579Z
Learning: In the `CodecFromConfig` function in the Go `encoding/interfaces.go` file, if none of the chain configuration conditions match, it's acceptable to default to returning `&DACodecV0{}` because, in the current logic, we can only deduce the codec version as the function implements, and the logic is complete.
📚 Learning: 2024-10-17T04:13:14.579Z
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/interfaces.go:95-108
Timestamp: 2024-10-17T04:13:14.579Z
Learning: In the `CodecFromConfig` function in the Go `encoding/interfaces.go` file, if none of the chain configuration conditions match, it's acceptable to default to returning `&DACodecV0{}` because, in the current logic, we can only deduce the codec version as the function implements, and the logic is complete.

Applied to files:

  • encoding/interfaces_test.go
  • encoding/codecv10.go
  • encoding/da.go
  • encoding/interfaces.go
📚 Learning: 2024-10-18T03:40:09.800Z
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/codecv1_types.go:105-116
Timestamp: 2024-10-18T03:40:09.800Z
Learning: The code in `encoding/codecv1_types.go`, specifically the `Encode` method in `daBatchV1`, has been updated. Previous comments regarding hardcoded byte offsets may be outdated.

Applied to files:

  • encoding/interfaces_test.go
  • encoding/codecv10.go
  • encoding/da.go
  • encoding/interfaces.go
📚 Learning: 2024-10-17T05:40:03.610Z
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/codecv0.go:387-401
Timestamp: 2024-10-17T05:40:03.610Z
Learning: In `DACodecV0`, methods like `EstimateChunkL1CommitBatchSizeAndBlobSize`, `EstimateBatchL1CommitBatchSizeAndBlobSize`, and `JSONFromBytes` are intentionally left as no-ops (returning zero or nil) to maintain a consistent interface across codecs and prevent the caller from needing conditional logic.

Applied to files:

  • encoding/interfaces_test.go
  • encoding/codecv10.go
  • encoding/da.go
  • encoding/interfaces.go
📚 Learning: 2024-10-17T08:47:58.627Z
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/codecv0_types.go:231-239
Timestamp: 2024-10-17T08:47:58.627Z
Learning: Constants like `daBatchV0OffsetSkippedL1MessageBitmap`, `daBatchOffsetVersion`, `daBatchV0OffsetL1MessagePopped`, and `daBatchOffsetDataHash` are defined in `da.go` file.

Applied to files:

  • encoding/da.go
  • encoding/interfaces.go
📚 Learning: 2024-10-17T08:49:05.064Z
Learnt from: colinlyguo
Repo: scroll-tech/da-codec PR: 25
File: encoding/codecv2.go:222-223
Timestamp: 2024-10-17T08:49:05.064Z
Learning: In the function `NewDABatchFromBytes` in `encoding/codecv2.go`, the assignments of `parentBatchHash` and `blobVersionedHash` are correct as implemented.

Applied to files:

  • encoding/da.go
🧬 Code graph analysis (3)
encoding/codecv10.go (4)
encoding/codecv9.go (1)
  • DACodecV9 (29-31)
encoding/interfaces.go (1)
  • CodecV10 (98-98)
encoding/codecv8.go (1)
  • DACodecV8 (30-32)
encoding/codecv7.go (1)
  • DACodecV7 (20-22)
encoding/da.go (1)
encoding/interfaces.go (7)
  • CodecV9 (97-97)
  • CodecV10 (98-98)
  • CodecV4 (92-92)
  • CodecV5 (93-93)
  • CodecV6 (94-94)
  • CodecV7 (95-95)
  • CodecV8 (96-96)
encoding/interfaces.go (1)
encoding/codecv10.go (1)
  • NewDACodecV10 (7-16)
⏰ 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). (1)
  • GitHub Check: tests
🔇 Additional comments (11)
encoding/codecv10.go (1)

1-16: LGTM!

The DACodecV10 implementation correctly follows the established embedding pattern. The struct embeds DACodecV9, and the constructor properly initializes the forcedVersion to CodecV10, ensuring the codec reports the correct version while inheriting all behavior from V9.

encoding/da.go (4)

828-832: LGTM!

The GetHardforkName function correctly prioritizes IsGalileoV2 check before falling through to the Galileo case, returning "galileoV2" for the new hardfork.


855-859: LGTM!

The GetCodecVersion function correctly returns CodecV10 when GalileoV2 is active, with proper ordering of hardfork checks.


887-887: LGTM!

CodecV10 is correctly added to the compression-enabled codec versions for chunk processing.


901-901: LGTM!

CodecV10 is correctly added to the compression-enabled codec versions for batch processing.

encoding/interfaces_test.go (2)

29-30: LGTM!

Test cases correctly updated: CodecV10 now maps to DACodecV10, and CodecV11 serves as the new undefined version boundary.


56-72: LGTM!

The "GalileoV2 active" test case correctly configures all prerequisite hardfork times (including GalileoV2Time) and expects DACodecV10 to be returned.

encoding/interfaces.go (3)

98-98: LGTM!

CodecV10 correctly added to the CodecVersion enum, following the established iota sequence.


124-125: LGTM!

The CodecFromVersion switch case correctly handles CodecV10 by returning the result of NewDACodecV10().


133-135: LGTM!

The CodecFromConfig function correctly prioritizes the IsGalileoV2 check before IsGalileo, ensuring proper hardfork detection since a chain at GalileoV2 would also satisfy the Galileo condition.

go.mod (1)

7-7: Dependency update is appropriate and necessary for GalileoV2 hardfork support.

The PR code extensively uses params.ChainConfig methods including IsGalileoV2(timestamp) and the GalileoV2Time field. The updated pseudo-version v1.10.14-0.20251127071535-dd8541508584 (commit from 2025-11-27) must include these hardfork detection capabilities to compile successfully.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member

@yiweichi yiweichi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I wonder why do we need this codecV10? seems it's exactly as same as codecV9.

@Thegaram
Copy link
Contributor Author

LGTM, I wonder why do we need this codecV10? seems it's exactly as same as codecV9.

We need to use a different version in rollup-relayer (v10 instead of v9) so that the coordinator/prover can decide which protocol version this is. It's a lot of boilerplate code/processes that we should try to eliminate in the future.

@Thegaram Thegaram merged commit afa161a into main Nov 28, 2025
4 checks passed
@Thegaram Thegaram deleted the feat-add-codecv10-for-galileov2 branch November 28, 2025 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants