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

Add finalized flag to chain data requests #254

Merged
merged 6 commits into from
Nov 9, 2022

Conversation

arnetheduck
Copy link
Contributor

Whether or not a request pertains to the finalized section of the chain (per the view of the client fork choice) is somewhat cumbersome to discover.

This PR adds a boolean that allows clients to distinguish a response that has been finalized and thus is unlikely to change from one that may still change over time (specially when using slot-based requests).

A flag like this can also be used for the purpose of verifying that a checkpoint root indeed is part of chain history and is likely to remain as such, as discussed in
#226

Whether or not a request pertains to the finalized section of the chain
(per the view of the client fork choice) is somewhat cumbersome to
discover.

This PR adds a boolean that allows clients to distinguish a response
that has been finalized and thus is unlikely to change from one that may
still change over time (specially when using slot-based requests).

A flag like this can also be used for the purpose of verifying that a
checkpoint root indeed is part of chain history and is likely to remain
as such, as discussed in
ethereum#226
@arnetheduck arnetheduck mentioned this pull request Oct 28, 2022
@ajsutton
Copy link
Contributor

ajsutton commented Nov 2, 2022

Makes sense to me. For most if not all of these APIs Teku already knows if it's finalized as part of gathering the data anyway so just a matter of plumbing to expose it.

@dapplion
Copy link
Collaborator

dapplion commented Nov 2, 2022

Is there demand for this feature outside of the Checkpoint Sync API? Shouldn't be too hard to support, but would appreciate more rationale

@arnetheduck
Copy link
Contributor Author

Is there demand

well .. finalized data is .. final - means services that follow the chain can do the same optimizations as us when it comes to single vs multiple histories making it easier for them to correctly handle reorgs etc. ie many of the popular tools have had this wrong and it's extremely annoying when they don't display the correct information - this gives them a reliable source.

Copy link
Collaborator

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on-board with the idea, and I guess we don't need new version numbers for the endpoints because we're just adding fields, like we did with execution_optimistic.

I think this would be easy to implement in Lighthouse but won't really know for sure until we dig into it. Would be happy to merge this PR in the meantime.

Copy link
Collaborator

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM also after some more eyes.

@rolfyone rolfyone merged commit b3c4def into ethereum:master Nov 9, 2022
@arnetheduck arnetheduck deleted the finalized-flag branch September 28, 2023 15:01
@Falehfale Falehfale mentioned this pull request Sep 28, 2023
Closed
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

Successfully merging this pull request may close these issues.

None yet

5 participants