feat: fetch blobs via /eth/v1/beacon/blobs from beacon node #1244
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the beacon node client to use the new /eth/v1/beacon/blobs API endpoint instead of the deprecated /eth/v1/beacon/blob_sidecars endpoint. This change is necessary for compatibility with Fusaka and PeerDAS upgrades.
- Updates API endpoint from
/eth/v1/beacon/blob_sidecarsto/eth/v1/beacon/blobs - Simplifies blob retrieval by using versioned hash filtering in the API request
- Removes manual blob hash verification logic since the new API handles filtering server-side
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rollup/da_syncer/blob_client/beacon_node_client.go | Updates beacon node client to use new blobs API with simplified response handling |
| params/version.go | Increments patch version from 5.9.5 to 5.9.6 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughPatch version bumped in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as BlobClient
participant BN as BeaconNode (blobs API)
rect rgba(220,235,255,0.35)
note over C,BN: Request single blob by versioned hash
C->>BN: GET /eth/v1/beacon/blobs?slot=<slot>&versioned_hashes=0x...
BN-->>C: 200 OK { data: ["0x..."] }
end
alt Exactly one blob returned
C->>C: decode hex -> bytes
C->>C: validate len == expectedBlobSize (lenBlobBytes)
alt length valid
C-->>Caller: return kzg4844.Blob
else invalid length
C-->>Caller: error "invalid blob length"
end
else zero or >1 blobs
C-->>Caller: error "unexpected blob count"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
♻️ Duplicate comments (1)
rollup/da_syncer/blob_client/beacon_node_client.go (1)
112-112: Fix query parameter construction.Query parameters must not be included in the path segment passed to
url.JoinPath. This can lead to improper URL encoding and API call failures. Additionally, the formatversioned_hashes=[%s]appears incorrect—the brackets should not be literal characters in the URL query string.Apply this diff to properly construct the URL with query parameters:
- blobSidecarPath, err := url.JoinPath(c.apiEndpoint, beaconNodeBlobEndpoint, fmt.Sprintf("%d?versioned_hashes=[%s]", slot, versionedHash)) + blobPath, err := url.JoinPath(c.apiEndpoint, beaconNodeBlobEndpoint, fmt.Sprintf("%d", slot)) if err != nil { return nil, fmt.Errorf("failed to join path, err: %w", err) } + + // Add query parameters + parsedURL, err := url.Parse(blobPath) + if err != nil { + return nil, fmt.Errorf("failed to parse URL, err: %w", err) + } + q := parsedURL.Query() + q.Add("versioned_hashes", versionedHash.Hex()) + parsedURL.RawQuery = q.Encode() + blobSidecarPath := parsedURL.String() + req, err := http.NewRequestWithContext(ctx, "GET", blobSidecarPath, nil)
🧹 Nitpick comments (1)
rollup/da_syncer/blob_client/beacon_node_client.go (1)
149-155: Direct conversion to kzg4844.Blob is safe. lenBlobBytes is 131072—exactly the [131072]byte size of kzg4844.Blob—and the runtime length check prevents any mismatch. You may optionally replace the conversion with an explicit copy for clarity:var b kzg4844.Blob copy(b[:], blobBytes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
params/version.go(1 hunks)rollup/da_syncer/blob_client/beacon_node_client.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rollup/da_syncer/blob_client/beacon_node_client.go (2)
common/bytes.go (1)
FromHex(29-37)crypto/kzg4844/kzg4844.go (1)
Blob(31-31)
⏰ 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 (4)
params/version.go (1)
27-27: LGTM!Version patch increment is appropriate for the new blob API feature.
rollup/da_syncer/blob_client/beacon_node_client.go (3)
31-31: LGTM!Endpoint update aligns with the new Beacon Node blobs API as specified in the PR objectives.
135-147: LGTM!The response validation logic appropriately handles edge cases where the beacon node returns zero or multiple blobs for the requested versioned hash. Error messages provide clear context for debugging.
170-172: LGTM!The
BlobsResptype correctly models the new/eth/v1/beacon/blobsAPI response structure with an array of hex-encoded blob strings.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rollup/da_syncer/blob_client/beacon_node_client.go(4 hunks)rollup/da_syncer/blob_client/blob_client.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- rollup/da_syncer/blob_client/blob_client.go
🧰 Additional context used
🧬 Code graph analysis (1)
rollup/da_syncer/blob_client/beacon_node_client.go (2)
crypto/kzg4844/kzg4844.go (1)
Blob(31-31)common/bytes.go (1)
FromHex(29-37)
⏰ 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). (5)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: build-mock-ccc-geth
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
rollup/da_syncer/blob_client/beacon_node_client.go (3)
31-31: LGTM: Endpoint updated to new API.The endpoint constant correctly reflects the migration from the deprecated
blob_sidecarsto the newblobsAPI.
158-163: LGTM: Thorough response validation.The validation ensures exactly one blob is returned, providing clear error messages for both zero and multiple blob cases. This prevents incorrect data from propagating.
186-188: LGTM: Clean response type definition.The
BlobsRespstruct appropriately models the new API response format with blobs as hex strings.
Thegaram
changed the title
1 participant
1. Purpose or design rationale of this PR
With Fusaka and PeerDAS,
/eth/v1/beacon/blob_sidecarsis now deprecated, we should use the new/eth/v1/beacon/blobsAPI instead. This API does not return the KZG commitment and proof, but it does allow filtering by blob versioned hash.References:
This PR has not been tested yet, and should not be merged at this point.
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
Bug Fixes
Chores