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
Simplify sync protocol and update to calculate optimistic heads #2746
Simplify sync protocol and update to calculate optimistic heads #2746
Changes from all commits
25f2efa
013e814
e104164
7718872
c4f7097
06af629
6fa1970
7de1495
c30662b
402c663
916193b
2f618f7
25d88fe
257c241
de89238
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.
Does that mean the light-client should fail if there is a skip period? This seems to be a fairly normal path when a client stops running for a few das.
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.
The light-client can request historic
LightClientUpdate
from the network. It needs at least one update per period to follow along, as it only knowscurrent_sync_committee
andnext_sync_committee
and can only verifyLightClientUpdate
from those periods.However, what is still suboptimal is the case where
finalized_update
is in a different period thanattested_update
, but this is not a problem introduced by this PR. Another tricky case to tackle for the case whereattested_update
is in a different period than the committee which signed it, which probably even requires some heuristics to figure out (as this case depends on there being missed slots at the start of an epoch). For now, these edge cases are all ignored, and the updates are only accepted if all offinalized_update
,attested_update
, and the sync committee signing it come from the same sync committee period.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.
Yes. agree on the updating path will trace sync-comm linkage. Also agree that this is not the issue raised by this PR.
With regard to the edge case...it could cause some weird behaviors temporarily. For example,
apply_light_client_update
is called due to timeout. Then, when there are validupdate.finalized_header
arrives, they will get rejected.Again, this behavior could be better handled if we assume that the light-client can make request for specific LightClientUpdates when it times out or fall out of sync with the current stream of updates. The sync logic would becomes a lot cleaner to be separated into two sync mode: skip-sync mode and normal sync mode.
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.
If the
optimistic_header
was older, I guess it should also be updated here (tofinalized_header
).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.
Does this mean that it could be the case the store.finalized_header is not actually a finalized header, when the apply_light_client_update is called through update timeout?
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.
This was the case in the old version as well, but it was called just
header
there.finalized_header
here seems to have a different meaning than in other contexts, it's justfinalized
for the light client (it won't revert it anymore). Agree that the naming is suboptimal. Likewise, theoptimistic_header
also seems to have a different meaning from the one discussed as part of the merge effort.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.
hmm...if this is the intended "finalization" for the light-client, that is not great.
In the case of timeout, why not just go to the network and ask for a committee changing update? I know that in this spec, we have not specify how to get that information. In any implementation, the light client is going to have to be able to ask for historic updates corresponding to some "sync-committee". If that is available, the finalization of just taking the
store.best_valid_update
is not great. I doubt that real client implementation is going to take this route.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.
If sync committee participation is low, and none of the blocks exceeds the 2/3 majority for a day, there still needs to be a way to proceed though. Not sure how realistic that is for mainnet.
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 think that is fine. If that indeed happens once in blue moon, the light client would stop working syncing. The manual fix for light client operator is to use a newly acquired, trusted starting point. The code owner could also update their client's hard coded starting point. In a way, these manual interventions should be considered desirable because we have unexpected level of participation.
However, if that happens a lot, I think that is more of an incentive design issue. We should consider how to fix that at the protocol level.
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.
Light clients are intended to be able to follow the chain in as similar a way to regular clients as possible. And one of the ethereum staking protocol's core design goals has all along been to have some path to be able to continue making progress under >1/3 offline conditions. So the light client protocol should include some way to do that.
(I'm assuming light clients are going to be used in a lot of contexts, including automated ones, where manual intervention is hard and should be left to resolving 51% attacks)
What is a better alternative to taking
store.best_valid_update
? The regular ethereum protocol advances during the non-finalization case by using the LMD GHOST fork choice rule, which follows the chain that has the most validators supporting it.store.best_valid_update
approximates that. Is there a better approximation?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 would suggest that the light-client's ability to continue could just rely on the "data source", which are invariably backed by full nodes. The exact meaning of data source is not well defined yet because the networking layer could be portal-network, a LES-like p2p network, or a server-client RPC pairing.
When the light-client experiences a timeout or falls behind the current sync-comm, i.e. the incoming Updates are not good enough to advance its finalized_header, the client would revert to a skip-sync mode. In a skip-sync mode, the client asks the "data source" for an update that would advance its sync-committee. A light client does not advance until it somehow find a way to access "finality". Because finality is guaranteed to be found in some data sources, a light client is stuck because it couldn't access the correct data sources (i.e. correct updates).
The guarantee of a light client would find a way to advance should depends on a light client having a way to find the right updates. Again, networking is not defined yet; once it is defined, we can evaluate at what conditions the light-client might not be able to find the appropriate updates.