Add LightClientHeader wrapper
#3190
Conversation
In Altair, light client sync protocol exchanges `BeaconBlockHeader` structures for tracking current progress. Wrapping `BeaconBlockHeader` inside a `LightClientHeader` allows future extensions of this header, e.g., to also track `ExecutionPayloadHeader`. Note: This changes the JSON REST format by adding a `beacon` nesting. For SSZ, the serialization format stays same (but overall root changes).
There was a problem hiding this comment.
Although the fact that it modifying the Altair spec is kinda cheating, I think it is cleaner for consensus specs. So I personally prefer this PR over #3151 solution.
/cc @dapplion, @wemeetagain for third opinions. The discussion was here: #3151 (review)
| ### `LightClientHeader` | ||
|
|
||
| ```python | ||
| class LightClientHeader(Container): | ||
| # Beacon block header | ||
| beacon: BeaconBlockHeader | ||
| ``` |
There was a problem hiding this comment.
Maybe add some notes about why we have this wrapper for future compatibility.
There was a problem hiding this comment.
Added comments, and also the stub for is_valid_light_client_header
|
The SSZ format stays the same, which is key, as otherwise we would break compatibility of gossip, potentially disconnecting nodes. As for JSON, clients will have to learn how to deal with the extra Note that no release for beacon-APIs was cut yet that formally locks in the JSON endpoints. |
|
Actually, this one would kill the entirety of #3150 until |
The light client data structures were changed to accommodate additional fields in future forks (e.g., to also hold execution data). There is a minor change to the JSON serialization, where the `header` properties are now nested inside a `LightClientHeader`. The SSZ serialization remains compatible. See ethereum/consensus-specs#3190
The light client data structures were changed to accommodate additional fields in future forks (e.g., to also hold execution data). There is a minor change to the JSON serialization, where the `header` properties are now nested inside a `LightClientHeader`. The SSZ serialization remains compatible. See ethereum/consensus-specs#3190 and ethereum/beacon-APIs#287
There was a problem hiding this comment.
I think it's worth it to make a breaking change on altair spec now that adoption is low to prevent more complexity via legacy code. I'm in favor of #3151 (see my comment for rationale) so we should commit in that direction now
The light client data structures were changed to accommodate additional fields in future forks (e.g., to also hold execution data). There is a minor change to the JSON serialization, where the `header` properties are now nested inside a `LightClientHeader`. The SSZ serialization remains compatible. See ethereum/consensus-specs#3190 and ethereum/beacon-APIs#287
The light client data structures were changed to accommodate additional fields in future forks (e.g., to also hold execution data). There is a minor change to the JSON serialization, where the `header` properties are now nested inside a `LightClientHeader`. The SSZ serialization remains compatible. See ethereum/consensus-specs#3190 and ethereum/beacon-APIs#287
The light client data structures were changed to accommodate additional fields in future forks (e.g., to also hold execution data). There is a minor change to the JSON serialization, where the `header` properties are now nested inside a `LightClientHeader`. The SSZ serialization remains compatible. See ethereum/consensus-specs#3190 and ethereum/beacon-APIs#287
In Altair, light client sync protocol exchanges
BeaconBlockHeaderstructures for tracking current progress. WrappingBeaconBlockHeaderinside aLightClientHeaderallows future extensions of this header, e.g., to also trackExecutionPayloadHeader.Note: This changes the JSON REST format by adding a
beaconnesting. For SSZ, the serialization format stays same (but overall root changes).