-
-
Notifications
You must be signed in to change notification settings - Fork 369
feat: update weak subjectivity calculation for electra #7735
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
base: unstable
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7735 +/- ##
=========================================
Coverage 50.14% 50.14%
=========================================
Files 605 605
Lines 40687 40726 +39
Branches 2223 2222 -1
=========================================
+ Hits 20402 20422 +20
- Misses 20243 20262 +19
Partials 42 42 🚀 New features to boost your workflow:
|
Performance Report✔️ no performance regression detected Full benchmark results
|
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.
just for reference, this ethereum/consensus-specs#4179 is the actual PR
balanceChurnLimit: number, // Note this is not the same as churnLimit in `computeWeakSubjectivityPeriodFromConstituentsPhase0` | ||
minWithdrawabilityDelay: number | ||
): number { | ||
const t = totalBalanceByIncrement; // Keep t as increment for now. Multiply final result by EFFECTIVE_BALANCE_INCREMENT |
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.
we should put comments above the variable, not at the end of the line
@@ -97,6 +129,18 @@ export function computeWeakSubjectivityPeriodFromConstituents( | |||
return wsPeriod; | |||
} | |||
|
|||
export function computeWeakSubjectivityPeriodFromConstituentsElectra( | |||
totalBalanceByIncrement: number, | |||
balanceChurnLimit: number, // Note this is not the same as churnLimit in `computeWeakSubjectivityPeriodFromConstituentsPhase0` |
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 function signature is really not ideal, it would be better to get it closer to how the spec works but I can see why we do it like this as well
Related spec ethereum/consensus-specs#4243
Closes #7645