Skip to content

perf(fork-choice): precompute per-chain attestation scores (FCR)#9227

Draft
twoeths wants to merge 6 commits intofeature/fast-confirmationfrom
te/fcr_precomputeChainAttestationScores
Draft

perf(fork-choice): precompute per-chain attestation scores (FCR)#9227
twoeths wants to merge 6 commits intofeature/fast-confirmationfrom
te/fcr_precomputeChainAttestationScores

Conversation

@twoeths
Copy link
Copy Markdown
Contributor

@twoeths twoeths commented Apr 17, 2026

Motivation

  • Compute attestation scores in batch, following Lighthouse (sigp/lighthouse#8951).
  • Also leverage state.getEffectiveBalanceIncrementsZeroInactive() to loop through validators fast (no per-index toValue() deserialization).

Description

  • Replace per-block getAttestationScore with a per-chain precomputeChainAttestationScores + isOneConfirmedWithScore pair. One pass over validators walks each vote's parent chain until it lands on a canonical-chain block; a single suffix sum materialises the score for every block on the chain — O(V × depth + B) vs. the previous O(B × V × depth).
  • Add unslashedActiveBalances to FastConfirmationBalanceSource (pre-zeroed for inactive + slashed) so the hot loop never touches the beacon state.
  • Structured as 6 self-contained commits (see the commit log); the old isOneConfirmed / getAttestationScore / ensureVoteMaps path is removed in the final commit.

part of #8837

AI Assistance Disclosure

Designed and implemented with Claude Code (Opus 4.7).

twoeths and others added 6 commits April 17, 2026 16:34
Add `unslashedActiveBalances` to `FastConfirmationBalanceSource`, populated
via `state.getEffectiveBalanceIncrementsZeroInactive()` in `getBalanceSource`.
Mirrors Lighthouse's `unslashed_balance` snapshot — preparing for the
per-validator hot loop in `precomputeChainAttestationScores` to avoid
beacon-state access on every iteration.

No behavior change: no callers consume the new field yet.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add three read-only accessors that expose ProtoArray internals needed by the
per-chain precompute coming in the next commit:

- `getNodeIndices(root)` — all variant indices for a block root (Gloas-aware)
- `getProtoNodeView()` — read-only `{nodes}` for parent-walking
- `getVoteNextIndices()` — read-only handle on per-validator vote indices

Hot-loop callers hoist these once and then do plain array access, avoiding
per-element function-call overhead (up to O(V × depth) iterations per
precompute invocation).

`getLatestMessage` stays in place — it is still used by the slot-range support
functions. Its removal is deferred to the C2 follow-up.

No behavior change: no callers consume the new accessors yet.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `precomputeChainAttestationScores` — a per-chain batch replacement for the
spec's per-block `get_attestation_score`. One pass over validators walks each
vote's parent chain until it lands on a canonical-chain block; a single suffix
sum then materialises the score for every block on the chain.

Reduces cost from O(B × V × depth) to O(V × depth + B). Mirrors Lighthouse's
`precompute_chain_attestation_scores` (sigp/lighthouse#8951), with a Lodestar
refinement: `voteNextIndices[i]` is already the ProtoArray node index, so the
walk skips the per-validator `indices.get(vote_root)` hash lookup.

The hot loop reads only `balanceSource.unslashedActiveBalances`, the hoisted
ProtoArray nodes view, and `voteNextIndices` — no beacon-state access. A small
fast-path cache (`lastVoteIdx` / `lastLandedPos`) skips the parent walk for
consecutive validators voting for the same target, mirroring Lighthouse.

The function is not yet called by any rule. It is covered by a new equivalence
test matrix that asserts per-block equality against the still-live
`getAttestationScore` for nine fixtures (linear chain, off-chain forks,
equivocators, slashed, null votes, degenerate chain, deep forks crossing
terminalSlot, mixed filters) plus a direct Gloas-variant-collapse unit test.

Also fix the test `makeState` mock so its `getEffectiveBalanceIncrementsZeroInactive`
zeroes slashed entries — matches production semantics and is required by the
equivalence test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ed scores

Add `isOneConfirmedWithScore` (spec `is_one_confirmed` with the attestation
score passed in, so the caller can precompute per-chain) and the helper
`getPrecomputedScoreOrThrow` (loud miss, not silent zero).

In `findLatestConfirmedDescendant`, precompute chain scores once at the top
over `head → latestConfirmedRoot` and reuse for both loops. Scores are valid
for loop 2's chain (a tip-side prefix of loop 1's) by the chain-prefix
argument — documented inline with the precompute call.

`isConfirmedChainSafe` still uses the legacy `isOneConfirmed` + `getAttestationScore`
path; migrated in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Precompute chain scores once over `confirmedRoot → startRoot` with
sourceKey "previous", then swap the loop's `isOneConfirmed` call for
`isOneConfirmedWithScore`. The legacy `isOneConfirmed` now has zero callers
but stays in the codebase for one more commit — deletion is the final step.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Delete `isOneConfirmed`, `getAttestationScore`, `ensureVoteMaps`, and the
now-orphaned `isDescendantCached`. All callers of the fast confirmation rule
go through `isOneConfirmedWithScore` + `precomputeChainAttestationScores`
after steps 4-5.

Also drop:
- `voteWeightBySource` and `isDescendantByRootPair` from `FastConfirmationCache`
  (sole consumers were the deleted functions).
- The unused `sourceKey` parameter from `precomputeChainAttestationScores`.

Tests:
- Migrate the `isOneConfirmed` unit test in `fastConfirmation.test.ts` to
  exercise `isOneConfirmedWithScore` via a precomputed score.
- Rewrite the equivalence test as a regression test with hand-constructed
  expected scores, since the legacy `getAttestationScore` is gone. Caught
  and fixed one incorrect expected value in fixture 8 along the way.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes the fast confirmation logic by introducing precomputeChainAttestationScores, which reduces the complexity of attestation score calculations from O(B * V * depth) to O(V * depth + B) through a single-pass validator walk and suffix sum. The review feedback suggests further performance refinements in this hot path, specifically replacing Map and standard Array with Int32Array and Float64Array to reduce hashing overhead and optimize memory usage.

Comment on lines +356 to 361
const indexToPosition = new Map<number, number>();
for (let pos = 0; pos < chain.length; pos++) {
for (const nodeIdx of ctx.getNodeIndices(chain[pos])) {
indexToPosition.set(nodeIdx, pos);
}
}
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.

high

Using a Map<number, number> for indexToPosition in this hot path introduces significant hashing overhead, especially since it's queried up to $O(V \times \text{depth})$ times per tick (where $V$ is the validator count, e.g., 500k+). Since nodeIdx is an index into the protoNodes array, we can use a Int32Array initialized with -1 for much faster $O(1)$ access.

Suggested change
const indexToPosition = new Map<number, number>();
for (let pos = 0; pos < chain.length; pos++) {
for (const nodeIdx of ctx.getNodeIndices(chain[pos])) {
indexToPosition.set(nodeIdx, pos);
}
}
const indexToPosition = new Int32Array(protoNodes.length).fill(-1);
for (let pos = 0; pos < chain.length; pos++) {
for (const nodeIdx of ctx.getNodeIndices(chain[pos])) {
indexToPosition[nodeIdx] = pos;
}
}

// Case 1 — landed. `cur` is a variant index of some `chain[pos]`.
// The suffix sum at the end of this function propagates this vote's
// contribution to every position closer to terminal.
const hit = indexToPosition.get(cur);
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.

high

If indexToPosition is refactored to use an Int32Array, the lookup should be updated to check against the sentinel value.

Suggested change
const hit = indexToPosition.get(cur);
const hit = indexToPosition[cur];
if (hit !== -1) {
landedPos = hit;


cache.voteWeightBySource.set(sourceKey, voteMap);
}
const scoreAtPosition = new Array<number>(chain.length).fill(0);
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.

medium

For performance-critical numeric arrays, using a TypedArray like Float64Array is generally preferred over a standard Array to ensure better memory layout and consistent performance in V8.

Suggested change
const scoreAtPosition = new Array<number>(chain.length).fill(0);
const scoreAtPosition = new Float64Array(chain.length);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant