Remove blob signing #369

realbigsean · 2023-10-31T18:31:17Z

Changes for ethereum/consensus-specs#3531

update the BlobSidecar struct in the GET blob_sidecars endpoint
remove SignedBlobSidecar type
use BlindedBeaconBlock structs rather than BlindedBlockContents structs
update BlockContents to wrap a BeaconBlock, an array of KZGCommitment's an array of KZGProof's, and an array of Blob's

My thinking here is we want to maintain a stateless flow (the unblinded flow) so we will have to pass along blobs to the VC. And in this case I think we might as well pass along proofs so they don't need to be recomputed. In the stateful (blinded) flow we only need to pass along the object we're signing, so I think it can be kept maximally simple. The downside is it's less intuitive which types should be expected when on an endpoint like /eth/v3/validator/blocks and what the types represent exactly.

The API changes here imply this block production flow:

Stateless

BN gets payload, commitments, proofs, blobs from the EL
BN assembles a block using the commitments and payload
VC receives the block, blobs, and proofs from the BN
VC Signs the block and returns everything
BN computes the commitment inclusion proof, assembles the sidecars, publishes

Stateful (blinded)

BN gets payload, commitments, proofs, blobs from the EL
BN assembles a block using the commitments and payload
BN caches payload, commitments, proofs, blobs
VC receives the blinded block
VC signs and returns the blinded block
BN unblinds the block, computes the commitment inclusion proof, assembles the sidecars using cached proofs and blobs, publishes

This reverts commit f0ffbb8.

ralexstokes

looks good from a high-level review

mcdee · 2023-11-01T10:57:23Z

types/deneb/blob_sidecar.yaml

@@ -6,83 +6,27 @@ Deneb:
    minItems: 0
    maxItems: 6

+  CommitmentInclusionProof:


Given that we use KZGCommitment elsewhere, should we refer to this as KZGCommitmentInclusionProof or is this considered more generic?

Updated field name to kzg_commitment_inclusion_proof ethereum/consensus-specs@4a609ce

updated both the struct name and field name here: e9d0538

rolfyone · 2023-11-02T10:09:07Z

apis/validator/block.v3.yaml

@@ -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"


remove this blockContents too?

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

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

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.

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).

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

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.

right, thats the idea of statelessness

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

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.

g11tech · 2023-11-03T18:19:43Z

types/deneb/block_contents.yaml

-        $ref: "./block.yaml#/Deneb/BlindedBeaconBlock"
-      blinded_blob_sidecars:
-        $ref: "./blob_sidecar.yaml#/Deneb/BlindedBlobSidecars"
+      kzg_proofs:


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)

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

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)

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.

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.

right, lodestar will soon be transition to SSZ now lol

g11tech · 2023-11-03T18:21:29Z

types/deneb/block_contents.yaml

similar comment as above

etan-status · 2023-11-04T12:15:43Z

types/deneb/block_contents.yaml

+    items:
+      $ref: '../primitive.yaml#/KZGProof'
+    minItems: 0
+    maxItems: 6


Should use the theoretical max here, MAX_BLOB_COMMITMENTS_PER_BLOCK, for stable SSZ transport if the number changes in future

Same for Blobs

ref: https://github.com/ethereum/builder-specs/blob/534e4f81276b8346d785ed9aba12c4c74b927ec6/specs/deneb/builder.md#blindedblobsbundle

+1 considering ssz transport

good catch!

updated! 9a7c8b6

`BlobSidecar` is no longer signed, instead use Merkle proof to link blobs with block. - ethereum/consensus-specs#3531 Associated beacon-API / builder-specs still TBD; minimal changes done to compile in similar style to previous spec, but not standardized yet. - ethereum/beacon-APIs#369 - ethereum/builder-specs#90

g11tech

i am onboard with the PR with a change that @etan-status proposed to keep the max length to MAX_BLOB_COMM... considering ssz transport encoding

…beacon-APIs into remove-blob-signing

realbigsean · 2023-11-06T14:14:24Z

Ok I think I've addressed the feedback so far

g11tech

lgtm

rolfyone · 2023-11-08T01:44:51Z

sorry for the less interesting bit, but changelog entry :)

etan-status

Looks good, my understanding is as follows:

Regular flow

BN1 -- BeaconBlockContents --> VC (with blobs + kzg_proofs)
VC -- SignedBeaconBlockContents --> BN2 --> libp2p (with blobs + kzg_proofs as is from VC to BN, just re-upload)

BN1 may be different from BN2

Blinded flow

MEV relay -- BlindedBeaconBlock --> BN --> VC
VC -- SignedBlindedBeaconBlock --> BN --> MEV relay
MEV relay -- SignedBeaconBlockContents --> BN --> libp2p

mcdee · 2023-11-08T09:56:37Z

@etan-status in your blinded flow part 1, the MEV relay currently provides an execution payload rather than an entire blinded beacon block.

With the advent of deneb this should change to the execution payload plus an array of blinded blob info. I think the blob info should be the blob roots plus the KZG commitments and KZG proofs, but would like to see if others on here concur.

etan-status · 2023-11-08T09:59:11Z

ah, right. but from BN --> VC the executionpayload is then combined with the beacon block, so the the VC it looks the same, regardless.

Update:

MEV relay -- blindedexecutionpayload --> BN -- BlindedBeaconBlock --> VC
VC -- SignedBlindedBeaconBlock --> BN --> MEV relay
MEV relay -- SignedBeaconBlockContents --> BN --> libp2p

etan-status · 2023-11-08T10:00:00Z

No need for KZG proofs / blob roots. there is no more signature on the blobs.

etan-status · 2023-11-08T10:00:35Z

And, in blinded flow, VC has to use the same MEV relay, so the same BN, to complete the flow. it is only in the unblinded flow where a different BN may be selected for publishing.

mcdee · 2023-11-08T10:02:25Z

No need for KZG proofs / blob roots. there is no more signature on the blobs.

Okay, so just the KZG commitments to place inside the block body.

etan-status · 2023-11-08T10:05:22Z

the hash_tree_root(kzg_commitments) would be sufficient. otherwise, there is not really blinding because the BN can identify what blobs were included by the relay, if they also observed the mempool.

mcdee · 2023-11-08T10:06:20Z

And, in blinded flow, VC has to use the same MEV relay, so the same BN...

What's the restriction here? Is the BN now holding state that must be passed to the MEV relay, or is it just the same condition at current that the BN knows which relay to talk to in order to carry out the unblinding?

etan-status · 2023-11-08T10:07:52Z

The latter. The BN knows the identity of the MEV relay who sent the bid, and also maintains the validator registration with that relay.

mcdee · 2023-11-08T10:08:34Z

the hash_tree_root(kzg_commitments) would be sufficient. otherwise, there is not really blinding because the BN can identify what blobs were included by the relay, if they also observed the mempool.

But BlindedBeaconBlock has the full KZG commitments in its definition.

Also, blinding is really to avoid leaking bundles that could be picked apart, so not sure that knowing which blob transactions are in a block is an issue.

etan-status · 2023-11-08T10:10:43Z

Sure, just saying, the protocol would work perfectly well even if BlindedBeaconBlock only contained hash_tree_root(kzg_commitments). If blinding doesn't try to hide the blob transactions, that's fine (but also kinda pointless) to include the full kzg_commitments in BlindedBeaconBlock.

rolfyone

LGTM

The `BlobSidecar` construction has been moved to the relay and is no longer done by the BN / VC in blinded flow. Builder bid contents have been shrinked from full `BlindedBlobBundle` to `blob_kzg_commitments`. - ethereum/builder-specs#90 - ethereum/beacon-APIs#369

realbigsean added 10 commits October 30, 2023 12:18

Revert "Add blob signing endpoints (ethereum#302)"

b5367bc

This reverts commit f0ffbb8.

add back things we still need

b43b5a6

update blob sidecar structure and block contents structure

131c291

fix lints

7efb8e5

fix path for signed block header

117e795

experiment adding type dropdown

a8f3aa9

try again

82f8667

delete discriminator

f4b053e

revert oapi version bump

5825d08

remove kzg commitments from block contents

c3775fc

ralexstokes approved these changes Oct 31, 2023

View reviewed changes

james-prysm mentioned this pull request Oct 31, 2023

Update Beacon API endpoints publishBlindedBlockV2,publishBlindedBlock,ProduceBlockV3Response and related use endpoints to remove blob signing prysmaticlabs/prysm#13151

Closed

mcdee reviewed Nov 1, 2023

View reviewed changes

rolfyone reviewed Nov 2, 2023

View reviewed changes

update commitment inclusion proof name

e9d0538

ralexstokes mentioned this pull request Nov 2, 2023

Drop "blinded blob sidecar" concept after using signed header to authenticate blobs ethereum/builder-specs#90

Merged

g11tech approved these changes Nov 2, 2023

View reviewed changes

pawanjay176 mentioned this pull request Nov 2, 2023

Sidecar inclusion proof sigp/lighthouse#4900

Merged

9 tasks

rolfyone mentioned this pull request Nov 3, 2023

beacon-api - remove blob signing Consensys/teku#7651

Closed

g11tech reviewed Nov 3, 2023

View reviewed changes

types/deneb/block_contents.yaml

kzg_proofs:

Copy link

Contributor

g11tech Nov 3, 2023

Choose a reason for hiding this comment

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

similar comment as above

etan-status mentioned this pull request Nov 4, 2023

Update Deneb for v1.4.0-beta.4 #370

Closed

etan-status reviewed Nov 4, 2023

View reviewed changes

etan-status mentioned this pull request Nov 4, 2023

update Deneb for blob sidecar inclusion proofs status-im/nimbus-eth2#5565

Merged

Merge branch 'master' into remove-blob-signing

ac84066

g11tech reviewed Nov 6, 2023

View reviewed changes

g11tech mentioned this pull request Nov 6, 2023

feat: implement blob sidecars with inclusion proof instead of signtaures ChainSafe/lodestar#6089

Merged

update array max sizes to 4096

9a7c8b6

Merge branch 'remove-blob-signing' of https://github.com/realbigsean/…

c76afea

…beacon-APIs into remove-blob-signing

g11tech approved these changes Nov 6, 2023

View reviewed changes

marioevz mentioned this pull request Nov 8, 2023

feat: Implement blob sidecar signatures removal marioevz/blobber#28

Merged

Merge branch 'master' into remove-blob-signing

1616e11

etan-status approved these changes Nov 8, 2023

View reviewed changes

mcdee approved these changes Nov 8, 2023

View reviewed changes

rolfyone approved these changes Nov 9, 2023

View reviewed changes

rolfyone merged commit c097f1a into ethereum:master Nov 9, 2023
2 checks passed

StefanBratanov mentioned this pull request Nov 9, 2023

rename old block contents Consensys/teku#7682

Closed

2 tasks

etan-status mentioned this pull request Nov 13, 2023

update Deneb for latest builder-specs flow status-im/nimbus-eth2#5598

Merged

james-prysm mentioned this pull request Nov 14, 2023

remove blob sidecar signing ethereum/remote-signing-api#8

Merged

3 tasks

realbigsean mentioned this pull request Nov 16, 2023

Sidecar inclusion api updates sigp/lighthouse#4927

Merged

james-prysm mentioned this pull request Nov 29, 2023

Beacon API: update Deneb endpoints after removing blob signing prysmaticlabs/prysm#13235

Merged

8 tasks

tersec mentioned this pull request Dec 3, 2023

Update to current (deprecated, but) version of produceBlindedBlock status-im/nimbus-eth2#5639

Merged

sebastianst mentioned this pull request Jan 18, 2024

eth: Add tests of Beacon API response structs & update Beacon API client ethereum-optimism/optimism#9077

Merged

Remove blob signing #369

Remove blob signing #369

Conversation

realbigsean commented Oct 31, 2023

ralexstokes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g11tech Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

realbigsean Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

g11tech left a comment

Choose a reason for hiding this comment

realbigsean commented Nov 6, 2023

g11tech left a comment

Choose a reason for hiding this comment

rolfyone commented Nov 8, 2023

etan-status left a comment • edited Loading

Choose a reason for hiding this comment

mcdee commented Nov 8, 2023

etan-status commented Nov 8, 2023 • edited Loading

etan-status commented Nov 8, 2023

etan-status commented Nov 8, 2023

mcdee commented Nov 8, 2023

etan-status commented Nov 8, 2023

mcdee commented Nov 8, 2023

etan-status commented Nov 8, 2023

mcdee commented Nov 8, 2023

etan-status commented Nov 8, 2023

rolfyone left a comment

Choose a reason for hiding this comment

g11tech Nov 3, 2023 •

edited

Loading

realbigsean Nov 6, 2023 •

edited

Loading

etan-status left a comment •

edited

Loading

etan-status commented Nov 8, 2023 •

edited

Loading