-
Notifications
You must be signed in to change notification settings - Fork 252
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
Ignore sync committee contributions that are a subset of ones we've already seen #5440
Ignore sync committee contributions that are a subset of ones we've already seen #5440
Conversation
public static final int VALID_CONTRIBUTION_AND_PROOF_SET_SIZE = 512; | ||
public static final int VALID_SYNC_COMMITTEE_MESSAGE_SET_SIZE = 512; |
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 it still breaks, maybe increase this to be large enough for 2 slots.. i have vague memories...
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.
It should be all good now. Having that too low would make it more likely to pass as we'd process more messages than we need to. But for sync committee messages I'm pretty confident there's no need to keep two slots since we ignore anything not from the current slot anyway.
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.
Yep, all good now.
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.
LGTM
PR Description
Sync sync contribution validation rules to ignore any contribution that we've seen a non-strict superset for already.
Also reduced the cache size for sync committee gossip. There are only 512 members of the sync committee and we automatically ignore messages that aren't for the current slot so at most we need to remember 512 seen messages.
Fixed Issue(s)
fixes #5304
Documentation
doc-change-required
label to this PR if updates are required.Changelog