-
Notifications
You must be signed in to change notification settings - Fork 973
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
Handle empty aggregation bits as discussed in #1713 #1780
Conversation
9458547
to
47ed5b6
Compare
Note that this change is a bit at odds with the comment here -- #1732 (comment) If we go through with this PR and with #1732, we should probably explicitly disallow 0-participation aggregates on the gossip layer |
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 looks good.
Seems to be consensus that this is the way to go.
Note: This PR is a specific/explicit solution to attestation inclusion. 0xc0 sigs/emptiness will come up again in other contexts in Phase 1
tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py
Outdated
Show resolved
Hide resolved
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.
handled a bit of minor PR feedback. ready for merge!
Fix #1713
Marking this as substantive, since it technically breaks existing behavior, if there is any attestation with 0 participants on-chain.
In the process of making the empty-participants case invalid, some tests didn't make sense anymore. Others were broken. Removed the invalid tests, fixed the broken ones, and added more testing (mostly related to rewards/penalties participation changes, re-using functions of the empty/almost-empty cases)
Edit: forced push 2nd commit that handles additional tests. Changed rng seeding to be different between test cases.