Backport blob schedule to deneb#4313
Conversation
| # 6 subnets | ||
| BLOB_SIDECAR_SUBNET_COUNT: 6 | ||
| # 6 blobs | ||
| MAX_BLOBS_PER_BLOCK: 6 |
There was a problem hiding this comment.
removing this constant might be more problematic then what we discussed in the other PR #4322 (comment) as it is consensus critical and at least lodestar vc enforces this right now at startup
however we can make this more lax now already to avoid potential interop issues
There was a problem hiding this comment.
it sounds like people are more willing to wear the pain than i'd like so whatever is decided on that other ticket is arguably equally applicable here.
| return entry["MAX_BLOBS_PER_BLOCK"] | ||
|
|
||
| # Only for testing. Should never reach this in the real world. | ||
| return min(entry["MAX_BLOBS_PER_BLOCK"] for entry in BLOB_SCHEDULE) |
There was a problem hiding this comment.
wouldn't it be better to use max blobs of the smallest epoch instead? Since BLOB_SCHEDULE can have entries that decrease max blobs it might be unexpected if such a entry is picked for unscheduled epochs
There was a problem hiding this comment.
Eh it shouldn't really matter. This line will only be executed in tests.
There was a problem hiding this comment.
was mostly thinking about kurtosis testing, if you override the blob schedule it's possible to reach this line as well, but might not be worth to change this now
There was a problem hiding this comment.
The real issue with this line is that under the minimal preset, it will return 6 for all forks (including Electra and Fulu). But there's really not a good solution to this other than revamping the testing infrastructure so that minimal uses realistic fork epoch values.
There was a problem hiding this comment.
I think the fork epochs in minimal doesn't have to be the same as mainnet, but they should be different numbers not all the same. Why are all the same?
It worries me that we add this because people will read this, in general, not as something for testing but prescription for their clients. The comment fixes this, but it would be neater to don't have things in the spec that the clients should implement other way.
Another separate thing:
What about adding to the blob schedule (Epoch:0, MAX_BLOBS_PER_BLOCK: 0) it is a more elegant way to represent that there are no blobs before.
|
This PR doesn't have the necessary support to be merged. Going to close it. |
**Motivation** The PR to backport blob schedule to deneb/electra (ethereum/consensus-specs#4313) has been closed and the entries for deneb/electra have been removed from CL spec as well since ethereum/consensus-specs#4341. **Description** - Remove blob schedule entries for deneb/electra - Update `getMaxBlobsPerBlock` to default to electra limit - Only return `BLOB_SCHEDULE` from `GET /eth/v1/config/spec` api if fulu is scheduled
This PR moves the blob schedule from Fulu to Deneb. It was added here: