Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Define libp2p protocol for light client sync #2802
Define libp2p protocol for light client sync #2802
Changes from 1 commit
56363cd
52a741f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, should it be
beacon_chain/
orlight_client/
?I see that it's a "beacon chain" light client, but for implementers & service providers, maybe there some benefits of distinguishing them?
@etan-status what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on this.
beacon_chain
fits for these reasons:beacon_chain
status
,ping
,metadata
,goodbye
messages, so it cannot solely uselight_client
, at least on libp2plight_client
fits for these reasons:Other impacted API should also be considered, e.g., REST, should it have it under the light client umbrella, or the beacon chain one? What about portal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with either way too! Let's keep it to
beacon_chain
now unless someone has a strong objection one day.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: There may be some room to optimize it, based on how many full nodes enable the light client protocol.
e.g., if most nodes are altruistic and enable light client protocol, maybe we only need to assign the sync committee aggregators to publish the updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it may work with just aggregators, but it's too early to tell. Also have to consider that the topic also extends beyond just full nodes, with all light clients also being subscribed to it, so I erred towards having a few more broadcasts for now. Originally, I implemented it with just the proposer sending the update, but that has the obvious downside of the slot not appearing at all if the proposer does not implement the light client sync protocol – in general, it still worked surprisingly well though.
The messages involved here are tiny (
BeaconBlockHeader
+SyncAggregate
essentially), and the additional broadcast lines up with the general concept to reward the sync committee for enabling light clients to sync. Furthermore, the sync committee already sends messages at 1/3 into the block, so sending the second message around that time also groups up those tasks nicely.