ChainSafe / ChainSafe/lodestar
Unify churn-limit helper naming to allow non-cache-requiring reuse
- Dominant language
- TypeScript
- Stars
- 1.4k
- Forks
- 483
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 150
Description
## Description
The churn-limit helpers in `packages/state-transition/src/util/validator.ts` have an inconsistent naming convention around whether they require an `EpochCache`:
| Function | Requires cache? |
|---|---|
| `getBalanceChurnLimit(increments, quotient, min)` | No (pure) |
| `getBalanceChurnLimitFromCache(epochCtx)` | Yes |
| `getActivationChurnLimit(epochCtx)` | Yes |
| `getExitChurnLimit(epochCtx)` | Yes |
| `getConsolidationChurnLimit(fork, epochCtx)` | Yes |
| `getActivationExitChurnLimit(epochCtx)` | Yes |
The balance-churn helper distinguishes its pure and cache-backed forms with a `FromCache` suffix, but the exit/activation/consolidation helpers exist only in cache-requiring form and take the *base* name. There is no pure `(config, increments)` counterpart for them, and the base name is already occupied by the cache version.
This came up in #9625 (Gloas `compute_weak_subjectivity_period`, EIP-8061). The plain-state weak-subjectivity path has only `(config, totalActiveBalanceIncrements)` and no `EpochCache`, so it could not reuse the existing exit/activation/consolidation helpers. To avoid the anti-pattern of constructing a fake/partial `EpochCache` cast, that PR introduced `getGloasChurnLimits(config, increments)`, which duplicates the quotient and min choices already encoded in the individual helpers.
Dependency: #9625
## Proposed change
Extract pure `(config, increments)` cores for the exit/activation/consolidation churn limits and have the cache-requiring functions delegate to them, mirroring the existing `getBalanceChurnLimit` / `getBalanceChurnLimitFromCache` split. This requires settling the naming inconsistency — either renaming the cache-taking helpers to `...FromCache` (freeing the base names for the pure cores) or an agreed alternative — which touches call sites across the Gloas processing code (exits, consolidations, registry updates), hence it was kept out of scope in #9625.
Once the pure cores exist, `getGloasChurnLimits` should be removed/reduced so the quotient/min choices live in a single place. And those functions should be called directly in the Gloas branch of weak subjectivity computation.
## Acceptance criteria
- Pure, non-cache-requiring forms exist for exit/activation/consolidation churn limits.
- Cache-requiring helpers delegate to the pure forms (single source of truth for quotients/mins).
- `getGloasChurnLimits` is removed or reduced to a thin delegator.
- Naming is consistent across the churn-limit helpers.
- Existing unit and spec tests remain green.
Contributor guide
Research direction
Start in packages/state-transition/src/util/validator.ts and review the existing cache-backed churn-limit helpers alongside getBalanceChurnLimit and getBalanceChurnLimitFromCache. Trace their call sites in the Gloas processing code and the weak-subjectivity path introduced for #9625. Done means pure forms, delegated cache forms, consistent naming, and green unit and spec tests without duplicated quotient or minimum choices.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- blockchain
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100