Skip to content

Conversation

@Thegaram
Copy link

@Thegaram Thegaram commented Oct 9, 2025

1. Purpose or design rationale of this PR

These changes are needed when we use l2geth and a Go SDK in rollup-relayer.

2. 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

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change
  • Yes

Summary by CodeRabbit

  • New Features

    • Versioned blob transaction sidecars with constructor and utility accessors.
    • Per-cell proof computation and batch verification for KZG-backed blobs.
    • Hex JSON/text encoding for KZG primitives (blob, commitment, proof).
  • Dependencies

    • Go toolchain bumped to 1.22.
  • Chores

    • Release patch version incremented to 7.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR merges KZG and blob changes from upstream, updating dependencies and adding new cell-based proof functionality for data availability sampling. The changes upgrade Go version, update KZG-related dependencies, and introduce versioned blob sidecars with support for cell proofs.

  • Upgraded Go version from 1.21 to 1.22 and updated multiple dependencies including KZG libraries
  • Added cell proof functionality with new methods for computing and verifying cell proofs
  • Introduced versioned blob sidecars (v0 and v1) to support different proof types

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
params/version.go Incremented patch version from 5 to 6
go.mod Updated Go version to 1.22 and upgraded KZG and other dependencies
crypto/kzg4844/kzg4844_test.go Updated import path and added cell proof tests with new benchmark loop pattern
crypto/kzg4844/kzg4844_gokzg.go Updated import path and added cell proof computation and verification functions
crypto/kzg4844/kzg4844_ckzg_nocgo.go Added wasip1 build constraint and stub functions for cell proofs
crypto/kzg4844/kzg4844_ckzg_cgo.go Updated import paths, modified trusted setup loading, and added cell proof implementations
crypto/kzg4844/kzg4844.go Added JSON marshaling, cell proof functions, and fixed typo in comment
core/types/blob_tx.go Added versioned blob sidecars, cell proof support, and enhanced encoding/decoding

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds versioned BlobTxSidecar (v0/v1) with conversion, validation, copy and accessor APIs; implements per-cell KZG proof compute/verify paths (CKZG and GoKZG) plus serialization hooks; updates KZG bindings and dependencies; bumps Go toolchain to 1.22 and increments params patch version.

Changes

Cohort / File(s) Summary
BlobTx Versioning & Sidecar API
core/types/blob_tx.go
Add BlobSidecarVersion0, BlobSidecarVersion1; add public Version byte to BlobTxSidecar; add NewBlobTxSidecar, CellProofsAt, ToV1, ValidateBlobCommitmentHashes, Copy; introduce internal blobTxWithBlobs interface and per-version types blobTxWithBlobsV0/...V1 with assign()/tx(); update encoding/decoding to handle v0/v1 and conversion to v1.
KZG Core API & Serialization
crypto/kzg4844/kzg4844.go
Add CellProofsPerBlob = 128; add ComputeCellProofs() and VerifyCellProofs() (dispatch to CKZG/GoKZG); add MarshalText/UnmarshalJSON (text/hex) for Blob, Commitment, Proof.
CKZG (cgo) Implementation
crypto/kzg4844/kzg4844_ckzg_cgo.go
Update build tags/imports to new bindings; add g1Lag handling in trusted setup; add ckzgComputeCellProofs() and ckzgVerifyCellProofBatch() using existing C APIs; adjust init/conversions and error handling.
CKZG (no-cgo) Stubs
crypto/kzg4844/kzg4844_ckzg_nocgo.go
Update build tags; add stub ckzgComputeCellProofs() and ckzgVerifyCellProofBatch() that panic on unsupported platforms.
GoKZG Implementation
crypto/kzg4844/kzg4844_gokzg.go
Replace import with go-eth-kzg; add gokzgComputeCellProofs() and gokzgVerifyCellProofBatch() for per-cell proof computation and batch verification.
KZG Tests & Benchmarks
crypto/kzg4844/kzg4844_test.go
Update imports; add TestCKZGCells, TestGoKZGCells; add benchmarks BenchmarkCKZGComputeCellProofs, BenchmarkGOKZGComputeCellProofs; add shared test/benchmark helpers.
Dependency & Toolchain Updates
go.mod
Bump go to 1.22 and add toolchain go1.22.12; replace crate-crypto/go-kzg-4844 with crate-crypto/go-eth-kzg v1.4.0; upgrade ethereum/c-kzg-4844 to v2; bump consensys/gnark-crypto and various golang.org/x/* and indirect deps.
Version Bump
params/version.go
Increment VersionPatch from 6 to 7.
CI Tooling
.github/workflows/l2geth_ci.yml
Bump CI Go versions to 1.22.x for multiple jobs; alter check job conditional to if: false.
Docker Build Images
Dockerfile, Dockerfile.alltools, Dockerfile.mockccc, Dockerfile.mockccc.alpine
Update builder/base images to Go 1.22 toolchains (image tag changes across Dockerfiles).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant App as Application
    participant Sidecar as BlobTxSidecar
    participant KZG as kzg4844
    participant Backend as CKZG/GoKZG

    App->>Sidecar: NewBlobTxSidecar(version, blobs, commitments, proofs)
    Sidecar-->>App: sidecar (Version set)

    App->>Sidecar: ToV1()
    Sidecar->>KZG: ComputeCellProofs(blob)
    KZG->>Backend: dispatch compute (CKZG or GoKZG)
    Backend-->>KZG: []Proof
    KZG-->>Sidecar: attach cell proofs (set Version = v1)

    App->>Sidecar: ValidateBlobCommitmentHashes(hashes)
    Sidecar->>KZG: VerifyCellProofs(blobs, commitments, proofs)
    KZG->>Backend: dispatch verify (batch)
    Backend-->>KZG: verify result
    KZG-->>Sidecar: validation result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

bump-version

Suggested reviewers

  • jonastheis
  • georgehao

Poem

🐰 I hopped in with blobs and a tiny new plan,
Sidecars now versioned — v0 to v1 I ran,
KZG proofs computed, attached with a grin,
Go rose to 1.22 and patch bumped to seven! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly indicates that this PR introduces KZG and blob changes merged from upstream, accurately reflecting the scope of updates and following conventional commit style.
Description Check ✅ Passed The pull request description follows the repository template, including all required sections: purpose, commit type validation, version update confirmation, and breaking change declaration, with each field completed and accurately reflecting the PR’s details.
✨ 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 chore-update-kzg-to-latest-upstream-version

📜 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 418c1ab and 6f3752e.

📒 Files selected for processing (1)
  • crypto/kzg4844/kzg4844_test.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • crypto/kzg4844/kzg4844_test.go
⏰ 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). (2)
  • GitHub Check: test
  • GitHub Check: Analyze (go)

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

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/types/blob_tx.go (1)

90-97: Bug: iterates over Blobs while indexing Commitments

Can panic or leave hashes uninitialized when lengths differ. Iterate over commitments.

Apply this diff:

 func (sc *BlobTxSidecar) BlobHashes() []common.Hash {
   hasher := sha256.New()
-  h := make([]common.Hash, len(sc.Commitments))
-  for i := range sc.Blobs {
-    h[i] = kzg4844.CalcBlobHashV1(hasher, &sc.Commitments[i])
-  }
+  n := len(sc.Commitments)
+  h := make([]common.Hash, n)
+  for i := 0; i < n; i++ {
+    h[i] = kzg4844.CalcBlobHashV1(hasher, &sc.Commitments[i])
+  }
   return h
 }
🧹 Nitpick comments (6)
core/types/blob_tx.go (2)

79-87: Constructor: consider basic invariants

Optionally validate lengths (len(blobs) == len(commitments); for v1, len(proofs) == blobs*CellProofsPerBlob) to fail fast on malformed inputs.


115-133: Avoid copying 128KiB arrays in loop (addr-of-range var)

Passing &blob takes address of the range variable (full copy each iter). Use index to take address of slice element.

-    for _, blob := range sc.Blobs {
-      cellProofs, err := kzg4844.ComputeCellProofs(&blob)
+    for i := range sc.Blobs {
+      cellProofs, err := kzg4844.ComputeCellProofs(&sc.Blobs[i])
         if err != nil {
           return err
         }
-      proofs = append(proofs, cellProofs...)
+      proofs = append(proofs, cellProofs...)
     }
crypto/kzg4844/kzg4844_gokzg.go (1)

100-116: Preallocate result slice to avoid extra allocs

Small efficiency win when converting proofs.

- var p []Proof
- for _, proof := range proofs {
-   p = append(p, (Proof)(proof))
- }
- return p, nil
+ p := make([]Proof, len(proofs))
+ for i, proof := range proofs {
+   p[i] = Proof(proof)
+ }
+ return p, nil
crypto/kzg4844/kzg4844_test.go (1)

213-227: Assert the expected cell-proof count

ComputeCellProofs is expected to return exactly CellProofsPerBlob entries per blob. Adding an explicit len(proofsX) == CellProofsPerBlob assertion would catch regressions in the backends (or future library upgrades) before they cascade into sidecar handling.

crypto/kzg4844/kzg4844.go (1)

154-163: Validate slice lengths before delegating

We document strict expectations on blobs, commitments, and proofs, but the function forwards mismatched slices straight into the backend. Add early guards (e.g. len(blobs) == len(commitments) and len(proofs) == len(blobs)*CellProofsPerBlob) so callers get deterministic, high-signal errors instead of opaque backend failures.

crypto/kzg4844/kzg4844_ckzg_cgo.go (1)

142-151: Preallocate the proof slice

proofs has a fixed length (len(proofs) from CKZG). Initializing p := make([]Proof, len(proofs)) and assigning by index avoids repeated reallocations and makes the intent clearer. It’s a cheap win on this hot path.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0697268 and 0322750.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (8)
  • core/types/blob_tx.go (8 hunks)
  • crypto/kzg4844/kzg4844.go (3 hunks)
  • crypto/kzg4844/kzg4844_ckzg_cgo.go (4 hunks)
  • crypto/kzg4844/kzg4844_ckzg_nocgo.go (2 hunks)
  • crypto/kzg4844/kzg4844_gokzg.go (2 hunks)
  • crypto/kzg4844/kzg4844_test.go (6 hunks)
  • go.mod (5 hunks)
  • params/version.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
crypto/kzg4844/kzg4844_ckzg_nocgo.go (1)
crypto/kzg4844/kzg4844.go (3)
  • Blob (42-42)
  • Commitment (55-55)
  • Proof (68-68)
crypto/kzg4844/kzg4844_gokzg.go (1)
crypto/kzg4844/kzg4844.go (3)
  • Blob (42-42)
  • Proof (68-68)
  • Commitment (55-55)
crypto/kzg4844/kzg4844.go (1)
common/hexutil/json.go (1)
  • UnmarshalFixedJSON (100-105)
crypto/kzg4844/kzg4844_test.go (1)
crypto/kzg4844/kzg4844.go (6)
  • VerifyBlobProof (147-152)
  • BlobToCommitment (110-115)
  • ComputeCellProofs (168-173)
  • Blob (42-42)
  • VerifyCellProofs (157-162)
  • Commitment (55-55)
core/types/blob_tx.go (4)
crypto/kzg4844/kzg4844.go (6)
  • Blob (42-42)
  • Commitment (55-55)
  • Proof (68-68)
  • CellProofsPerBlob (39-39)
  • ComputeCellProofs (168-173)
  • CalcBlobHashV1 (177-186)
rlp/raw.go (2)
  • SplitList (112-121)
  • Split (63-69)
rlp/decode.go (2)
  • List (581-581)
  • DecodeBytes (91-105)
core/types/transaction.go (1)
  • BlobTxType (54-54)
crypto/kzg4844/kzg4844_ckzg_cgo.go (2)
common/hexutil/hexutil.go (1)
  • MustDecode (80-86)
crypto/kzg4844/kzg4844.go (3)
  • Blob (42-42)
  • Proof (68-68)
  • Commitment (55-55)
🪛 OSV Scanner (2.2.3)
go.mod

[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities in github.com/docker/docker

(GO-2022-0390)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Symlink Attack in Libcontainer and Docker Engine in github.com/docker/docker

(GO-2022-0649)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Arbitrary File Override in Docker Engine in github.com/docker/docker

(GO-2022-0708)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions in github.com/docker/docker

(GO-2022-0985)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Container build can leak any path on the host into the container in github.com/docker/docker

(GO-2022-1107)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) Insufficiently restricted permissions on data directory in github.com/docker/docker

(GO-2024-2500)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Classic builder cache poisoning in github.com/docker/docker

(GO-2024-2512)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby Docker cp broken with debian containers in github.com/docker/docker

(GO-2024-2521)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Unexpected chmod of host files via 'docker cp' in Moby Docker Engine in github.com/docker/docker

(GO-2024-2913)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing in github.com/docker/docker

(GO-2024-2914)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby firewalld reload removes bridge network isolation in github.com/docker/docker

(GO-2025-3829)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities

(GHSA-2mm7-x5h6-5pvq)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) Insufficiently restricted permissions on data directory

(GHSA-3fwx-pjgw-3558)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby firewalld reload removes bridge network isolation

(GHSA-4vq8-7jfc-9cvp)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Path Traversal in Moby builder

(GHSA-6hwg-w5jg-9c6x)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Symlink Attack in Libcontainer and Docker Engine

(GHSA-g7v2-2qxx-wjrw)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Secret insertion into debug log in Docker

(GHSA-j249-ghv5-7mxv)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: /sys/devices/virtual/powercap accessible by default to containers

(GHSA-jq35-85cj-fj4p)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby's external DNS requests from 'internal' networks could lead to data exfiltration

(GHSA-mq39-4gv4-mvpx)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Docker Authentication Bypass

(GHSA-qrqr-3x5j-2xw9)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions

(GHSA-rc4r-wh2q-q6c4)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Arbitrary File Override in Docker Engine

(GHSA-v4h8-794j-g8mm)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: docker cp allows unexpected chmod of host files in Moby Docker Engine

(GHSA-v994-f8vw-g7j4)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Container build can leak any path on the host into the container

(GHSA-vp35-85q5-9f25)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing

(GHSA-xmmx-7jpf-fx42)


[HIGH] 22-22: github.com/docker/docker 1.4.2-0.20180625184442-8e610b2b55bf: Classic builder cache poisoning

(GHSA-xw73-rw38-6vjc)


[HIGH] 64-64: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto

(GO-2025-3487)


[HIGH] 64-64: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange

(GHSA-hcg3-q754-cr77)

⏰ 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). (2)
  • GitHub Check: test
  • GitHub Check: Analyze (go)
🔇 Additional comments (10)
params/version.go (1)

27-27: Patch bump looks good

No issues spotted.

go.mod (3)

18-20: KZG deps switch aligns with new cell-proof APIs

Upgrading to go-eth-kzg v1.4.0 and c-kzg-4844/v2 v2.1.5 is consistent with the new APIs added in crypto/kzg4844. LGTM.

Also applies to: 25-25


3-5: Go/toolchain bump: ensure CI/build images match

Pin CI runners and builder images to Go 1.22.12 to avoid drift with the toolchain directive.


64-64: Upgrade golang.org/x/crypto to a patched release
Multiple subpackages (sha3, hkdf, openpgp, pbkdf2, scrypt, ssh, ripemd160, etc.) are imported; bump the module from v0.32.0 to the latest patched version and verify compatibility by running the full test suite.

core/types/blob_tx.go (3)

35-45: Sidecar versioning constants: clear and minimal

Definitions read well and match intended semantics.


167-178: Deep copy via slices.Clone is appropriate

Good use of slices.Clone for fixed-size array elements; Copy() looks correct.


356-424: Decoding logic separation (canonical vs network, v0 vs v1) looks sound

RLP splitting approach and payload assignment are consistent. Nice.

crypto/kzg4844/kzg4844_ckzg_nocgo.go (2)

17-17: Build tag: wasip1 exclusion added

Appropriate for non-cgo environments. LGTM.


64-75: Stub functions for cell proofs: OK for unsupported platforms

Panicking stubs maintain API surface consistently.

crypto/kzg4844/kzg4844_gokzg.go (1)

23-23: Dependency switch to go-eth-kzg

Ensure v1.4.0 exports ComputeCells, ComputeCellsAndKZGProofs, and VerifyCellKZGProofBatch as used here.

Based on learnings

Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/l2geth_ci.yml (1)

93-94: Align goimports job with Go 1.22 toolchain

All other jobs now use Go 1.22, but this step still provisions Go 1.18. That binary ships the Go 1.18 parser, which cannot format or even parse Go 1.22 syntax (e.g., range-over-integers). As soon as the repo adopts any 1.22-only feature, goimports will fail and break the CI lint job. Please bump this step to Go 1.22.x as well so the formatter matches the rest of the toolchain.

🧹 Nitpick comments (2)
core/types/blob_tx.go (2)

79-87: Consider adding validation to the constructor.

The constructor accepts a version byte and slice parameters without validation. While the validation might be deferred to usage sites, consider validating at construction time to prevent creating sidecars in invalid states.

For example:

  • Check that version is either BlobSidecarVersion0 or BlobSidecarVersion1
  • Verify that slice lengths are consistent (e.g., len(blobs) == len(commitments))
  • For version 0, check len(proofs) == len(blobs)
  • For version 1, check len(proofs) == len(blobs) * kzg4844.CellProofsPerBlob

356-423: Complex but correct decoding logic for multiple encoding formats.

The method correctly handles:

  1. Canonical encoding (blob tx without sidecars)
  2. Network encoding with legacy v0 sidecars
  3. Network encoding with versioned v1 sidecars

The logic uses RLP structure inspection to distinguish between formats, which is sound. However, the complexity could be reduced for maintainability.

Consider extracting helper functions to improve readability:

// isCanonicalEncoding checks if the input is a canonical blob tx encoding (without sidecar)
func isCanonicalEncoding(input []byte) (bool, error) {
    firstElem, _, err := rlp.SplitList(input)
    if err != nil {
        return false, err
    }
    firstElemKind, _, _, err := rlp.Split(firstElem)
    if err != nil {
        return false, err
    }
    return firstElemKind != rlp.List, nil
}

// isLegacySidecar checks if the sidecar encoding is v0 (legacy)
func isLegacySidecar(input []byte) (bool, error) {
    firstElem, _, err := rlp.SplitList(input)
    if err != nil {
        return false, err
    }
    _, _, secondElem, err := rlp.Split(firstElem)
    if err != nil {
        return false, err
    }
    secondElemKind, _, _, err := rlp.Split(secondElem)
    if err != nil {
        return false, err
    }
    return secondElemKind == rlp.List, nil
}

Then simplify the decode method:

func (tx *BlobTx) decode(input []byte) error {
    canonical, err := isCanonicalEncoding(input)
    if err != nil {
        return err
    }
    if canonical {
        return rlp.DecodeBytes(input, tx)
    }

    legacy, err := isLegacySidecar(input)
    if err != nil {
        return err
    }

    var payload blobTxWithBlobs
    if legacy {
        payload = new(blobTxWithBlobsV0)
    } else {
        payload = new(blobTxWithBlobsV1)
    }

    if err := rlp.DecodeBytes(input, payload); err != nil {
        return err
    }
    sc := new(BlobTxSidecar)
    if err := payload.assign(sc); err != nil {
        return err
    }
    *tx = *payload.tx()
    tx.Sidecar = sc
    return nil
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0322750 and e29abc7.

📒 Files selected for processing (3)
  • .github/workflows/l2geth_ci.yml (5 hunks)
  • core/types/blob_tx.go (8 hunks)
  • crypto/kzg4844/kzg4844_ckzg_cgo.go (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
crypto/kzg4844/kzg4844_ckzg_cgo.go (2)
common/hexutil/hexutil.go (1)
  • MustDecode (80-86)
crypto/kzg4844/kzg4844.go (3)
  • Blob (42-42)
  • Proof (68-68)
  • Commitment (55-55)
core/types/blob_tx.go (3)
crypto/kzg4844/kzg4844.go (6)
  • Blob (42-42)
  • Commitment (55-55)
  • Proof (68-68)
  • CellProofsPerBlob (39-39)
  • ComputeCellProofs (168-173)
  • CalcBlobHashV1 (177-186)
rlp/raw.go (2)
  • SplitList (112-121)
  • Split (63-69)
rlp/decode.go (2)
  • List (581-581)
  • DecodeBytes (91-105)
⏰ 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: test
  • GitHub Check: Analyze (go)
🔇 Additional comments (6)
core/types/blob_tx.go (6)

35-45: LGTM! Clear versioning constants with comprehensive documentation.

The version constants are well-defined with clear explanations of their intended use cases (full blob verification vs. data availability sampling).


99-113: LGTM! Excellent defensive programming with comprehensive bounds checking.

The method correctly validates:

  • Sidecar version compatibility
  • Blob index bounds
  • Proof array integrity

Error messages are descriptive and will help diagnose issues during debugging.


151-165: LGTM! Correct validation logic with descriptive error messages.

The method properly validates commitment hashes by:

  • Checking length consistency
  • Computing expected hashes using kzg4844.CalcBlobHashV1
  • Comparing against provided hashes
  • Returning informative errors on mismatch

167-178: LGTM! Correct deep-copy implementation.

The method correctly deep-copies the sidecar. The comment accurately explains that slices.Clone performs a deep copy for fixed-size byte arrays (kzg4844.Blob [131072]byte, kzg4844.Commitment [48]byte, kzg4844.Proof [48]byte).


180-227: LGTM! Clean interface design for handling versioned sidecars.

The blobTxWithBlobs interface and its implementations (blobTxWithBlobsV0, blobTxWithBlobsV1) provide a clean abstraction for encoding/decoding different sidecar versions. The assign() method on v1 correctly validates the version byte.


329-354: LGTM! Comprehensive encoding logic with proper error handling.

The switch statement correctly handles:

  • Transactions without sidecars
  • Version 0 sidecars (legacy format)
  • Version 1 sidecars (with cell proofs)
  • Unsupported versions (returns error)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e29abc7 and 43aace9.

📒 Files selected for processing (2)
  • .github/workflows/l2geth_ci.yml (5 hunks)
  • params/version.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • params/version.go
⏰ 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). (2)
  • GitHub Check: test
  • GitHub Check: Analyze (go)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
crypto/kzg4844/kzg4844_test.go (1)

234-239: Consider removing or updating the benchmark output comment.

The hardcoded benchmark output comment may become stale as the code evolves. Consider removing it or noting that it represents a specific machine/configuration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 43aace9 and 418c1ab.

📒 Files selected for processing (5)
  • Dockerfile (2 hunks)
  • Dockerfile.alltools (1 hunks)
  • Dockerfile.mockccc (1 hunks)
  • Dockerfile.mockccc.alpine (1 hunks)
  • crypto/kzg4844/kzg4844_test.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crypto/kzg4844/kzg4844_test.go (1)
crypto/kzg4844/kzg4844.go (5)
  • BlobToCommitment (110-115)
  • ComputeCellProofs (168-173)
  • Blob (42-42)
  • VerifyCellProofs (157-162)
  • Commitment (55-55)
⏰ 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). (2)
  • GitHub Check: test
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
crypto/kzg4844/kzg4844_test.go (1)

24-24: LGTM! Import updated to go-eth-kzg.

The import path change aligns with the PR objectives to merge KZG updates from upstream.

@Thegaram Thegaram merged commit 9aa8b3f into develop Oct 17, 2025
14 checks passed
@Thegaram Thegaram deleted the chore-update-kzg-to-latest-upstream-version branch October 17, 2025 05:43
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.

4 participants