ethereum / beacon-APIs

Collection of RESTful APIs provided by Ethereum Beacon nodes
https://ethereum.github.io/beacon-APIs/
Creative Commons Zero v1.0 Universal
328 stars 167 forks source link

Add `eventstreamV2` #459

Open ensi321 opened 1 month ago

ensi321 commented 1 month ago

We need to communicate fork version during attestation and attester_slashing since it is modified by EIP-7549.

For example, "aggregation_bits":"0x01" will mean different validators in pre and post-EIP-7549.

This PR adds eventstreamV2 which has version in attestation and attester_slashing

Related to #445

nflaig commented 1 month ago

I am in favor of bumping the whole api to v2 instead of just single topics (previous discussion) but I think we should use this opportunity to make other topics more forward compatible to not have to bump the api again at some (unknown) point in the future.

Further topics we could wrap inside version container are

In the worst case if those never happen to change in the future, it would at least make the format of events more consistent

rolfyone commented 1 month ago

i think if we're wanting to go this way we want to put version on every type...

One option would be to just add versioned_attestation to event stream v1... It lets us version out the attestations with no change to the eventstream, and just change the single message we're wanting to update. in future we can do the same at any point with any other event message... This is probably a MVP type solution if we updated V1.

This lets us not duplicate our eventstream framework, im not sure if that only affects our specific endpoints... currently teku has a whole framework around the event stream so this might be a bit of fun (not saying to avoid it, but just as context)

nflaig commented 1 month ago

One option would be to just add versioned_attestation to event stream v1

If we do that, does the attestation topic just handle phase0 attestations then?

This lets us not duplicate our eventstream framework

yeah, we need more feedback from others on the implementation complexity here, might not be as trivial as bumping other apis, it's also a bit more effort on Lodestar side to do this.