ChainSafe / ChainSafe/lodestar

Unify churn-limit helper naming to allow non-cache-requiring reuse

Open
#9,626 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.