-
Notifications
You must be signed in to change notification settings - Fork 201
Add getBlobs endpoint #546
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
557d749
Add getBlobs endpoint
nflaig aa3b921
Add changelog entry
nflaig a70801d
Merge branch 'master' into nflaig/get-blobs
nflaig a63410b
Merge branch 'master' into nflaig/get-blobs
nflaig 269b406
Use versioned_hashes instead of indices for filtering
nflaig bcbb342
Update description
nflaig File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| get: | ||
| operationId: getBlobs | ||
| summary: Get blobs | ||
| description: | | ||
| Retrieves blobs for a given block id. | ||
| Depending on `Accept` header it can be returned either as json or as bytes serialized by SSZ. | ||
|
|
||
| If the `indices` parameter is specified, only the blobs with the specified indices will be returned. There are no guarantees | ||
| for the returned blobs in terms of ordering. | ||
|
|
||
| After the Fulu fork, only supernodes (which custody all data columns) are required to return blobs. Clients may implement | ||
| blob reconstruction logic for non-super nodes. | ||
| tags: | ||
| - Beacon | ||
| parameters: | ||
| - name: block_id | ||
| in: path | ||
| required: true | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/parameters/BlockId" | ||
| - name: indices | ||
| in: query | ||
| description: Array of indices for blobs to request for in the specified block. Returns all blobs in the block if not specified. | ||
| required: false | ||
| schema: | ||
| type: array | ||
| uniqueItems: true | ||
| items: | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Uint64" | ||
| responses: | ||
| "200": | ||
| description: "Successful response" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| title: GetBlobsResponse | ||
| type: object | ||
| required: [execution_optimistic, finalized, data] | ||
| properties: | ||
| execution_optimistic: | ||
nflaig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/ExecutionOptimistic" | ||
| finalized: | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Finalized" | ||
| data: | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Blobs" | ||
nflaig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `List[Blob, MAX_BLOB_COMMITMENTS_PER_BLOCK]` bytes. Use Accept header to choose this response type" | ||
| "400": | ||
| description: "The block ID supplied could not be parsed" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/ErrorMessage" | ||
| example: | ||
| code: 400 | ||
| message: "Invalid block ID: current" | ||
| "404": | ||
| description: "Block not found" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/ErrorMessage" | ||
| example: | ||
| code: 404 | ||
| message: "Block not found" | ||
| "406": | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/responses/NotAcceptable" | ||
| "500": | ||
| $ref: "../../../beacon-node-oapi.yaml#/components/responses/InternalError" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I'm not sure how useful is it for users to get a list of unordered blobs without indices. If this is intended for L2s, we might as well implement retrieval by version hashes?
#332
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.
seems like from the previous discussion it might be better, would we still keep the
block_idin that case as noted in #332 (comment)? We index data columns sidecars by block root or slot so dropping this would require additional indexingThere 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.
yep you're right, I'm bringing this up because the intent of this endpoint is for L2 users - I'm not exactly sure how they use it, but i assume they have
block_idandversion_hash, and so they can retrieve the blob they need?With the
getBlobSidecarendpoint, they're able to do this by computing theversion_hashof individual sidecars fromkzg_commitment- but without the sidecar metadata this won't work anymore.I see a few possible options:
block.kzg_commitmentsI think #3 may be the least work on the beacon node, and also keep the API clean?
Uh oh!
There was an error while loading. Please reload this page.
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.
Not sure how they use it either but since the blob sidecars endpoint required
block_idalready it should be fine(3) is the simplest for us and that's how I implemented it for now but using
version_hashesfor filtering should be easy change, if we go with (3) it makes sense to add explicit note that an ordered list must be returnedThere 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.
For Scroll, it would be more convenient to have a "get blob(s) by versioned hash(es)" endpoint (without block id).
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.
It seems like from @Tristan-Wilson 's description slot numbers have concrete debugging and diagnostic advantages, too.
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.
In that case, from our perspective, I don't see much benefit to adding a new API.
Or is the idea here that later when we'll have much more blobs in each block, the response would grow too large? In that case, filtering based on versioned hashes would be useful.
Uh oh!
There was an error while loading. Please reload this page.
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.
the reason for the new api is that we no longer use and store
BlobSidecarson the CL side once Fusaka goes live, see #524 (comment)yeah, just to avoid returning data you don't need and with increasing blob count this matters more
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.
Chiming in a little - I understand the OP folks are having some issues with plans to deprecate the blob_sidecars endpoint
Fulu deprecates the /eth/v1/beacon/blob_sidecars Beacon node endpoint, replacing it with /eth/v1/beacon/blobs. However, this new endpoint only provides the raw blobs, without the new kzg commitments (for cell proofs), without which verification doesn't work. So we need to investigate how we want to support this new endpoint for untrusted beacon nodes.This is an endpoint we are currently using in production for data indexing as well, and would appreciate if it could be retained.
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.
The reason for deprecating the
blob_sidecarsendpoint is because blob sidecars no longer exist after Fulu, we only have data column sidecars.I'd rather update the response of the blobs endpoint to include additional data as previously suggested by @jimmygchen