diff --git a/packages/beacon-node/src/chain/errors/executionPayloadBid.ts b/packages/beacon-node/src/chain/errors/executionPayloadBid.ts index 000f77d5db57..6506e87e2ea4 100644 --- a/packages/beacon-node/src/chain/errors/executionPayloadBid.ts +++ b/packages/beacon-node/src/chain/errors/executionPayloadBid.ts @@ -5,6 +5,7 @@ export enum ExecutionPayloadBidErrorCode { BUILDER_NOT_ELIGIBLE = "EXECUTION_PAYLOAD_BID_ERROR_BUILDER_NOT_ELIGIBLE", INVALID_BUILDER_VERSION = "EXECUTION_PAYLOAD_BID_ERROR_INVALID_BUILDER_VERSION", NON_ZERO_EXECUTION_PAYMENT = "EXECUTION_PAYLOAD_BID_ERROR_NON_ZERO_EXECUTION_PAYMENT", + INCOMPATIBLE_WITH_HEAD = "EXECUTION_PAYLOAD_BID_ERROR_INCOMPATIBLE_WITH_HEAD", BID_ALREADY_KNOWN = "EXECUTION_PAYLOAD_BID_ERROR_BID_ALREADY_KNOWN", BID_TOO_LOW = "EXECUTION_PAYLOAD_BID_ERROR_BID_TOO_LOW", BID_TOO_HIGH = "EXECUTION_PAYLOAD_BID_ERROR_BID_TOO_HIGH", @@ -33,6 +34,13 @@ export type ExecutionPayloadBidErrorType = builderIndex: BuilderIndex; executionPayment: number; } + | { + code: ExecutionPayloadBidErrorCode.INCOMPATIBLE_WITH_HEAD; + slot: Slot; + parentBlockRoot: RootHex; + parentBlockHash: RootHex; + headBlockRoot: RootHex; + } | { code: ExecutionPayloadBidErrorCode.BID_ALREADY_KNOWN; builderIndex: BuilderIndex; diff --git a/packages/beacon-node/src/chain/prepareNextSlot.ts b/packages/beacon-node/src/chain/prepareNextSlot.ts index a991db1231ff..ce088598d62d 100644 --- a/packages/beacon-node/src/chain/prepareNextSlot.ts +++ b/packages/beacon-node/src/chain/prepareNextSlot.ts @@ -173,7 +173,7 @@ export class PrepareNextSlotScheduler { // Apply parent payload once here as it's reused by EL prep and SSE emit below let stateAfterParentPayload: IBeaconStateViewBellatrix = updatedPrepareState; if (isStatePostGloas(updatedPrepareState)) { - // Spec: should_build_on_full(store, head) — see produceBlockBody.ts for context. + // Spec: should_build_on_full(store, head, slot) - see produceBlockBody.ts for context. if (this.chain.forkChoice.shouldBuildOnFull(updatedHead, prepareSlot)) { parentBlockHash = updatedPrepareState.latestExecutionPayloadBid.blockHash; // Skip applying parent payload unless we're proposing the next slot or have to emit payload_attributes events diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index db4fcaae965d..1ccc3056a7b6 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -276,7 +276,7 @@ export async function produceBlockBody( let parentExecutionRequests: gloas.ExecutionRequests; // Apply parent payload once here as it's reused by EL prep and voluntary exit filtering below let stateAfterParentPayload: IBeaconStateViewBellatrix = currentState; - // Spec: should_build_on_full(store, head). `parentBlock` is the proposer's head + // Spec: should_build_on_full(store, head, slot). `parentBlock` is the proposer's head // (set by chain.getProposerHead(slot)). Returns false when the PTC majority signalled // the blob data is not available or the payload was not timely, forcing a build on EMPTY (reorg). const isBuildingOnFull = this.forkChoice.shouldBuildOnFull(parentBlock, blockSlot); diff --git a/packages/beacon-node/src/chain/seenCache/seenExecutionPayloadBids.ts b/packages/beacon-node/src/chain/seenCache/seenExecutionPayloadBids.ts index 624484e958f7..39046a234af1 100644 --- a/packages/beacon-node/src/chain/seenCache/seenExecutionPayloadBids.ts +++ b/packages/beacon-node/src/chain/seenCache/seenExecutionPayloadBids.ts @@ -1,4 +1,4 @@ -import {BuilderIndex, Slot} from "@lodestar/types"; +import {BuilderIndex, RootHex, Slot} from "@lodestar/types"; import {MapDef} from "@lodestar/utils"; /** @@ -7,29 +7,42 @@ import {MapDef} from "@lodestar/utils"; const SLOTS_RETAINED = 2; /** - * Tracks execution payload bids we've already seen per (slot, builder). + * Tracks execution payload bids we've already seen per + * (slot, builder, parent block hash, parent block root). */ export class SeenExecutionPayloadBids { - private readonly builderIndexesBySlot = new MapDef>(() => new Set()); + private readonly branchesByBuilderBySlot = new MapDef>>( + () => new MapDef>(() => new Set()) + ); private lowestPermissibleSlot: Slot = 0; - isKnown(slot: Slot, builderIndex: BuilderIndex): boolean { - return this.builderIndexesBySlot.get(slot)?.has(builderIndex) === true; + isKnown(slot: Slot, builderIndex: BuilderIndex, parentBlockHash: RootHex, parentBlockRoot: RootHex): boolean { + return ( + this.branchesByBuilderBySlot.get(slot)?.get(builderIndex)?.has(branchKey(parentBlockHash, parentBlockRoot)) === + true + ); } - add(slot: Slot, builderIndex: BuilderIndex): void { + add(slot: Slot, builderIndex: BuilderIndex, parentBlockHash: RootHex, parentBlockRoot: RootHex): void { if (slot < this.lowestPermissibleSlot) { throw Error(`slot ${slot} < lowestPermissibleSlot ${this.lowestPermissibleSlot}`); } - this.builderIndexesBySlot.getOrDefault(slot).add(builderIndex); + this.branchesByBuilderBySlot + .getOrDefault(slot) + .getOrDefault(builderIndex) + .add(branchKey(parentBlockHash, parentBlockRoot)); } prune(currentSlot: Slot): void { this.lowestPermissibleSlot = Math.max(currentSlot - SLOTS_RETAINED, 0); - for (const slot of this.builderIndexesBySlot.keys()) { + for (const slot of this.branchesByBuilderBySlot.keys()) { if (slot < this.lowestPermissibleSlot) { - this.builderIndexesBySlot.delete(slot); + this.branchesByBuilderBySlot.delete(slot); } } } } + +function branchKey(parentBlockHash: RootHex, parentBlockRoot: RootHex): string { + return `${parentBlockHash}:${parentBlockRoot}`; +} diff --git a/packages/beacon-node/src/chain/validation/executionPayloadBid.ts b/packages/beacon-node/src/chain/validation/executionPayloadBid.ts index 60a6926e842e..e499aa6e03ad 100644 --- a/packages/beacon-node/src/chain/validation/executionPayloadBid.ts +++ b/packages/beacon-node/src/chain/validation/executionPayloadBid.ts @@ -1,4 +1,5 @@ import {PublicKey} from "@chainsafe/blst"; +import {IForkChoice, ProtoBlock} from "@lodestar/fork-choice"; import {PAYLOAD_BUILDER_VERSION} from "@lodestar/params"; import { computeEpochAtSlot, @@ -8,7 +9,7 @@ import { isGasLimitTargetCompatible, isStatePostGloas, } from "@lodestar/state-transition"; -import {ValidatorIndex, gloas} from "@lodestar/types"; +import {RootHex, Slot, ValidatorIndex, gloas} from "@lodestar/types"; import {byteArrayEquals, toHex, toRootHex} from "@lodestar/utils"; import {getShufflingDependentRoot} from "../../util/dependentRoot.js"; import {ExecutionPayloadBidError, ExecutionPayloadBidErrorCode, GossipAction} from "../errors/index.js"; @@ -36,12 +37,45 @@ const BID_INCREMENT_CAP_GWEI = 10_000_000; * Return the minimum value a new bid must have to be forwarded given the current highest bid. * Division before multiplication to stay within safe integer range for max gwei values. */ -export function getMinBidValue(currentHighestBid: number): number { +function getMinBidValue(currentHighestBid: number): number { const relativeIncrement = Math.floor(currentHighestBid / 10_000) * BID_INCREMENT_BPS; const increment = Math.min(Math.max(BID_INCREMENT_FLOOR_GWEI, relativeIncrement), BID_INCREMENT_CAP_GWEI); return currentHighestBid + increment; } +/** + * Check whether a bid builds on one of the paths compatible with the local head branch. + * + * The direct parent path is always allowed for proposer-boost reorgs. Otherwise the bid + * must build on the local head's full or empty payload variant, as selected for its slot. + */ +function isBidCompatibleWithHead( + forkChoice: IForkChoice, + head: ProtoBlock, + bidSlot: Slot, + bidParentBlockRoot: RootHex, + bidParentBlockHash: RootHex +): boolean { + const buildsOnParentBlock = bidParentBlockRoot === head.parentRoot; + const buildsOnParentPayload = bidParentBlockHash === head.parentBlockHash; + + if (buildsOnParentBlock && buildsOnParentPayload) { + return true; + } + + if (bidParentBlockRoot !== head.blockRoot) { + return false; + } + + const buildsOnHeadPayload = bidParentBlockHash === head.executionPayloadBlockHash; + + if (forkChoice.shouldBuildOnFull(head, bidSlot)) { + return buildsOnHeadPayload; + } + + return buildsOnParentPayload; +} + export async function validateApiExecutionPayloadBid( chain: IBeaconChain, signedExecutionPayloadBid: gloas.SignedExecutionPayloadBid @@ -61,8 +95,8 @@ async function validateExecutionPayloadBid( signedExecutionPayloadBid: gloas.SignedExecutionPayloadBid ): Promise<{proposerIndex: ValidatorIndex}> { const bid = signedExecutionPayloadBid.message; - const parentBlockRootHex = toRootHex(bid.parentBlockRoot); - const parentBlockHashHex = toRootHex(bid.parentBlockHash); + const bidParentBlockRoot = toRootHex(bid.parentBlockRoot); + const bidParentBlockHash = toRootHex(bid.parentBlockHash); // [IGNORE] `bid.slot` is the current slot, or the next slot (`bid.slot - 1` is current), allowing for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. if ( @@ -76,14 +110,40 @@ async function validateExecutionPayloadBid( }); } + // [IGNORE] The bid is compatible with the current head branch. + const head = chain.forkChoice.getHead(); + if (!isBidCompatibleWithHead(chain.forkChoice, head, bid.slot, bidParentBlockRoot, bidParentBlockHash)) { + throw new ExecutionPayloadBidError(GossipAction.IGNORE, { + code: ExecutionPayloadBidErrorCode.INCOMPATIBLE_WITH_HEAD, + slot: bid.slot, + parentBlockRoot: bidParentBlockRoot, + parentBlockHash: bidParentBlockHash, + headBlockRoot: head.blockRoot, + }); + } + + // [IGNORE] this is the first signed bid seen with a valid signature from the given builder for + // the tuple `(bid.slot, bid.parent_block_hash, bid.parent_block_root)`. + // Entries are only added after signature verification, so known tuples can be dropped before + // state regeneration and the other expensive validation steps. + if (chain.seenExecutionPayloadBids.isKnown(bid.slot, bid.builderIndex, bidParentBlockHash, bidParentBlockRoot)) { + throw new ExecutionPayloadBidError(GossipAction.IGNORE, { + code: ExecutionPayloadBidErrorCode.BID_ALREADY_KNOWN, + builderIndex: bid.builderIndex, + slot: bid.slot, + parentBlockRoot: bidParentBlockRoot, + parentBlockHash: bidParentBlockHash, + }); + } + // [IGNORE] `bid.parent_block_root` is the hash tree root of a known beacon block in fork choice. // Moved earlier than the spec ordering so we can derive the proposer dependent root for the // proposer-preferences lookup below from a known fork-choice block. - const parentBlock = chain.forkChoice.getBlockHexDefaultStatus(parentBlockRootHex); + const parentBlock = chain.forkChoice.getBlockHexDefaultStatus(bidParentBlockRoot); if (parentBlock === null) { throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.UNKNOWN_BLOCK_ROOT, - parentBlockRoot: parentBlockRootHex, + parentBlockRoot: bidParentBlockRoot, }); } @@ -122,7 +182,7 @@ async function validateExecutionPayloadBid( throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.NO_MATCHING_PROPOSER_PREFERENCES, slot: bid.slot, - parentBlockRoot: parentBlockRootHex, + parentBlockRoot: bidParentBlockRoot, dependentRoot: "unknown", }); } @@ -132,7 +192,7 @@ async function validateExecutionPayloadBid( throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.NO_MATCHING_PROPOSER_PREFERENCES, slot: bid.slot, - parentBlockRoot: parentBlockRootHex, + parentBlockRoot: bidParentBlockRoot, dependentRoot: dependentRootHex, }); } @@ -143,7 +203,7 @@ async function validateExecutionPayloadBid( .catch(() => { throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.UNKNOWN_BLOCK_ROOT, - parentBlockRoot: parentBlockRootHex, + parentBlockRoot: bidParentBlockRoot, }); }); @@ -208,11 +268,11 @@ async function validateExecutionPayloadBid( // payload's hash) and EMPTY parents (EMPTY/PENDING variants carry the inherited parent // payload's hash, since the new block doesn't have its own payload). Variant carries the // executed payload's gas_limit, which we use as `parent_gas_limit` below. - const parentPayloadVariant = chain.forkChoice.getBlockHexAndBlockHash(parentBlockRootHex, parentBlockHashHex); + const parentPayloadVariant = chain.forkChoice.getBlockHexAndBlockHash(bidParentBlockRoot, bidParentBlockHash); if (parentPayloadVariant === null || parentPayloadVariant.executionPayloadBlockHash === null) { throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.UNKNOWN_PARENT_BLOCK_HASH, - parentBlockHash: parentBlockHashHex, + parentBlockHash: bidParentBlockHash, }); } @@ -245,23 +305,12 @@ async function validateExecutionPayloadBid( }); } - // [IGNORE] this is the first signed bid seen with a valid signature from the given builder for this slot. - if (chain.seenExecutionPayloadBids.isKnown(bid.slot, bid.builderIndex)) { - throw new ExecutionPayloadBidError(GossipAction.IGNORE, { - code: ExecutionPayloadBidErrorCode.BID_ALREADY_KNOWN, - builderIndex: bid.builderIndex, - slot: bid.slot, - parentBlockRoot: parentBlockRootHex, - parentBlockHash: parentBlockHashHex, - }); - } - // [IGNORE] this bid is the highest value bid seen for the tuple // `(bid.slot, bid.parent_block_hash, bid.parent_block_root)`. // As a DoS prevention measure, the bid must also exceed the current highest bid by a minimum // increment, see https://github.com/ethereum/consensus-specs/pull/4831. This prevents spam // from builders submitting numerous bids with minimal value increments. - const bestBid = chain.executionPayloadBidPool.getBestBid(bid.slot, parentBlockHashHex, parentBlockRootHex); + const bestBid = chain.executionPayloadBidPool.getBestBid(bid.slot, bidParentBlockHash, bidParentBlockRoot); if (bestBid !== null && bid.value < getMinBidValue(bestBid.message.value)) { throw new ExecutionPayloadBidError(GossipAction.IGNORE, { code: ExecutionPayloadBidErrorCode.BID_TOO_LOW, @@ -306,8 +355,20 @@ async function validateExecutionPayloadBid( }); } + // Repeat the seen check after the awaited signature verification to prevent concurrent bids + // for the same builder and tuple from both passing validation. + if (chain.seenExecutionPayloadBids.isKnown(bid.slot, bid.builderIndex, bidParentBlockHash, bidParentBlockRoot)) { + throw new ExecutionPayloadBidError(GossipAction.IGNORE, { + code: ExecutionPayloadBidErrorCode.BID_ALREADY_KNOWN, + builderIndex: bid.builderIndex, + slot: bid.slot, + parentBlockRoot: bidParentBlockRoot, + parentBlockHash: bidParentBlockHash, + }); + } + // Valid - chain.seenExecutionPayloadBids.add(bid.slot, bid.builderIndex); + chain.seenExecutionPayloadBids.add(bid.slot, bid.builderIndex, bidParentBlockHash, bidParentBlockRoot); return {proposerIndex: proposerPreferences.message.validatorIndex}; } diff --git a/packages/beacon-node/src/network/processor/index.ts b/packages/beacon-node/src/network/processor/index.ts index 357bb967e48e..8006a265f917 100644 --- a/packages/beacon-node/src/network/processor/index.ts +++ b/packages/beacon-node/src/network/processor/index.ts @@ -461,7 +461,8 @@ export class NetworkProcessor { break; } case GossipType.execution_payload_bid: { - // instead of searching for the message root, this searches for the parent root + // Search for the parent independently of current head compatibility. A bid may arrive + // before its parent block, so compatibility is checked later during gossip validation. const parentBlockRoot = getParentBlockRootFromSignedExecutionPayloadBidSerialized(message.msg.data); const parentBlockHash = getParentBlockHashFromSignedExecutionPayloadBidSerialized(message.msg.data); if ( diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 19d0566fce39..3732f848111d 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -388,7 +388,7 @@ export class ForkChoice implements IForkChoice { return this.protoArray.shouldExtendPayload(blockRoot, this.proposerBoostRoot); } - /** Spec: should_build_on_full(store, head) */ + /** Spec: should_build_on_full(store, head, slot) */ shouldBuildOnFull(head: ProtoBlock, slot: Slot): boolean { return this.protoArray.shouldBuildOnFull(head, slot); } diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index 9f582a4c54df..4a1b26a37d5f 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -270,7 +270,7 @@ export interface IForkChoice { getBlockHexDefaultStatus(blockRoot: RootHex): ProtoBlock | null; getBlockHexAndBlockHash(blockRoot: RootHex, blockHash: RootHex): ProtoBlock | null; shouldExtendPayload(blockRoot: RootHex): boolean; - /** Spec: should_build_on_full(store, head) */ + /** Spec: should_build_on_full(store, head, slot) */ shouldBuildOnFull(head: ProtoBlock, slot: Slot): boolean; getFinalizedBlock(): ProtoBlock; getJustifiedBlock(): ProtoBlock; diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 4ea61f6e7e0f..b234f70d3b24 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -855,7 +855,7 @@ export class ProtoArray { } /** - * Spec: should_build_on_full(store, head) + * Spec: should_build_on_full(store, head, slot) * * The proposer is forced to build on the EMPTY variant (effectively reorging) * when the PTC majority voted that the blob data is not available or that the diff --git a/packages/fork-choice/test/unit/protoArray/gloas.test.ts b/packages/fork-choice/test/unit/protoArray/gloas.test.ts index d77fb776982e..f99537953362 100644 --- a/packages/fork-choice/test/unit/protoArray/gloas.test.ts +++ b/packages/fork-choice/test/unit/protoArray/gloas.test.ts @@ -1143,7 +1143,7 @@ describe("Gloas Fork Choice", () => { }); }); - describe("shouldBuildOnFull() — Spec: should_build_on_full(store, head)", () => { + describe("shouldBuildOnFull() - Spec: should_build_on_full(store, head, slot)", () => { let protoArray: ProtoArray; beforeEach(() => {