-
Notifications
You must be signed in to change notification settings - Fork 15
Support v1 sidecar for blob tx #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds versioned blob sidecars (v0/v1) with conversion and validation APIs, integrates cell-level KZG proof computation/verification, extends blob/commitment/proof hex IO and versioned blob-hash helpers, updates tests/benchmarks, and upgrades dependencies and Go toolchain. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as BlobTx.encode / decode
participant BlobTx
participant Sidecar as BlobTxSidecar
participant V0 as blobTxWithBlobsV0
participant V1 as blobTxWithBlobsV1
Note over Caller,BlobTx: Encoding path chooses representation
Caller->>BlobTx: encode()
BlobTx->>Sidecar: has sidecar?
alt no sidecar
BlobTx-->Caller: encode canonical tx only
else sidecar present
Sidecar-->>BlobTx: read Version
alt Version == 0
BlobTx->>V0: use v0 encoder
V0-->BlobTx: v0 payload
else Version == 1
BlobTx->>V1: use v1 encoder
V1-->BlobTx: v1 payload (includes cell proofs)
else unsupported
BlobTx-->Caller: return error
end
end
sequenceDiagram
participant Decoder as BlobTx.decode
participant Reader as input bytes
participant BlobTx
participant Sidecar
participant V0 as blobTxWithBlobsV0
participant V1 as blobTxWithBlobsV1
Decoder->>Reader: read input
Reader-->>Decoder: bytes
Decoder->>BlobTx: parse tx core
Decoder->>Sidecar: detect sidecar form (none / v0 / v1)
alt network or canonical v0
Decoder->>V0: parse v0 layout
V0-->>Sidecar: build sidecar (v0)
else v1
Decoder->>V1: parse v1 layout (cell proofs)
V1-->>Sidecar: build sidecar (v1)
end
BlobTx->>Sidecar: attach via withSidecar()
Decoder-->Caller: return BlobTx
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
There was a problem hiding this 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
🧹 Nitpick comments (8)
crypto/kzg4844/kzg4844_gokzg.go (1)
104-116: Preallocate result slice in ComputeCellProofs.Minor efficiency/readability improvement.
func gokzgComputeCellProofs(blob *Blob) ([]Proof, error) { gokzgIniter.Do(gokzgInit) _, proofs, err := context.ComputeCellsAndKZGProofs((*gokzg4844.Blob)(blob), 0) if err != nil { - return []Proof{}, err + return nil, err } - var p []Proof - for _, proof := range proofs { - p = append(p, (Proof)(proof)) - } + 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: Test failure messages could be clearer.Failure strings in cell‑proof tests refer to “at point”. Consider updating messages to “cell proofs” for accuracy.
core/types/tx_blob_test.go (2)
47-75: Size assertions look good; add error checks for MarshalBinary.Guard against unexpected encoding errors to avoid false positives.
- withBlobsEnc, _ := withBlobs.MarshalBinary() - withoutBlobsEnc, _ := withoutBlobs.MarshalBinary() + withBlobsEnc, err := withBlobs.MarshalBinary() + if err != nil { t.Fatalf("marshal with blobs: %v", err) } + withoutBlobsEnc, err := withoutBlobs.MarshalBinary() + if err != nil { t.Fatalf("marshal without blobs: %v", err) }
77-81: Init-time KZG in tests: consider lazy init to improve test ergonomics.Top-level computation of
emptyBlobCommit/emptyBlobProofcan panic early if backends/setup are unavailable. Optionally compute inTestMainor per-test.crypto/kzg4844/kzg4844.go (2)
156-165: Add fast-fail argument checks for VerifyCellProofs.Help callers catch misuse before crossing FFI boundaries.
func VerifyCellProofs(blobs []Blob, commitments []Commitment, proofs []Proof) error { + if len(blobs) != len(commitments) { + return fmt.Errorf("blobs/commitments length mismatch: %d vs %d", len(blobs), len(commitments)) + } + if want := len(blobs) * CellProofsPerBlob; len(proofs) != want { + return fmt.Errorf("invalid number of cell proofs: got %d, want %d", len(proofs), want) + } if useCKZG.Load() { return ckzgVerifyCellProofBatch(blobs, commitments, proofs) } return gokzgVerifyCellProofBatch(blobs, commitments, proofs) }
177-188: Limit CalcBlobHashV1 to SHA‑256 explicitly or document broader acceptance.Current check (
hasher.Size() == 32) admits non‑SHA‑256 32‑byte digests. Either document this or assertsha256.Newvia type assertion.core/types/tx_blob.go (2)
89-97: Guard against Blobs/Commitments length mismatch in BlobHashes.Avoid possible panic when
len(Blobs) > len(Commitments).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) + if len(sc.Blobs) != n { + // Defensive: compute up to the min length to avoid panic; callers should validate separately. + n = min(len(sc.Blobs), n) + } + h := make([]common.Hash, n) + for i := 0; i < n; i++ { + h[i] = kzg4844.CalcBlobHashV1(hasher, &sc.Commitments[i]) + } return h }
115-133: ToV1: consider validating commitments vs computed cells, or document trust boundary.If proofs are derived from
Blobsonly, ensure later verification binds them toCommitments. At minimum, add a comment stating the trust assumption, or optionally verify each blob’s cell proofs against its commitment.
📜 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 (8)
core/types/tx_blob.go(6 hunks)core/types/tx_blob_test.go(3 hunks)crypto/kzg4844/kzg4844.go(2 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)
🧰 Additional context used
🧬 Code graph analysis (7)
crypto/kzg4844/kzg4844_ckzg_nocgo.go (1)
crypto/kzg4844/kzg4844.go (3)
Blob(44-44)Commitment(57-57)Proof(70-70)
crypto/kzg4844/kzg4844_test.go (1)
crypto/kzg4844/kzg4844.go (6)
VerifyBlobProof(149-154)BlobToCommitment(112-117)ComputeCellProofs(170-175)Blob(44-44)VerifyCellProofs(159-164)Commitment(57-57)
core/types/tx_blob_test.go (5)
crypto/crypto.go (1)
GenerateKey(258-260)crypto/kzg4844/kzg4844.go (5)
Blob(44-44)BlobToCommitment(112-117)ComputeBlobProof(141-146)Commitment(57-57)Proof(70-70)core/types/transaction.go (1)
Transaction(60-68)core/types/transaction_signing.go (2)
NewCurieSigner(304-306)MustSignNewTx(131-137)core/types/tx_blob.go (3)
BlobTx(48-69)NewBlobTxSidecar(80-87)BlobSidecarVersion0(39-39)
crypto/kzg4844/kzg4844_ckzg_cgo.go (2)
common/hexutil/hexutil.go (1)
MustDecode(80-86)crypto/kzg4844/kzg4844.go (3)
Blob(44-44)Proof(70-70)Commitment(57-57)
crypto/kzg4844/kzg4844_gokzg.go (1)
crypto/kzg4844/kzg4844.go (3)
Blob(44-44)Proof(70-70)Commitment(57-57)
crypto/kzg4844/kzg4844.go (1)
common/hexutil/json.go (1)
UnmarshalFixedJSON(100-105)
core/types/tx_blob.go (3)
crypto/kzg4844/kzg4844.go (6)
Blob(44-44)Commitment(57-57)Proof(70-70)CalcBlobHashV1(179-188)CellProofsPerBlob(41-41)ComputeCellProofs(170-175)rlp/raw.go (2)
SplitList(131-140)Split(80-86)rlp/decode.go (2)
List(551-551)DecodeBytes(92-106)
🪛 OSV Scanner (2.2.3)
go.mod
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities in github.com/docker/docker
(GO-2022-0390)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions in github.com/docker/docker
(GO-2022-0985)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Container build can leak any path on the host into the container in github.com/docker/docker
(GO-2022-1107)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) Insufficiently restricted permissions on data directory in github.com/docker/docker
(GO-2024-2500)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Classic builder cache poisoning in github.com/docker/docker
(GO-2024-2512)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby Docker cp broken with debian containers in github.com/docker/docker
(GO-2024-2521)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Unexpected chmod of host files via 'docker cp' in Moby Docker Engine in github.com/docker/docker
(GO-2024-2913)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing in github.com/docker/docker
(GO-2024-2914)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby firewalld reload removes bridge network isolation in github.com/docker/docker
(GO-2025-3829)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) Insufficiently restricted permissions on data directory
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby firewalld reload removes bridge network isolation
[HIGH] 19-19: github.com/docker/docker 1.6.2: Path Traversal in Moby builder
[HIGH] 19-19: github.com/docker/docker 1.6.2: Secret insertion into debug log in Docker
[HIGH] 19-19: github.com/docker/docker 1.6.2: /sys/devices/virtual/powercap accessible by default to containers
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby's external DNS requests from 'internal' networks could lead to data exfiltration
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker Authentication Bypass
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions
[HIGH] 19-19: github.com/docker/docker 1.6.2: docker cp allows unexpected chmod of host files in Moby Docker Engine
[HIGH] 19-19: github.com/docker/docker 1.6.2: Container build can leak any path on the host into the container
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing
[HIGH] 19-19: github.com/docker/docker 1.6.2: Classic builder cache poisoning
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Unbounded memory consumption in github.com/ethereum/go-ethereum
(GO-2023-2046)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Denial of Service in github.com/ethereum/go-ethereum
(GO-2024-2819)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to DoS via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Go-Ethereum vulnerable to denial of service via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to denial of service via crafted GraphQL query
⏰ 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: Analyze (go)
🔇 Additional comments (3)
crypto/kzg4844/kzg4844_ckzg_nocgo.go (1)
64-75: Stub APIs OK for unsupported platforms.Panicking stubs for cell-proof APIs match the pattern used by other ckzg nocgo functions. No action needed.
Please ensure callers are gated by
ckzgAvailable/useCKZGso these stubs can't be reached in normal builds.crypto/kzg4844/kzg4844_test.go (1)
108-111: Benchmarks use b.Loop(): ensure Go ≥ 1.24 or fallback to b.N.CI must run Go 1.24+; otherwise switch to the standard
for i := 0; i < b.N; i++ {}.Also applies to: 127-130, 148-151, 167-170, 187-190
go.mod (1)
3-3: The review comment is based on incorrect security data and should be disregarded.Web searches confirm that go-ethereum v1.10.26 has no public CVEs affecting this version, and docker/docker v1.6.2 CVEs (CVE-2014-8179, CVE-2015-3629, CVE-2015-3631) are from 2014–2015, not recent high-severity advisories.
The b.Loop() concern is actually satisfied: the tests in
crypto/kzg4844/kzg4844_test.go(lines 108, 127, 148, 167, 187, 249) do useb.Loop(), and Go 1.25.1 running in CI is forward-compatible with the go.mod pin of 1.24.0, so this requirement is met. The version mismatch is expected and poses no practical risk.Likely an incorrect or invalid review comment.
| // ckzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | ||
| func ckzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | ||
| ckzgIniter.Do(ckzgInit) | ||
| var ( | ||
| proofs = make([]ckzg4844.Bytes48, len(cellProofs)) | ||
| commits = make([]ckzg4844.Bytes48, 0, len(cellProofs)) | ||
| cellIndices = make([]uint64, 0, len(cellProofs)) | ||
| cells = make([]ckzg4844.Cell, 0, len(cellProofs)) | ||
| ) | ||
| // Copy over the cell proofs | ||
| for i, proof := range cellProofs { | ||
| proofs[i] = (ckzg4844.Bytes48)(proof) | ||
| } | ||
| // Blow up the commitments to be the same length as the proofs | ||
| for _, commitment := range commitments { | ||
| for range gokzg4844.CellsPerExtBlob { | ||
| commits = append(commits, (ckzg4844.Bytes48)(commitment)) | ||
| } | ||
| } | ||
| // Compute the cells and cell indices | ||
| for i := range blobs { | ||
| cellsI, err := ckzg4844.ComputeCells((*ckzg4844.Blob)(&blobs[i])) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| cells = append(cells, cellsI[:]...) | ||
| for idx := range len(cellsI) { | ||
| cellIndices = append(cellIndices, uint64(idx)) | ||
| } | ||
| } | ||
|
|
||
| valid, err := ckzg4844.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if !valid { | ||
| return errors.New("invalid proof") | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix invalid range and add validations in CKZG batch verifier.
for idx := range len(cellsI)is invalid Go; range overcellsIinstead.- Add early length checks to fail fast on malformed inputs.
func ckzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error {
ckzgIniter.Do(ckzgInit)
- var (
+ // Validate inputs early.
+ if len(blobs) != len(commitments) {
+ return fmt.Errorf("blobs/commitments length mismatch: %d vs %d", len(blobs), len(commitments))
+ }
+ cellsPer := len(gokzg4844.CellsPerExtBlob)
+ want := len(commitments) * cellsPer
+ if len(cellProofs) != want {
+ return fmt.Errorf("invalid number of cell proofs: got %d, want %d", len(cellProofs), want)
+ }
+
+ var (
proofs = make([]ckzg4844.Bytes48, len(cellProofs))
- commits = make([]ckzg4844.Bytes48, 0, len(cellProofs))
- cellIndices = make([]uint64, 0, len(cellProofs))
- cells = make([]ckzg4844.Cell, 0, len(cellProofs))
+ commits = make([]ckzg4844.Bytes48, len(cellProofs))
+ cellIndices = make([]uint64, 0, len(cellProofs))
+ cells = make([]ckzg4844.Cell, 0, len(cellProofs))
)
// Copy over the cell proofs
for i, proof := range cellProofs {
proofs[i] = (ckzg4844.Bytes48)(proof)
}
// Blow up the commitments to be the same length as the proofs
- for _, commitment := range commitments {
- for range gokzg4844.CellsPerExtBlob {
- commits = append(commits, (ckzg4844.Bytes48)(commitment))
- }
- }
+ idx := 0
+ for _, commitment := range commitments {
+ for j := 0; j < cellsPer; j++ {
+ commits[idx] = (ckzg4844.Bytes48)(commitment)
+ idx++
+ }
+ }
// Compute the cells and cell indices
for i := range blobs {
cellsI, err := ckzg4844.ComputeCells((*ckzg4844.Blob)(&blobs[i]))
if err != nil {
return err
}
cells = append(cells, cellsI[:]...)
- for idx := range len(cellsI) {
- cellIndices = append(cellIndices, uint64(idx))
- }
+ for idx := range cellsI {
+ cellIndices = append(cellIndices, uint64(idx))
+ }
}
valid, err := ckzg4844.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ckzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | |
| func ckzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | |
| ckzgIniter.Do(ckzgInit) | |
| var ( | |
| proofs = make([]ckzg4844.Bytes48, len(cellProofs)) | |
| commits = make([]ckzg4844.Bytes48, 0, len(cellProofs)) | |
| cellIndices = make([]uint64, 0, len(cellProofs)) | |
| cells = make([]ckzg4844.Cell, 0, len(cellProofs)) | |
| ) | |
| // Copy over the cell proofs | |
| for i, proof := range cellProofs { | |
| proofs[i] = (ckzg4844.Bytes48)(proof) | |
| } | |
| // Blow up the commitments to be the same length as the proofs | |
| for _, commitment := range commitments { | |
| for range gokzg4844.CellsPerExtBlob { | |
| commits = append(commits, (ckzg4844.Bytes48)(commitment)) | |
| } | |
| } | |
| // Compute the cells and cell indices | |
| for i := range blobs { | |
| cellsI, err := ckzg4844.ComputeCells((*ckzg4844.Blob)(&blobs[i])) | |
| if err != nil { | |
| return err | |
| } | |
| cells = append(cells, cellsI[:]...) | |
| for idx := range len(cellsI) { | |
| cellIndices = append(cellIndices, uint64(idx)) | |
| } | |
| } | |
| valid, err := ckzg4844.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs) | |
| if err != nil { | |
| return err | |
| } | |
| if !valid { | |
| return errors.New("invalid proof") | |
| } | |
| return nil | |
| } | |
| // ckzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | |
| func ckzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | |
| ckzgIniter.Do(ckzgInit) | |
| // Validate inputs early. | |
| if len(blobs) != len(commitments) { | |
| return fmt.Errorf("blobs/commitments length mismatch: %d vs %d", len(blobs), len(commitments)) | |
| } | |
| cellsPer := len(gokzg4844.CellsPerExtBlob) | |
| want := len(commitments) * cellsPer | |
| if len(cellProofs) != want { | |
| return fmt.Errorf("invalid number of cell proofs: got %d, want %d", len(cellProofs), want) | |
| } | |
| var ( | |
| proofs = make([]ckzg4844.Bytes48, len(cellProofs)) | |
| commits = make([]ckzg4844.Bytes48, len(cellProofs)) | |
| cellIndices = make([]uint64, 0, len(cellProofs)) | |
| cells = make([]ckzg4844.Cell, 0, len(cellProofs)) | |
| ) | |
| // Copy over the cell proofs | |
| for i, proof := range cellProofs { | |
| proofs[i] = (ckzg4844.Bytes48)(proof) | |
| } | |
| // Blow up the commitments to be the same length as the proofs | |
| idx := 0 | |
| for _, commitment := range commitments { | |
| for j := 0; j < cellsPer; j++ { | |
| commits[idx] = (ckzg4844.Bytes48)(commitment) | |
| idx++ | |
| } | |
| } | |
| // Compute the cells and cell indices | |
| for i := range blobs { | |
| cellsI, err := ckzg4844.ComputeCells((*ckzg4844.Blob)(&blobs[i])) | |
| if err != nil { | |
| return err | |
| } | |
| cells = append(cells, cellsI[:]...) | |
| for idx := range cellsI { | |
| cellIndices = append(cellIndices, uint64(idx)) | |
| } | |
| } | |
| valid, err := ckzg4844.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs) | |
| if err != nil { | |
| return err | |
| } | |
| if !valid { | |
| return errors.New("invalid proof") | |
| } | |
| return nil | |
| } |
| // gokzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | ||
| func gokzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | ||
| gokzgIniter.Do(gokzgInit) | ||
|
|
||
| var ( | ||
| proofs = make([]gokzg4844.KZGProof, len(cellProofs)) | ||
| commits = make([]gokzg4844.KZGCommitment, 0, len(cellProofs)) | ||
| cellIndices = make([]uint64, 0, len(cellProofs)) | ||
| cells = make([]*gokzg4844.Cell, 0, len(cellProofs)) | ||
| ) | ||
| // Copy over the cell proofs | ||
| for i, proof := range cellProofs { | ||
| proofs[i] = gokzg4844.KZGProof(proof) | ||
| } | ||
| // Blow up the commitments to be the same length as the proofs | ||
| for _, commitment := range commitments { | ||
| for range gokzg4844.CellsPerExtBlob { | ||
| commits = append(commits, gokzg4844.KZGCommitment(commitment)) | ||
| } | ||
| } | ||
| // Compute the cell and cell indices | ||
| for i := range blobs { | ||
| cellsI, err := context.ComputeCells((*gokzg4844.Blob)(&blobs[i]), 2) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| cells = append(cells, cellsI[:]...) | ||
| for idx := range len(cellsI) { | ||
| cellIndices = append(cellIndices, uint64(idx)) | ||
| } | ||
| } | ||
| return context.VerifyCellKZGProofBatch(commits, cellIndices, cells[:], proofs) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix compile errors and add validations in batch cell-proof verifier.
- Incorrect range:
for idx := range len(cellsI)is invalid Go; should range over the slice/array. - Wrong element type: building
cellsas[]*gokzg4844.Cellmismatches typical API expecting[]gokzg4844.Cell. - Missing sanity checks for input lengths.
Apply this diff:
func gokzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error {
gokzgIniter.Do(gokzgInit)
- var (
- proofs = make([]gokzg4844.KZGProof, len(cellProofs))
- commits = make([]gokzg4844.KZGCommitment, 0, len(cellProofs))
- cellIndices = make([]uint64, 0, len(cellProofs))
- cells = make([]*gokzg4844.Cell, 0, len(cellProofs))
- )
+ // Validate inputs early.
+ if len(blobs) != len(commitments) {
+ return fmt.Errorf("blobs/commitments length mismatch: %d vs %d", len(blobs), len(commitments))
+ }
+ cellsPer := len(gokzg4844.CellsPerExtBlob)
+ want := len(commitments) * cellsPer
+ if len(cellProofs) != want {
+ return fmt.Errorf("invalid number of cell proofs: got %d, want %d", len(cellProofs), want)
+ }
+
+ var (
+ proofs = make([]gokzg4844.KZGProof, len(cellProofs))
+ commits = make([]gokzg4844.KZGCommitment, len(cellProofs))
+ cellIndices = make([]uint64, 0, len(cellProofs))
+ cells = make([]gokzg4844.Cell, 0, len(cellProofs))
+ )
// Copy over the cell proofs
for i, proof := range cellProofs {
proofs[i] = gokzg4844.KZGProof(proof)
}
// Blow up the commitments to be the same length as the proofs
- for _, commitment := range commitments {
- for range gokzg4844.CellsPerExtBlob {
- commits = append(commits, gokzg4844.KZGCommitment(commitment))
- }
- }
+ idx := 0
+ for _, commitment := range commitments {
+ for j := 0; j < cellsPer; j++ {
+ commits[idx] = gokzg4844.KZGCommitment(commitment)
+ idx++
+ }
+ }
// Compute the cell and cell indices
for i := range blobs {
- cellsI, err := context.ComputeCells((*gokzg4844.Blob)(&blobs[i]), 2)
+ cellsI, err := context.ComputeCells((*gokzg4844.Blob)(&blobs[i]), 2)
if err != nil {
return err
}
- cells = append(cells, cellsI[:]...)
- for idx := range len(cellsI) {
- cellIndices = append(cellIndices, uint64(idx))
- }
+ cells = append(cells, cellsI[:]...)
+ for idx := range cellsI {
+ cellIndices = append(cellIndices, uint64(idx))
+ }
}
- return context.VerifyCellKZGProofBatch(commits, cellIndices, cells[:], proofs)
+ return context.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // gokzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | |
| func gokzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | |
| gokzgIniter.Do(gokzgInit) | |
| var ( | |
| proofs = make([]gokzg4844.KZGProof, len(cellProofs)) | |
| commits = make([]gokzg4844.KZGCommitment, 0, len(cellProofs)) | |
| cellIndices = make([]uint64, 0, len(cellProofs)) | |
| cells = make([]*gokzg4844.Cell, 0, len(cellProofs)) | |
| ) | |
| // Copy over the cell proofs | |
| for i, proof := range cellProofs { | |
| proofs[i] = gokzg4844.KZGProof(proof) | |
| } | |
| // Blow up the commitments to be the same length as the proofs | |
| for _, commitment := range commitments { | |
| for range gokzg4844.CellsPerExtBlob { | |
| commits = append(commits, gokzg4844.KZGCommitment(commitment)) | |
| } | |
| } | |
| // Compute the cell and cell indices | |
| for i := range blobs { | |
| cellsI, err := context.ComputeCells((*gokzg4844.Blob)(&blobs[i]), 2) | |
| if err != nil { | |
| return err | |
| } | |
| cells = append(cells, cellsI[:]...) | |
| for idx := range len(cellsI) { | |
| cellIndices = append(cellIndices, uint64(idx)) | |
| } | |
| } | |
| return context.VerifyCellKZGProofBatch(commits, cellIndices, cells[:], proofs) | |
| } | |
| // gokzgVerifyCellProofBatch verifies that the blob data corresponds to the provided commitment. | |
| func gokzgVerifyCellProofBatch(blobs []Blob, commitments []Commitment, cellProofs []Proof) error { | |
| gokzgIniter.Do(gokzgInit) | |
| // Validate inputs early. | |
| if len(blobs) != len(commitments) { | |
| return fmt.Errorf("blobs/commitments length mismatch: %d vs %d", len(blobs), len(commitments)) | |
| } | |
| cellsPer := len(gokzg4844.CellsPerExtBlob) | |
| want := len(commitments) * cellsPer | |
| if len(cellProofs) != want { | |
| return fmt.Errorf("invalid number of cell proofs: got %d, want %d", len(cellProofs), want) | |
| } | |
| var ( | |
| proofs = make([]gokzg4844.KZGProof, len(cellProofs)) | |
| commits = make([]gokzg4844.KZGCommitment, len(cellProofs)) | |
| cellIndices = make([]uint64, 0, len(cellProofs)) | |
| cells = make([]gokzg4844.Cell, 0, len(cellProofs)) | |
| ) | |
| // Copy over the cell proofs | |
| for i, proof := range cellProofs { | |
| proofs[i] = gokzg4844.KZGProof(proof) | |
| } | |
| // Blow up the commitments to be the same length as the proofs | |
| idx := 0 | |
| for _, commitment := range commitments { | |
| for j := 0; j < cellsPer; j++ { | |
| commits[idx] = gokzg4844.KZGCommitment(commitment) | |
| idx++ | |
| } | |
| } | |
| // Compute the cell and cell indices | |
| for i := range blobs { | |
| cellsI, err := context.ComputeCells((*gokzg4844.Blob)(&blobs[i]), 2) | |
| if err != nil { | |
| return err | |
| } | |
| cells = append(cells, cellsI[:]...) | |
| for idx := range cellsI { | |
| cellIndices = append(cellIndices, uint64(idx)) | |
| } | |
| } | |
| return context.VerifyCellKZGProofBatch(commits, cellIndices, cells, proofs) | |
| } |
🤖 Prompt for AI Agents
In crypto/kzg4844/kzg4844_gokzg.go around lines 118-150, fix the invalid range
and type mismatches and add simple sanity checks: change cells to
[]gokzg4844.Cell (not []*gokzg4844.Cell); expand each commitment with a
conventional for i := 0; i < gokzg4844.CellsPerExtBlob; i++ { commits =
append(commits, gokzg4844.KZGCommitment(commitment)) }; when computing cells,
capture the current start index (start := len(cells)), append the returned
array/slice via cells = append(cells, cellsI[:]...), then append cellIndices
with absolute positions using for idx := range cellsI { cellIndices =
append(cellIndices, uint64(start+idx)) }; add sanity checks before work (e.g.,
ensure len(cellProofs) == len(commits) after expansion and that
len(blobs)*gokzg4844.CellsPerExtBlob == len(cellProofs)) and return errors if
they fail; then call context.VerifyCellKZGProofBatch(commits, cellIndices,
cells, proofs).
There was a problem hiding this 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)
consensus/misc/eip4844/eip4844.go (1)
11-11: Remove unused global variable.The
blobGaspriceUpdateFractionvariable is no longer used after the CalcBlobFee function was updated to acceptupdateFractionas a parameter.Apply this diff to remove the dead code:
var ( minBlobGasPrice = big.NewInt(params.BlobTxMinBlobGasprice) - blobGaspriceUpdateFraction = big.NewInt(params.BlobTxBlobGaspriceUpdateFraction) )
📜 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)
consensus/misc/eip4844/eip4844.go(1 hunks)consensus/misc/eip4844/eip4844_test.go(1 hunks)go.mod(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
consensus/misc/eip4844/eip4844_test.go (2)
consensus/misc/eip4844/eip4844.go (1)
CalcBlobFee(25-27)params/protocol_params.go (1)
BlobTxBlobGaspriceUpdateFraction(170-170)
🪛 OSV Scanner (2.2.3)
go.mod
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities in github.com/docker/docker
(GO-2022-0390)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions in github.com/docker/docker
(GO-2022-0985)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Container build can leak any path on the host into the container in github.com/docker/docker
(GO-2022-1107)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) Insufficiently restricted permissions on data directory in github.com/docker/docker
(GO-2024-2500)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Classic builder cache poisoning in github.com/docker/docker
(GO-2024-2512)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby Docker cp broken with debian containers in github.com/docker/docker
(GO-2024-2521)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Unexpected chmod of host files via 'docker cp' in Moby Docker Engine in github.com/docker/docker
(GO-2024-2913)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing in github.com/docker/docker
(GO-2024-2914)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby firewalld reload removes bridge network isolation in github.com/docker/docker
(GO-2025-3829)
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) started with non-empty inheritable Linux process capabilities
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) Insufficiently restricted permissions on data directory
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby firewalld reload removes bridge network isolation
[HIGH] 19-19: github.com/docker/docker 1.6.2: Path Traversal in Moby builder
[HIGH] 19-19: github.com/docker/docker 1.6.2: Secret insertion into debug log in Docker
[HIGH] 19-19: github.com/docker/docker 1.6.2: /sys/devices/virtual/powercap accessible by default to containers
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby's external DNS requests from 'internal' networks could lead to data exfiltration
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker Authentication Bypass
[HIGH] 19-19: github.com/docker/docker 1.6.2: Docker supplementary group permissions not set up properly, allowing attackers to bypass primary group restrictions
[HIGH] 19-19: github.com/docker/docker 1.6.2: docker cp allows unexpected chmod of host files in Moby Docker Engine
[HIGH] 19-19: github.com/docker/docker 1.6.2: Container build can leak any path on the host into the container
[HIGH] 19-19: github.com/docker/docker 1.6.2: Moby (Docker Engine) is vulnerable to Ambiguous OCI manifest parsing
[HIGH] 19-19: github.com/docker/docker 1.6.2: Classic builder cache poisoning
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Unbounded memory consumption in github.com/ethereum/go-ethereum
(GO-2023-2046)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Denial of Service in github.com/ethereum/go-ethereum
(GO-2024-2819)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to DoS via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Go-Ethereum vulnerable to denial of service via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to denial of service via crafted GraphQL query
⏰ 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: Analyze (go)
🔇 Additional comments (5)
consensus/misc/eip4844/eip4844_test.go (1)
71-71: LGTM!The test correctly adapts to the new
CalcBlobFeesignature by passingparams.BlobTxBlobGaspriceUpdateFractionas the second argument, maintaining the same behavior as the previous implementation.consensus/misc/eip4844/eip4844.go (1)
25-26: Breaking API change properly handled in-codebase; verify external dependency impact.The
CalcBlobFeefunction signature change from a single parameter to acceptingupdateFractionas a second parameter is a breaking change. Verification confirms the only in-codebase caller (in the test file) has been correctly updated to pass both arguments.However, since this is a public API, consider whether this function is exposed in external packages or imports that would be affected by this breaking change.
go.mod (3)
16-16: Clarify the rationale for ethereum/go-ethereum v1.10.26 given PR objective to sync recent features.The PR states it "syncs features from ethereum/go-ethereum" for EIP-4844 blob sidecar support, yet the pinned version (v1.10.26) appears significantly behind current upstream. Confirm whether:
- Features are backported from a newer version to v1.10.26, or
- v1.10.26 is intended and already contains the necessary EIP-4844 support, or
- A version update is required to align with upstream.
Also note the replacement of crate-crypto/go-kzg-4844 with ethereum/go-eth-kzg and the addition of ethereum/c-kzg-4844/v2 aligns with the cell-proofs feature; verify this transition is complete across the codebase.
Also applies to: 22-23
15-15: Approve cryptography and standard library updates.The updates to gnark-crypto (v0.12.1 → v0.16.0), testify (v1.8.4 → v1.10.0), and all x/* packages (crypto, sync, sys, text, tools) are in line with Go 1.24.0 compatibility and modern best practices for security and stability.
Also applies to: 58-58, 61-66
19-19: Confirm docker/docker v1.6.2 scope before prioritizing upgrade.docker/docker v1.6.2 does contain the reported HIGH severity vulnerabilities. However, it's used only in
p2p/simulations/adapters/(for process re-execution in simulation scenarios) andcmd/p2psim/(a development utility tool), with no evidence of production code importing these packages.Verify: Is
cmd/p2psimorp2p/simulationsshipped/used at runtime in production? If dev-only, the risk is limited to development environments. If shipped, upgrade to a patched version promptly.
| module github.com/morph-l2/go-ethereum | ||
|
|
||
| go 1.21 | ||
| go 1.24.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify Go 1.24.0 compatibility with cryptography library updates.
The Go toolchain upgrade from 1.21 to 1.24.0 is substantial. With concurrent updates to gnark-crypto (v0.16.0), go-eth-kzg (v1.4.0), and c-kzg-4844/v2 (v2.1.5), verify that all dependencies compile and function correctly with the new Go version.
🏁 Script executed:
#!/bin/bash
# Description: Verify that dependencies are compatible with Go 1.24.0 and resolve correctly.
# Check for build compatibility and any unresolved indirect dependencies.
# Ensure go.mod is properly formatted and all modules resolve
go mod tidy
go mod verify
# Attempt a build to catch any Go version compatibility issues
go build ./...
# Check for any discrepancies in module graph
go mod graph | head -20Length of output: 10018
Go 1.24.0 introduces breaking changes that prevent the code from compiling.
The build fails with two distinct errors:
-
mobile/geth.go:152–undefined: debug.Memsize: Thedebug.Memsizefunction was removed from Go's standard library. This API existed in Go 1.21 but is no longer available in Go 1.24.0. This originates from the vendoredethereum/go-ethereumv1.10.26 code. -
golang.org/x/tools– Invalid array length: The pinned versionv0.21.1-0.20240508182429-e35e4ccd0d2dhas an invalid array length calculation (-delta * deltaevaluates to a negative constant), indicating this prerelease version is incompatible with Go 1.24.0.
Actions required:
- Update
github.com/ethereum/go-ethereumto a version compatible with Go 1.24, or backport thedebug.Memsizeremoval inmobile/geth.go - Update
golang.org/x/toolsto a stable release compatible with Go 1.24 (e.g.,v0.22.0or later once available)
🤖 Prompt for AI Agents
In go.mod around line 3, the project is set to Go 1.24.0 which exposes two
incompatibilities: vendored github.com/ethereum/go-ethereum (uses removed
debug.Memsize) and golang.org/x/tools pinned to a prerelease with an invalid
array length. Fix by updating the go-ethereum dependency to a release that
supports Go 1.24 (or edit mobile/geth.go to remove/replace debug.Memsize usage
if you must patch locally), and update golang.org/x/tools to a stable release
compatible with Go 1.24 (e.g., v0.22.0 or later); then run go get
github.com/ethereum/go-ethereum@<new-version> and go get
golang.org/x/tools@v0.22.0 (or chosen stable tag), run go mod tidy and rebuild
to verify compilation.
| github.com/edsrzf/mmap-go v1.0.0 | ||
| github.com/ethereum/c-kzg-4844/bindings/go v0.0.0-20230126171313-363c7d7593b4 | ||
| github.com/ethereum/c-kzg-4844/v2 v2.1.5 | ||
| github.com/ethereum/go-ethereum v1.10.26 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Address HIGH severity vulnerabilities in ethereum/go-ethereum 1.10.26.
OSV Scanner reports multiple HIGH severity issues:
- Unbounded memory consumption (GO-2023-2046)
- Denial of Service (GO-2024-2819)
- Malicious p2p message handling vulnerabilities (GHSA-4xc9-8hmq-j652, GHSA-ppjg-v974-84cm, GHSA-v9jh-j8px-98vq)
These are runtime vulnerabilities. Verify whether mitigations are in place (e.g., request size limits, message validation, network isolation) or if a version upgrade is feasible.
🌐 Web query:
What are the latest patches and security advisories for ethereum/go-ethereum versions after 1.10.26?
💡 Result:
Summary — notable patches & security advisories for ethereum/go-ethereum (post‑v1.10.26)
- Current stable line (latest published builds): geth 1.16.x (stable channel as of Oct 16, 2025). [1]
- Security advisory GHSA-q26p-9cq4-7fc2 (CVE‑2025‑24883 / GO‑2025‑3436): a DoS via malformed p2p handshake (introduced in 1.14.0); fixed in geth 1.14.13 and later — operators are advised to upgrade to 1.14.13+ (or any later stable 1.15/1.16 builds). Published Jan 30, 2025. [2][3]
- Project security page / vulnerability feed: GitHub security overview lists the GHSA entries (including the 2025 DoS advisory) and points to the repository’s vulnerabilities.json and the built‑in geth version‑check for automated checking. Use geth version-check to verify whether your installed version is affected. [4][2]
If you want, I can:
- list every intermediate release (1.11 → 1.12 → … → 1.16) with their release dates and changelog links; or
- run a version-check for a specific installed geth version and report whether it’s affected.
Sources:
[1] geth downloads (stable releases).
[2] GHSA-q26p-9cq4-7fc2 / OSV (CVE‑2025‑24883) advisory and patch info.
[3] News coverage / release notes for geth 1.14.13.
[4] ethereum/go-ethereum GitHub — Security / vulnerability disclosure.
Upgrade ethereum/go-ethereum to v1.14.13 or later; v1.10.26 is severely outdated and unpatched.
The latest stable geth version is 1.16.x (as of Oct 16, 2025), and v1.10.26 is 5+ major versions behind. Recent security advisories continue to be published; upgrading to v1.14.13 or later versions (1.15.x/1.16.x) is strongly advised. Use geth's built-in version-check tool to verify your installed version is not affected by known vulnerabilities.
🧰 Tools
🪛 OSV Scanner (2.2.3)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Unbounded memory consumption in github.com/ethereum/go-ethereum
(GO-2023-2046)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Denial of Service in github.com/ethereum/go-ethereum
(GO-2024-2819)
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to DoS via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: Go-Ethereum vulnerable to denial of service via malicious p2p message
[HIGH] 23-23: github.com/ethereum/go-ethereum 1.10.26: go-ethereum vulnerable to denial of service via crafted GraphQL query
🤖 Prompt for AI Agents
In go.mod around line 23, the project pins github.com/ethereum/go-ethereum to
v1.10.26 which is outdated and unpatched; update the dependency to at least
v1.14.13 (preferably the latest 1.15.x/1.16.x stable release). Edit go.mod to
require github.com/ethereum/go-ethereum v1.14.13 or newer, run the module update
commands (go get github.com/ethereum/go-ethereum@<version> and go mod tidy),
then rebuild and run the full test suite; finally verify the binary’s runtime
version with geth's built-in version-check tool to ensure the upgrade succeeded
and no new API changes break the code.
1. Purpose or design rationale of this PR
Syncd the features from ethereum go-ethereum.
Add Version1 BlobSidecar with cell proofs support for EIP-4844
Introduces Version1 BlobSidecar supporting granular cell proofs for individual blob verification.
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:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Improvements
Tests