Allow second sync committee message to propagate if first one stale#4015
Open
etan-status wants to merge 16 commits intoethereum:masterfrom
Open
Allow second sync committee message to propagate if first one stale#4015etan-status wants to merge 16 commits intoethereum:masterfrom
etan-status wants to merge 16 commits intoethereum:masterfrom
Conversation
The sync committee message signature does not sign over the slot, making it possible for stale messages to be adjusted and replayed, possibly preventing new sync committee messages to propagate. If such a message for an older head is received, allow a second message to propagate that refers to the current head as selected by fork choice, overriding the previous one. Also add a couple sanity checks, as in, disallowing messages referring to unknown or invalid blocks to spread, in line with existing rules for blob sidecars, parent block roots and attestations.
Contributor
Author
|
Implemented in Nimbus: status-im/nimbus-eth2#4953 |
jtraglia
previously approved these changes
Nov 22, 2024
mkalinin
reviewed
Nov 27, 2024
mkalinin
previously approved these changes
Dec 4, 2024
Contributor
mkalinin
left a comment
mkalinin
left a comment
There was a problem hiding this comment.
Looks good to me! But I’d like to see approves from other CL devs before merging
rolfyone
reviewed
Dec 10, 2024
mcdee
suggested changes
Dec 16, 2024
Member
|
If we are changing this, why in altair spec and not in fulu spec? Is this change in a EIP? Seems no more work has happened since December on this, what is the consensus about the likehood of this being approved? |
Contributor
Author
|
The way how the consensus-specs work is that later forks inherit the logic from earlier forks. This documentation change is documenting currently implemented behaviour.
It has 2 approvals |
jtraglia
dismissed stale reviews from mkalinin and themself
The base branch was changed.
etan-status
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
The sync committee message signature does not sign over the slot, making it possible for stale messages to be adjusted and replayed, possibly preventing new sync committee messages to propagate. If such a message for an older head is received, allow a second message to propagate that refers to the current head as selected by fork choice, overriding the previous one.
Also add a couple sanity checks, as in, disallowing messages referring to unknown or invalid blocks to spread, in line with existing rules for blob sidecars, parent block roots and attestations.