-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
Implement distributed aggregation selections #5344
Conversation
8899e32
to
bbbf68b
Compare
Performance Report✔️ no performance regression detected Full benchmark results
|
f5be68e
to
19342ae
Compare
19342ae
to
c3977a8
Compare
c3977a8
to
2ad316e
Compare
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.
Logic looks good to me. Good job with the helpful comments and generally clean code.
Leaving open for another pair of eyes.
Thanks for the review @wemeetagain! Yeah please let it open, still want to add unit tests and wait for more results from Obol testing this on their cluster |
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 code looks awesome and the comments were even better. Approving for code structure and high-fiving for quality! Reading the spec and implementation notes to familiarize myself.
e4925ec
to
78155a7
Compare
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.
Read thru again. LGTM
Again great attention to detail.
🎉 This PR is included in v1.8.0 🎉 |
Motivation
Implements ethereum/beacon-APIs#224 based on guide, closes #5334
Description
Additional context