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

Eip4844 beacon_block_and_blobs_sidecar gossip validation rules #3109

Open
tbenr opened this issue Nov 17, 2022 · 4 comments
Open

Eip4844 beacon_block_and_blobs_sidecar gossip validation rules #3109

tbenr opened this issue Nov 17, 2022 · 4 comments
Labels
Deneb was called: eip-4844

Comments

@tbenr
Copy link
Contributor

tbenr commented Nov 17, 2022

Current rules:

In addition to the gossip validations for the `beacon_block` topic from prior specifications, the following validations MUST pass before forwarding the `signed_beacon_block_and_blobs_sidecar` on the network.  
Alias `signed_beacon_block = signed_beacon_block_and_blobs_sidecar.beacon_block`, `block = signed_beacon_block.message`, `execution_payload = block.body.execution_payload`.
- _[REJECT]_ The KZG commitments of the blobs are all correctly encoded compressed BLS G1 Points.
  -- i.e. `all(bls.KeyValidate(commitment) for commitment in block.body.blob_kzg_commitments)`
- _[REJECT]_ The KZG commitments correspond to the versioned hashes in the transactions list.
  -- i.e. `verify_kzg_commitments_against_transactions(block.body.execution_payload.transactions, block.body.blob_kzg_commitments)`

Alias `sidecar = signed_beacon_block_and_blobs_sidecar.blobs_sidecar`.
- _[IGNORE]_ the `sidecar.beacon_block_slot` is for the current slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. `sidecar.beacon_block_slot == block.slot`.
- _[REJECT]_ the `sidecar.blobs` are all well formatted, i.e. the `BLSFieldElement` in valid range (`x < BLS_MODULUS`).
- _[REJECT]_ The KZG proof is a correctly encoded compressed BLS G1 Point -- i.e. `bls.KeyValidate(blobs_sidecar.kzg_aggregated_proof)`
  1. Shouldn't we add check sidecar.beacon_block_root == block.block.hash_tree_root()?
  2. Shouldn't we [REJECT] on slot and block_root checks since signed_beacon_block_and_blobs_sidecar itself is inconsistent thus invalid?
  3. Shouldn't we rely on kzg library verification here by running validate_blobs_sidecar and all the low level KZG\BLS checks will be done internally by the kzg library?

cc @terencechain

@djrtwo
Copy link
Contributor

djrtwo commented Nov 17, 2022

good points!

  1. I'm not sure we need the beacon slot and root in the sidecar gossip anymore because of the simultaneous delivery (especially if we make our range/root requests coupled as well)
  2. Yes, if we keep it, it should be REJECT
  3. We either need signature or full kzg validation as discussed in BlobSideCar mutable in gossip #3103

@hwwhww hwwhww added the Deneb was called: eip-4844 label Nov 17, 2022
@tbenr
Copy link
Contributor Author

tbenr commented Nov 17, 2022

  1. I'm not sure we need the beacon slot and root in the sidecar gossip anymore because of the simultaneous delivery (especially if we make our range/root requests coupled as well)

Yes, makes sense for the coupled container but if we leave the decoupled "byRange" methods (which was the latest direction if I recall correctly) hey might have more sense

@terencechain
Copy link
Contributor

  1. Shouldn't we rely on kzg library verification here by running validate_blobs_sidecar and all the low level KZG\BLS checks will be done internally by the kzg library?
  1. We either need signature or full kzg validation as discussed in BlobSideCar mutable in gossip #3103

I think we can make an argument to remove all the low-level kzg checks in either signature or full kzg validation cases. If the signature can be validated, is it still worth the low-level kzg checks before forwarding it to peers? They are cheap checks, so maybe why not

@tbenr
Copy link
Contributor Author

tbenr commented Dec 20, 2022

Since this has been merged, shouldn't we simplify\revisit the validations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb was called: eip-4844
Projects
None yet
Development

No branches or pull requests

5 participants
@djrtwo @hwwhww @tbenr @terencechain and others