diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index ab6e4774a0ba..896de7942a2b 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -6,6 +6,7 @@ import { ForkChoiceError, ForkChoiceErrorCode, NotReorgedReason, + getFinalizedExecutionBlockHash, getSafeExecutionBlockHash, } from "@lodestar/fork-choice"; import { @@ -443,8 +444,8 @@ export async function importBlock( * the current finalized block does not contain any execution payload at all (pre MERGE_EPOCH) or if it contains a * zero block hash (pre TTD) */ - const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice); - const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; + const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice, this.logger); + const finalizedBlockHash = getFinalizedExecutionBlockHash(this.forkChoice, this.logger); if (headBlockHash !== ZERO_HASH_HEX) { this.executionEngine .notifyForkchoiceUpdate( diff --git a/packages/beacon-node/src/chain/blocks/importExecutionPayload.ts b/packages/beacon-node/src/chain/blocks/importExecutionPayload.ts index dac0b486c374..289e346994ec 100644 --- a/packages/beacon-node/src/chain/blocks/importExecutionPayload.ts +++ b/packages/beacon-node/src/chain/blocks/importExecutionPayload.ts @@ -1,8 +1,12 @@ import {routes} from "@lodestar/api"; -import {ExecutionStatus, PayloadExecutionStatus, getSafeExecutionBlockHash} from "@lodestar/fork-choice"; +import { + ExecutionStatus, + PayloadExecutionStatus, + getFinalizedExecutionBlockHash, + getSafeExecutionBlockHash, +} from "@lodestar/fork-choice"; import {DataAvailabilityStatus, isStatePostGloas} from "@lodestar/state-transition"; import {isErrorAborted} from "@lodestar/utils"; -import {ZERO_HASH_HEX} from "../../constants/index.js"; import {ExecutionPayloadStatus} from "../../execution/index.js"; import {isQueueErrorAborted} from "../../util/queue/index.js"; import {BeaconChain} from "../chain.js"; @@ -245,8 +249,8 @@ export async function importExecutionPayload( // 7. Queue notifyForkchoiceUpdate to engine api const head = this.forkChoice.getHead(); if (!this.opts.disableImportExecutionFcU && blockRootHex === head.blockRoot) { - const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice); - const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; + const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice, this.logger); + const finalizedBlockHash = getFinalizedExecutionBlockHash(this.forkChoice, this.logger); this.executionEngine.notifyForkchoiceUpdate(fork, blockHashHex, safeBlockHash, finalizedBlockHash).catch((e) => { if (!isErrorAborted(e) && !isQueueErrorAborted(e)) { this.logger.error("Error pushing notifyForkchoiceUpdate()", {blockHashHex, finalizedBlockHash}, e); diff --git a/packages/beacon-node/src/chain/prepareNextSlot.ts b/packages/beacon-node/src/chain/prepareNextSlot.ts index 695ff8145ea9..4bc771975bdf 100644 --- a/packages/beacon-node/src/chain/prepareNextSlot.ts +++ b/packages/beacon-node/src/chain/prepareNextSlot.ts @@ -1,6 +1,6 @@ import {routes} from "@lodestar/api"; import {ChainForkConfig} from "@lodestar/config"; -import {getSafeExecutionBlockHash} from "@lodestar/fork-choice"; +import {getFinalizedExecutionBlockHash, getSafeExecutionBlockHash} from "@lodestar/fork-choice"; import { ForkPostBellatrix, ForkSeq, @@ -23,7 +23,7 @@ import { } from "@lodestar/state-transition"; import {Bytes32, Slot, ValidatorIndex} from "@lodestar/types"; import {Logger, fromHex, isErrorAborted, sleep} from "@lodestar/utils"; -import {GENESIS_SLOT, ZERO_HASH_HEX} from "../constants/constants.js"; +import {GENESIS_SLOT} from "../constants/constants.js"; import {BuilderStatus} from "../execution/builder/http.js"; import {Metrics} from "../metrics/index.js"; import {ClockEvent} from "../util/clock.js"; @@ -223,9 +223,8 @@ export class PrepareNextSlotScheduler { computeTimeAtSlot(this.config, prepareSlot, this.chain.genesisTime) - Date.now() / 1000; this.metrics?.blockPayload.payloadAdvancePrepTime.observe(preparationTime); - const safeBlockHash = getSafeExecutionBlockHash(this.chain.forkChoice); - const finalizedBlockHash = - this.chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; + const safeBlockHash = getSafeExecutionBlockHash(this.chain.forkChoice, this.logger); + const finalizedBlockHash = getFinalizedExecutionBlockHash(this.chain.forkChoice, this.logger); // awaiting here instead of throwing an async call because there is no other task // left for scheduler and this gives nice semantics to catch and log errors in the diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index 20592fa5cc12..e631e9f9e760 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -1,6 +1,11 @@ import {BitArray} from "@chainsafe/ssz"; import {ChainForkConfig} from "@lodestar/config"; -import {IForkChoice, ProtoBlock, getSafeExecutionBlockHash} from "@lodestar/fork-choice"; +import { + IForkChoice, + ProtoBlock, + getFinalizedExecutionBlockHash, + getSafeExecutionBlockHash, +} from "@lodestar/fork-choice"; import { BUILDER_INDEX_SELF_BUILD, ForkName, @@ -265,8 +270,8 @@ export async function produceBlockBody( // TODO GLOAS: support non self-building here, the block type differentiation between // full and blinded no longer makes sense in gloas, it might be a good idea to move // this into a completely separate function and have pre/post gloas more separated - const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice); - const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; + const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice, this.logger); + const finalizedBlockHash = getFinalizedExecutionBlockHash(this.forkChoice, this.logger); // TODO GLOAS: post-Gloas, proposer feeRecipient is also carried (signed) in // ProposerPreferencesPool. Consider using this unified cache instead // see https://github.com/ChainSafe/lodestar/issues/9379 @@ -411,8 +416,8 @@ export async function produceBlockBody( throw new Error("Expected Bellatrix state for execution block production"); } - const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice); - const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; + const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice, this.logger); + const finalizedBlockHash = getFinalizedExecutionBlockHash(this.forkChoice, this.logger); const feeRecipient = requestedFeeRecipient ?? this.beaconProposerCache.getOrDefault(proposerIndex); const feeRecipientType = requestedFeeRecipient ? "requested" diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts index bd42ee5f75b7..3cedb42a200e 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts @@ -314,8 +314,8 @@ describe("api/validator - produceBlockV3", () => { syncCommitteeBits: ssz.altair.SyncCommitteeBits.defaultValue(), syncCommitteeSignature: G2_POINT_AT_INFINITY, }); - modules.forkChoice.getJustifiedBlock.mockReturnValue({} as ProtoBlock); - modules.forkChoice.getFinalizedBlock.mockReturnValue({} as ProtoBlock); + modules.forkChoice.getConfirmedBlock.mockReturnValue(generateProtoBlock()); + modules.forkChoice.getFinalizedBlock.mockReturnValue(generateProtoBlock()); modules.chain["executionEngine"].payloadIdCache = new PayloadIdCache(); modules.chain["executionEngine"].notifyForkchoiceUpdate.mockResolvedValue("0x"); diff --git a/packages/beacon-node/test/unit/chain/prepareNextSlot.test.ts b/packages/beacon-node/test/unit/chain/prepareNextSlot.test.ts index aac4fcdbf052..843c957af7b1 100644 --- a/packages/beacon-node/test/unit/chain/prepareNextSlot.test.ts +++ b/packages/beacon-node/test/unit/chain/prepareNextSlot.test.ts @@ -122,7 +122,8 @@ describe("PrepareNextSlot scheduler", () => { getForkStub.mockReturnValue(ForkName.bellatrix); chainStub.recomputeForkChoiceHead.mockReturnValue({...zeroProtoBlock, slot: SLOTS_PER_EPOCH - 3} as ProtoBlock); chainStub.predictProposerHead.mockReturnValue({...zeroProtoBlock, slot: SLOTS_PER_EPOCH - 3} as ProtoBlock); - forkChoiceStub.getFinalizedBlock.mockReturnValue({} as ProtoBlock); + forkChoiceStub.getConfirmedBlock.mockReturnValue({...zeroProtoBlock, slot: SLOTS_PER_EPOCH - 3} as ProtoBlock); + forkChoiceStub.getFinalizedBlock.mockReturnValue({...zeroProtoBlock, slot: SLOTS_PER_EPOCH - 3} as ProtoBlock); updateBuilderStatus.mockReturnValue(void 0); const state = generateCachedBellatrixState(); vi.spyOn(state.epochCtx, "getBeaconProposer").mockReturnValue(proposerIndex); diff --git a/packages/beacon-node/test/utils/typeGenerator.ts b/packages/beacon-node/test/utils/typeGenerator.ts index 7886df877050..0be6c48ee1c7 100644 --- a/packages/beacon-node/test/utils/typeGenerator.ts +++ b/packages/beacon-node/test/utils/typeGenerator.ts @@ -1,4 +1,4 @@ -import {ExecutionStatus, ProtoBlock} from "@lodestar/fork-choice"; +import {ExecutionStatus, PayloadStatus, ProtoBlock} from "@lodestar/fork-choice"; import {DataAvailabilityStatus} from "@lodestar/state-transition"; import {Slot, phase0, ssz} from "@lodestar/types"; import {fromHex} from "@lodestar/utils"; @@ -40,6 +40,8 @@ export function generateProtoBlock(overrides: Partial = {}): ProtoBl unrealizedFinalizedRoot: ZERO_HASH_HEX, timeliness: false, + payloadStatus: PayloadStatus.FULL, + parentBlockHash: null, ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, dataAvailabilityStatus: DataAvailabilityStatus.PreData, diff --git a/packages/fork-choice/src/forkChoice/safeBlocks.ts b/packages/fork-choice/src/forkChoice/safeBlocks.ts index c8be98840e74..e8a93a25c173 100644 --- a/packages/fork-choice/src/forkChoice/safeBlocks.ts +++ b/packages/fork-choice/src/forkChoice/safeBlocks.ts @@ -1,6 +1,7 @@ -import {ZERO_HASH_HEX} from "@lodestar/params"; +import {GENESIS_SLOT, ZERO_HASH_HEX} from "@lodestar/params"; import {Root, RootHex} from "@lodestar/types"; -import {fromHex} from "@lodestar/utils"; +import {LogLevel, Logger, fromHex} from "@lodestar/utils"; +import {HEX_ZERO_HASH, ProtoBlock, isGloasBlock} from "../protoArray/interface.js"; import {IForkChoice} from "./interface.js"; /** @@ -19,17 +20,56 @@ export function getSafeBeaconBlockRoot(fc: IForkChoice): Root { } /** - * Get execution payload hash for the safe block + * Get execution payload hash to report as `safeBlockHash` in `engine_forkchoiceUpdated`. * - * https://github.com/ethereum/consensus-specs/blob/master/fork_choice/safe-block.md#get_safe_execution_block_hash + * Pre-Gloas: the confirmed block's own payload hash. + * Post-Gloas: the confirmed block's bid `parent_block_hash` — under ePBS the block's own + * payload may not yet be confirmed canonical, so we report the parent EL block which has + * been (the bid commits to extending it). + * + * https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.13/specs/bellatrix/fast-confirmation.md#new-get_safe_execution_block_hash */ -export function getSafeExecutionBlockHash(forkChoice: IForkChoice): RootHex { - const confirmedRoot = forkChoice.getConfirmedRoot(); - if (confirmedRoot) { - const confirmedBlock = forkChoice.getBlockHexDefaultStatus(confirmedRoot); - if (confirmedBlock?.executionPayloadBlockHash) { - return confirmedBlock.executionPayloadBlockHash; - } +export function getSafeExecutionBlockHash(forkChoice: IForkChoice, logger?: Pick): RootHex { + const confirmedBlock = forkChoice.getConfirmedBlock(); + if (confirmedBlock === null) { + logger?.warn("Confirmed block not found, using zero safe execution block hash", { + confirmedRoot: forkChoice.getConfirmedRoot(), + }); + return ZERO_HASH_HEX; } - return ZERO_HASH_HEX; + + return getExecutionBlockHash(confirmedBlock, logger); +} + +/** + * Get execution payload hash to report as `finalizedBlockHash` in `engine_forkchoiceUpdated`. + * Mirrors `getSafeExecutionBlockHash`: post-Gloas returns the bid `parent_block_hash`. + * + * https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.13/specs/gloas/fork-choice.md#notify_forkchoice_updated + */ +export function getFinalizedExecutionBlockHash(forkChoice: IForkChoice, logger?: Pick): RootHex { + return getExecutionBlockHash(forkChoice.getFinalizedBlock(), logger); +} + +function getExecutionBlockHash(block: ProtoBlock, logger?: Pick): RootHex { + if (isGloasBlock(block)) { + return block.parentBlockHash; + } + + // The genesis block body carries a default execution payload, so it is not an execution block + // per the spec (`is_execution_block`); its proto-array payload hash comes from the anchor + // state header instead of a block body and must not be reported as safe. + if (block.slot === GENESIS_SLOT) { + return HEX_ZERO_HASH; + } + + if (block.executionPayloadBlockHash === null) { + logger?.warn("Execution payload block hash not found, using zero hash", { + blockRoot: block.blockRoot, + slot: block.slot, + }); + return HEX_ZERO_HASH; + } + + return block.executionPayloadBlockHash; } diff --git a/packages/fork-choice/src/protoArray/interface.ts b/packages/fork-choice/src/protoArray/interface.ts index 11ba86a71f7e..5f48059e60a2 100644 --- a/packages/fork-choice/src/protoArray/interface.ts +++ b/packages/fork-choice/src/protoArray/interface.ts @@ -48,7 +48,7 @@ export enum PayloadStatus { /** * Check if a block is in the Gloas fork (ePBS enabled) */ -export function isGloasBlock(block: ProtoBlock): boolean { +export function isGloasBlock(block: ProtoBlock): block is ProtoBlock & {parentBlockHash: RootHex} { return block.parentBlockHash !== null; } diff --git a/packages/fork-choice/test/unit/forkChoice/safeBlocks.test.ts b/packages/fork-choice/test/unit/forkChoice/safeBlocks.test.ts new file mode 100644 index 000000000000..72acd93a369b --- /dev/null +++ b/packages/fork-choice/test/unit/forkChoice/safeBlocks.test.ts @@ -0,0 +1,179 @@ +import {describe, expect, it, vi} from "vitest"; +import {GENESIS_SLOT} from "@lodestar/params"; +import {DataAvailabilityStatus} from "@lodestar/state-transition"; +import {RootHex, Slot} from "@lodestar/types"; +import {IForkChoice} from "../../../src/forkChoice/interface.js"; +import {getFinalizedExecutionBlockHash, getSafeExecutionBlockHash} from "../../../src/forkChoice/safeBlocks.js"; +import {ExecutionStatus, HEX_ZERO_HASH, PayloadStatus, ProtoBlock} from "../../../src/protoArray/interface.js"; + +function buildBlock(opts: { + blockRoot: RootHex; + executionPayloadBlockHash: RootHex | null; + parentBlockHash: RootHex | null; + slot?: Slot; +}): ProtoBlock { + const common = { + slot: opts.slot ?? 1, + blockRoot: opts.blockRoot, + parentRoot: "0x00", + stateRoot: "0x00", + targetRoot: "0x00", + justifiedEpoch: 0, + justifiedRoot: "0x00", + finalizedEpoch: 0, + finalizedRoot: "0x00", + unrealizedJustifiedEpoch: 0, + unrealizedJustifiedRoot: "0x00", + unrealizedFinalizedEpoch: 0, + unrealizedFinalizedRoot: "0x00", + timeliness: true, + payloadStatus: PayloadStatus.FULL, + parentBlockHash: opts.parentBlockHash, + }; + if (opts.executionPayloadBlockHash === null) { + return { + ...common, + executionPayloadBlockHash: null, + executionStatus: ExecutionStatus.PreMerge, + dataAvailabilityStatus: DataAvailabilityStatus.PreData, + }; + } + return { + ...common, + executionPayloadBlockHash: opts.executionPayloadBlockHash, + executionPayloadNumber: 0, + executionPayloadGasLimit: 30_000_000, + executionStatus: ExecutionStatus.Valid, + dataAvailabilityStatus: DataAvailabilityStatus.Available, + }; +} + +function mockForkChoice(confirmed: ProtoBlock | null, finalized: ProtoBlock): IForkChoice { + return { + getConfirmedBlock: () => confirmed, + getConfirmedRoot: () => confirmed?.blockRoot ?? "0xconfirmed", + getFinalizedBlock: () => finalized, + } as unknown as IForkChoice; +} + +describe("safeBlocks - getSafeExecutionBlockHash", () => { + it("pre-Gloas: returns the confirmed block's own executionPayloadBlockHash", () => { + const confirmed = buildBlock({ + blockRoot: "0xaa", + executionPayloadBlockHash: "0xpayloadA", + parentBlockHash: null, + }); + const fc = mockForkChoice(confirmed, confirmed); + expect(getSafeExecutionBlockHash(fc)).toBe("0xpayloadA"); + }); + + it("pre-Bellatrix: returns ZERO_HASH_HEX when executionPayloadBlockHash is null", () => { + const warn = vi.fn(); + const confirmed = buildBlock({ + blockRoot: "0xaa", + executionPayloadBlockHash: null, + parentBlockHash: null, + }); + const fc = mockForkChoice(confirmed, confirmed); + expect(getSafeExecutionBlockHash(fc, {warn})).toBe(HEX_ZERO_HASH); + expect(warn).toHaveBeenCalledWith("Execution payload block hash not found, using zero hash", { + blockRoot: confirmed.blockRoot, + slot: confirmed.slot, + }); + }); + + it("post-Gloas: returns the confirmed block's bid.parent_block_hash, not its own payload hash", () => { + const confirmed = buildBlock({ + blockRoot: "0xaa", + executionPayloadBlockHash: "0xpayloadA", + parentBlockHash: "0xparentEL", + }); + const fc = mockForkChoice(confirmed, confirmed); + expect(getSafeExecutionBlockHash(fc)).toBe("0xparentEL"); + }); + + it("returns ZERO_HASH_HEX and logs when the confirmed block is not found", () => { + const warn = vi.fn(); + const finalized = buildBlock({ + blockRoot: "0xbb", + executionPayloadBlockHash: "0xpayloadF", + parentBlockHash: null, + }); + const fc = mockForkChoice(null, finalized); + + expect(getSafeExecutionBlockHash(fc, {warn})).toBe(HEX_ZERO_HASH); + expect(warn).toHaveBeenCalledWith("Confirmed block not found, using zero safe execution block hash", { + confirmedRoot: "0xconfirmed", + }); + }); + + it("pre-Gloas genesis anchor: returns ZERO_HASH_HEX, not the state's payload header hash", () => { + const confirmed = buildBlock({ + blockRoot: "0xaa", + executionPayloadBlockHash: "0xfromStateHeader", + parentBlockHash: null, + slot: GENESIS_SLOT, + }); + const fc = mockForkChoice(confirmed, confirmed); + expect(getSafeExecutionBlockHash(fc)).toBe(HEX_ZERO_HASH); + }); + + it("Gloas genesis anchor: returns the bid.parent_block_hash", () => { + const confirmed = buildBlock({ + blockRoot: "0xaa", + executionPayloadBlockHash: "0xpayloadA", + parentBlockHash: "0xparentEL", + slot: GENESIS_SLOT, + }); + const fc = mockForkChoice(confirmed, confirmed); + expect(getSafeExecutionBlockHash(fc)).toBe("0xparentEL"); + }); +}); + +describe("safeBlocks - getFinalizedExecutionBlockHash", () => { + it("pre-Gloas: returns the finalized block's own executionPayloadBlockHash", () => { + const finalized = buildBlock({ + blockRoot: "0xbb", + executionPayloadBlockHash: "0xpayloadF", + parentBlockHash: null, + }); + const fc = mockForkChoice(finalized, finalized); + expect(getFinalizedExecutionBlockHash(fc)).toBe("0xpayloadF"); + }); + + it("pre-Bellatrix: returns ZERO_HASH_HEX when executionPayloadBlockHash is null", () => { + const warn = vi.fn(); + const finalized = buildBlock({ + blockRoot: "0xbb", + executionPayloadBlockHash: null, + parentBlockHash: null, + }); + const fc = mockForkChoice(finalized, finalized); + expect(getFinalizedExecutionBlockHash(fc, {warn})).toBe(HEX_ZERO_HASH); + expect(warn).toHaveBeenCalledWith("Execution payload block hash not found, using zero hash", { + blockRoot: finalized.blockRoot, + slot: finalized.slot, + }); + }); + + it("post-Gloas: returns the finalized block's bid.parent_block_hash, not its own payload hash", () => { + const finalized = buildBlock({ + blockRoot: "0xbb", + executionPayloadBlockHash: "0xpayloadF", + parentBlockHash: "0xparentEL", + }); + const fc = mockForkChoice(finalized, finalized); + expect(getFinalizedExecutionBlockHash(fc)).toBe("0xparentEL"); + }); + + it("pre-Gloas genesis anchor: returns ZERO_HASH_HEX, not the state's payload header hash", () => { + const finalized = buildBlock({ + blockRoot: "0xbb", + executionPayloadBlockHash: "0xfromStateHeader", + parentBlockHash: null, + slot: GENESIS_SLOT, + }); + const fc = mockForkChoice(finalized, finalized); + expect(getFinalizedExecutionBlockHash(fc)).toBe(HEX_ZERO_HASH); + }); +});