-
Notifications
You must be signed in to change notification settings - Fork 171
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
Add blob download endpoint (getBlobs) #286
Changes from 16 commits
5502da1
85a715d
e702695
fd02cd2
ca3dd1b
09ca1dd
709ccd6
652b865
f300afe
b1bbe28
30b2d05
1d8e599
39efcbe
1ec6931
fe595c3
27ac4b8
7f5e792
7ee3525
4ccaaa6
4ded8d7
61b4663
27e4451
e3dfda2
354ff9f
eb7bc1f
df43224
775e93c
a6d1c36
096aadc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
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. | ||
It is recommended that blob retrieval for finalized blocks should by done by slot else the underlying index might not be available. | ||
jimmygchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
tags: | ||
- Beacon | ||
parameters: | ||
jimmygchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- 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 speicfied. | ||
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 | ||
properties: | ||
data: | ||
type: array | ||
items: | ||
oneOf: | ||
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlobSidecar" | ||
minItems: 0 | ||
maxItems: 4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will likely have to change in line with ethereum/consensus-specs#3338 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the latest discussions & changes in the PR, it looks like we may not need to change this value here. A new |
||
application/octet-stream: | ||
schema: | ||
description: "SSZ serialized block bytes. Use Accept header to choose this response type" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's this returning? eg. if i set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for raising this, great point - I think it would be good to be consistent with other endpoints that accepts array type query params (e.g. `getStateValidators), i.e. not guaranteeing order, unless there are specific reasons for guaranteeing the order when indices are specified? From
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i haven't looked at the SSZ, hopefully its easy to get the blob id from the object.. assuming thats fine... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just wondering if the ordering is important, so if i query some weird order, do i need to process them in some specific order, and is that possible based on the return data if we're not returning in the same order... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if the ordering is important tbh, but we do have |
||
"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: "Blob sidecar not found" | ||
content: | ||
application/json: | ||
schema: | ||
$ref: "../../../beacon-node-oapi.yaml#/components/schemas/ErrorMessage" | ||
example: | ||
code: 404 | ||
message: "Blob sidecar not found" | ||
jimmygchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"500": | ||
$ref: "../../../beacon-node-oapi.yaml#/components/responses/InternalError" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
Deneb: | ||
BlobSidecar: | ||
type: object | ||
description: "The `BlobSidecar` object from the Deneb CL spec." | ||
properties: | ||
block_root: | ||
$ref: "../primitive.yaml#/Root" | ||
index: | ||
$ref: "../primitive.yaml#/Uint64" | ||
slot: | ||
$ref: "../primitive.yaml#/Uint64" | ||
block_parent_root: | ||
$ref: "../primitive.yaml#/Root" | ||
proposer_index: | ||
$ref: "../primitive.yaml#/Uint64" | ||
blob: | ||
$ref: "../primitive.yaml#/Blob" | ||
kzg_commitment: | ||
$ref: '../primitive.yaml#/KZGCommitment' | ||
kzg_proof: | ||
$ref: '../primitive.yaml#/KZGProof' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
Deneb: | ||
BeaconBlockBodyCommon: | ||
# An abstract object to collect the common fields between the BeaconBlockBody and the BlindedBeaconBlockBody objects | ||
type: object | ||
description: "The [`BeaconBlockBody`](https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#beaconblockbody) object from the CL Deneb spec." | ||
properties: | ||
randao_reveal: | ||
allOf: | ||
- $ref: '../primitive.yaml#/Signature' | ||
- description: "The RanDAO reveal value provided by the validator." | ||
jimmygchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
eth1_data: | ||
$ref: '../eth1.yaml#/Eth1Data' | ||
graffiti: | ||
$ref: '../primitive.yaml#/Graffiti' | ||
proposer_slashings: | ||
type: array | ||
items: | ||
$ref: '../proposer_slashing.yaml#/ProposerSlashing' | ||
attester_slashings: | ||
type: array | ||
items: | ||
$ref: '../attester_slashing.yaml#/AttesterSlashing' | ||
attestations: | ||
type: array | ||
items: | ||
$ref: '../attestation.yaml#/Attestation' | ||
deposits: | ||
type: array | ||
items: | ||
$ref: '../deposit.yaml#/Deposit' | ||
voluntary_exits: | ||
type: array | ||
items: | ||
$ref: '../voluntary_exit.yaml#/SignedVoluntaryExit' | ||
sync_aggregate: | ||
$ref: '../altair/sync_aggregate.yaml#/Altair/SyncAggregate' | ||
bls_to_execution_changes: | ||
type: array | ||
items: | ||
$ref: '../bls_to_execution_change.yaml#/SignedBLSToExecutionChange' | ||
blob_kzg_commitments: | ||
type: array | ||
items: | ||
$ref: '../primitive.yaml#/KZGCommitment' | ||
|
||
BeaconBlockBody: | ||
allOf: | ||
- $ref: '#/Deneb/BeaconBlockBodyCommon' | ||
- type: object | ||
properties: | ||
execution_payload: | ||
$ref: './execution_payload.yaml#/Deneb/ExecutionPayload' | ||
|
||
BeaconBlock: | ||
description: "The [`BeaconBlock`](https://github.com/ethereum/consensus-specs/blob/master/specs/Deneb/beacon-chain.md#beaconblock) object from the CL Deneb spec." | ||
allOf: | ||
- $ref: '../altair/block.yaml#/Altair/BeaconBlockCommon' | ||
- type: object | ||
properties: | ||
body: | ||
$ref: '#/Deneb/BeaconBlockBody' | ||
|
||
SignedBeaconBlock: | ||
type: object | ||
description: "The [`SignedBeaconBlock`](https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#signedbeaconblock) object envelope from the CL Deneb spec." | ||
properties: | ||
message: | ||
$ref: "#/Deneb/BeaconBlock" | ||
signature: | ||
$ref: "../primitive.yaml#/Signature" | ||
|
||
BlindedBeaconBlockBody: | ||
description: "A variant of the [`BeaconBlockBody`](https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#beaconblockbody) object from the CL Deneb spec, which contains a transactions root rather than a full transactions list." | ||
allOf: | ||
- $ref: '#/Deneb/BeaconBlockBodyCommon' | ||
- type: object | ||
properties: | ||
execution_payload_header: | ||
$ref: './execution_payload.yaml#/Deneb/ExecutionPayloadHeader' | ||
|
||
BlindedBeaconBlock: | ||
description: "A variant of the the [`BeaconBlock`](https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#beaconblock) object from the CL Deneb spec, which contains a `BlindedBeaconBlockBody` rather than a `BeaconBlockBody`." | ||
allOf: | ||
- $ref: '../altair/block.yaml#/Altair/BeaconBlockCommon' | ||
- type: object | ||
properties: | ||
body: | ||
$ref: '#/Deneb/BlindedBeaconBlockBody' | ||
|
||
SignedBlindedBeaconBlock: | ||
type: object | ||
description: "A variant of the the the [`SignedBeaconBlock`](https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#signedbeaconblock) object envelope from the CL Deneb spec, which contains a `BlindedBeaconBlock` rather than a `BeaconBlock`." | ||
properties: | ||
message: | ||
$ref: "#/Deneb/BlindedBeaconBlock" | ||
signature: | ||
$ref: "../primitive.yaml#/Signature" |
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.
and similar changes to the query path?
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.
Yeah I had that originally but dropped it. There was a suggestion a while ago to remove it, and I think it's a fair enough reason, although I also think
blob_sidecars
make sense too given it's the structure we return and consistent with the spec name. I don't have a strong opinion here though.#286 (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.
i think in previous version the comment made sense as blobs were sort of different from the sidecars aggregate object, but not blobSideCar is an independent object, i means its just not taxing on the mind to understand in a glance what they return.
but yes i wont strongly push it either but weakly in favor of change now
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.
Thanks @g11tech, I think that sounds reasonable to me, so:
/eth/v1/beacon/blob_sidecars/{block_id}
Any other thoughts on this @mcdee @rolfyone?
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 impartial on this one :)
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.
Thanks for the inputs, I'll wait another few days before updating this to see if anyone else have an opinion on this.
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.
Updated to
blob_sidecars
in a6d1c36