feat: minimal PTC caching with single previous_ptc field#13
feat: minimal PTC caching with single previous_ptc field#13lodekeeper wants to merge 7 commits intounstablefrom
Conversation
Implements alternative to consensus-specs#4992 for addressing ChainSafe#4979's epoch-boundary PTC bug with minimal state changes. Spec: lodekeeper/consensus-specs@feat/minimal-ptc-caching Changes: - Add previousPtc field to gloas BeaconState SSZ type (~4KB, single Vector) - New processPtcUpdate() called at START of process_epoch, before effective balance updates alter the weighted selection - New computePayloadTimelinessCommitteeAtSlot() for on-demand single-slot PTC computation (extracted from epoch-batch computation) - EpochCache: replace previousPayloadTimelinessCommittees (full epoch array) with previousEpochLastSlotPtc (single Uint32Array) - getPayloadTimelinessCommittee() returns cached previousEpochLastSlotPtc for epoch-boundary case, epochCtx cache for current epoch - Initialize previousPtc to zeros in upgradeStateToGloas and genesis - Update specrefs for compute_ptc, get_ptc, process_ptc_update Key design decisions: - No per-slot state rotation (unlike ChainSafe#4992) - Only the last slot of the previous epoch is cached in state because process_payload_attestation enforces data.slot + 1 == state.slot - All epoch PTCs still computed eagerly in epochCtx for duties API - get_ptc_assignment preserved (unlike ChainSafe#4992 which removed it) Co-authored-by: Nico Flaig <nflaig@users.noreply.github.com>
- compute_ptc and process_ptc_update are custom functions not in official gloas spec → remove <spec fn> tags, document as plain spec without ethspecify tracking - get_ptc entry was using placeholder hash 'minimal-ptc-caching' → restore official get_ptc specref (hash ae15f761) at the implementation source - specrefs are now in sync: ethspecify process + check both pass locally
|
|
||
| // Compute PTC for all slots in the prev/current epoch | ||
| let previousPayloadTimelinessCommittees: Uint32Array[] = []; | ||
| const previousEpochLastSlotPtc = new Uint32Array(0); |
There was a problem hiding this comment.
this should be retrieved from state and set in the cache, or alternative is to only use the cache for current epoch and use field in state for this case instead
There was a problem hiding this comment.
Fixed — now loads previousPtc from state into the epochCtx cache in createFromState:
previousEpochLastSlotPtc = new Uint32Array((state as CachedBeaconStateGloas).previousPtc.getAll());This ensures epoch-boundary PTC lookups work correctly after state loading (checkpoint sync, replay). The cache still handles current epoch PTCs, and previousEpochLastSlotPtc is only used for the last slot of the previous epoch.
Also renamed the SSZ field previousEpochLastPtc → previousPtc to match the consensus-specs naming (previous_ptc).
Address review feedback: previousEpochLastSlotPtc in epochCtx was initialized to empty Uint32Array(0) instead of being loaded from state.previousPtc. Now reads from state on createFromState so epoch-boundary PTC lookups work correctly after state loading (e.g. checkpoint sync, state replay). Also rename SSZ field previousEpochLastPtc → previousPtc to match consensus-specs naming convention (previous_ptc).
c4ab249 to
a9e701c
Compare
| // [New in Gloas:EIP7732] Cache last-slot PTC before epoch processing changes effective balances | ||
| if (fork >= ForkSeq.gloas) { | ||
| processPtcUpdate(postState as CachedBeaconStateGloas); | ||
| } |
There was a problem hiding this comment.
shoudln't this be part of process epoch as per spec?
There was a problem hiding this comment.
Good catch — moved processPtcUpdate into processEpoch (epoch/index.ts) as the first operation, matching the spec where process_ptc_update is the first step inside process_epoch. stateTransition.ts is now clean (no diff from unstable).
process_ptc_update is defined as the first step inside process_epoch in the consensus spec, not a separate call before it. Move from stateTransition.ts into epoch/index.ts as first operation before processJustificationAndFinalization, gated by fork >= ForkSeq.gloas.
…state directly The state field previous_epoch_last_ptc IS the cache — reading it is O(1). No need to duplicate it in epochCache. - Remove previousEpochLastSlotPtc from EpochCache (field, constructor, createFromState, clone, afterProcessEpoch) - getPayloadTimelinessCommittee now only handles current epoch (throws for previous) - processPayloadAttestation reads state.previousEpochLastPtc directly for epoch boundary case - Remove unused CachedBeaconStateGloas import from epochCache
|
Closing — spec changes are under discussion upstream. Nico will bring these to Lodestar once the spec settles. Branch preserved for reference. |
Alternative to consensus-specs#4992 that addresses #4979's epoch-boundary PTC bug with minimal state changes.
CL Spec
Branch: lodekeeper/consensus-specs@feat/minimal-ptc-caching
Design
Single
previous_ptcfield in BeaconState (~4KB) vs ChainSafe#4992's two fields + per-slot rotation (~8KB + per-slot state mutation).Key differences from ChainSafe#4992:
previous_ptc)previous_ptc+current_ptc)process_slotsmodifiedget_ptc_assignmentHow it works:
compute_ptc(state, slot)— pure computation (extracted from oldget_ptc)process_ptc_update— new step at START ofprocess_epoch, savescompute_ptc(state, state.slot)tostate.previous_ptcbefore effective balance updatesget_ptc(state, slot)— returns cachedprevious_ptcfor epoch-boundary case (last slot of previous epoch), delegates tocompute_ptcotherwiseWhy only one slot needs caching:
process_payload_attestationenforcesdata.slot + 1 == state.slot. The cross-epoch case (where effective balances differ) only occurs when processing a block at the first slot of epoch N with payload attestations from the last slot of epoch N-1.Implementation changes:
packages/types/src/gloas/sszTypes.ts— addPayloadTimelinessCommitteetype +previousPtctoBeaconStatepackages/state-transition/src/util/seed.ts— newcomputePayloadTimelinessCommitteeAtSlot()packages/state-transition/src/util/gloas.ts— newprocessPtcUpdate()packages/state-transition/src/stateTransition.ts— callprocessPtcUpdatebefore epoch processingpackages/state-transition/src/cache/epochCache.ts— replace full previous-epoch array with singlepreviousEpochLastSlotPtcpackages/state-transition/src/slot/upgradeStateToGloas.ts— initializepreviousPtcat fork boundarypackages/state-transition/src/util/genesis.ts— initializepreviousPtcat genesis