diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 6dfd3fda6925..7e045e4ad6b0 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -265,9 +265,20 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand } throw e; } finally { + const proposerIndex = signedBlock.message.proposerIndex; + // A block with a verified proposer signature counts as a proposal of its proposer in fork choice whether it is + // imported or not, so the equivocation view does not depend on how many repeat proposals are validated + if (chain.seenBlockProposers.hasBlockRoot(slot, proposerIndex, blockRootHex)) { + const receiveDelaySec = seenTimestampSec - computeTimeAtSlot(config, slot, chain.genesisTime); + chain.forkChoice.onSignedBlockHeader( + slot, + proposerIndex, + blockRootHex, + chain.forkChoice.isPtcTimely(slot, receiveDelaySec) + ); + } // The block received from the network may have established an equivocation, either by conflicting // with a previously observed block root (REPEAT_PROPOSAL) or with a root observed during validation - const proposerIndex = signedBlock.message.proposerIndex; if (chain.seenBlockProposers.isEquivocating(slot, proposerIndex)) { chain.processProposerEquivocation(slot, proposerIndex); } diff --git a/packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts b/packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts index ad51fdabdf53..d76b791a9040 100644 --- a/packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts +++ b/packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts @@ -8,6 +8,7 @@ import { DataAvailabilityStatus, computeAnchorCheckpoint, computeEpochAtSlot, + computeStartSlotAtEpoch, getEffectiveBalanceIncrementsZeroed, getTemporaryBlockHeader, processSlots, @@ -163,6 +164,13 @@ describe("LodestarForkChoice", () => { /** * finalized - slot 8 (finalized 1) - slot 12 - slot 16 (finalized 2) - slot 20 - slot 24 (finalized 3) - slot 28 - slot 32 (finalized 4) */ + it("isPtcTimely - only for the current slot before the PTC deadline", () => { + forkChoice.updateTime(16); + expect(forkChoice.isPtcTimely(16, 1)).toBe(true); + expect(forkChoice.isPtcTimely(16, config.SECONDS_PER_SLOT - 1)).toBe(false); + expect(forkChoice.isPtcTimely(15, 1)).toBe(false); + }); + it("prune - should prune old blocks", () => { const {blockHeader} = computeAnchorCheckpoint(config, anchorState); const finalizedRoot = ssz.phase0.BeaconBlockHeader.hashTreeRoot(blockHeader); @@ -269,7 +277,14 @@ describe("LodestarForkChoice", () => { executionStatus, dataAvailabilityStatus ); + const signedProposals = (forkChoice as unknown as {signedProposals: Map}).signedProposals; + expect(signedProposals.has(8)).toBe(true); forkChoice.prune(hashBlock(block16.message)); + const finalizedSlot = computeStartSlotAtEpoch(forkChoice.getFinalizedCheckpoint().epoch); + expect([...signedProposals.keys()].filter((slot) => slot < finalizedSlot)).toEqual([]); + // signed blocks for pruned slots are not recorded anymore + forkChoice.onSignedBlockHeader(finalizedSlot - 1, 0, hashBlock(block08.message), true); + expect(signedProposals.has(finalizedSlot - 1)).toBe(false); expect(forkChoice.getAllAncestorBlocks(hashBlock(block16.message), PayloadStatus.FULL).length).toBeWithMessage( 1, "getAllAncestorBlocks returns the finalized block itself as the last (boundary) node" diff --git a/packages/beacon-node/test/unit/network/processor/gossipHandlers.test.ts b/packages/beacon-node/test/unit/network/processor/gossipHandlers.test.ts index bbec2d65d6db..e80257757cdf 100644 --- a/packages/beacon-node/test/unit/network/processor/gossipHandlers.test.ts +++ b/packages/beacon-node/test/unit/network/processor/gossipHandlers.test.ts @@ -65,7 +65,12 @@ describe("getGossipHandlers", () => { }); it("imports a signature-verified REPEAT_PROPOSAL (equivocating) block into fork choice but keeps IGNORE", async () => { - const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: true}); + const {processBlock, threw, onSignedBlockHeader} = await runBeaconBlockRepeatProposal(denebConfig, { + recorded: true, + }); + // the signed block counts as a proposal in fork choice regardless of the import + expect(onSignedBlockHeader).toHaveBeenCalledOnce(); + expect(onSignedBlockHeader.mock.calls[0]).toEqual([1, 3, expect.any(String), true]); // imported so LMD-GHOST can weigh it ... expect(processBlock).toHaveBeenCalledOnce(); @@ -74,7 +79,10 @@ describe("getGossipHandlers", () => { }); it("does not import a REPEAT_PROPOSAL block whose root was not recorded (unverified 3rd+ proposal)", async () => { - const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: false}); + const {processBlock, threw, onSignedBlockHeader} = await runBeaconBlockRepeatProposal(denebConfig, { + recorded: false, + }); + expect(onSignedBlockHeader).not.toHaveBeenCalled(); expect(processBlock).not.toHaveBeenCalled(); expect(threw).toBe(true); @@ -162,7 +170,11 @@ async function runBeaconBlockProcessingError( async function runBeaconBlockRepeatProposal( config: BeaconConfig, {recorded}: {recorded: boolean} -): Promise<{processBlock: ReturnType; threw: boolean}> { +): Promise<{ + processBlock: ReturnType; + threw: boolean; + onSignedBlockHeader: ReturnType; +}> { const logger = testLogger(); const peerIdStr = "16Uiu2HAmTestGossipPeer" as PeerIdStr; const signedBlock = ssz.deneb.SignedBeaconBlock.defaultValue(); @@ -201,10 +213,16 @@ async function runBeaconBlockRepeatProposal( } const processBlock = vi.fn().mockResolvedValue(undefined); + const onSignedBlockHeader = vi.fn(); const chain = { clock: new ClockStopped(1), custodyConfig: {sampledColumns: [], custodyColumns: []} as unknown as CustodyConfig, emitter: new ChainEventEmitter(), + forkChoice: { + onSignedBlockHeader, + isPtcTimely: vi.fn().mockReturnValue(true), + } as unknown as IBeaconChain["forkChoice"], + genesisTime: 0, getBlobsTracker: {triggerGetBlobs: vi.fn()}, logger, processBlock, @@ -257,7 +275,7 @@ async function runBeaconBlockRepeatProposal( await new Promise((resolve) => setTimeout(resolve, 0)); await new Promise((resolve) => setTimeout(resolve, 0)); - return {processBlock, threw}; + return {processBlock, threw, onSignedBlockHeader}; } function getExecutionBlockError( diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 60d8fc8d5f3a..16c5035f7a6c 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -89,6 +89,8 @@ export type UpdateAndGetHeadOpt = // the initial vote epoch for all validators const INIT_VOTE_SLOT: Slot = 0; +/** Two signed block roots establish a proposer equivocation, the first two seen are also the most timely */ +const MAX_SIGNED_PROPOSALS_PER_SLOT_PROPOSER = 2; /** * Provides an implementation of "Ethereum Consensus -- Beacon Chain Fork Choice": @@ -154,6 +156,13 @@ export class ForkChoice implements IForkChoice { private validatedAttestationDatas = new Set(); /** Boost the entire branch with this proposer root as the leaf */ private proposerBoostRoot: RootHex | null = null; + /** + * Block roots signed by a proposer for a slot with their PTC timeliness, imported or not. Two roots establish + * an equivocation and the first two seen are the most timely, so later ones are not kept. + */ + private readonly signedProposals = new MapDef>>( + () => new MapDef(() => new Map()) + ); /** Score to use in proposer boost, evaluated lazily from justified balances */ private justifiedProposerBoostScore: bigint | null = null; /** The current effective balances */ @@ -938,6 +947,7 @@ export class ForkChoice implements IForkChoice { }; this.protoArray.onBlock(protoBlock, currentSlot, proposerBoostRoot); + this.recordSignedProposal(slot, block.proposerIndex, blockRootHex, protoBlock.ptcTimeliness); if (isProposerBoostBlock) { this.proposerBoostRoot = blockRootHex; @@ -1065,6 +1075,19 @@ export class ForkChoice implements IForkChoice { } } + /** + * Record a block signed by `proposerIndex` for `slot` that was seen but not imported, e.g. a repeat proposal + * ignored on gossip. Imported blocks are recorded by onBlock. The spec reads proposer equivocations from + * store.blocks, ie. only valid blocks. Counting signed blocks instead keeps the view consistent across nodes + * that bound how many repeat proposals they validate. + */ + onSignedBlockHeader(slot: Slot, proposerIndex: ValidatorIndex, blockRoot: RootHex, ptcTimely: boolean): void { + if (slot < computeStartSlotAtEpoch(this.fcStore.finalizedCheckpoint.epoch)) { + return; + } + this.recordSignedProposal(slot, proposerIndex, blockRoot, ptcTimely); + } + /** * Small different from the spec: * We already call is_slashable_attestation_data() and is_valid_indexed_attestation @@ -1330,6 +1353,12 @@ export class ForkChoice implements IForkChoice { */ prune(finalizedRoot: RootHex): ProtoBlock[] { const prunedNodes = this.protoArray.maybePrune(finalizedRoot); + const finalizedSlot = computeStartSlotAtEpoch(this.fcStore.finalizedCheckpoint.epoch); + for (const slot of this.signedProposals.keys()) { + if (slot < finalizedSlot) { + this.signedProposals.delete(slot); + } + } const prunedCount = prunedNodes.length; for (let i = 0; i < this.voteNextSlots.length; i++) { const currentIndex = this.voteCurrentIndices[i]; @@ -1750,8 +1779,13 @@ export class ForkChoice implements IForkChoice { * Child class can overwrite this for testing purpose. */ protected isBlockPtcTimely(block: BeaconBlock, blockDelaySec: number): boolean { + return this.isPtcTimely(block.slot, blockDelaySec); + } + + /** Return true if a block for `slot` received `receiveDelaySec` after the slot start is PTC-timely */ + isPtcTimely(slot: Slot, receiveDelaySec: number): boolean { const ptcThresholdMs = this.config.getSlotComponentDurationMs(this.config.PAYLOAD_ATTESTATION_DUE_BPS); - return this.fcStore.currentSlot === block.slot && blockDelaySec * 1000 < ptcThresholdMs; + return this.fcStore.currentSlot === slot && receiveDelaySec * 1000 < ptcThresholdMs; } /** @@ -1802,7 +1836,7 @@ export class ForkChoice implements IForkChoice { // Parent is weak and from the previous slot: apply boost only if there are no early // equivocations, ie. no other PTC-timely block at the parent's slot from the same proposer. - return !this.protoArray.hasEquivocatingBlock( + return !this.hasEquivocatingBlock( parentBlock.proposerIndex, parentBlock.slot, parentBlock.blockRoot, @@ -1820,7 +1854,42 @@ export class ForkChoice implements IForkChoice { private isProposerEquivocation(block: ProtoBlock): boolean { // Any known sibling counts, timely or not. Timeliness only matters for withholding the boost from // the next proposer, the reorg itself is safe to attempt whenever the equivocation is visible. - return this.protoArray.hasEquivocatingBlock(block.proposerIndex, block.slot, block.blockRoot, false); + return this.hasEquivocatingBlock(block.proposerIndex, block.slot, block.blockRoot, false); + } + + private recordSignedProposal( + slot: Slot, + proposerIndex: ValidatorIndex, + blockRoot: RootHex, + ptcTimely: boolean + ): void { + const ptcTimelyByRoot = this.signedProposals.getOrDefault(slot).getOrDefault(proposerIndex); + if (ptcTimelyByRoot.size < MAX_SIGNED_PROPOSALS_PER_SLOT_PROPOSER && !ptcTimelyByRoot.has(blockRoot)) { + ptcTimelyByRoot.set(blockRoot, ptcTimely); + } + } + + /** + * Return true if a block other than `excludeRoot` at `slot` was signed by `proposerIndex`. + * `should_apply_proposer_boost` only counts PTC-timely blocks (`ptcTimelyOnly`), `is_proposer_equivocation` + * counts any known block. + */ + private hasEquivocatingBlock( + proposerIndex: ValidatorIndex, + slot: Slot, + excludeRoot: RootHex, + ptcTimelyOnly: boolean + ): boolean { + const ptcTimelyByRoot = this.signedProposals.get(slot)?.get(proposerIndex); + if (ptcTimelyByRoot === undefined) { + return false; + } + for (const [blockRoot, ptcTimely] of ptcTimelyByRoot) { + if (blockRoot !== excludeRoot && (ptcTimely || !ptcTimelyOnly)) { + return true; + } + } + return false; } /** diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index ee46b66ca803..ccd42adcceec 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -1,5 +1,14 @@ import {DataAvailabilityStatus, EffectiveBalanceIncrements, IBeaconStateView} from "@lodestar/state-transition"; -import {AttesterSlashing, BeaconBlock, Epoch, IndexedAttestation, Root, RootHex, Slot} from "@lodestar/types"; +import { + AttesterSlashing, + BeaconBlock, + Epoch, + IndexedAttestation, + Root, + RootHex, + Slot, + ValidatorIndex, +} from "@lodestar/types"; import { BlockExecutionStatus, LVHExecResponse, @@ -188,6 +197,13 @@ export interface IForkChoice { * https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.3/specs/phase0/fork-choice.md#on_attester_slashing */ onAttesterSlashing(slashing: AttesterSlashing): void; + /** + * Record a signed block that was seen but not imported, e.g. a repeat proposal ignored on gossip, so it counts + * as a proposer equivocation for `should_apply_proposer_boost` and `get_proposer_head` + */ + onSignedBlockHeader(slot: Slot, proposerIndex: ValidatorIndex, blockRoot: RootHex, ptcTimely: boolean): void; + /** Return true if a block for `slot` received `receiveDelaySec` after the slot start is PTC-timely */ + isPtcTimely(slot: Slot, receiveDelaySec: number): boolean; /** * Process PTC (Payload Timeliness Committee) messages from a block * Updates the PTC votes for the attested beacon block diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index d29323e23bd7..bdad633e9f33 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1,7 +1,7 @@ import {BitArray} from "@chainsafe/ssz"; import {EFFECTIVE_BALANCE_INCREMENT, GENESIS_EPOCH, GENESIS_SLOT, PTC_SIZE} from "@lodestar/params"; import {DataAvailabilityStatus, computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; -import {Epoch, RootHex, Slot, ValidatorIndex} from "@lodestar/types"; +import {Epoch, RootHex, Slot} from "@lodestar/types"; import {bitCount, toRootHex} from "@lodestar/utils"; import {ForkChoiceError, ForkChoiceErrorCode} from "../forkChoice/errors.js"; import {LVHExecError, LVHExecErrorCode, ProtoArrayError, ProtoArrayErrorCode} from "./errors.js"; @@ -2007,41 +2007,6 @@ export class ProtoArray { return this.getNodeByIndex(nodeIndex); } - /** - * Return true if a block other than `excludeRoot` at `slot` was proposed by `proposerIndex`. - * `should_apply_proposer_boost` only counts PTC-timely blocks (`ptcTimelyOnly`), `is_proposer_equivocation` - * counts any known block. - * - * Iterates unique block roots (via the canonical variant) since `slot`, `proposerIndex` and - * `ptcTimeliness` are block-level properties identical across payload-status variants. - */ - hasEquivocatingBlock( - proposerIndex: ValidatorIndex, - slot: Slot, - excludeRoot: RootHex, - ptcTimelyOnly: boolean - ): boolean { - for (const root of this.indices.keys()) { - if (root === excludeRoot) { - continue; - } - const nodeIndex = this.getDefaultNodeIndex(root); - if (nodeIndex === undefined) { - continue; - } - const node = this.nodes[nodeIndex]; - if ( - node !== undefined && - node.slot === slot && - node.proposerIndex === proposerIndex && - (node.ptcTimeliness || !ptcTimelyOnly) - ) { - return true; - } - } - return false; - } - /** * Return MUTABLE ProtoBlock for blockRoot with explicit payload status * diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts index e7f102735023..1c09d043d6d5 100644 --- a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -214,6 +214,8 @@ describe("Forkchoice / GetProposerHead", () => { headBlock: ProtoBlockWithWeight; /** Imported alongside the head, to simulate an equivocation */ siblingBlock?: ProtoBlockWithWeight; + /** false when the sibling was only seen on gossip, ie. recorded in fork choice without being imported */ + siblingImported?: boolean; expectReorg: boolean; currentSlot?: Slot; secFromSlot?: number; @@ -315,6 +317,14 @@ describe("Forkchoice / GetProposerHead", () => { siblingBlock: equivocatingHeadBlock, expectReorg: true, }, + { + id: "Reorg weak equivocating head when the equivocating block was seen but not imported", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, timeliness: true}, + siblingBlock: equivocatingHeadBlock, + siblingImported: false, + expectReorg: true, + }, { id: "Reorg weak equivocating head even if parent is weak", parentBlock: {...baseParentHeadBlock, weight: 211}, @@ -373,6 +383,7 @@ describe("Forkchoice / GetProposerHead", () => { parentBlock, headBlock, siblingBlock, + siblingImported, expectReorg, currentSlot: proposalSlot, secFromSlot, @@ -382,14 +393,19 @@ describe("Forkchoice / GetProposerHead", () => { it(`${id}`, async () => { protoArr.onBlock(parentBlock, parentBlock.slot, null); protoArr.onBlock(headBlock, headBlock.slot, null); - if (siblingBlock) { + if (siblingBlock && siblingImported !== false) { protoArr.onBlock(siblingBlock, siblingBlock.slot, null); } const currentSlot = proposalSlot ?? headBlock.slot + 1; const currentSecFromSlot = secFromSlot ?? 0; protoArr.applyScoreChanges({ - attestationDeltas: [0, parentBlock.weight, headBlock.weight, ...(siblingBlock ? [siblingBlock.weight] : [])], + attestationDeltas: [ + 0, + parentBlock.weight, + headBlock.weight, + ...(siblingBlock && siblingImported !== false ? [siblingBlock.weight] : []), + ], proposerBoost: null, justifiedEpoch: genesisEpoch, justifiedRoot: genesisRoot, @@ -402,6 +418,14 @@ describe("Forkchoice / GetProposerHead", () => { proposerBoost: true, proposerBoostReorg: true, }); + if (siblingBlock) { + forkChoice.onSignedBlockHeader( + siblingBlock.slot, + siblingBlock.proposerIndex, + siblingBlock.blockRoot, + siblingBlock.ptcTimeliness + ); + } const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead( headBlock, diff --git a/packages/fork-choice/test/unit/forkChoice/shouldApplyProposerBoost.test.ts b/packages/fork-choice/test/unit/forkChoice/shouldApplyProposerBoost.test.ts index 99342fe903f4..b9f7d69eff03 100644 --- a/packages/fork-choice/test/unit/forkChoice/shouldApplyProposerBoost.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/shouldApplyProposerBoost.test.ts @@ -61,7 +61,7 @@ function setup({ store = makeStore(), childSlot = headSlot, }: { - sibling?: {ptcTimeliness: boolean; proposerIndex?: ValidatorIndex} | null; + sibling?: {ptcTimeliness: boolean; proposerIndex?: ValidatorIndex; imported?: boolean} | null; parentVotes?: number; store?: IForkChoiceStore; childSlot?: Slot; @@ -72,7 +72,7 @@ function setup({ const protoArray = ProtoArray.initialize(toProtoBlock(genesisSlot, genesisRoot, false), genesisSlot); protoArray.onBlock(toProtoBlock(parentSlot, genesisRoot, true, {proposerIndex: PARENT_PROPOSER}), parentSlot, null); - if (sibling) { + if (sibling && sibling.imported !== false) { protoArray.onBlock( toProtoBlock(parentSlot, genesisRoot, true, { blockRoot: SIBLING_ROOT, @@ -91,6 +91,14 @@ function setup({ ); const forkChoice = new ForkChoice(gloasConfig, store, protoArray, VALIDATOR_COUNT, null, {proposerBoost: true}); + if (sibling) { + forkChoice.onSignedBlockHeader( + parentSlot, + sibling.proposerIndex ?? PARENT_PROPOSER, + SIBLING_ROOT, + sibling.ptcTimeliness + ); + } if (parentVotes > 0) { protoArray.applyScoreChanges({ @@ -136,6 +144,22 @@ describe("Forkchoice / shouldApplyProposerBoost", () => { expect(appliedBoost(protoArray, childRoot)).toBe(BOOST_SCORE); }); + it("withholds boost when the PTC-timely equivocating sibling was seen but not imported", () => { + const {forkChoice, protoArray, childRoot} = setup({sibling: {ptcTimeliness: true, imported: false}}); + + forkChoice.updateHead(); + + expect(appliedBoost(protoArray, childRoot)).toBe(0n); + }); + + it("applies boost when the equivocating sibling seen but not imported is not PTC-timely", () => { + const {forkChoice, protoArray, childRoot} = setup({sibling: {ptcTimeliness: false, imported: false}}); + + forkChoice.updateHead(); + + expect(appliedBoost(protoArray, childRoot)).toBe(BOOST_SCORE); + }); + it("applies boost when the sibling was proposed by a different proposer", () => { const {forkChoice, protoArray, childRoot} = setup({ sibling: {ptcTimeliness: true, proposerIndex: PARENT_PROPOSER + 1}, diff --git a/packages/fork-choice/test/unit/forkChoice/shouldOverrideForkChoiceUpdate.test.ts b/packages/fork-choice/test/unit/forkChoice/shouldOverrideForkChoiceUpdate.test.ts index f6583437944b..3a88b352d0ec 100644 --- a/packages/fork-choice/test/unit/forkChoice/shouldOverrideForkChoiceUpdate.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/shouldOverrideForkChoiceUpdate.test.ts @@ -334,6 +334,14 @@ describe("Forkchoice / shouldOverrideForkChoiceUpdate", () => { proposerBoost: true, proposerBoostReorg: true, }); + if (siblingBlock) { + forkChoice.onSignedBlockHeader( + siblingBlock.slot, + siblingBlock.proposerIndex, + siblingBlock.blockRoot, + siblingBlock.ptcTimeliness + ); + } const result = forkChoice.shouldOverrideForkChoiceUpdate(headBlock, secFromSlot, currentSlot);