Skip to content

update Deneb for blob sidecar inclusion proofs#5565

Merged
etan-status merged 7 commits into
unstablefrom
dev/etan/df-proof
Nov 6, 2023
Merged

update Deneb for blob sidecar inclusion proofs#5565
etan-status merged 7 commits into
unstablefrom
dev/etan/df-proof

Conversation

@etan-status

@etan-status etan-status commented Nov 4, 2023

Copy link
Copy Markdown
Contributor

BlobSidecar is no longer signed, instead use Merkle proof to link blobs with block.

Associated beacon-API / builder-specs still TBD; minimal changes done to compile in similar style to previous spec, but not standardized yet.

`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
@github-actions

github-actions Bot commented Nov 5, 2023

Copy link
Copy Markdown

Unit Test Results

         9 files  ±  0    1 098 suites  ±0   31m 45s ⏱️ + 4m 35s
  3 945 tests +52    3 598 ✔️ +2  347 💤 +50  0 ±0 
16 048 runs  +72  15 650 ✔️  - 3  398 💤 +75  0 ±0 

Results for commit d88f1d3. ± Comparison against base commit e4dacc3.

♻️ This comment has been updated with latest results.

Comment on lines +435 to +436
# [REJECT] The sidecar's blob is valid as verified by `verify_blob_kzg_proof(
# blob_sidecar.blob, blob_sidecar.kzg_commitment, blob_sidecar.kzg_proof)`.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Double-checked with @mratsim , the proposer sig check is the faster one, so doing it before the KZG check: "pairings are 2 levels deep in KZG and 1 deep in BLS signature (just after deserialization and sanity checks that points are on curve and non-infinity)"

Batching of the two verifications would be clunky, likely not worth it.

Comment on lines +413 to +414
kzg_proofs: blobsBundle.proofs,
blobs: blobsBundle.blobs)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sidecars are no longer sent back and forth, as they no longer need a signature. Instead, just the kzg_proofs and blobs are sent.

kzg_proofs and blobs are downloaded by the VC and then submitted again to the BN to support use case where the block is produced by a different BN than the one who is broadcasting. The BN then converts the SignedDenebBlockContents to SignedBeaconBlock + BlobSidecar list

Comment on lines +97 to +99
KzgProofs* = List[KzgProof, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]
Blobs* = List[Blob, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]
BlobRoots* = List[Eth2Digest, Limit MAX_BLOB_COMMITMENTS_PER_BLOCK]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure where to put them and what to put as the correct limit. beacon-API uses 0 .. MAX_BLOBS_PER_BLOCK in informal metadata description, builder-API uses MAX_BLOB_COMMITMENTS_PER_BLOCK in a more explicit specification. In practice, it doesn't matter as the SSZ messages are not hash-tree-rooted in neither of the two protocols. If they were hash_tree_rooted, using the theoretical capacity (MAX_BLOB_COMMITMENTS_PER_BLOCK) makes more sense (that is also what they use for the KzgCommitments inside consensus, where HTR matters).


# https://github.com/ethereum/builder-specs/blob/534e4f81276b8346d785ed9aba12c4c74b927ec6/specs/deneb/builder.md#blindedblobsidecar
BlindedBlobSidecar* = object
block_root*: Eth2Digest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

BlindedBlobSidecar may go away entirely, but the specs are still in flux:

For now, have done the minimal changes to it to make the flow work in theory, but the spec may optimize further and get rid of the BlindedBlobSidecar altogether.

@etan-status

etan-status commented Nov 5, 2023

Copy link
Copy Markdown
Contributor Author

This should be all that's needed to get us going with the new blob sidecar flavor, as an initial PR upgrading to v1.4.0-beta.4 test vectors.

  • Remove SignedBlobSidecar, and replace with new BlobSidecar conceptually
  • Change BlobSidecar, and replace with kzg_proofs + blobs lists conceptually
  • BN converts signed_block (kzg_commitments) + kzg_proofs + blobs lists to BlobSidecar and puts in cryptographic link via Merkle proof, instead of VC putting in a signature
  • Beacon-APIs updated for new flow
  • Builder-APIs flow updated minimally, it still uses BlindedBlobSidecar -> BlobSidecar flow; may become simpler in the future

Notably missing:

  • Final beacon-API and builder-API specs
  • Gossip per-chunk validation: per-chunk validation of libp2p req/resp chunk #5557
  • I think there could also be more sanity checking, but that's not a new problem. As in, any time one receives via REST a signed block, validate (minimally) that KZG proofs are correct, and that inclusion proofs for sidecars are correct. Helper functions are there. Not high prio as it only affects trusted REST API I think.
  • Same for VC, could check that proofs are valid before signing a block.

@etan-status etan-status marked this pull request as ready for review November 5, 2023 09:30
@etan-status etan-status merged commit d8a7f0d into unstable Nov 6, 2023
@etan-status etan-status deleted the dev/etan/df-proof branch November 6, 2023 06:48
etan-status added a commit that referenced this pull request May 16, 2026
- #4732 KZG primitives added
- #4785 added gossip validation for BlobSidecar but did not check KZG
- #4844 added req/resp for BlobSidecar but did not validate responses
- #5445 fixed KZG validate_blobs in state transition (Opt[bool] jank)
- ethereum/consensus-specs#3531 new spec hits
- #5565 new spec, + fixed gossip missing inclusion proof + KZG checks
- #6109 goerli hardening, + fixed Req/Resp missing inclusion proof check
- #6741 copies the req/resp gap to Fulu
- #8254 removes the Deneb blob_quarantine
- This one now finally adds the missing KZG checks to req/resp
etan-status added a commit that referenced this pull request May 16, 2026
- #4732 KZG primitives added
- #4785 added gossip validation for BlobSidecar but did not check KZG
- #4844 added req/resp for BlobSidecar but did not validate responses
- #5445 fixed KZG validate_blobs in state transition (Opt[bool] jank)
- ethereum/consensus-specs#3531 new spec hits
- #5565 new spec, + fixed gossip missing inclusion proof + KZG checks
- #6109 goerli hardening, + fixed Req/Resp missing inclusion proof check
- #6741 copies the remaining req/resp missing KZG check gap to Fulu
- #8254 removes the Deneb blob_quarantine
- This one now finally adds the missing KZG checks to req/resp
tersec pushed a commit that referenced this pull request May 17, 2026
- #4732 KZG primitives added
- #4785 added gossip validation for BlobSidecar but did not check KZG
- #4844 added req/resp for BlobSidecar but did not validate responses
- #5445 fixed KZG validate_blobs in state transition (Opt[bool] jank)
- ethereum/consensus-specs#3531 new spec hits
- #5565 new spec, + fixed gossip missing inclusion proof + KZG checks
- #6109 goerli hardening, + fixed Req/Resp missing inclusion proof check
- #6741 copies the remaining req/resp missing KZG check gap to Fulu
- #8254 removes the Deneb blob_quarantine
- This one now finally adds the missing KZG checks to req/resp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants