Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify fork boundary op pool behavior #458

Closed
realbigsean opened this issue Jul 11, 2024 · 3 comments
Closed

Clarify fork boundary op pool behavior #458

realbigsean opened this issue Jul 11, 2024 · 3 comments

Comments

@realbigsean
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator

nflaig commented Jul 12, 2024

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants