Skip to content

cl/phase1/forkchoice: use the incremental indexed weight store for the GLOAS head#21694

Open
yperbasis wants to merge 11 commits into
mainfrom
yperbasis/caplin-incremental-forkchoice-weights
Open

cl/phase1/forkchoice: use the incremental indexed weight store for the GLOAS head#21694
yperbasis wants to merge 11 commits into
mainfrom
yperbasis/caplin-incremental-forkchoice-weights

Conversation

@yperbasis

@yperbasis yperbasis commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

Compute the GLOAS fork-choice head (getHeadGloas) from the incremental
indexedWeightStore instead of the full-scan weightStore. The index is
maintained on every GLOAS attestation (see #21698) but was not previously read
for head selection. Because that maintenance is GLOAS-gated, the index is cold
at the pre-GLOAS→GLOAS transition; getHeadGloas seeds it from latestMessages
on first use so it is not missing votes cast before activation.

Scoped to GLOAS only — pre-GLOAS fork choice is unchanged.

Changes

  • getHeadGloas scores via headWeightStore(cs) (the index), fetching the
    justified checkpoint state before acquiring f.mu (a nil state degrades to
    zero attestation weight, as before — same pattern as pre-GLOAS getHead).
  • indexedWeightStore is now structural: it stores only
    (validator, slot, payloadPresent) per target root and reads balance +
    active/slashed status fresh from the per-query justified checkpoint state
    in GetAttestationScore, instead of caching balances that go stale when the
    checkpoint advances. IndexVote no longer does a per-vote getCheckpointState
    lookup.
  • RemoveVote is now allocation-free (in-place compaction). It runs per
    attestation on the GLOAS vote path, so the previous fresh-slice-per-call was a
    CPU/GC hotspot under the fork-choice lock on high-validator-count networks.
  • onNewFinalized drops indexed votes for finalized-away roots.
  • On the first getHeadGloas, headWeightStore seeds the index once from the
    full latestMessages snapshot (seedFromLatestMessages, clear-then-rebuild).
    Index maintenance only begins at GLOAS activation, so without the seed the head
    would undercount attestation weight at the transition until validators
    re-attest.
  • Removed dead index API: the unused GetIndexedWeightStore accessor,
    GetWeightStore (orphaned once getHeadGloas switched to headWeightStore;
    NewWeightStore stays for isHeadWeak), Invalidate (cleared votes but left
    seeded set, so any future caller would silently undercount weights), and the
    write-only version counter.

Testing

  • TestIndexedWeightStoreMatchesFullScan: starts from a cold index, has
    headWeightStore seed it, then asserts it returns the same GetWeight /
    GetAttestationScore as the trusted full-scan weightStore for every node in
    the filtered tree (non-vacuous) — exercising the real seed path.
  • TestSeedFromLatestMessagesIsIdempotent: the seed mirrors latestMessages
    once and is a no-op thereafter (no double-counting).
  • TestRemoveVoteCompactsInPlace: removal keeps the other voters and drops the
    root once empty.
  • TestPruneFinalizedDropsFinalizedAndUnknownRoots: votes targeting roots at or
    below the finalized slot, or missing from the fork graph, are dropped; votes
    for live roots above it survive.
  • TestOnNewFinalizedPrunesIndexedVotes: the onNewFinalized wiring empties the
    index once finalization passes every indexed root. Both pruning tests were
    mutation-checked: they fail against a no-op pruneFinalized.
  • Existing fork-choice tests and cl/phase1/forkchoice: don't maintain GLOAS indexed weight store pre-GLOAS #21698's GLOAS/pre-GLOAS maintenance tests pass;
    go test ./cl/phase1/forkchoice/... and scoped golangci-lint clean.

Follow-up

The indexed store and its transition seed are superseded by the delta-propagation
rework tracked in #21704 (the canonical proto-array model both Prysm and
Lighthouse ship).

@yperbasis yperbasis requested a review from Copilot June 9, 2026 11:05
@yperbasis yperbasis added the Caplin Caplin: Consensus Layer, Beacon API label Jun 9, 2026
@yperbasis yperbasis added this to the 3.6.0 milestone Jun 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Caplin’s fork-choice head selection to compute head weights incrementally using the existing indexedWeightStore (instead of per-GetHead full validator scans / sampled approximations), and wires this approach into both pre-GLOAS and GLOAS head paths. It also removes the probabilistic/sampled head-weight path and adds a differential test validating indexed vs full-scan weight calculations.

Changes:

  • Switch pre-GLOAS getHead and GLOAS getHeadGloas to score candidates via an incremental indexedWeightStore seeded with the per-query justified checkpoint state.
  • Make indexedWeightStore structural (root → vote entries) and read balances/active/slashed status fresh from the checkpoint state during scoring; prune indexed votes on finalization.
  • Remove probabilistic head getter plumbing and add a new diff test + benchmark comparing indexed vs full-scan scoring.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
cmd/caplin/caplin1/run.go Removes probabilistic head-weight sampling flag/wiring when constructing forkchoice store.
cl/spectest/consensus_tests/fork_choice.go Updates spectest forkchoice store construction for new constructor signature.
cl/phase1/forkchoice/weight_store.go Adds headWeightStore(cs) to reuse the incremental indexed store during head walks.
cl/phase1/forkchoice/weight_store_indexed.go Removes stored balances from indexed votes, adds per-pass checkpointState, adds finalized pruning, and updates scoring to read balances fresh.
cl/phase1/forkchoice/weight_store_diff_test.go Adds differential test ensuring indexed store matches full-scan store + a benchmark comparison.
cl/phase1/forkchoice/utils.go Prunes indexed votes when new finalized checkpoint is observed.
cl/phase1/forkchoice/get_head.go Replaces legacy vote accounting/sampling with incremental weight scoring for both GLOAS and pre-GLOAS head selection.
cl/phase1/forkchoice/forkchoice.go Removes probabilistic head getter field/param; constructor signature updated accordingly.
cl/phase1/forkchoice/fork_choice_test.go Updates tests for the constructor signature change and adds an end-to-end ex-ante proposer boost vs attestations test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cl/phase1/forkchoice/weight_store_diff_test.go Outdated
Comment thread cl/phase1/forkchoice/weight_store_diff_test.go
Comment thread cl/phase1/forkchoice/weight_store_indexed.go
Comment thread cl/phase1/forkchoice/get_head.go
Comment thread cl/phase1/forkchoice/get_head.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread cl/phase1/forkchoice/get_head.go
Comment thread cl/phase1/forkchoice/weight_store_diff_test.go
@yperbasis yperbasis marked this pull request as draft June 9, 2026 11:37
@yperbasis yperbasis added the Glamsterdam https://eips.ethereum.org/EIPS/eip-7773 label Jun 9, 2026
@yperbasis yperbasis modified the milestones: 3.6.0, 3.7.0 Jun 9, 2026
@yperbasis yperbasis force-pushed the yperbasis/caplin-incremental-forkchoice-weights branch from d3df030 to c360d5c Compare June 9, 2026 13:21
@yperbasis yperbasis changed the title cl/phase1/forkchoice: compute fork-choice head weights incrementally cl/phase1/forkchoice: use indexed weight store for GLOAS head Jun 9, 2026
getHeadGloas scored the head with the full-scan weightStore while the
incremental indexedWeightStore — already maintained on every attestation —
went unused. Score the GLOAS head via the index instead.

Make the index structural: store only (validator, slot, payloadPresent) per
target root and read balance + active/slashed status fresh from a per-query
justified checkpoint state in GetAttestationScore, instead of caching balances
that go stale when the checkpoint advances. Drop indexed votes for
finalized-away roots in onNewFinalized.

Pre-GLOAS fork choice is unchanged.

Validated by a differential test asserting the index equals the full-scan
weightStore for GetWeight/GetAttestationScore across the filtered tree.
@yperbasis yperbasis force-pushed the yperbasis/caplin-incremental-forkchoice-weights branch from c360d5c to b8aac17 Compare June 9, 2026 13:37
Resolves the on_attestation.go conflict by taking #21698's GLOAS-gated index
maintenance (the stale IndexVote comment this branch fixed was removed there).

Since #21698 gates indexed-vote maintenance to the GLOAS vote path, the
differential test's pre-GLOAS fixtures no longer populate the index, so
buildExAnteStore now mirrors latestMessages into the index directly.
@yperbasis yperbasis modified the milestones: 3.7.0, 3.6.0 Jun 9, 2026
yperbasis added 2 commits June 9, 2026 15:54
…ial test

headWeightStore documents that f.mu must be held for the scoring pass;
keep that contract in the test (getCheckpointState stays outside the lock,
mirroring production).
RemoveVote allocated a fresh slice per call to filter out a validator's entry.
On the GLOAS vote path it runs per attestation, so on high-validator-count
networks it is a top CPU consumer and GC source under the fork-choice lock
(the cost #21698 removed pre-GLOAS but which returns under GLOAS now that
getHeadGloas reads the index). Compact the entry out in place instead.
@yperbasis yperbasis changed the title cl/phase1/forkchoice: use indexed weight store for GLOAS head cl/phase1/forkchoice: use the incremental indexed weight store for the GLOAS head Jun 9, 2026
@yperbasis yperbasis requested a review from Copilot June 9, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread cl/phase1/forkchoice/weight_store_indexed.go
Comment thread cl/phase1/forkchoice/weight_store_diff_test.go
… the benchmark

GetAttestationScore converted entry.ValidatorIndex to int before the bounds
check, so an index past math.MaxInt (e.g. on a 32-bit build) could wrap negative
and bypass it. Bounds-check the uint64 first, convert only once it is in range.
Also hold f.mu across the differential benchmark's scoring loops, matching the
headWeightStore contract the test already follows.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread cl/phase1/forkchoice/weight_store.go
Comment thread cl/phase1/forkchoice/weight_store.go Outdated
Comment thread cl/phase1/forkchoice/weight_store_diff_test.go
yperbasis added 2 commits June 9, 2026 17:06
…mark cs

GetIndexedWeightStore has no callers and now exposes a store that returns 0
unless primed via the unexported setCheckpointState, so remove it. The
differential benchmark ignored getCheckpointState's error/nil result and could
silently measure the degenerate cs==nil path; assert it like the correctness
test does.
getHeadGloas reads the indexedWeightStore, but index maintenance is GLOAS-gated
(#21698): pre-GLOAS attestations fill latestMessages without touching the index.
At the pre-GLOAS->GLOAS boundary GetHead flips from the full-scan store (which
reads all of latestMessages) to the index, still empty — so attestation weight
is understated until validators re-attest over ~1 epoch, risking a wrong head if
a fork straddles the transition.

The spec keeps store.latest_messages as a single dict never reset across forks,
and Prysm/Lighthouse keep one continuous per-validator vote store; Erigon already
maintains latestMessages continuously, so make the derived index match it at
first use: headWeightStore imports the full latestMessages snapshot into the
index once (clear-then-rebuild, so early GLOAS votes are not double-counted).

The differential test now starts from a cold index and asserts headWeightStore
seeds it to equal the full-scan store; a new test covers seed idempotency.

Interim fix; superseded by the delta-propagation rework tracked in #21704.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread cl/phase1/forkchoice/weight_store_indexed_test.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread cl/phase1/forkchoice/weight_store_indexed.go
@yperbasis yperbasis marked this pull request as ready for review June 10, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Caplin Caplin: Consensus Layer, Beacon API Glamsterdam https://eips.ethereum.org/EIPS/eip-7773 performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants