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

Clarify fork boundary op pool behavior #458

Closed realbigsean closed 2 months ago

realbigsean commented 2 months ago

Not sure if this has already been discussed, but GET queries to the op pool around the fork boundary return a list of what could be mixed message types, some pre-electra and some post-electra. It's not clear how this should be handled considering we are adding version: electra to the top-level of the JSON, implying the message should be interpreted as containing electra structs.

In Lighthouse we're opting to convert all messages to their electra version in electra, but this may be unexpected as it is unspecified.

mehdi-aouadi commented 2 months ago

In Teku we went with returning Electra messages only and ignored the other ones. This behaviour sounds inline with the decision of dropping pre Electra messages once we reach the fork. If we take as example the Attestations, the first block post Electra will contain Electra attestations only and IMO the GET queries should be inline with that. Repackaging non Electra messages to their Electra version might be an easier implementation but didn't look accurate to me. I'm wondering what would be the use cases and which approach would be the best

nflaig commented 2 months ago

We had a quick discussion on the PR that we don't want mixed attestation arrays. I tend to agree that it might be better to just drop them from the response as I would expect the only reason why they are still in the op pool is that it's not worth it to prune it at fork boundry.

realbigsean commented 2 months ago

switched this to filtering in LH i think that makes more sense, thanks for the feedback guys 🙏