Add Electra blob sidecars type to support max blobs from EIP-7691

[nflaig]

Adds new BlobSidecars type to Electra with max items of 9 to match MAX_BLOBS_PER_BLOCK_ELECTRA from EIP-7691.

In addition this PR makes the metadata fields added in #441 and the Eth-Consensus-Version header required. Implementations should make sure to support this at latest in their Electra release.

[mcdee]

The change to the existing endpoint to now require Eth-Consensus-Version is technically breaking, it's worth doing a quick check of the endpoint on existing beacon nodes to confirm if they supply this, and if not to raise an issue so that they are aware of this and can address it ahead of time.

But in general LGTM.

[nflaig]

The change to the existing endpoint to now require Eth-Consensus-Version is technically breaking, it's worth doing a quick check of the endpoint on existing beacon nodes to confirm if they supply this

I updated the PR description to highlight this, Lodestar already sets this header in the stable release.

[mcdee]

Sure, but it's still a breaking change to an existing endpoint which is unexpected and may not be picked up by client teams without a nudge.

[nflaig]

From references of the previous PR, looks like at least Prysm and Teku should be fine as well, so there is only Nimbus, Lighthouse and Grandine left to check

[rkapka]

Prysm does already return the metadata

[Tumas]

Grandine returns the metadata as well

[rolfyone]

LGTM

[dapplion]

This API returns JSON, why is it relevant this maxItems value? Even for SSZ, in it's serialized form the max value is not relevant

[nflaig]

It returns both JSON and SSZ

beacon-APIs/apis/beacon/blob_sidecars/blob_sidecars.yaml

Lines 50 to 52 in aa1be25

	application/octet-stream:
	schema:
	description: "SSZ serialized `BlobSidecars` bytes. Use Accept header to choose this response type"

Even for SSZ, in it's serialized form the max value is not relevant

I quickly mentioned this in discord, we could just use MAX_BLOB_COMMITMENTS_PER_BLOCK which is 4096 as max items which is what we use in Lodestar for the list limit.

[tersec]

It seems better to discard invalid data as quickly as possible, and anything past MAX_BLOBS_PER_BLOCK_ELECTRA is invalid. No need to build in capacity in the rest of the system to handle it, either on the client or server end, for theoretical possibilities which even if working in isolation aren't useful as part of a broader system.

[dapplion]

You are talking about a consumer of the REST API, not a p2p network with adversarial peers. I think MAX_BLOB_COMMITMENTS_PER_BLOCK is fine and we don't have to deal with forks on this type anymore

[tersec]

Unclear that https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlobSidecars will even be useful/salient in Fulu; the beacon API might end up with a new getColumnSidecars or similar endpoint. A similar discussion has been happening in the req/resp area, where the v3 Fulu-only getBlobsByRange was removed in lieu of keeping only the column one.

[nflaig]

we can always change this to MAX_BLOB_COMMITMENTS_PER_BLOCK in a follow-up PR, I don't feel strongly about this as both approaches work

[rolfyone]

@tersec @dapplion is the above conversation resolved? I don't want to merge until there's consensus...

[nflaig]

@tersec @dapplion is the above conversation resolved? I don't want to merge until there's consensus...

can be changed in a follow-up PR if someone feels strongly about it, discussion has stopped for a week now