add galileo CodecV9 #66
Conversation
WalkthroughAdds CodecV9 support: a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as Config
participant Factory as CodecFromConfig
participant ForkCheck as IsGalileo
participant V9 as NewDACodecV9
participant V8 as NewDACodecV8
Config->>Factory: request codec for blockTimestamp
Factory->>ForkCheck: IsGalileo(blockTimestamp)?
alt Galileo active
ForkCheck-->>Factory: true
Factory->>V9: return NewDACodecV9()
V9-->>Factory: *DACodecV9 instance*
else Galileo not active
ForkCheck-->>Factory: false
Factory->>V8: evaluate other forks -> NewDACodecV8()
V8-->>Factory: *DACodecV8 instance*
end
sequenceDiagram
autonumber
participant Caller as NewDABatch caller
participant Codec as DACodecV9
participant Compressor as Compressor/Zstd
participant Blob as BlobBuilder
Caller->>Codec: NewDABatch(batch)
Codec->>Codec: validate batch non-empty & consistency
Codec->>Compressor: compress payload (maybe)
alt compression compatible
Compressor-->>Codec: compressed payload
Codec->>Blob: constructBlob(metadata, compressed)
Blob-->>Codec: canonical blob, commit, versioned hash
Codec-->>Caller: DABatch with blob, commitment, hash
else incompatible
Compressor-->>Codec: indicate not compatible / skip
Codec->>Blob: constructBlob(uncompressed)
Codec-->>Caller: DABatch with uncompressed blob
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (14)📓 Common learnings📚 Learning: 2024-10-18T03:40:09.800ZApplied to files:
📚 Learning: 2024-10-17T05:40:03.610ZApplied to files:
📚 Learning: 2024-10-17T04:13:14.579ZApplied to files:
📚 Learning: 2024-10-17T08:49:05.064ZApplied to files:
📚 Learning: 2024-10-17T07:33:28.436ZApplied to files:
📚 Learning: 2024-10-17T04:15:29.946ZApplied to files:
📚 Learning: 2024-10-16T18:43:44.520ZApplied to files:
📚 Learning: 2024-10-17T05:41:29.398ZApplied to files:
📚 Learning: 2024-10-17T04:16:36.614ZApplied to files:
📚 Learning: 2024-10-17T08:47:58.627ZApplied to files:
📚 Learning: 2024-10-16T18:17:33.741ZApplied to files:
📚 Learning: 2024-10-18T03:41:31.377ZApplied to files:
📚 Learning: 2024-10-17T03:55:56.330ZApplied to files:
🧬 Code graph analysis (2)encoding/codecv9.go (4)
encoding/da.go (1)
⏰ 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)
🔇 Additional comments (8)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
encoding/codecv9.go(1 hunks)encoding/interfaces.go(2 hunks)go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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.
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.
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.
📚 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.gogo.modencoding/codecv9.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.gogo.modencoding/codecv9.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.gogo.modencoding/codecv9.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:
go.mod
🧬 Code graph analysis (2)
encoding/interfaces.go (1)
encoding/codecv9.go (1)
NewDACodecV9(7-14)
encoding/codecv9.go (3)
encoding/codecv8.go (1)
DACodecV8(30-32)encoding/interfaces.go (1)
CodecV9(97-97)encoding/codecv7.go (1)
DACodecV7(20-22)
🪛 OSV Scanner (2.2.4)
go.mod
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: Denial of service in message decoding in github.com/btcsuite/btcd
(GO-2022-1098)
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: Consensus failures in github.com/btcsuite/btcd
(GO-2024-2818)
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: Consensus failure in github.com/btcsuite/btcd
(GO-2024-3189)
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: btcd did not correctly re-implement Bitcoin Core's "FindAndDelete()" functionality
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: btcd mishandles witness size checking
[CRITICAL] 18-18: github.com/btcsuite/btcd 0.20.1-beta: btcd susceptible to consensus failures
[HIGH] 20-20: github.com/consensys/gnark-crypto 0.16.0: Unchecked memory allocation during vector deserialization in github.com/consensys/gnark-crypto
(GO-2025-4087)
[HIGH] 20-20: github.com/consensys/gnark-crypto 0.16.0: gnark-crypto allows unchecked memory allocation during vector deserialization
[HIGH] 35-35: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 35-35: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto/ssh/agent
(GO-2025-4116)
[HIGH] 35-35: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
⏰ 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 (8)
encoding/codecv9.go (2)
3-5: LGTM! Clean codec inheritance pattern.The struct definition correctly follows the established embedding pattern, inheriting all functionality from CodecV8.
7-14: Constructor is correctly implemented.The
NewDACodecV9()function properly initializes the codec with the correct version flag. TheIsGalileomethod is defined in the externalgit.colasdn.top/scroll-tech/go-ethereum/paramspackage and is actively used in theCodecFromConfigrouting logic. No issues found.encoding/interfaces.go (3)
97-97: LGTM! Correct enum extension.The
CodecV9constant is properly added to theCodecVersionenum following the sequential pattern.
121-122: LGTM! Proper codec factory routing.The
CodecV9case correctly returnsNewDACodecV9()following the established pattern for codec instantiation.
130-132: LGTM! Correct chronological codec selection.The
IsGalileocheck is properly positioned first to ensure the newest codec is selected when applicable, with the previousIsFeynmancheck correctly moved to the else-if branch.go.mod (3)
3-3: Verify CI/CD compatibility with Go 1.22.The toolchain upgrade to Go 1.22 looks appropriate. Ensure that all CI/CD pipelines and development environments support this version.
35-35: Update golang.org/x/crypto to a patched version compatible with go-ethereum.The codebase does not directly import golang.org/x/crypto subpackages (only standard library
crypto/sha256andcrypto/randare used). However,golang.org/x/crypto v0.32.0is pulled transitively by go-ethereum. GO-2025-3487 is patched in v0.35.0 and GO-2025-4116 is patched in v0.43.0.Since SSH is not used directly in this codebase, verify whether go-ethereum's internal usage of x/crypto exposes either vulnerability and whether the current go-ethereum version (v1.10.14-0.20251113125950-906b730d541d) is compatible with the patched versions. If so, update go-ethereum to a version requiring a patched x/crypto, or explicitly upgrade x/crypto in go.mod if the go-ethereum constraint allows.
7-7: Verify that go-ethereum v1.10.14-0.20251113125950-906b730d541d includes theIsGalileomethod.The codebase calls
chainCfg.IsGalileo(startBlockTimestamp)inencoding/interfaces.go:130to select CodecV9 for Galileo support. However, this method must be provided by the go-ethereum dependency, and I cannot independently confirm it exists in the specified pseudo-version. Ensure this version includes the requiredIsGalileomethod onparams.ChainConfigand that the code compiles and runs correctly with the updated dependency.
There was a problem hiding this comment.
Remember that we need to bump da-codec version in both rollup-relayer and l2geth (I think previously we forgot about l2geth).
There was a problem hiding this comment.
blob-uploader depends on da-code as well, Do we already have plan to update blob-uploader here to support codecv9?
There was a problem hiding this comment.
Nevermind, it's already covered on pr https://github.com/scroll-tech/scroll/pull/1752/files#diff-516f715f986f16ab3c638b2e3652d9bf4af757c33fd880a0964dea26c2d48f2dR1053.
* fix: consider RLE blocks in zstd compatibility check (#64) * fix compability * fmt * override behavior * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Ho <[email protected]> Co-authored-by: georgehao <[email protected]> Co-authored-by: Copilot <[email protected]>
Purpose or design rationale of this PR
This PR adds CodecV9 for Galileo. This codec is identical with CodecV8, except that it uses version
9in both the batch header and blob envelope.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:
Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Tests
Chores