Skip to content
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

Remove blob signing #369

Merged
merged 15 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions apis/beacon/blocks/blinded_blocks.v2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ post:
successful. The beacon node is expected to integrate the new block into its state, and
therefore validate the block internally, however blocks which fail the validation are still
broadcast but a different status code is returned (202). Pre-Bellatrix, this endpoint will accept
a `SignedBeaconBlock`. After Deneb, this additionally instructs the beacon node to broadcast all given
signed blobs. The broadcast behaviour may be adjusted via the `broadcast_validation`
a `SignedBeaconBlock`. The broadcast behaviour may be adjusted via the `broadcast_validation`
query parameter.
parameters:
- name: broadcast_validation
Expand Down Expand Up @@ -55,7 +54,7 @@ post:
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Altair.SignedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Bellatrix.SignedBlindedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Capella.SignedBlindedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Deneb.SignedBlindedBlockContents"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Deneb.SignedBlindedBeaconBlock"
application/octet-stream:
schema:
description: "SSZ serialized block bytes. Use content type header to indicate that SSZ data is contained in the request body."
Expand Down
5 changes: 2 additions & 3 deletions apis/beacon/blocks/blinded_blocks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ post:
successful. The beacon node is expected to integrate the new block into its state, and
therefore validate the block internally, however blocks which fail the validation are still
broadcast but a different status code is returned (202). Pre-Bellatrix, this endpoint will accept
a `SignedBeaconBlock`. After Deneb, this additionally instructs the beacon node to broadcast all given
signed blobs.
a `SignedBeaconBlock`.
parameters:
- in: header
schema:
Expand All @@ -33,7 +32,7 @@ post:
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Altair.SignedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Bellatrix.SignedBlindedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Capella.SignedBlindedBeaconBlock"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Deneb.SignedBlindedBlockContents"
- $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Deneb.SignedBlindedBeaconBlock"
application/octet-stream:
schema:
description: "SSZ serialized block bytes. Use content type header to indicate that SSZ data is contained in the request body."
Expand Down
2 changes: 1 addition & 1 deletion apis/validator/blinded_block.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ get:
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Altair.BeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Bellatrix.BlindedBeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Capella.BlindedBeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlindedBlockContents"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlindedBeaconBlock"
application/octet-stream:
schema:
description: "SSZ serialized block bytes. Use Accept header to choose this response type, version string is sent in header `Eth-Consensus-Version`."
Expand Down
4 changes: 2 additions & 2 deletions apis/validator/block.v3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ get:
example: "12345"
data:
oneOf:
- $ref: '../../beacon-node-oapi.yaml#/components/schemas/BeaconBlock'
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/BeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Altair.BeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Bellatrix.BeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Bellatrix.BlindedBeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Capella.BeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Capella.BlindedBeaconBlock"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlockContents"
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this blockContents too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where it's a little confusing and asymmetric. BlockContents is still used in the unblinded flow, because in order to keep it stateless we need to pass blobs along with the block back and forth. So this endpoint can return Deneb.BlockContents or Deneb.BlindedBeaconBlock

Copy link
Contributor

Choose a reason for hiding this comment

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

i would prefer a separate call by validator to pull blobs if it plans to reroute the publish since the data transfer is non trivial and will also resolve code spaghetti in the api, to address latency, validator call concurrently pull blobs while its signing the block

Copy link
Contributor

Choose a reason for hiding this comment

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

That would not meet the requirement for statelessness, as it would be possible for the beacon node to exit between returning the data in this call and a separate request being made for the blobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i would prefer a separate call by validator to pull blobs if it plans to reroute the publish since the data transfer is non trivial and will also resolve code spaghetti in the api, to address latency, validator call concurrently pull blobs while its signing the block

I'm not sure this can be done without reintroducing the state requirement. Potentially we could set up an avenue for BNs to query ELs to check their local mempool for blobs matching a commitment, but I don't think this is really a great solution either. It might be better to mitigate the expense of transferring blob data by preferring SSZ transfer or preferring the blinded flow even when not using an external builder (this does put you at risk of missing a block proposal if you are using fallback between BNs however).

Copy link
Contributor

Choose a reason for hiding this comment

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

theoretically, the VC doesn't even need access to the blobs. it can just sign the block and let the blob be handled by the BN, as in, creating the BlobSidecar from SignedBeaconBlock + KzgProofs / Blobs

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, it needs access, because the BN that they are sent to may not have the blobs anymore, if they were originally fetched from a different one.

Copy link
Contributor

Choose a reason for hiding this comment

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

right, thats the idea of statelessness

Copy link
Contributor Author

Choose a reason for hiding this comment

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

v2 was only created for deneb... if its causing us to keep a container we don't need, thats sub-optimal...

right I think I missed your point. Agree we should deprecate V2

Copy link
Member

Choose a reason for hiding this comment

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

Just wanted to mention that keeping the blobs in the unblinded flow response makes it really easy to update our testing tools for the blob p2p scenarios in testnets.

- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlindedBlockContents"
- $ref: "../../beacon-node-oapi.yaml#/components/schemas/Deneb.BlindedBeaconBlock"
application/octet-stream:
schema:
description: "SSZ serialized block or blinded block bytes. Use Accept header to choose this response type, version string is sent in header `Eth-Consensus-Version` and block type in `Eth-Blinded-Payload`."
Expand Down
10 changes: 0 additions & 10 deletions beacon-node-oapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -369,22 +369,12 @@ components:
$ref: './types/deneb/block_contents.yaml#/Deneb/SignedBlockContents'
Deneb.BlindedBeaconBlock:
$ref: './types/deneb/block.yaml#/Deneb/BlindedBeaconBlock'
Deneb.SignedBlindedBlockContents:
$ref: './types/deneb/block_contents.yaml#/Deneb/SignedBlindedBlockContents'
Deneb.BlindedBlockContents:
$ref: './types/deneb/block_contents.yaml#/Deneb/BlindedBlockContents'
Deneb.SignedBlindedBeaconBlock:
$ref: './types/deneb/block.yaml#/Deneb/SignedBlindedBeaconBlock'
Blob:
$ref: './types/primitive.yaml#/Blob'
Deneb.BlobSidecars:
$ref: './types/deneb/blob_sidecar.yaml#/Deneb/BlobSidecars'
Deneb.SignedBlobSidecar:
$ref: './types/deneb/blob_sidecar.yaml#/Deneb/SignedBlobSidecar'
Deneb.BlindedBlobSidecar:
$ref: './types/deneb/blob_sidecar.yaml#/Deneb/BlindedBlobSidecar'
Deneb.SignedBlindedBlobSidecar:
$ref: './types/deneb/blob_sidecar.yaml#/Deneb/SignedBlindedBlobSidecar'
Node:
$ref: './types/fork_choice.yaml#/Node'
ExtraData:
Expand Down
78 changes: 11 additions & 67 deletions types/deneb/blob_sidecar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,83 +6,27 @@ Deneb:
minItems: 0
maxItems: 6

KZGCommitmentInclusionProof:
type: array
items:
$ref: "../primitive.yaml#/Bytes32"
minItems: 17
maxItems: 17

BlobSidecar:
type: object
description: "A blob sidecar as defined in the Deneb consensus 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'
signed_block_header:
$ref: "../block.yaml#/SignedBeaconBlockHeader"
kzg_commitment_inclusion_proof:
$ref: '#/Deneb/KZGCommitmentInclusionProof'

SignedBlobSidecars:
type: array
items:
$ref: '#/Deneb/SignedBlobSidecar'
minItems: 0
maxItems: 6

SignedBlobSidecar:
type: object
description: "The `SignedBlobSidecar` object envelope from the CL Deneb spec."
properties:
message:
$ref: "#/Deneb/BlobSidecar"
signature:
$ref: "../primitive.yaml#/Signature"

BlindedBlobSidecars:
type: array
items:
$ref: '#/Deneb/BlindedBlobSidecar'
minItems: 0
maxItems: 6

BlindedBlobSidecar:
type: object
description: "A blob sidecar with the SSZ root of the blob rather than the full blob."
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_root:
$ref: "../primitive.yaml#/Root"
kzg_commitment:
$ref: '../primitive.yaml#/KZGCommitment'
kzg_proof:
$ref: '../primitive.yaml#/KZGProof'

SignedBlindedBlobSidecars:
type: array
items:
$ref: '#/Deneb/SignedBlindedBlobSidecar'
minItems: 0
maxItems: 6

SignedBlindedBlobSidecar:
type: object
description: "A variant of the `SignedBlobSidecar` object envelope from the CL Deneb spec, which contains a `BlindedBlobSidecar` rather than a `BlobSidecar`."
properties:
message:
$ref: "#/Deneb/BlindedBlobSidecar"
signature:
$ref: "../primitive.yaml#/Signature"
45 changes: 22 additions & 23 deletions types/deneb/block_contents.yaml
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
Deneb:
KZGProofs:
type: array
items:
$ref: '../primitive.yaml#/KZGProof'
minItems: 0
maxItems: 4096

Blobs:
type: array
items:
$ref: "../primitive.yaml#/Blob"
minItems: 0
maxItems: 4096

BlockContents:
type: object
description: "The required object for block production according to the Deneb CL spec."
properties:
block:
$ref: "./block.yaml#/Deneb/BeaconBlock"
blob_sidecars:
$ref: "./blob_sidecar.yaml#/Deneb/BlobSidecars"

BlindedBlockContents:
type: object
description: "The required object for blinded block production according to the Deneb CL spec."
properties:
blinded_block:
$ref: "./block.yaml#/Deneb/BlindedBeaconBlock"
blinded_blob_sidecars:
$ref: "./blob_sidecar.yaml#/Deneb/BlindedBlobSidecars"
kzg_proofs:
Copy link
Contributor

@g11tech g11tech Nov 3, 2023

Choose a reason for hiding this comment

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

if we are assuming statelessness, are we requiring here for the beacon to build inclusion proofs for the blobs before transmitting them (since this is what validator is fetching)

Copy link
Contributor

Choose a reason for hiding this comment

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

The validator can compute them on its own, the inclusion proof can be computed solely from block.

Copy link
Contributor

Choose a reason for hiding this comment

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

correct however what i was clarifying that since validator publishes signed block contents, the intercepting beacon is would be building the proofs and publishing blocks (which is fine or may be good as well that while validator is signing the beacon can build the proofs and patch them in as validator starts publish flow although just a matter of 20-30ms as such)

(this comment is about SignedBlockContents)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking in implementation, a beacon node could start building the inclusion proofs and cache them when it builds a block. But should also have the ability to compute them on the fly (on a cache miss). So in the case where you publish through the BN building the block it'd be faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

It takes milliseconds to compute those Merkle proofs, as they are solely rooted inside BeaconBlock (~2 MB structure, and no need to do expensive BeaconState reconstruction). no need to optimize for this, especially while there are still clients that use JSON instead of SSZ encoding to access these endpoints.

Copy link
Contributor

Choose a reason for hiding this comment

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

right, lodestar will soon be transition to SSZ now lol

$ref: "#/Deneb/KZGProofs"
blobs:
$ref: "#/Deneb/Blobs"

SignedBlockContents:
type: object
description: "The required signed components of block production according to the Deneb CL spec."
properties:
signed_block:
$ref: "./block.yaml#/Deneb/SignedBeaconBlock"
signed_blob_sidecars:
$ref: "./blob_sidecar.yaml#/Deneb/SignedBlobSidecars"

SignedBlindedBlockContents:
type: object
description: "The required signed components of block production according to the Deneb CL spec."
properties:
signed_blinded_block:
$ref: "./block.yaml#/Deneb/SignedBlindedBeaconBlock"
signed_blinded_blob_sidecars:
$ref: "./blob_sidecar.yaml#/Deneb/SignedBlindedBlobSidecars"

kzg_proofs:
$ref: "#/Deneb/KZGProofs"
blobs:
$ref: "#/Deneb/Blobs"
1 change: 1 addition & 0 deletions types/primitive.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,4 @@ KZGProof:
format: hex
pattern: "^0x[a-fA-F0-9]{96}$"
description: "A G1 curve point. Used for verifying that the `KZGCommitment` for a given `Blob` is correct."

Loading