-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
fix: make sure shuffling is calculated when querying next epoch proposers #7156
Conversation
Performance Report✔️ no performance regression detected 🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
Didn't change anything in regard to |
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.
Pushed it right before I left for dinner. Will check when I get home and fix |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7156 +/- ##
============================================
- Coverage 49.18% 49.17% -0.01%
============================================
Files 598 598
Lines 39801 39801
Branches 2082 2081 -1
============================================
- Hits 19576 19574 -2
- Misses 20184 20186 +2
Partials 41 41 |
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.
Tested the branch on mainnet, issue seems resolved, leaving it up to @twoeths to approve
yep.. its deployed on feat2 |
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, this looks simpler and thanks for keeping state-transition sync @matthewkeil
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
…sers (#7156) * feat: make getBeaconProposersNextEpoch async to await for shuffling calculation * fix: build error in getBeaconProposersNextEpoch * fix: failed unit test * refactor: use tuyen suggestion to await for shuffling keeping state-transition sync
🎉 This PR is included in v1.23.0 🎉 |
Description
Make pulling of next proposers async to await for shuffling calculation
Motivation
Error was found on 1.23 release after #7120 was merged with unstable due to change made in #7130
Notes
@twoeths I made
getBeaconProposersNextEpoch
async to resolve this but curious if you would prefer and async helper function instead. This is the only place wheregetBeaconProposersNextEpoch
gets called so will be an easy switch if you want to keep everything onEpochCache
sync. Just let me know and will fix it quickly. Currently the proposers are cached on theepochCtx
so i could just pass that through the free function to maintain the caching or we can just recompute on each API call if there will not be many. Just let me know which way you prefer to go but I built it like this because it seemed like the best way forward.