From 843d2208ac3362a54da374fda6e343738173cc9d Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 1 May 2026 14:23:40 +0800 Subject: [PATCH 1/6] feat(fork-choice): expose getViableHeads() for compliance suite The fork-choice compliance suite (consensus-specs#3831) introduces a `viable_for_head_roots_and_weights` check that asserts the leaves and weights of the internal `filter_block_tree(store, justified.root)`. Head equivalence alone is too weak to catch filtered-tree regressions. Add `IForkChoice.getViableHeads()` that returns the viable-for-head leaves of the filtered tree paired with their weights: - A node is a filter_block_tree leaf iff it is viable AND has no viable descendants. `maybeUpdateBestChildAndDescendant` only sets `bestChild` to a child that `nodeLeadsToViableHead`, so `bestChild === undefined` iff no viable descendant. Combined with `nodeIsViableForHead`, this matches the spec exactly. Same approach as Teku's `ForkChoiceStrategy.getChainHeads`. - Weight is converted from internal increment units to Gwei on read (compliance fixtures expect Gwei). Note the proposer-boost score contribution is also stored in increments and may round down on minimal preset (102 vs 102.4 ETH); attestation weight is exact, the rounding only surfaces while boost is active. Mainnet boost values are integer ETH so this is a non-issue there. --- packages/fork-choice/src/forkChoice/forkChoice.ts | 4 ++++ packages/fork-choice/src/forkChoice/interface.ts | 1 + packages/fork-choice/src/protoArray/protoArray.ts | 13 ++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 9ea39c53ce29..c573b414eabb 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -594,6 +594,10 @@ export class ForkChoice implements IForkChoice { return this.protoArray.nodes.filter((node) => node.bestChild === undefined); } + getViableHeads(): {root: RootHex; weight: number}[] { + return this.protoArray.getViableHeads(this.fcStore.currentSlot); + } + /** This is for the debug API only */ getAllNodes(): ProtoNode[] { return this.protoArray.nodes; diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index 63b5cd983291..0a8767e39c30 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -128,6 +128,7 @@ export interface IForkChoice { * Retrieves all possible chain heads (leaves of fork choice tree). */ getHeads(): ProtoBlock[]; + getViableHeads(): {root: RootHex; weight: number}[]; /** * Retrieve all nodes for the debug API. */ diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index cdba276e74ca..e26434e8c8d6 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1,5 +1,5 @@ import {BitArray} from "@chainsafe/ssz"; -import {GENESIS_EPOCH, PTC_SIZE} from "@lodestar/params"; +import {EFFECTIVE_BALANCE_INCREMENT, GENESIS_EPOCH, PTC_SIZE} from "@lodestar/params"; import {DataAvailabilityStatus, computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {Epoch, RootHex, Slot} from "@lodestar/types"; import {bitCount, toRootHex} from "@lodestar/utils"; @@ -1541,6 +1541,17 @@ export class ProtoArray { return correctJustified && correctFinalized; } + /** Weight is converted from increment units to Gwei. */ + getViableHeads(currentSlot: Slot): {root: RootHex; weight: number}[] { + const result: {root: RootHex; weight: number}[] = []; + for (const node of this.nodes) { + if (node.bestChild === undefined && this.nodeIsViableForHead(node, currentSlot)) { + result.push({root: node.blockRoot, weight: node.weight * EFFECTIVE_BALANCE_INCREMENT}); + } + } + return result; + } + /** * Return `true` if `node` is equal to or a descendant of the finalized node. * This function helps improve performance of nodeIsViableForHead a lot by avoiding From 59c571562aab5e37f5302e18e9a7404f635e2ec0 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 1 May 2026 14:24:06 +0800 Subject: [PATCH 2/6] feat(beacon-node): integrate fork_choice_compliance spec runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the consensus-specs Fork Choice Compliance suite (#3831) into the existing `forkChoiceTest` runner. The on-disk layout matches the standard spec-test layout (`tests///fork_choice_compliance////`), so it slots in alongside `fork_choice` and `sync` runners. Three test-only accommodations the compliance fixtures require: 1. `bls_setting: 2` — every compliance fixture uses placeholder signatures. Pass `validSignatures: testcase.meta?.bls_setting !== BigInt(1)` to `chain.processBlock` so verification short-circuits. Standard `fork_choice` fixtures use `bls_setting: 1` so behavior there is unchanged. 2. `BLOCK_ERROR_ALREADY_KNOWN` — compliance fixtures intentionally re-import the same block (`dup_shift` mutations in their `meta.yaml`). Spec semantics for `on_block(store, known_block)` is a no-op success. Production block import correctly rejects with ALREADY_KNOWN; this runner treats that case as success only when the step is `valid: true`. 3. Cross-epoch attestation shuffling — `on_attestation` decodes aggregation_bits using the state at the attestation's target checkpoint, not the head state. The runner now resolves the right shuffling via ShufflingCache + regen (mirroring the production validation path) instead of `headState.epochCtx.getIndexedAttestation`, which only worked when the attestation's epoch happened to be in the head's epoch cache (±1 epoch) and broke on cross-epoch fork attestations surfaced by the compliance suite. Adds support for two compliance-only check fields: - `viable_for_head_roots_and_weights` (consensus-specs#3831): compared via `getViableHeads()`. Both sides are sorted by root before comparison since the spec doesn't fix order. - `head_payload_status` (gloas): mapped between our internal enum ordering (PENDING=0, EMPTY=1, FULL=2) and spec ordering (EMPTY=0, FULL=1, PENDING=2). Pass rate against the latest comptests workflow `small.tar.gz` artifact: fulu/fork_choice_compliance: 253/1472 cases pass (17.2%) Top remaining failures: - ~80% `Invalid proposer boost root` — consensus-specs#4807 introduced a `block.proposer_index == get_beacon_proposer_index(head_state)` guard in `update_proposer_boost_root` that we do not yet implement; affects all forks (not just gloas equivocation handling). Tracked for follow-up alongside #9233. - ~1% `Invalid viable heads` — proposer-boost rounding on minimal preset (see `getViableHeads()` weight note). --- .../test/spec/presets/fork_choice.test.ts | 53 ++++++++++++++++--- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/fork_choice.test.ts b/packages/beacon-node/test/spec/presets/fork_choice.test.ts index bdc72a6b4e61..29326a81d0e4 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.test.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.test.ts @@ -15,6 +15,7 @@ import { ForkPreFulu, ForkPreGloas, ForkSeq, + SLOTS_PER_EPOCH, } from "@lodestar/params"; import {InputType} from "@lodestar/spec-test-util"; import { @@ -25,6 +26,7 @@ import { createCachedBeaconState, createPubkeyCache, createSingleSignatureSetFromComponents, + getIndexedAttestation, getPayloadAttestationDataSigningRoot, isExecutionStateType, isGloasStateType, @@ -57,9 +59,11 @@ import { verifyExecutionPayloadEnvelope, verifyExecutionPayloadEnvelopeSignature, } from "../../../src/chain/blocks/verifyExecutionPayloadEnvelope.js"; +import {BlockError, BlockErrorCode} from "../../../src/chain/errors/blockError.js"; import {BeaconChain, ChainEvent} from "../../../src/chain/index.js"; import {defaultChainOptions} from "../../../src/chain/options.js"; import {RegenCaller} from "../../../src/chain/regen/index.js"; +import {getShufflingForAttestationVerification} from "../../../src/chain/validation/attestation.js"; import {validateFuluBlockDataColumnSidecars} from "../../../src/chain/validation/dataColumnSidecar.js"; import {ZERO_HASH_HEX} from "../../../src/constants/constants.js"; import {ExecutionPayloadStatus} from "../../../src/execution/engine/interface.js"; @@ -187,12 +191,26 @@ const forkChoiceTest = logger.debug(`Step ${i}/${stepsLen} attestation`, {root: step.attestation, valid: isValid}); const attestation = testcase.attestations.get(step.attestation); if (!attestation) throw Error(`No attestation ${step.attestation}`); - const headState = chain.getHeadState() as BeaconStateView; const attDataRootHex = toHexString(sszTypesFor(fork).AttestationData.hashTreeRoot(attestation.data)); - const indexedAttestation = headState.cachedState.epochCtx.getIndexedAttestation( - ForkSeq[fork], - attestation + + // `on_attestation` decodes aggregation_bits with the shuffling at the attestation's + // target checkpoint, not the head state — resolve it via ShufflingCache + regen so + // cross-epoch fork attestations (surfaced by the compliance suite) decode correctly. + const attHeadBlock = chain.forkChoice.getBlockHexDefaultStatus(toHex(attestation.data.beaconBlockRoot)); + if (attHeadBlock === null) { + logger.debug(`Step ${i}/${stepsLen} skip attestation: head block unknown`, { + blockRoot: toHex(attestation.data.beaconBlockRoot), + }); + continue; + } + const attEpoch = Math.floor(attestation.data.slot / SLOTS_PER_EPOCH); + const shuffling = await getShufflingForAttestationVerification( + chain, + attEpoch, + attHeadBlock, + RegenCaller.validateGossipAttestation ); + const indexedAttestation = getIndexedAttestation(shuffling, ForkSeq[fork], attestation); try { chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); if (!isValid) throw Error("Expect error since this is a negative test"); @@ -463,10 +481,19 @@ const forkChoiceTest = seenTimestampSec: tickTime, validBlobSidecars: BlobSidecarValidation.Full, importAttestations: AttestationImportOpt.Force, + validSignatures: testcase.meta?.bls_setting !== BigInt(1), }); if (!isValid) throw Error("Expect error since this is a negative test"); } catch (e) { - if (isValid || (e as Error).message === "Expect error since this is a negative test") throw e; + // Spec `on_block` is a no-op success on a known block; lodestar's production + // import path rejects with ALREADY_KNOWN. Treat as success in the spec runner. + if (isValid && e instanceof BlockError && e.type.code === BlockErrorCode.ALREADY_KNOWN) { + logger.debug(`Step ${i}/${stepsLen} block already known — treating as no-op success`, { + id: step.block, + }); + } else if (isValid || (e as Error).message === "Expect error since this is a negative test") { + throw e; + } } } @@ -597,15 +624,23 @@ const forkChoiceTest = ); } if (step.checks.head_payload_status !== undefined) { - // Map our PayloadStatus enum to spec's numbering: - // Spec: EMPTY=0, FULL=1, PENDING=2 - // Ours: PENDING=0, EMPTY=1, FULL=2 + // Spec: EMPTY=0, FULL=1, PENDING=2; Ours: PENDING=0, EMPTY=1, FULL=2 const payloadStatusToSpec: Record = {0: 2, 1: 0, 2: 1}; expect(payloadStatusToSpec[head.payloadStatus]).toEqualWithMessage( bnToNum(step.checks.head_payload_status), `Invalid head payload status at step ${i}` ); } + if (step.checks.viable_for_head_roots_and_weights !== undefined) { + const expected = step.checks.viable_for_head_roots_and_weights + .map((entry) => ({root: entry.root, weight: bnToNum(entry.weight)})) + .sort((a, b) => a.root.localeCompare(b.root)); + const actual = (chain.forkChoice as ForkChoice) + .getViableHeads() + .map(({root, weight}) => ({root, weight})) + .sort((a, b) => a.root.localeCompare(b.root)); + expect(actual).toEqualWithMessage(expected, `Invalid viable heads at step ${i}`); + } if (step.checks.should_override_forkchoice_update) { const currentSlot = Math.floor(tickTime / (config.SLOT_DURATION_MS / 1000)); const result = chain.forkChoice.shouldOverrideForkChoiceUpdate( @@ -856,6 +891,7 @@ type Checks = { block_root: RootHex; votes: (boolean | null)[]; }; + viable_for_head_roots_and_weights?: {root: RootHex; weight: bigint}[]; }; }; @@ -911,4 +947,5 @@ function isCheck(step: Step): step is Checks { specTestIterator(path.join(ethereumConsensusSpecsTests.outputDir, "tests", ACTIVE_PRESET), { fork_choice: {type: RunnerType.default, fn: forkChoiceTest({onlyPredefinedResponses: false})}, sync: {type: RunnerType.default, fn: forkChoiceTest({onlyPredefinedResponses: true})}, + fork_choice_compliance: {type: RunnerType.default, fn: forkChoiceTest({onlyPredefinedResponses: false})}, }); From 35cca8cc11d930c8668f32f05f91312a11cb110c Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 1 May 2026 14:24:28 +0800 Subject: [PATCH 3/6] chore(scripts): download fork-choice compliance test artifacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `scripts/download-compliance-fc-tests.sh` plus an npm script wrapper (`pnpm download-compliance-fc-tests`) for fetching consensus-specs Fork Choice Compliance test fixtures. These fixtures are NOT in the standard `download-spec-tests` bundle — the consensus-specs `release.yml` workflow does not invoke `make comptests`. Instead they are produced by the scheduled "Compliance Tests" workflow (`comptests.yml`) and published only as a transient GitHub Actions artifact (`.tar.gz`). The script mirrors Prysm's `hack/compliance-fc-report.sh` resolution order and handles three modes: --tarball Use a local .tar.gz file --run-id Download artifact from a specific GH Actions run (default) Auto-fetch the latest successful run via `gh` The artifact is extracted into `packages/beacon-node/spec-tests/` alongside the release-tarball output. Top-level paths in the artifact are `tests///fork_choice_compliance/...`, which do not collide with anything emitted by the standard release tarball, so extraction is purely additive — no path remapping needed in the runner. The GH artifact API may return either raw `.tar.gz` (when `upload-artifact` used `archive: false`, current workflow behavior) or a zip-wrapped one. The script sniffs the first two magic bytes and unwraps the zip case if needed. Default config is `small` (~89 MB compressed, ~950 MB extracted, ~2900 cases). Cache lives at `$LODESTAR_COMPLIANCE_FC_CACHE` (default `/var/tmp/lodestar_compliance_fc_cache`). CI is intentionally not wired in this PR — the runner cleanly skips the compliance suite when the fixtures are absent, so existing `test:spec:*` jobs are unaffected. Devs and reviewers run on demand: pnpm download-compliance-fc-tests pnpm test:spec:minimal -t fork_choice_compliance --- packages/beacon-node/package.json | 1 + scripts/download-compliance-fc-tests.sh | 179 ++++++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100755 scripts/download-compliance-fc-tests.sh diff --git a/packages/beacon-node/package.json b/packages/beacon-node/package.json index ff5cf52f6ebb..785902e998b5 100644 --- a/packages/beacon-node/package.json +++ b/packages/beacon-node/package.json @@ -100,6 +100,7 @@ "test:sim": "vitest run test/sim/**/*.test.ts", "test:sim:blobs": "vitest run test/sim/4844-interop.test.ts", "download-spec-tests": "node --loader=ts-node/esm test/spec/downloadTests.ts", + "download-compliance-fc-tests": "../../scripts/download-compliance-fc-tests.sh", "test:spec:bls": "vitest run --project spec-minimal test/spec/bls/", "test:spec:general": "vitest run --project spec-minimal test/spec/general/", "test:spec:minimal": "vitest run --project spec-minimal test/spec/presets/", diff --git a/scripts/download-compliance-fc-tests.sh b/scripts/download-compliance-fc-tests.sh new file mode 100755 index 000000000000..432d06afd085 --- /dev/null +++ b/scripts/download-compliance-fc-tests.sh @@ -0,0 +1,179 @@ +#!/usr/bin/env bash +# Download consensus-specs fork-choice compliance test fixtures. +# See `--help` for usage. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +OUTPUT_DIR="$REPO_ROOT/packages/beacon-node/spec-tests" +CACHE_DIR="${LODESTAR_COMPLIANCE_FC_CACHE:-/var/tmp/lodestar_compliance_fc_cache}" +GH_REPO="ethereum/consensus-specs" +GH_WORKFLOW="Compliance Tests" + +CONFIG="small" +TARBALL="" +RUN_ID="" + +usage() { + cat <<'EOF' +Usage: download-compliance-fc-tests.sh [options] + +Options: + --config Generator config to fetch (default: small) + --tarball Use a local .tar.gz file + --run-id Download artifact from a specific GH run id + --repo Override consensus-specs repo + --workflow Override workflow name + -h, --help Show this help + +Output: + packages/beacon-node/spec-tests/tests///fork_choice_compliance/ + +Compliance fixtures live alongside the standard spec-test tarball output; +extraction does not overwrite release-tarball files because the compliance +generator only emits `fork_choice_compliance/` subtrees. + +Environment: + LODESTAR_COMPLIANCE_FC_CACHE Tarball cache dir + (default: /var/tmp/lodestar_compliance_fc_cache) + GITHUB_TOKEN Picked up by `gh` for artifact downloads +EOF +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --config) CONFIG="$2"; shift 2 ;; + --tarball) TARBALL="$2"; shift 2 ;; + --run-id) RUN_ID="$2"; shift 2 ;; + --repo) GH_REPO="$2"; shift 2 ;; + --workflow) GH_WORKFLOW="$2"; shift 2 ;; + -h|--help) usage; exit 0 ;; + *) echo "Unknown argument: $1" >&2; usage; exit 1 ;; + esac +done + +case "$CONFIG" in + tiny|small|standard) ;; + *) echo "Invalid --config: $CONFIG (must be tiny, small, or standard)" >&2; exit 1 ;; +esac + +ARTIFACT_NAME="${CONFIG}.tar.gz" + +require_gh() { + command -v gh >/dev/null 2>&1 || { + echo "error: 'gh' CLI required for this resolution mode" >&2 + echo " install from https://cli.github.com or pass --tarball" >&2 + exit 1 + } +} + +extract_tarball() { + local tarball="$1" + if [[ ! -f "$tarball" ]]; then + echo "error: tarball not found at $tarball" >&2 + exit 1 + fi + echo "Extracting $tarball -> $OUTPUT_DIR" + mkdir -p "$OUTPUT_DIR" + tar -xzf "$tarball" -C "$OUTPUT_DIR" +} + +resolve_latest_run() { + require_gh + echo "Resolving latest successful '$GH_WORKFLOW' run on $GH_REPO..." >&2 + gh -R "$GH_REPO" run list \ + --workflow "$GH_WORKFLOW" \ + --status success \ + --limit 1 \ + --json databaseId \ + --jq '.[0].databaseId' +} + +download_run_artifact() { + local run_id="$1" + require_gh + local cache_path="$CACHE_DIR/run-$run_id" + mkdir -p "$cache_path" + local target="$cache_path/$ARTIFACT_NAME" + + if [[ -f "$target" ]]; then + echo "Using cached $target" >&2 + printf '%s' "$target" + return + fi + + echo "Looking up artifact $ARTIFACT_NAME in run $run_id ($GH_REPO)..." >&2 + # Capture the full listing before selecting the first id: `gh --paginate | head -n1` would + # kill gh with SIGPIPE once head exits, which pipefail turns into a silent script abort. + local artifact_ids artifact_id + artifact_ids=$(gh api "repos/$GH_REPO/actions/runs/$run_id/artifacts" --paginate \ + --jq ".artifacts[] | select(.name == \"$ARTIFACT_NAME\") | .id") + artifact_id=$(printf '%s\n' "$artifact_ids" | head -n 1) + + if [[ -z "$artifact_id" ]]; then + echo "error: artifact named '$ARTIFACT_NAME' not found in run $run_id" >&2 + exit 1 + fi + + local raw="$cache_path/raw.bin" + echo "Downloading artifact id=$artifact_id" >&2 + gh api "repos/$GH_REPO/actions/artifacts/$artifact_id/zip" > "$raw" + + # The artifact may be raw .tar.gz (1f 8b) or zip-wrapped (50 4b "PK"). + local magic + magic=$(head -c 2 "$raw" | od -An -tx1 | tr -d ' \n') + case "$magic" in + 504b) + command -v unzip >/dev/null 2>&1 || { + echo "error: 'unzip' required to unwrap zip-wrapped artifact" >&2 + exit 1 + } + echo "Artifact is zip-wrapped, unwrapping..." >&2 + # Unzip into a scratch dir so the tarball search cannot pick up a previously cached + # tarball of a different config living in the same per-run cache directory. + local unzip_dir="$cache_path/unzip.$$" + mkdir -p "$unzip_dir" + (cd "$unzip_dir" && unzip -o -q "$raw") + rm -f "$raw" + local found + found=$(find "$unzip_dir" -name "$ARTIFACT_NAME" -type f -print -quit || true) + if [[ -z "$found" ]]; then + found=$(find "$unzip_dir" -name '*.tar.gz' -type f -print -quit || true) + fi + if [[ -z "$found" ]]; then + echo "error: no .tar.gz found inside zip-wrapped artifact" >&2 + exit 1 + fi + mv "$found" "$target" + rm -rf "$unzip_dir" + ;; + 1f8b) + echo "Artifact is a raw gzip, using directly." >&2 + mv "$raw" "$target" + ;; + *) + echo "error: unknown artifact format (magic=$magic)" >&2 + exit 1 + ;; + esac + + printf '%s' "$target" +} + +if [[ -n "$TARBALL" ]]; then + extract_tarball "$TARBALL" +elif [[ -n "$RUN_ID" ]]; then + TAR_PATH=$(download_run_artifact "$RUN_ID") + extract_tarball "$TAR_PATH" +else + RUN_ID=$(resolve_latest_run) + if [[ -z "$RUN_ID" ]]; then + echo "error: no successful '$GH_WORKFLOW' runs found on $GH_REPO" >&2 + exit 1 + fi + echo "Latest successful run: $RUN_ID" + TAR_PATH=$(download_run_artifact "$RUN_ID") + extract_tarball "$TAR_PATH" +fi + +echo "Done." From 6649551420ea14e50390b06e8fe9365b2d6f5133 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 1 Jul 2026 03:44:47 -0400 Subject: [PATCH 4/6] fix(fork-choice): restrict getViableHeads to justified subtree, dedupe gloas variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corrections to the compliance-only getViableHeads getter, plus an API-surface trim: - Restrict leaves to descendants of the justified checkpoint block: the spec's get_filtered_block_tree is rooted at store.justified_checkpoint, so iterating all proto-array nodes over-included FFG-viable leaves hanging off the finalized checkpoint on branches not under the current justified checkpoint (~87 of 100 'Invalid viable heads' compliance failures were this, not proposer-boost rounding as previously assumed). The descendant walk follows parent links to the justified root — a slot-based getAncestor lookup returns the node itself for leaves whose own slot equals the justified epoch start slot — and stops early once it passes the justified block's slot (parent slots are non-increasing). - Emit each block root at most once: gloas payload-status variants share a blockRoot and more than one can be a leaf (EMPTY and FULL are both parented under PENDING), which produced duplicate roots that broke the index-paired comparison in the compliance check. Which variant's weight the spec expects is part of the open gloas vote-attribution divergence; keep the heaviest qualifying variant deterministically. - Drop getViableHeads from IForkChoice: it is compliance-test-only and its single caller accesses the ForkChoice class directly. Verified against consensus-specs compliance vectors (run 28413328593, minimal fulu): viable-head root-set failures 100 -> 0; standard minimal fork_choice+sync spectests unaffected (397 passed). --- .../fork-choice/src/forkChoice/interface.ts | 1 - .../fork-choice/src/protoArray/protoArray.ts | 45 +++++++++++++++++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index 0a8767e39c30..63b5cd983291 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -128,7 +128,6 @@ export interface IForkChoice { * Retrieves all possible chain heads (leaves of fork choice tree). */ getHeads(): ProtoBlock[]; - getViableHeads(): {root: RootHex; weight: number}[]; /** * Retrieve all nodes for the debug API. */ diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index e26434e8c8d6..65b11ccf77ab 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1543,13 +1543,50 @@ export class ProtoArray { /** Weight is converted from increment units to Gwei. */ getViableHeads(currentSlot: Slot): {root: RootHex; weight: number}[] { - const result: {root: RootHex; weight: number}[] = []; + // Mirror the spec's `get_filtered_block_tree`, which is rooted at the store's justified + // checkpoint: a viable head is a leaf (no viable descendant, i.e. `bestChild === undefined`) + // that descends from the justified checkpoint block AND is itself viable for head. Iterating + // all nodes without the justified-descendant filter would wrongly include FFG-viable leaves + // that hang off the finalized checkpoint on a branch not under the current justified checkpoint. + const justifiedStatus = this.getDefaultVariant(this.justifiedRoot); + const justifiedSlot = + justifiedStatus !== undefined ? this.getNode(this.justifiedRoot, justifiedStatus)?.slot : undefined; + // Gloas payload-status variants share a blockRoot and more than one can be a leaf (EMPTY and + // FULL are both parented under PENDING); the spec's filtered tree is keyed by block root, so + // emit each root at most once. Which variant's weight the spec expects is an open gloas + // divergence — deterministically keep the heaviest qualifying variant. + const weightByRoot = new Map(); for (const node of this.nodes) { - if (node.bestChild === undefined && this.nodeIsViableForHead(node, currentSlot)) { - result.push({root: node.blockRoot, weight: node.weight * EFFECTIVE_BALANCE_INCREMENT}); + if (node.bestChild !== undefined || !this.nodeIsViableForHead(node, currentSlot)) { + continue; + } + if (this.justifiedEpoch !== GENESIS_EPOCH && !this.descendsFromJustified(node, justifiedSlot)) { + continue; + } + const prev = weightByRoot.get(node.blockRoot); + if (prev === undefined || node.weight > prev) { + weightByRoot.set(node.blockRoot, node.weight); } } - return result; + return Array.from(weightByRoot.entries()).map(([root, weight]) => ({ + root, + weight: weight * EFFECTIVE_BALANCE_INCREMENT, + })); + } + + /** + * True if `node` is the justified checkpoint block or one of its descendants. + * Parent slots are non-increasing, so the walk stops once it passes the justified block's slot. + */ + private descendsFromJustified(node: ProtoNode, justifiedSlot: Slot | undefined): boolean { + let current: ProtoNode | undefined = node; + while (current !== undefined && (justifiedSlot === undefined || current.slot >= justifiedSlot)) { + if (current.blockRoot === this.justifiedRoot) { + return true; + } + current = current.parent !== undefined ? this.getNodeByIndex(current.parent) : undefined; + } + return false; } /** From 01d5438bd4cb0cba29a2135228da005df2ee2ddb Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 1 Jul 2026 03:45:22 -0400 Subject: [PATCH 5/6] test(spec): align fork-choice runner with spec test-format semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes to the shared fork-choice runner, each anchored to a test-format or spec contract (not compliance-suite identity), so they apply uniformly to fork_choice/sync/fork_choice_compliance: - Register fork_choice_compliance in coveredTestRunners: without it every other spec suite iterating the same fixture tree (ssz_static, operations, sanity, ...) throws 'No test runner for fork_choice_compliance' once compliance fixtures are downloaded. - Attestation timing precondition: spec validate_on_attestation requires get_current_slot(store) >= data.slot + 1. Lodestar enforces this 1-slot delay in the gossip/attestation-pool layer, not forkChoice.onAttestation; the runner calls onAttestation directly, so replicate the precondition (Lighthouse's runner excuses the same case post-hoc: DidntFail tolerated when data.slot >= current_slot). Clears all 298 negative-attestation compliance failures. - Attestation step hardening: an unknown beacon block root on a valid:true step now fails loudly instead of being silently skipped, and the shuffling resolution runs inside the negative-test try/catch so that errors on valid:false steps (e.g. attesting to a future block) count as the expected rejection instead of crashing the case. - Skip gloas ePBS signature verification unless bls_setting == 1: the test format mandates skipping verification for placeholder signatures (bls_setting: 2), which the block-import path already honors via validSignatures. Extend the same handling to the execution_payload and payload_attestation_message steps (Teku disables BLS at spec level for bls_setting IGNORED; Lighthouse compiles compliance under fake_crypto). - Split viable_for_head_roots_and_weights into exact root-set equality plus a 1.2 ETH weight tolerance: the root set is epoch-determined and must match exactly (this strictness caught the getViableHeads justified-subtree bug; Teku's containsAll would not), while weights are stored in EFFECTIVE_BALANCE_INCREMENT units so the proposer-boost score quantizes to whole ETH; the divergence is bounded by (100 - gcd(P,100) + P)/100 = 1.2 ETH independent of preset. Compliance (run 28413328593, minimal): fulu 1459/1472 (99.1%), remaining 11 proposer-boost-root divergences + 1-2 timeouts; gloas 200/1472, blocked by processPayloadAttestation verifying signatures unconditionally (ignores verifySignatures; not batched in getBlockSignatureSets) and a payload-status vote-attribution divergence (weight diffs are exact multiples of 32 ETH between EMPTY/FULL variants of the same root) — both documented follow-ups, out of scope here. Known accommodation limit: ALREADY_KNOWN treated as no-op success cannot re-apply proposer boost for a timely duplicate the way spec on_block re-processing would. Standard minimal fork_choice+sync: 397 passed, unchanged. --- .../test/spec/presets/fork_choice.test.ts | 138 ++++++++++++------ .../test/spec/utils/specTestIterator.ts | 1 + 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/fork_choice.test.ts b/packages/beacon-node/test/spec/presets/fork_choice.test.ts index 29326a81d0e4..62b81cd3c869 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.test.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.test.ts @@ -8,6 +8,7 @@ import {CheckpointWithHex, ExecutionStatus, ForkChoice} from "@lodestar/fork-cho import {testLogger} from "@lodestar/logger/test-utils"; import { ACTIVE_PRESET, + EFFECTIVE_BALANCE_INCREMENT, ForkPostDeneb, ForkPostFulu, ForkPostGloas, @@ -15,7 +16,6 @@ import { ForkPreFulu, ForkPreGloas, ForkSeq, - SLOTS_PER_EPOCH, } from "@lodestar/params"; import {InputType} from "@lodestar/spec-test-util"; import { @@ -23,6 +23,7 @@ import { BeaconStateView, DataAvailabilityStatus, IBeaconStateViewGloas, + computeEpochAtSlot, createCachedBeaconState, createPubkeyCache, createSingleSignatureSetFromComponents, @@ -193,25 +194,43 @@ const forkChoiceTest = if (!attestation) throw Error(`No attestation ${step.attestation}`); const attDataRootHex = toHexString(sszTypesFor(fork).AttestationData.hashTreeRoot(attestation.data)); + // Spec `validate_on_attestation` requires `get_current_slot(store) >= attestation.data.slot + 1` + // (an attestation may only influence fork choice from the slot AFTER it was created). Lodestar + // enforces this 1-slot delay in the gossip/attestation-pool layer, not in `forkChoice.onAttestation`, + // so replicate the precondition here since the runner calls `onAttestation` directly. + const currentSlot = Math.floor(tickTime / (config.SLOT_DURATION_MS / 1000)); + if (currentSlot < attestation.data.slot + 1) { + if (isValid) { + throw Error(`Attestation not yet 1 slot old but marked valid at step ${i}`); + } + logger.debug( + `Step ${i}/${stepsLen} skip attestation: not yet 1 slot old (spec on_attestation rejects)`, + { + attSlot: attestation.data.slot, + currentSlot, + } + ); + continue; + } + // `on_attestation` decodes aggregation_bits with the shuffling at the attestation's // target checkpoint, not the head state — resolve it via ShufflingCache + regen so // cross-epoch fork attestations (surfaced by the compliance suite) decode correctly. + // The resolution runs inside the try so that errors on `valid: false` steps (e.g. + // attesting to a future block) count as the expected rejection. const attHeadBlock = chain.forkChoice.getBlockHexDefaultStatus(toHex(attestation.data.beaconBlockRoot)); - if (attHeadBlock === null) { - logger.debug(`Step ${i}/${stepsLen} skip attestation: head block unknown`, { - blockRoot: toHex(attestation.data.beaconBlockRoot), - }); - continue; + if (attHeadBlock === null && isValid) { + throw Error(`Attestation beacon block root unknown to fork choice at step ${i}`); } - const attEpoch = Math.floor(attestation.data.slot / SLOTS_PER_EPOCH); - const shuffling = await getShufflingForAttestationVerification( - chain, - attEpoch, - attHeadBlock, - RegenCaller.validateGossipAttestation - ); - const indexedAttestation = getIndexedAttestation(shuffling, ForkSeq[fork], attestation); try { + if (attHeadBlock === null) throw Error("Unknown attestation head block (expected rejection)"); + const shuffling = await getShufflingForAttestationVerification( + chain, + computeEpochAtSlot(attestation.data.slot), + attHeadBlock, + RegenCaller.validateGossipAttestation + ); + const indexedAttestation = getIndexedAttestation(shuffling, ForkSeq[fork], attestation); chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); if (!isValid) throw Error("Expect error since this is a negative test"); } catch (e) { @@ -269,27 +288,31 @@ const forkChoiceTest = ); } - // Signature verification, matching the `validateGossipPayloadAttestationMessage` flow - const validatorPubkey = pubkeyCache.get(payloadAttestationMessage.validatorIndex); - if (!validatorPubkey) { - throw Error(`Unknown validator index ${payloadAttestationMessage.validatorIndex}`); - } - const signatureSet = createSingleSignatureSetFromComponents( - validatorPubkey, - getPayloadAttestationDataSigningRoot(beaconConfig, payloadAttestationMessage.data), - payloadAttestationMessage.signature - ); - let signatureValidity: boolean; - try { - signatureValidity = await chain.bls.verifySignatureSets([signatureSet], { - verifyOnMainThread: true, - batchable: true, - priority: true, - }); - } catch { - signatureValidity = false; + // Signature verification (matching `validateGossipPayloadAttestationMessage`) — skipped for + // bls_setting !== 1: compliance fixtures use placeholder signatures (bls_setting: 2), so the + // spec reference runner does not verify. Mirror the block-import accommodation (`validSignatures`). + if (testcase.meta?.bls_setting === BigInt(1)) { + const validatorPubkey = pubkeyCache.get(payloadAttestationMessage.validatorIndex); + if (!validatorPubkey) { + throw Error(`Unknown validator index ${payloadAttestationMessage.validatorIndex}`); + } + const signatureSet = createSingleSignatureSetFromComponents( + validatorPubkey, + getPayloadAttestationDataSigningRoot(beaconConfig, payloadAttestationMessage.data), + payloadAttestationMessage.signature + ); + let signatureValidity: boolean; + try { + signatureValidity = await chain.bls.verifySignatureSets([signatureSet], { + verifyOnMainThread: true, + batchable: true, + priority: true, + }); + } catch { + signatureValidity = false; + } + if (!signatureValidity) throw Error("Invalid payload attestation signature"); } - if (!signatureValidity) throw Error("Invalid payload attestation signature"); chain.forkChoice.notifyPtcMessages( blockRoot, @@ -524,16 +547,20 @@ const forkChoiceTest = ); verifyExecutionPayloadEnvelope(beaconConfig, blockState as IBeaconStateViewGloas, envelope.message); - // Verify signature - const sigValid = await verifyExecutionPayloadEnvelopeSignature( - beaconConfig, - blockState as IBeaconStateViewGloas, - pubkeyCache, - envelope, - blockState.latestBlockHeader.proposerIndex, - chain.bls - ); - if (!sigValid) throw Error("Invalid execution payload envelope signature"); + // Verify signature — skipped for bls_setting !== 1: compliance fixtures use placeholder + // signatures (bls_setting: 2), so the spec reference runner does not verify. Mirror the + // block-import accommodation (`validSignatures` above). + if (testcase.meta?.bls_setting === BigInt(1)) { + const sigValid = await verifyExecutionPayloadEnvelopeSignature( + beaconConfig, + blockState as IBeaconStateViewGloas, + pubkeyCache, + envelope, + blockState.latestBlockHeader.proposerIndex, + chain.bls + ); + if (!sigValid) throw Error("Invalid execution payload envelope signature"); + } // Add predefined VALID status for the payload's block hash so the EL mock accepts it executionEngineBackend.addPredefinedPayloadStatus(blockHash, { @@ -639,7 +666,28 @@ const forkChoiceTest = .getViableHeads() .map(({root, weight}) => ({root, weight})) .sort((a, b) => a.root.localeCompare(b.root)); - expect(actual).toEqualWithMessage(expected, `Invalid viable heads at step ${i}`); + + // The set of viable heads is determined by justified/finalized epochs, not weight, + // so it must match exactly. Comparing the root sets (not a subset) also rejects a + // degenerate empty result. + expect(actual.map((h) => h.root)).toEqualWithMessage( + expected.map((h) => h.root), + `Invalid viable head roots at step ${i}` + ); + + // Weights are stored in EFFECTIVE_BALANCE_INCREMENT units, so the proposer-boost + // score is floored to whole ETH while the spec keeps Gwei precision. Attestation + // weight is exact, so the per-head divergence is only the boost quantization, bounded + // by (100 - gcd(PROPOSER_SCORE_BOOST, 100) + PROPOSER_SCORE_BOOST) / 100 = 1.2 ETH, + // independent of SLOTS_PER_EPOCH and total balance. Compare within that bound. + const tolerance = 1.2 * EFFECTIVE_BALANCE_INCREMENT; + for (const [k, {root, weight}] of actual.entries()) { + const diff = Math.abs(weight - expected[k].weight); + expect( + diff, + `Viable head ${root} weight off by ${diff} Gwei (> 1.2 ETH) at step ${i}` + ).toBeLessThanOrEqual(tolerance); + } } if (step.checks.should_override_forkchoice_update) { const currentSlot = Math.floor(tickTime / (config.SLOT_DURATION_MS / 1000)); diff --git a/packages/beacon-node/test/spec/utils/specTestIterator.ts b/packages/beacon-node/test/spec/utils/specTestIterator.ts index cfe2689ee437..76e18f5ab0aa 100644 --- a/packages/beacon-node/test/spec/utils/specTestIterator.ts +++ b/packages/beacon-node/test/spec/utils/specTestIterator.ts @@ -34,6 +34,7 @@ const coveredTestRunners = [ "finality", "fork", "fork_choice", + "fork_choice_compliance", "sync", "fork", "genesis", From b1bf28a092a1d773bf662fdcc71965e5526e3f8a Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Mon, 6 Jul 2026 08:35:25 -0400 Subject: [PATCH 6/6] refactor(fork-choice): collapse justified-slot lookup in getViableHeads Use the single getDefaultNodeIndex + getNodeByIndex idiom already used elsewhere in protoArray instead of the two-hop getDefaultVariant + getNode lookup. Semantically identical for all variant cases (pre-gloas FULL, gloas PENDING, absent root); drops one redundant indices map lookup and removes an unreachable throw path. --- packages/fork-choice/src/protoArray/protoArray.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 65b11ccf77ab..26e72f8e048e 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1548,9 +1548,8 @@ export class ProtoArray { // that descends from the justified checkpoint block AND is itself viable for head. Iterating // all nodes without the justified-descendant filter would wrongly include FFG-viable leaves // that hang off the finalized checkpoint on a branch not under the current justified checkpoint. - const justifiedStatus = this.getDefaultVariant(this.justifiedRoot); - const justifiedSlot = - justifiedStatus !== undefined ? this.getNode(this.justifiedRoot, justifiedStatus)?.slot : undefined; + const justifiedIndex = this.getDefaultNodeIndex(this.justifiedRoot); + const justifiedSlot = justifiedIndex !== undefined ? this.getNodeByIndex(justifiedIndex)?.slot : undefined; // Gloas payload-status variants share a blockRoot and more than one can be a leaf (EMPTY and // FULL are both parented under PENDING); the spec's filtered tree is keyed by block root, so // emit each root at most once. Which variant's weight the spec expects is an open gloas