feat: implement compute_weak_subjectivity_period for Gloas (EIP-8061)#9625
feat: implement compute_weak_subjectivity_period for Gloas (EIP-8061)#9625markolazic01 wants to merge 2 commits into
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements the weak subjectivity period calculation for the Gloas fork, introducing getGloasChurnLimits and computeWeakSubjectivityPeriodFromConstituentsGloas along with corresponding unit tests and spec reference updates. The review feedback suggests optimizing computeWeakSubjectivityPeriod by moving the getGloasChurnLimits call inside the Gloas-specific conditional block. This prevents unnecessary execution and potential NaN errors on older forks where Gloas configuration parameters might not be defined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const churnLimitsGloas = getGloasChurnLimits(config, totalActiveBalanceIncrements); | ||
|
|
||
| return isForkPostGloas(fork) | ||
| ? computeWeakSubjectivityPeriodFromConstituentsGloas( | ||
| totalActiveBalanceIncrements, | ||
| getBalanceChurnLimit( | ||
| totalActiveBalanceIncrements, | ||
| config.CHURN_LIMIT_QUOTIENT, | ||
| config.MIN_PER_EPOCH_CHURN_LIMIT_ELECTRA | ||
| ), | ||
| churnLimitsGloas.exit, | ||
| churnLimitsGloas.activation, | ||
| churnLimitsGloas.consolidation, | ||
| config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY | ||
| ) | ||
| : computeWeakSubjectivityPeriodFromConstituentsPhase0( | ||
| activeIndices.length, | ||
| totalActiveBalanceIncrements, | ||
| getChurnLimit(config, activeIndices.length), | ||
| config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY | ||
| ); | ||
| : isForkPostElectra(fork) | ||
| ? computeWeakSubjectivityPeriodFromConstituentsElectra( | ||
| totalActiveBalanceIncrements, | ||
| getBalanceChurnLimit( | ||
| totalActiveBalanceIncrements, | ||
| config.CHURN_LIMIT_QUOTIENT, | ||
| config.MIN_PER_EPOCH_CHURN_LIMIT_ELECTRA | ||
| ), | ||
| config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY | ||
| ) | ||
| : computeWeakSubjectivityPeriodFromConstituentsPhase0( | ||
| activeIndices.length, | ||
| totalActiveBalanceIncrements, | ||
| getChurnLimit(config, activeIndices.length), | ||
| config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY | ||
| ); |
There was a problem hiding this comment.
Currently, getGloasChurnLimits is called unconditionally, even for pre-Gloas forks. If Gloas configuration parameters (like CHURN_LIMIT_QUOTIENT_GLOAS) are not defined or initialized on older forks or certain networks, this will result in NaN values being computed and passed around, which is unnecessary and potentially error-prone. Moving this calculation inside the isForkPostGloas(fork) conditional block ensures it is only executed when actually needed.
if (isForkPostGloas(fork)) {
const churnLimitsGloas = getGloasChurnLimits(config, totalActiveBalanceIncrements);
return computeWeakSubjectivityPeriodFromConstituentsGloas(
totalActiveBalanceIncrements,
churnLimitsGloas.exit,
churnLimitsGloas.activation,
churnLimitsGloas.consolidation,
config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY
);
}
return isForkPostElectra(fork)
? computeWeakSubjectivityPeriodFromConstituentsElectra(
totalActiveBalanceIncrements,
getBalanceChurnLimit(
totalActiveBalanceIncrements,
config.CHURN_LIMIT_QUOTIENT,
config.MIN_PER_EPOCH_CHURN_LIMIT_ELECTRA
),
config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY
)
: computeWeakSubjectivityPeriodFromConstituentsPhase0(
activeIndices.length,
totalActiveBalanceIncrements,
getChurnLimit(config, activeIndices.length),
config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY
);There was a problem hiding this comment.
Noted this in the PR desc, it is not perfect but it is imo harmless and much more readable as is.
Willing to change this if needed ofc.
Motivation
Implements the Gloas
compute_weak_subjectivity_period(modified in EIP-8061),which the weak-subjectivity guide updated to weight churn asymmetrically.
Resolves the gap left by #9305, which implements EIP-8061.
Description
computeWeakSubjectivityPeriodFromConstituentsGloas, following thePhase0/Electra
FromConstituentspattern (caller supplies pre-computed churnlimits; function does only the period math).
deltaper spec:2/3 * exit + 1/3 * activation + consolidation,flooring each weighted term independently, then apply the increment→Gwei
scaling as the Electra path does.
computeWeakSubjectivityPeriodandcomputeWeakSubjectivityPeriodCachedState(Gloas is post-Electra, so theorder matters).
getGloasChurnLimitshelper for the plain-state path, which has noEpochCacheto hand the existing epochCtx-based churn helpers.compute_weak_subjectivity_period#gloasin specrefs (remove thealpha.3 "not yet implemented" exception, point it at the implementation).
integer Gwei arithmetic, covering the exit MIN floor, the activation cap, and
the exit/activation asymmetry EIP-8061 introduces.
Closes #9337
getGloasChurnLimitsis called before the fork specificcomputeWeakSubjectivitycalls for readability, it also runs on pre-Gloas forks but is negligible.getGloasChurnLimitsis a function made due to reasons of existing naming entanglement:getBalanceChurnLimit-> requires no epoch cachegetBalanceChurnLimitFromCache-> requires cachegetActivationChurnLimit(and other wrappers) -> require cacheThis didn't allow for elegant creation of 'non-cache-requiring' wrappers or helpers, and that is why the most elegant solution seemed to be
getGloasChurnLimits.This topic should be addressed in another PR, I'll open a follow-up issue for it.
AI Assistance Disclosure
Used Claude to navigate the repo, review the diffs, and derive/verify the unit test values; it also drafted parts of this description. All code was authored and applied manually.