Add Gloas boilerplate #7728
Add Gloas boilerplate #7728
Conversation
|
@macladson can you add this de-duplications? |
| @@ -1137,6 +1193,7 @@ impl HttpJsonRpc { | |||
| new_payload_v3: capabilities.contains(ENGINE_NEW_PAYLOAD_V3), | |||
| new_payload_v4: capabilities.contains(ENGINE_NEW_PAYLOAD_V4), | |||
| new_payload_v5: capabilities.contains(ENGINE_NEW_PAYLOAD_V5), | |||
| new_payload_v6: capabilities.contains(ENGINE_NEW_PAYLOAD_V6), | |||
There was a problem hiding this comment.
Why add a fictional v6 before it's actually used?
There was a problem hiding this comment.
See my comment regarding the new block type
There was a problem hiding this comment.
btw new_payload_v5 doesn't exist in Fusaka, so we may be able to use v5 for G*
There was a problem hiding this comment.
Ooh I was under the impression that both new_payload and get_payload would always increment each fork. In that case @dapplion is probably right and we can exclude these in the future.
| .deconstruct(); | ||
|
|
||
| ( | ||
| BeaconBlock::Gloas(BeaconBlockGloas { |
There was a problem hiding this comment.
What's the point of adding a new block type before we actually need it? Could we wait for the first feature in Gloas?
There was a problem hiding this comment.
I suppose we could wait, but we will inevitably end up with this block type anyway so I would consider what I've added as boilerplate. Plus it reduces the overhead of devs adding new features. For example, if we have multiple people working on their own feature forks they can all reuse this code rather than all having to implement it themselves.
You could similarly argue that this entire PR is unnecessary since we don't have any Gloas features yet, but that misses the point imo
There was a problem hiding this comment.
Yeah I'm fine with defining BeaconBlockGloas
There was a problem hiding this comment.
Makes sense, I'm convinced
Proposed Changes
Adds the required boilerplate code for the Gloas (Glamsterdam) hard fork. This allows PRs testing Gloas-candidate features to test fork transition.
This also includes de-duplication of post-Bellatrix readiness notifiers from #6797 (credit to @dapplion)
Additional Info
Not all tests have been updated yet.