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: couple beacon block and blob sidecar for p2p #3046

Merged
merged 7 commits into from
Oct 26, 2022
Merged
Changes from 2 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
36 changes: 17 additions & 19 deletions specs/eip4844/p2p-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ The specification of these changes continues in the same format as the network s
- [Containers](#containers)
- [`BlobsSidecar`](#blobssidecar)
- [`SignedBlobsSidecar`](#signedblobssidecar)
- [`SignedBeaconBlockAndBlobsSidecar`](#signedbeaconblockandblobssidecar)
- [The gossip domain: gossipsub](#the-gossip-domain-gossipsub)
- [Topics and messages](#topics-and-messages)
- [Global topics](#global-topics)
- [`beacon_block`](#beacon_block)
- [`blobs_sidecar`](#blobs_sidecar)
- [`beacon_block_and_blob_sidecar`](#beacon_block_and_blob_sidecar)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems we use "blobs sidecar" everywhere but here where you use "blob sidecar". Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo, I will fix. Thanks!

- [Transitioning the gossip](#transitioning-the-gossip)
- [The Req/Resp domain](#the-reqresp-domain)
- [Messages](#messages)
Expand Down Expand Up @@ -58,6 +58,14 @@ class SignedBlobsSidecar(Container):
signature: BLSSignature
```

### `SignedBeaconBlockAndBlobsSidecar`

```python
class SignedBeaconBlockAndBlobsSidecar(Container):
beacon_block: SignedBeaconBlock
blobs_sidecar: SignedBlobsSidecar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Verifying the sidecar signature helps ensure the KZG commitments are worth verifying, but would it really result in DoS / performance issues when anyone changes it to invalid data? Technically we compare the sidecar against the KZG commitments in the beacon block already, so we do not have to sign the sidecar.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I don't think we need to sign the side car. This also informs whether or not we send signed or unsigned side cars in blobs by range/root request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree that comparing KZG commitments in the beacon block is sufficient. We don't need the sidecar signature

@realbigsean range/root request will be decoupled and it's unsigned there

```

## The gossip domain: gossipsub

Some gossip meshes are upgraded in the fork of EIP4844 to support upgraded types.
Expand All @@ -75,33 +83,25 @@ The new topics along with the type of the `data` field of a gossipsub message ar

| Name | Message Type |
| - | - |
| `beacon_block` | `SignedBeaconBlock` (modified) |
| `blobs_sidecar` | `SignedBlobsSidecar` (new) |
| `beacon_block_and_blob_sidecar` | `SignedBeaconBlockAndBlobsSidecar` (new) |

Note that the `ForkDigestValue` path segment of the topic separates the old and the new `beacon_block` topics.

#### Global topics

EIP4844 changes the type of the global beacon block topic and introduces a new global topic for blobs-sidecars.
EIP4844 introduces a new global topic for beacon block and blobs-sidecars.

##### `beacon_block`
##### `beacon_block_and_blob_sidecar`

The *type* of the payload of this topic changes to the (modified) `SignedBeaconBlock` found in EIP4844.
This topic is used to propagate new signed and coupled beacon blocks and blobs sidecars to all nodes on the networks.

In addition to the gossip validations for this topic from prior specifications,
the following validations MUST pass before forwarding the `signed_beacon_block` on the network.
Alias `block = signed_beacon_block.message`, `execution_payload = block.body.execution_payload`.
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)`

##### `blobs_sidecar`

This topic is used to propagate data blobs included in any given beacon block.

The following validations MUST pass before forwarding the `signed_blobs_sidecar` on the network;
Alias `sidecar = signed_blobs_sidecar.message`.
Alias `signed_blobs_sidecar = signed_beacon_block_and_blobs_sidecar.blobs_sidecar`, `sidecar = signed_blobs_sidecar.message`.
- _[IGNORE]_ the `sidecar.beacon_block_slot` is for the current slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. `sidecar.beacon_block_slot == current_slot`.
terencechain marked this conversation as resolved.
Show resolved Hide resolved
- _[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)`
Expand All @@ -113,8 +113,6 @@ Alias `sidecar = signed_blobs_sidecar.message`.
- _[IGNORE]_ The sidecar is the first sidecar with valid signature received for the `(proposer_index, sidecar.beacon_block_slot)` combination,
where `proposer_index` is the validator index of the beacon block proposer of `sidecar.beacon_block_slot`
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove this condition now that it's coupled


Note that a sidecar may be propagated before or after the corresponding beacon block.

Once both sidecar and beacon block are received, `validate_blobs_sidecar` can unlock the data-availability fork-choice dependency.
terencechain marked this conversation as resolved.
Show resolved Hide resolved

### Transitioning the gossip
Expand Down