From aa6c1bb31a7828ab26bacb6aa009089c19e8a68d Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sat, 4 Apr 2026 13:27:45 +0100 Subject: [PATCH 1/2] refactor: reorder payload status constants --- .../opPools/aggregatedAttestationPool.test.ts | 4 +- .../test/utils/validationData/attestation.ts | 4 +- .../fork-choice/src/protoArray/interface.ts | 6 +-- .../fork-choice/src/protoArray/protoArray.ts | 45 +++++++++---------- .../test/unit/forkChoice/forkChoice.test.ts | 2 +- .../protoArray/executionStatusUpdates.test.ts | 2 +- .../test/unit/protoArray/gloas.test.ts | 2 +- 7 files changed, 32 insertions(+), 33 deletions(-) diff --git a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts index c785dd691a74..c34517d7589f 100644 --- a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -70,7 +70,7 @@ describe.skip(`getAttestationsForBlock vc=${vc}`, () => { dataAvailabilityStatus: DataAvailabilityStatus.PreData, parentBlockHash: null, - payloadStatus: 2, // PayloadStatus.FULL + payloadStatus: PayloadStatus.FULL, }, originalState.slot ); @@ -98,7 +98,7 @@ describe.skip(`getAttestationsForBlock vc=${vc}`, () => { dataAvailabilityStatus: DataAvailabilityStatus.PreData, parentBlockHash: null, - payloadStatus: 2, // PayloadStatus.FULL + payloadStatus: PayloadStatus.FULL, }, slot, null diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index 1894a9b89b39..f963360493ec 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -1,5 +1,5 @@ import {BitArray, toHexString} from "@chainsafe/ssz"; -import {ExecutionStatus, IForkChoice, ProtoBlock} from "@lodestar/fork-choice"; +import {ExecutionStatus, IForkChoice, PayloadStatus, ProtoBlock} from "@lodestar/fork-choice"; import {testLogger} from "@lodestar/logger/test-utils"; import {DOMAIN_BEACON_ATTESTER} from "@lodestar/params"; import { @@ -82,7 +82,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { dataAvailabilityStatus: DataAvailabilityStatus.PreData, parentBlockHash: null, - payloadStatus: 2, // PayloadStatus.FULL + payloadStatus: PayloadStatus.FULL, }; const shufflingCache = new ShufflingCache(null, null, {}, [ diff --git a/packages/fork-choice/src/protoArray/interface.ts b/packages/fork-choice/src/protoArray/interface.ts index cae46fdacd55..d2f952edcf2d 100644 --- a/packages/fork-choice/src/protoArray/interface.ts +++ b/packages/fork-choice/src/protoArray/interface.ts @@ -41,9 +41,9 @@ export enum ExecutionStatus { * Spec: gloas/fork-choice.md#constants */ export enum PayloadStatus { - PENDING = 0, - EMPTY = 1, - FULL = 2, + EMPTY = 0, + FULL = 1, + PENDING = 2, } /** diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index d32944a6f3a9..c5017587225e 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -30,11 +30,12 @@ const ZERO_HASH_HEX = toRootHex(Buffer.alloc(32, 0)); /** Pre-Gloas: single element, FULL index (for backward compatibility) */ type PreGloasVariantIndex = number; /** - * Post-Gloas: array length is 2 or 3 - * - Length 2: [PENDING_INDEX, EMPTY_INDEX] when payload hasn't arrived yet - * - Length 3: [PENDING_INDEX, EMPTY_INDEX, FULL_INDEX] when payload has arrived + * Post-Gloas: always length 3, indexed by PayloadStatus enum value + * - [EMPTY_INDEX, FULL_INDEX | undefined, PENDING_INDEX] + * - PayloadStatus.EMPTY=0, PayloadStatus.FULL=1, PayloadStatus.PENDING=2 + * - FULL starts as undefined until execution payload arrives */ -type GloasVariantIndices = [number, number] | [number, number, number]; +type GloasVariantIndices = [number, number | undefined, number]; type VariantIndices = PreGloasVariantIndex | GloasVariantIndices; export class ProtoArray { @@ -49,10 +50,10 @@ export class ProtoArray { /** * Maps block root to array of node indices for each payload status variant * - * Array structure: [PENDING, EMPTY, FULL] where indices correspond to PayloadStatus enum values - * - number[0] = PENDING variant index (PayloadStatus.PENDING = 0) - * - number[1] = EMPTY variant index (PayloadStatus.EMPTY = 1) - * - number[2] = FULL variant index (PayloadStatus.FULL = 2) + * Array structure: [EMPTY, FULL, PENDING] indexed by PayloadStatus enum values + * - number[0] = EMPTY variant index (PayloadStatus.EMPTY = 0) + * - number[1] = FULL variant index (PayloadStatus.FULL = 1), undefined until payload arrives + * - number[2] = PENDING variant index (PayloadStatus.PENDING = 2) * * Note: undefined array elements indicate that variant doesn't exist for this block */ @@ -143,7 +144,7 @@ export class ProtoArray { }); } - // Gloas: return the specified variant, or PENDING if not specified + // Gloas: return the specified variant return variantOrArr[payloadStatus]; } @@ -441,11 +442,11 @@ export class ProtoArray { // Check if parent exists by getting variants array const parentVariants = this.indices.get(block.parentRoot); if (parentVariants != null) { - const anyParentIndex = Array.isArray(parentVariants) ? parentVariants[0] : parentVariants; + const anyParentIndex = Array.isArray(parentVariants) ? parentVariants[PayloadStatus.PENDING] : parentVariants; const anyParentNode = this.nodes[anyParentIndex]; if (!isGloasBlock(anyParentNode)) { - // Fork transition: parent is pre-Gloas, so it only has FULL variant at variants[0] + // Fork transition: parent is pre-Gloas, so it only has FULL variant parentIndex = anyParentIndex; } else { // Both blocks are Gloas: determine which parent payload status to extend @@ -481,9 +482,9 @@ export class ProtoArray { const emptyIndex = this.nodes.length; this.nodes.push(emptyNode); - // Store both variants in the indices array - // [PENDING, EMPTY, undefined] - FULL will be added later if payload arrives - this.indices.set(block.blockRoot, [pendingIndex, emptyIndex]); + // Store variants indexed by PayloadStatus enum values + // [EMPTY=0, FULL=1(undefined), PENDING=2] - FULL set when payload arrives + this.indices.set(block.blockRoot, [emptyIndex, undefined, pendingIndex]); // Update bestChild pointers if (parentIndex !== undefined) { @@ -603,7 +604,7 @@ export class ProtoArray { const fullIndex = this.nodes.length; this.nodes.push(fullNode); - // Add FULL variant to the indices array + // Append FULL variant to the indices array variants[PayloadStatus.FULL] = fullIndex; // Update bestChild for PENDING node (may now prefer FULL over EMPTY) @@ -1060,7 +1061,7 @@ export class ProtoArray { }); } - // For Gloas, PENDING variant (index 0) is always the smallest since it's inserted first + // For Gloas, PENDING variant is always the smallest since it's inserted first const finalizedIndex = Array.isArray(variants) ? variants[PayloadStatus.PENDING] : variants; if (finalizedIndex < this.pruneThreshold) { @@ -1397,7 +1398,7 @@ export class ProtoArray { * Gloas: Returns (root, payloadStatus) based on actual node state */ getAncestor(blockRoot: RootHex, ancestorSlot: Slot): ProtoNode { - // Get any variant to check the block (use variants[0]) + // Get any variant to check the block (PENDING for Gloas, FULL for pre-Gloas) const variantOrArr = this.indices.get(blockRoot); if (variantOrArr == null) { throw new ForkChoiceError({ @@ -1406,13 +1407,11 @@ export class ProtoArray { }); } - const blockIndex = Array.isArray(variantOrArr) ? variantOrArr[0] : variantOrArr; + const blockIndex = Array.isArray(variantOrArr) ? variantOrArr[PayloadStatus.PENDING] : variantOrArr; const block = this.nodes[blockIndex]; - // If block is at or before queried slot, return PENDING variant (or FULL for pre-Gloas) + // If block is at or before queried slot, return any variant (PENDING for Gloas, FULL for pre-Gloas) if (block.slot <= ancestorSlot) { - // For pre-Gloas: only FULL exists at variants[0] - // For Gloas: PENDING is at variants[0] return block; } @@ -1428,7 +1427,7 @@ export class ProtoArray { }); } - let parentIndex = Array.isArray(parentVariants) ? parentVariants[0] : parentVariants; + let parentIndex = Array.isArray(parentVariants) ? parentVariants[PayloadStatus.PENDING] : parentVariants; let parentBlock = this.nodes[parentIndex]; // Walk backwards while parent.slot > ancestorSlot @@ -1444,7 +1443,7 @@ export class ProtoArray { }); } - parentIndex = Array.isArray(nextParentVariants) ? nextParentVariants[0] : nextParentVariants; + parentIndex = Array.isArray(nextParentVariants) ? nextParentVariants[PayloadStatus.PENDING] : nextParentVariants; parentBlock = this.nodes[parentIndex]; } diff --git a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts index df68c53632dd..af229b9955d0 100644 --- a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts @@ -159,7 +159,7 @@ describe("Forkchoice", () => { bestDescendant: undefined, parent: 0, weight: 0, - payloadStatus: 2, // Pre-Gloas blocks always have PAYLOAD_STATUS_FULL + payloadStatus: PayloadStatus.FULL, // Pre-Gloas blocks always have PAYLOAD_STATUS_FULL }); }); diff --git a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts index d4cddf32c483..11ee26246800 100644 --- a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts +++ b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts @@ -302,7 +302,7 @@ describe("executionStatus / invalidate all postmerge chain", () => { // findHead returns ProtoNode // For pre-Gloas blocks, this should have blockRoot "0" and payloadStatus FULL (2) expect(fcHead.blockRoot).toBe("0"); - expect(fcHead.payloadStatus).toBe(2); // PayloadStatus.FULL + expect(fcHead.payloadStatus).toBe(PayloadStatus.FULL); }); }); diff --git a/packages/fork-choice/test/unit/protoArray/gloas.test.ts b/packages/fork-choice/test/unit/protoArray/gloas.test.ts index aff01435db4f..17796151d98f 100644 --- a/packages/fork-choice/test/unit/protoArray/gloas.test.ts +++ b/packages/fork-choice/test/unit/protoArray/gloas.test.ts @@ -82,7 +82,7 @@ describe("Gloas Fork Choice", () => { const variants = (protoArray as any).indices.get("0x02"); expect(variants).toBeDefined(); - // Gloas: variants[PENDING] and variants[EMPTY] should be defined + // Gloas: variants indexed by PayloadStatus [EMPTY, FULL|undefined, PENDING] expect(variants[PayloadStatus.PENDING]).toBeDefined(); expect(variants[PayloadStatus.EMPTY]).toBeDefined(); expect(variants[PayloadStatus.FULL]).toBeUndefined(); From 352cdf260f7ee18ebf9b8fd4491d902adad193ab Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sat, 4 Apr 2026 13:41:41 +0100 Subject: [PATCH 2/2] address bot review --- .../stateCache/persistentCheckpointsCache.ts | 4 +-- .../fork-choice/src/protoArray/protoArray.ts | 30 ++++++++----------- .../protoArray/executionStatusUpdates.test.ts | 2 +- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index 8d0e04ce7451..7a09e50764a6 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -1,6 +1,6 @@ import {routes} from "@lodestar/api"; import {BeaconConfig} from "@lodestar/config"; -import {CheckpointWithPayloadStatus} from "@lodestar/fork-choice"; +import {CheckpointWithPayloadStatus, PayloadStatus} from "@lodestar/fork-choice"; import {IBeaconStateView, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {Epoch, RootHex, phase0} from "@lodestar/types"; import {Logger, MapDef, fromHex, sleep, toHex, toRootHex} from "@lodestar/utils"; @@ -930,8 +930,6 @@ export function toCheckpointHexPayload(checkpoint: phase0.Checkpoint, payloadPre * @throws Error if checkpoint has PENDING payload status (ambiguous which variant to use) */ export function fcCheckpointToHexPayload(checkpoint: CheckpointWithPayloadStatus): CheckpointHexPayload { - const PayloadStatus = {PENDING: 0, EMPTY: 1, FULL: 2} as const; - if (checkpoint.payloadStatus === PayloadStatus.PENDING) { throw Error( `Cannot convert checkpoint with PENDING payload status at epoch ${checkpoint.epoch} root ${checkpoint.rootHex}` diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index c5017587225e..01ec73c50588 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -938,32 +938,26 @@ export class ProtoArray { * Get payload status tiebreaker for fork choice comparison * Spec: gloas/fork-choice.md#new-get_payload_status_tiebreaker * - * For PENDING nodes: always returns 0 - * For EMPTY/FULL variants from slot n-1: implements tiebreaker logic based on should_extend_payload - * For older blocks: returns node.payloadStatus + * For PENDING or not-previous-slot: returns node.payloadStatus + * For EMPTY/FULL from slot n-1: returns hardcoded tiebreaker values per spec + * - EMPTY → 1, FULL+extend → 2, FULL+!extend → 0 * - * Note: pre-gloas logic won't reach here. Pre-Gloas blocks have different roots, so they are always resolved by the weight and root tiebreaker before reaching here. + * Note: pre-gloas logic won't reach here. Pre-Gloas blocks have different roots, + * so they are always resolved by the weight and root tiebreaker before reaching here. */ private getPayloadStatusTiebreaker(node: ProtoNode, currentSlot: Slot, proposerBoostRoot: RootHex | null): number { - // PENDING nodes always return PENDING (no tiebreaker needed) - // PENDING=0, EMPTY=1, FULL=2 - if (node.payloadStatus === PayloadStatus.PENDING) { + // PENDING or not from previous slot: return raw PayloadStatus value + if (node.payloadStatus === PayloadStatus.PENDING || node.slot + 1 !== currentSlot) { return node.payloadStatus; } - // For Gloas: check if from previous slot - if (node.slot + 1 !== currentSlot) { - return node.payloadStatus; - } - - // For previous slot blocks in Gloas, decide between FULL and EMPTY - // based on should_extend_payload + // Previous slot EMPTY/FULL: use spec-defined tiebreaker values (not PayloadStatus enum values) + // Spec: return 1 for EMPTY, 2 for FULL+extend, 0 for FULL+!extend if (node.payloadStatus === PayloadStatus.EMPTY) { - return PayloadStatus.EMPTY; + return 1; } // FULL - check should_extend_payload - const shouldExtend = this.shouldExtendPayload(node.blockRoot, proposerBoostRoot); - return shouldExtend ? PayloadStatus.FULL : PayloadStatus.PENDING; + return this.shouldExtendPayload(node.blockRoot, proposerBoostRoot) ? 2 : 0; } /** @@ -1061,7 +1055,7 @@ export class ProtoArray { }); } - // For Gloas, PENDING variant is always the smallest since it's inserted first + // For Gloas, PENDING has the smallest node index since it's pushed to this.nodes first const finalizedIndex = Array.isArray(variants) ? variants[PayloadStatus.PENDING] : variants; if (finalizedIndex < this.pruneThreshold) { diff --git a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts index 11ee26246800..6d2ed55b62f4 100644 --- a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts +++ b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts @@ -300,7 +300,7 @@ describe("executionStatus / invalidate all postmerge chain", () => { const fcHead = fc.findHead("0", 3); it("pre merge block should be the FC head", () => { // findHead returns ProtoNode - // For pre-Gloas blocks, this should have blockRoot "0" and payloadStatus FULL (2) + // For pre-Gloas blocks, this should have blockRoot "0" and payloadStatus FULL expect(fcHead.blockRoot).toBe("0"); expect(fcHead.payloadStatus).toBe(PayloadStatus.FULL); });