From 992ba3ba1e7e37ce031de5b6cd18cd4c15e35856 Mon Sep 17 00:00:00 2001 From: NC Date: Fri, 22 Mar 2024 15:23:51 +0800 Subject: [PATCH 01/11] Init --- packages/api/src/beacon/routes/debug.ts | 1 + .../api/test/unit/beacon/testData/debug.ts | 1 + .../opPools/aggregatedAttestationPool.test.ts | 5 +- packages/beacon-node/test/utils/state.ts | 2 + .../beacon-node/test/utils/typeGenerator.ts | 2 + .../test/utils/validationData/attestation.ts | 2 + .../config/src/chainConfig/configs/mainnet.ts | 3 + .../config/src/chainConfig/configs/minimal.ts | 3 + packages/config/src/chainConfig/types.ts | 6 + .../fork-choice/src/forkChoice/forkChoice.ts | 223 +++++++++++++++++- .../fork-choice/src/forkChoice/interface.ts | 2 + .../fork-choice/src/protoArray/interface.ts | 3 + .../fork-choice/test/perf/forkChoice/util.ts | 2 + .../test/unit/forkChoice/forkChoice.test.ts | 6 +- .../unit/forkChoice/getProposerHead.test.ts | 218 +++++++++++++++++ .../protoArray/executionStatusUpdates.test.ts | 2 + .../unit/protoArray/getCommonAncestor.test.ts | 4 + .../test/unit/protoArray/protoArray.test.ts | 6 + packages/validator/src/util/params.ts | 3 + 19 files changed, 481 insertions(+), 13 deletions(-) create mode 100644 packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts diff --git a/packages/api/src/beacon/routes/debug.ts b/packages/api/src/beacon/routes/debug.ts index 84eed0af04c9..99403e61fe58 100644 --- a/packages/api/src/beacon/routes/debug.ts +++ b/packages/api/src/beacon/routes/debug.ts @@ -34,6 +34,7 @@ const protoNodeSszType = new ContainerType( parentRoot: stringType, stateRoot: stringType, targetRoot: stringType, + timeliness: ssz.Boolean, justifiedEpoch: ssz.Epoch, justifiedRoot: stringType, finalizedEpoch: ssz.Epoch, diff --git a/packages/api/test/unit/beacon/testData/debug.ts b/packages/api/test/unit/beacon/testData/debug.ts index 6b65d610d16f..3ceda4574605 100644 --- a/packages/api/test/unit/beacon/testData/debug.ts +++ b/packages/api/test/unit/beacon/testData/debug.ts @@ -40,6 +40,7 @@ export const testData: GenericServerTestCases = { weight: 1, bestChild: "1", bestDescendant: "1", + timeliness: false, }, ], }, 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 a9ce54e9d3f4..e47ce48f57c1 100644 --- a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -64,6 +64,8 @@ describe(`getAttestationsForBlock vc=${vc}`, () => { unrealizedFinalizedRoot: toHexString(finalizedCheckpoint.root), executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, }, originalState.slot ); @@ -87,6 +89,7 @@ describe(`getAttestationsForBlock vc=${vc}`, () => { unrealizedFinalizedRoot: toHexString(finalizedCheckpoint.root), executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge, + timeliness: false, }, slot ); @@ -159,7 +162,7 @@ describe(`getAttestationsForBlock vc=${vc}`, () => { return {state, pool}; }, fn: ({state, pool}) => { - pool.getAttestationsForBlock(state.config.getForkName(state.slot), forkchoice, state); + pool.getAttestationsForBlock(forkchoice, state); }, }); } diff --git a/packages/beacon-node/test/utils/state.ts b/packages/beacon-node/test/utils/state.ts index a0fa42be555e..c9dc79d34ed0 100644 --- a/packages/beacon-node/test/utils/state.ts +++ b/packages/beacon-node/test/utils/state.ts @@ -153,5 +153,7 @@ export const zeroProtoBlock: ProtoBlock = { unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: ZERO_HASH_HEX, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }; diff --git a/packages/beacon-node/test/utils/typeGenerator.ts b/packages/beacon-node/test/utils/typeGenerator.ts index cdaccd005c8e..90986a60eb4b 100644 --- a/packages/beacon-node/test/utils/typeGenerator.ts +++ b/packages/beacon-node/test/utils/typeGenerator.ts @@ -40,6 +40,8 @@ export function generateProtoBlock(overrides: Partial = {}): ProtoBl unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: ZERO_HASH_HEX, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, ...overrides, diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index fa3c4d479ade..24e599852d86 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -75,6 +75,8 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: ZERO_HASH_HEX, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }; diff --git a/packages/config/src/chainConfig/configs/mainnet.ts b/packages/config/src/chainConfig/configs/mainnet.ts index 9d060330d201..883688ca821b 100644 --- a/packages/config/src/chainConfig/configs/mainnet.ts +++ b/packages/config/src/chainConfig/configs/mainnet.ts @@ -81,6 +81,9 @@ export const chainConfig: ChainConfig = { // --------------------------------------------------------------- // 40% PROPOSER_SCORE_BOOST: 40, + REORG_HEAD_WEIGHT_THRESHOLD: 20, + REORG_PARENT_WEIGHT_THRESHOLD: 160, + REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2, // Deposit contract // --------------------------------------------------------------- diff --git a/packages/config/src/chainConfig/configs/minimal.ts b/packages/config/src/chainConfig/configs/minimal.ts index 6c0a13d8abb2..23cd14e763ec 100644 --- a/packages/config/src/chainConfig/configs/minimal.ts +++ b/packages/config/src/chainConfig/configs/minimal.ts @@ -78,6 +78,9 @@ export const chainConfig: ChainConfig = { // --------------------------------------------------------------- // 40% PROPOSER_SCORE_BOOST: 40, + REORG_HEAD_WEIGHT_THRESHOLD: 20, + REORG_PARENT_WEIGHT_THRESHOLD: 160, + REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2, // Deposit contract // --------------------------------------------------------------- diff --git a/packages/config/src/chainConfig/types.ts b/packages/config/src/chainConfig/types.ts index 3e0844118290..657c8a6c14b4 100644 --- a/packages/config/src/chainConfig/types.ts +++ b/packages/config/src/chainConfig/types.ts @@ -58,6 +58,9 @@ export type ChainConfig = { // Fork choice PROPOSER_SCORE_BOOST: number; + REORG_HEAD_WEIGHT_THRESHOLD: number; + REORG_PARENT_WEIGHT_THRESHOLD: number; + REORG_MAX_EPOCHS_SINCE_FINALIZATION: number; // Deposit contract DEPOSIT_CHAIN_ID: number; @@ -114,6 +117,9 @@ export const chainConfigTypes: SpecTypes = { // Fork choice PROPOSER_SCORE_BOOST: "number", + REORG_HEAD_WEIGHT_THRESHOLD: "number", + REORG_PARENT_WEIGHT_THRESHOLD: "number", + REORG_MAX_EPOCHS_SINCE_FINALIZATION: "number", // Deposit contract DEPOSIT_CHAIN_ID: "number", diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 374bc65542ee..2b5f45b76fb6 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -1,5 +1,5 @@ import {toHexString} from "@chainsafe/ssz"; -import {fromHex} from "@lodestar/utils"; +import {Logger, fromHex} from "@lodestar/utils"; import {SLOTS_PER_HISTORICAL_ROOT, SLOTS_PER_EPOCH, INTERVALS_PER_SLOT} from "@lodestar/params"; import {bellatrix, Slot, ValidatorIndex, phase0, allForks, ssz, RootHex, Epoch, Root} from "@lodestar/types"; import { @@ -45,9 +45,21 @@ import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalan export type ForkChoiceOpts = { proposerBoostEnabled?: boolean; + proposerBoostReorgEnabled?: boolean; computeUnrealized?: boolean; }; +export enum UpdateHeadOpt { + GetCanonicialHead, // Skip getProposerHead + GetProposerHead, // With getProposerHead + GetPredictedProposerHead, // With predictProposerHead +} + +export type UpdateAndGetHeadOpt = + | {mode: UpdateHeadOpt.GetCanonicialHead} + | {mode: UpdateHeadOpt.GetProposerHead; slot: Slot} + | {mode: UpdateHeadOpt.GetPredictedProposerHead; slot: Slot}; + /** * Provides an implementation of "Ethereum Consensus -- Beacon Chain Fork Choice": * @@ -107,6 +119,7 @@ export class ForkChoice implements IForkChoice { private readonly fcStore: IForkChoiceStore, /** The underlying representation of the block DAG. */ private readonly protoArray: ProtoArray, + private readonly logger?: Logger, private readonly opts?: ForkChoiceOpts ) { this.head = this.updateHead(); @@ -154,6 +167,29 @@ export class ForkChoice implements IForkChoice { return this.head; } + /** + * + * A multiplexer to wrap around the traditional `updateHead()` according to the scenario + * Scenarios as follow: + * Prepare to propose in the next slot: getHead() -> predictProposerHead() + * Proposing in the current slot: updateHead() -> getProposerHead() + * Others eg. initializing forkchoice, importBlock: updateHead() + */ + updateAndGetHead(opt: UpdateAndGetHeadOpt): ProtoBlock { + const {mode} = opt; + + const canonicialHeadBlock = mode === UpdateHeadOpt.GetPredictedProposerHead ? this.getHead() : this.updateHead(); + switch (mode) { + case UpdateHeadOpt.GetPredictedProposerHead: + return this.predictProposerHead(canonicialHeadBlock, opt.slot); + case UpdateHeadOpt.GetProposerHead: + return this.getProposerHead(canonicialHeadBlock, opt.slot); + case UpdateHeadOpt.GetCanonicialHead: + default: + return canonicialHeadBlock; + } + } + /** * Get the proposer boost root */ @@ -161,6 +197,119 @@ export class ForkChoice implements IForkChoice { return this.proposerBoostRoot ?? HEX_ZERO_HASH; } + /** + * To predict the proposer head of the next slot. That is, to predict if proposer-boost-reorg could happen. + * Reason why we can't be certain is because information of the head block is not fully available yet + * since the current slot hasn't ended especially the attesters' votes. + * + * There is a chance we mispredict. + * + * By calling this function, we assume we are the proposer of next slot + * + * https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/bellatrix/fork-choice.md#should_override_forkchoice_update + */ + predictProposerHead(headBlock: ProtoBlock, currentSlot?: Slot): ProtoBlock { + // Skip re-org attempt if proposer boost (reorg) are disabled + if (!this.opts?.proposerBoostEnabled || !this.opts?.proposerBoostReorgEnabled) { + this.logger?.verbose("No proposer boot reorg prediction since the related flags are disabled"); + return headBlock; + } + + const parentBlock = this.protoArray.getBlock(headBlock.parentRoot); + const proposalSlot = headBlock.slot + 1; + currentSlot = currentSlot ?? this.fcStore.currentSlot; + + // No reorg if parentBlock isn't available + if (parentBlock === undefined) { + return headBlock; + } + + const prelimProposerHeadBlock = this.getPreliminaryProposerHead(headBlock, parentBlock, proposalSlot); + + if (prelimProposerHeadBlock === headBlock) { + return headBlock; + } + + const currentTimeOk = headBlock.slot === currentSlot; + if (!currentTimeOk) { + return headBlock; + } + + this.logger?.info("Current head is weak. Predicting next block to be built on parent of head"); + return parentBlock; + } + + /** + * + * This function takes in the canonical head block and determine the proposer head (canonical head block or its parent) + * https://github.com/ethereum/consensus-specs/pull/3034 for info about proposer boost reorg + * This function should only be called during block proposal and only be called after `updateHead()` in `updateAndGetHead()` + * + * Same as https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#get_proposer_head + */ + getProposerHead(headBlock: ProtoBlock, slot: Slot): ProtoBlock { + // Skip re-org attempt if proposer boost (reorg) are disabled + if (!this.opts?.proposerBoostEnabled || !this.opts?.proposerBoostReorgEnabled) { + this.logger?.verbose("No proposer boot reorg attempt since the related flags are disabled"); + return headBlock; + } + + const parentBlock = this.protoArray.getBlock(headBlock.parentRoot); + + // No reorg if parentBlock isn't available + if (parentBlock === undefined) { + return headBlock; + } + + const prelimProposerHeadBlock = this.getPreliminaryProposerHead(headBlock, parentBlock, slot); + + if (prelimProposerHeadBlock === headBlock) { + return headBlock; + } + + // No reorg if attempted reorg is more than a single slot + // Half of single_slot_reorg check in the spec is done in getPreliminaryProposerHead() + const currentTimeOk = headBlock.slot + 1 === slot; + if (!currentTimeOk) { + return headBlock; + } + + // No reorg if proposer boost is still in effect + const isProposerBoostWornOff = this.proposerBoostRoot !== headBlock.blockRoot; + if (!isProposerBoostWornOff) { + return headBlock; + } + + // No reorg if headBlock is "not weak" ie. headBlock's weight exceeds (REORG_HEAD_WEIGHT_THRESHOLD = 20)% of total attester weight + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_head_weak + const reorgThreshold = getCommitteeFraction(this.fcStore.justified.totalBalance, { + slotsPerEpoch: SLOTS_PER_EPOCH, + committeePercent: this.config.REORG_HEAD_WEIGHT_THRESHOLD, + }); + const headNode = this.protoArray.getNode(headBlock.blockRoot); + // If headNode is unavailable, give up reorg + if (headNode === undefined || headNode.weight >= reorgThreshold) { + return headBlock; + } + + // No reorg if parentBlock is "not strong" ie. parentBlock's weight is less than or equal to (REORG_PARENT_WEIGHT_THRESHOLD = 160)% of total attester weight + // https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#is_parent_strong + const parentThreshold = getCommitteeFraction(this.fcStore.justified.totalBalance, { + slotsPerEpoch: SLOTS_PER_EPOCH, + committeePercent: this.config.REORG_PARENT_WEIGHT_THRESHOLD, + }); + const parentNode = this.protoArray.getNode(parentBlock.blockRoot); + // If parentNode is unavailable, give up reorg + if (parentNode === undefined || parentNode.weight <= parentThreshold) { + return headBlock; + } + + // Reorg if all above checks fail + this.logger?.info("Will perform single-slot reorg to reorg out current weak head"); + + return parentBlock; + } + /** * Run the fork choice rule to determine the head. * Update the head cache. @@ -201,9 +350,9 @@ export class ForkChoice implements IForkChoice { if (this.opts?.proposerBoostEnabled && this.proposerBoostRoot) { const proposerBoostScore = this.justifiedProposerBoostScore ?? - getProposerScore(this.fcStore.justified.totalBalance, { + getCommitteeFraction(this.fcStore.justified.totalBalance, { slotsPerEpoch: SLOTS_PER_EPOCH, - proposerScoreBoost: this.config.PROPOSER_SCORE_BOOST, + committeePercent: this.config.PROPOSER_SCORE_BOOST, }); proposerBoost = {root: this.proposerBoostRoot, score: proposerBoostScore}; this.justifiedProposerBoostScore = proposerBoostScore; @@ -352,12 +501,13 @@ export class ForkChoice implements IForkChoice { const blockRoot = this.config.getForkTypes(slot).BeaconBlock.hashTreeRoot(block); const blockRootHex = toHexString(blockRoot); - // Add proposer score boost if the block is timely + // Assign proposer score boost if the block is timely // before attesting interval = before 1st interval + const isBeforeAttestingInterval = blockDelaySec < this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT; + const isTimely = this.fcStore.currentSlot === slot && isBeforeAttestingInterval; if ( this.opts?.proposerBoostEnabled && - this.fcStore.currentSlot === slot && - blockDelaySec < this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT && + isTimely && // only boost the first block we see this.proposerBoostRoot === null ) { @@ -451,6 +601,7 @@ export class ForkChoice implements IForkChoice { parentRoot: parentRootHex, targetRoot: toHexString(targetRoot), stateRoot: toHexString(block.stateRoot), + timeliness: isTimely, justifiedEpoch: stateJustifiedEpoch, justifiedRoot: toHexString(state.currentJustifiedCheckpoint.root), @@ -1202,6 +1353,57 @@ export class ForkChoice implements IForkChoice { () => this.fcStore.unrealizedJustified.balances ); } + + /** + * + * Common logic of get_proposer_head() and should_override_forkchoice_update() + * No one should be calling this function except these two + * + */ + private getPreliminaryProposerHead(headBlock: ProtoBlock, parentBlock: ProtoBlock, slot: Slot): ProtoBlock { + // No reorg if headBlock is on time + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_head_late + const isHeadLate = !headBlock.timeliness; + if (!isHeadLate) { + return headBlock; + } + + // No reorg if we are at epoch boundary where proposer shuffling could change + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_shuffling_stable + const isShufflingStable = slot % SLOTS_PER_EPOCH !== 0; + if (!isShufflingStable) { + return headBlock; + } + + // No reorg if headBlock and parentBlock are not ffg competitive + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_ffg_competitive + const {unrealizedJustifiedEpoch: headBlockCpEpoch, unrealizedJustifiedRoot: headBlockCpRoot} = headBlock; + const {unrealizedJustifiedEpoch: parentBlockCpEpoch, unrealizedJustifiedRoot: parentBlockCpRoot} = parentBlock; + const isFFGCompetitive = headBlockCpEpoch === parentBlockCpEpoch && headBlockCpRoot === parentBlockCpRoot; + if (!isFFGCompetitive) { + return headBlock; + } + + // No reorg if chain is not finalizing within REORG_MAX_EPOCHS_SINCE_FINALIZATION + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_finalization_ok + const epochsSinceFinalization = computeEpochAtSlot(slot) - this.getFinalizedCheckpoint().epoch; + const isFinalizationOk = epochsSinceFinalization <= this.config.REORG_MAX_EPOCHS_SINCE_FINALIZATION; + if (!isFinalizationOk) { + return headBlock; + } + + // -No reorg if we are not proposing on time.- + // Note: Skipping this check as store.time in Lodestar is stored in slot and not unix time + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_proposing_on_time + + // No reorg if this reorg spans more than a single slot + const parentSlotOk = parentBlock.slot + 1 === headBlock.slot; + if (!parentSlotOk) { + return headBlock; + } + + return parentBlock; + } } /** @@ -1261,11 +1463,12 @@ export function assertValidTerminalPowBlock( } } } - -export function getProposerScore( +// Approximate https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#calculate_committee_fraction +// Calculates proposer boost score when committeePercent = config.PROPOSER_SCORE_BOOST +export function getCommitteeFraction( justifiedTotalActiveBalanceByIncrement: number, - config: {slotsPerEpoch: number; proposerScoreBoost: number} + config: {slotsPerEpoch: number; committeePercent: number} ): number { const committeeWeight = Math.floor(justifiedTotalActiveBalanceByIncrement / config.slotsPerEpoch); - return Math.floor((committeeWeight * config.proposerScoreBoost) / 100); + return Math.floor((committeeWeight * config.committeePercent) / 100); } diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index fffbc3e4007f..b40e37f7568c 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -3,6 +3,7 @@ import {CachedBeaconStateAllForks} from "@lodestar/state-transition"; import {Epoch, Slot, ValidatorIndex, phase0, allForks, Root, RootHex} from "@lodestar/types"; import {ProtoBlock, MaybeValidExecutionStatus, LVHExecResponse, ProtoNode} from "../protoArray/interface.js"; import {CheckpointWithHex} from "./store.js"; +import {UpdateAndGetHeadOpt} from "./forkChoice.js"; export type CheckpointHex = { epoch: Epoch; @@ -77,6 +78,7 @@ export interface IForkChoice { getHeadRoot(): RootHex; getHead(): ProtoBlock; updateHead(): ProtoBlock; + updateAndGetHead(mode: UpdateAndGetHeadOpt): ProtoBlock; /** * Retrieves all possible chain heads (leaves of fork choice tree). */ diff --git a/packages/fork-choice/src/protoArray/interface.ts b/packages/fork-choice/src/protoArray/interface.ts index 003a3c8f9f1e..e59c366cbf59 100644 --- a/packages/fork-choice/src/protoArray/interface.ts +++ b/packages/fork-choice/src/protoArray/interface.ts @@ -78,6 +78,9 @@ export type ProtoBlock = BlockExecution & { unrealizedJustifiedRoot: RootHex; unrealizedFinalizedEpoch: Epoch; unrealizedFinalizedRoot: RootHex; + + // Indicate whether block arrives in a timely manner ie. before the 4 second mark + timeliness: boolean; }; /** diff --git a/packages/fork-choice/test/perf/forkChoice/util.ts b/packages/fork-choice/test/perf/forkChoice/util.ts index 1ad97d5b6c54..eace72d4d800 100644 --- a/packages/fork-choice/test/perf/forkChoice/util.ts +++ b/packages/fork-choice/test/perf/forkChoice/util.ts @@ -75,6 +75,8 @@ export function initializeForkChoice(opts: Opts): ForkChoice { executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, }; protoArr.onBlock(block, block.slot); diff --git a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts index 93aa28f8f2e5..fb72a6705c37 100644 --- a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts @@ -101,6 +101,8 @@ describe("Forkchoice", function () { executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, }; }; @@ -170,14 +172,14 @@ describe("Forkchoice", function () { // TODO: more unit tests for other apis }); -function getStateRoot(slot: number): RootHex { +export function getStateRoot(slot: number): RootHex { const root = Buffer.alloc(32, 0x00); root[0] = rootStateBytePrefix; root[31] = slot; return toHex(root); } -function getBlockRoot(slot: number): RootHex { +export function getBlockRoot(slot: number): RootHex { const root = Buffer.alloc(32, 0x00); root[0] = rootBlockBytePrefix; root[31] = slot; diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts new file mode 100644 index 000000000000..40108bd748c3 --- /dev/null +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -0,0 +1,218 @@ +import {describe, it, expect, beforeEach} from "vitest"; +import {fromHexString} from "@chainsafe/ssz"; +import {config} from "@lodestar/config/default"; +import {Slot} from "@lodestar/types"; +import {toHex} from "@lodestar/utils"; +import {SLOTS_PER_EPOCH} from "@lodestar/params"; +import {ForkChoice, IForkChoiceStore, ProtoArray, ExecutionStatus, ProtoBlock} from "../../../src/index.js"; +import {getBlockRoot, getStateRoot} from "./forkChoice.test.js"; + +type ProtoBlockWithWeight = ProtoBlock & {weight: number}; // weight of the block itself + +describe("Forkchoice / GetProposerHead", function () { + const genesisSlot = 0; + const genesisEpoch = 0; + const genesisRoot = "0x0000000000000000000000000000000000000000000000000000000000000000"; + + const parentSlot = genesisSlot + 1; + const headSlot = genesisSlot + 2; + + let protoArr: ProtoArray; + + const genesisBlock: Omit = { + slot: genesisSlot, + stateRoot: getStateRoot(genesisSlot), + parentRoot: toHex(Buffer.alloc(32, 0xff)), + blockRoot: getBlockRoot(genesisSlot), + + justifiedEpoch: genesisEpoch, + justifiedRoot: genesisRoot, + finalizedEpoch: genesisEpoch, + finalizedRoot: genesisRoot, + unrealizedJustifiedEpoch: genesisEpoch, + unrealizedJustifiedRoot: genesisRoot, + unrealizedFinalizedEpoch: genesisEpoch, + unrealizedFinalizedRoot: genesisRoot, + + executionPayloadBlockHash: null, + executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, + }; + + const baseHeadBlock: ProtoBlockWithWeight = { + slot: headSlot, + stateRoot: getStateRoot(headSlot), + parentRoot: getBlockRoot(parentSlot), + blockRoot: getBlockRoot(headSlot), + targetRoot: getBlockRoot(headSlot), + + justifiedEpoch: genesisEpoch, + justifiedRoot: genesisRoot, + finalizedEpoch: genesisEpoch, + finalizedRoot: genesisRoot, + unrealizedJustifiedEpoch: genesisEpoch, + unrealizedJustifiedRoot: genesisRoot, + unrealizedFinalizedEpoch: genesisEpoch, + unrealizedFinalizedRoot: genesisRoot, + + executionPayloadBlockHash: null, + executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, + + weight: 29, + }; + + const baseParentHeadBlock: ProtoBlockWithWeight = { + slot: parentSlot, + stateRoot: getStateRoot(parentSlot), + parentRoot: getBlockRoot(genesisSlot), + blockRoot: getBlockRoot(parentSlot), + targetRoot: getBlockRoot(parentSlot), + + justifiedEpoch: genesisEpoch, + justifiedRoot: genesisRoot, + finalizedEpoch: genesisEpoch, + finalizedRoot: genesisRoot, + unrealizedJustifiedEpoch: genesisEpoch, + unrealizedJustifiedRoot: genesisRoot, + unrealizedFinalizedEpoch: genesisEpoch, + unrealizedFinalizedRoot: genesisRoot, + + executionPayloadBlockHash: null, + executionStatus: ExecutionStatus.PreMerge, + + timeliness: false, + weight: 212, // 240 - 29 + 1 + }; + + const fcStore: IForkChoiceStore = { + currentSlot: genesisSlot + 1, + justified: { + checkpoint: {epoch: genesisEpoch, root: fromHexString(genesisBlock.blockRoot), rootHex: genesisBlock.blockRoot}, + balances: new Uint8Array(Array(32).fill(150)), + totalBalance: 32 * 150, + }, + unrealizedJustified: { + checkpoint: {epoch: genesisEpoch, root: fromHexString(genesisBlock.blockRoot), rootHex: genesisBlock.blockRoot}, + balances: new Uint8Array(Array(32).fill(150)), + }, + finalizedCheckpoint: { + epoch: genesisEpoch, + root: fromHexString(genesisBlock.blockRoot), + rootHex: genesisBlock.blockRoot, + }, + unrealizedFinalizedCheckpoint: { + epoch: genesisEpoch, + root: fromHexString(genesisBlock.blockRoot), + rootHex: genesisBlock.blockRoot, + }, + justifiedBalancesGetter: () => new Uint8Array(Array(32).fill(150)), + equivocatingIndices: new Set(), + }; + + // head block's weight < 30 is considered weak. parent block's total weight > 240 is considered strong + const testCases: { + id: string; + parentBlock: ProtoBlockWithWeight; + headBlock: ProtoBlockWithWeight; + expectReorg: boolean; + currentSlot?: Slot; + }[] = [ + { + id: "Case that meets all conditions to be re-orged", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock}, + expectReorg: true, + }, + { + id: "No reorg when head block is timly", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, timeliness: true}, + expectReorg: false, + }, + { + id: "No reorg when currenSlot is at epoch boundary", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock}, + expectReorg: false, + currentSlot: SLOTS_PER_EPOCH * 2, + }, + { + id: "No reorg when the blocks are not ffg competitive", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, unrealizedJustifiedEpoch: 1}, + expectReorg: false, + }, + { + id: "No reorg when the blocks are not ffg competitive 2", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, unrealizedJustifiedRoot: "-"}, + expectReorg: false, + }, + { + id: "No reorg if long unfinality", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock}, + expectReorg: false, + currentSlot: (genesisEpoch + 2) * SLOTS_PER_EPOCH + 1, + }, + { + id: "No reorg if reorg spans more than a single slot", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, slot: headSlot + 1}, + expectReorg: false, + }, + { + id: "No reorg if current slot is more than one slot from head block", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock}, + expectReorg: false, + currentSlot: headSlot + 2, + }, + { + id: "No reorg if head is strong", + parentBlock: {...baseParentHeadBlock}, + headBlock: {...baseHeadBlock, weight: 30}, + expectReorg: false, + }, + { + id: "No reorg if parent is weak", + parentBlock: {...baseParentHeadBlock, weight: 211}, + headBlock: {...baseHeadBlock}, + expectReorg: false, + }, + ]; + + beforeEach(() => { + protoArr = ProtoArray.initialize(genesisBlock, genesisSlot); + }); + + for (const {id, parentBlock, headBlock, expectReorg, currentSlot: proposalSlot} of testCases) { + it(`${id}`, async () => { + protoArr.onBlock(parentBlock, parentBlock.slot); + protoArr.onBlock(headBlock, headBlock.slot); + + const currentSlot = proposalSlot ?? headBlock.slot + 1; + protoArr.applyScoreChanges({ + deltas: [0, parentBlock.weight, headBlock.weight], + proposerBoost: null, + justifiedEpoch: genesisEpoch, + justifiedRoot: genesisRoot, + finalizedEpoch: genesisEpoch, + finalizedRoot: genesisRoot, + currentSlot, + }); + + const forkChoice = new ForkChoice(config, fcStore, protoArr, undefined, { + proposerBoostEnabled: true, + proposerBoostReorgEnabled: true, + }); + + const proposerHead = forkChoice.getProposerHead(headBlock, currentSlot); + + expect(proposerHead.blockRoot).toBe(expectReorg ? parentBlock.blockRoot : headBlock.blockRoot); + }); + } +}); diff --git a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts index e6916f24800f..9d007f1c27e9 100644 --- a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts +++ b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts @@ -106,6 +106,8 @@ function setupForkChoice(): ProtoArray { unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: "-", + timeliness: false, + ...executionData, }, block.slot diff --git a/packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts b/packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts index 766c02a15a23..e014615fd312 100644 --- a/packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts +++ b/packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts @@ -40,6 +40,8 @@ describe("getCommonAncestor", () => { unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: "-", + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }, 0 @@ -63,6 +65,8 @@ describe("getCommonAncestor", () => { unrealizedFinalizedEpoch: 0, unrealizedFinalizedRoot: "-", + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }, block.slot diff --git a/packages/fork-choice/test/unit/protoArray/protoArray.test.ts b/packages/fork-choice/test/unit/protoArray/protoArray.test.ts index c3bf8a0f439a..fdebd4bba645 100644 --- a/packages/fork-choice/test/unit/protoArray/protoArray.test.ts +++ b/packages/fork-choice/test/unit/protoArray/protoArray.test.ts @@ -30,6 +30,8 @@ describe("ProtoArray", () => { unrealizedFinalizedEpoch: genesisEpoch, unrealizedFinalizedRoot: stateRoot, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }, genesisSlot @@ -53,6 +55,8 @@ describe("ProtoArray", () => { unrealizedFinalizedEpoch: genesisEpoch, unrealizedFinalizedRoot: stateRoot, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }, genesisSlot + 1 @@ -76,6 +80,8 @@ describe("ProtoArray", () => { unrealizedFinalizedEpoch: genesisEpoch, unrealizedFinalizedRoot: stateRoot, + timeliness: false, + ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }, genesisSlot + 1 diff --git a/packages/validator/src/util/params.ts b/packages/validator/src/util/params.ts index 006ae3fadbbb..8ccaf9fe75ba 100644 --- a/packages/validator/src/util/params.ts +++ b/packages/validator/src/util/params.ts @@ -123,6 +123,9 @@ function getSpecCriticalParams(localConfig: ChainConfig): Record Date: Fri, 22 Mar 2024 15:47:07 +0800 Subject: [PATCH 02/11] Remove unnecessary code --- .../beacon-node/src/chain/forkChoice/index.ts | 7 +++- .../opPools/aggregatedAttestationPool.test.ts | 2 +- .../fork-choice/src/forkChoice/forkChoice.ts | 41 ++----------------- .../fork-choice/src/forkChoice/interface.ts | 2 - 4 files changed, 9 insertions(+), 43 deletions(-) diff --git a/packages/beacon-node/src/chain/forkChoice/index.ts b/packages/beacon-node/src/chain/forkChoice/index.ts index da50dd1f48ad..047fc0d24977 100644 --- a/packages/beacon-node/src/chain/forkChoice/index.ts +++ b/packages/beacon-node/src/chain/forkChoice/index.ts @@ -16,6 +16,7 @@ import { isMergeTransitionComplete, } from "@lodestar/state-transition"; +import {Logger} from "@lodestar/utils"; import {computeAnchorCheckpoint} from "../initState.js"; import {ChainEventEmitter} from "../emitter.js"; import {ChainEvent} from "../emitter.js"; @@ -35,7 +36,8 @@ export function initializeForkChoice( currentSlot: Slot, state: CachedBeaconStateAllForks, opts: ForkChoiceOpts, - justifiedBalancesGetter: JustifiedBalancesGetter + justifiedBalancesGetter: JustifiedBalancesGetter, + logger?: Logger ): ForkChoice { const {blockHeader, checkpoint} = computeAnchorCheckpoint(config, state); const finalizedCheckpoint = {...checkpoint}; @@ -75,6 +77,7 @@ export function initializeForkChoice( parentRoot: toHexString(blockHeader.parentRoot), stateRoot: toHexString(blockHeader.stateRoot), blockRoot: toHexString(checkpoint.root), + timeliness: true, // Optimisitcally assume is timely justifiedEpoch: justifiedCheckpoint.epoch, justifiedRoot: toHexString(justifiedCheckpoint.root), @@ -95,7 +98,7 @@ export function initializeForkChoice( }, currentSlot ), - + logger, opts ); } 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 e47ce48f57c1..2e0c3500bea5 100644 --- a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -162,7 +162,7 @@ describe(`getAttestationsForBlock vc=${vc}`, () => { return {state, pool}; }, fn: ({state, pool}) => { - pool.getAttestationsForBlock(forkchoice, state); + pool.getAttestationsForBlock(state.config.getForkName(state.slot), forkchoice, state); }, }); } diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 2b5f45b76fb6..0c746d921ff8 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -45,21 +45,9 @@ import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalan export type ForkChoiceOpts = { proposerBoostEnabled?: boolean; - proposerBoostReorgEnabled?: boolean; computeUnrealized?: boolean; }; -export enum UpdateHeadOpt { - GetCanonicialHead, // Skip getProposerHead - GetProposerHead, // With getProposerHead - GetPredictedProposerHead, // With predictProposerHead -} - -export type UpdateAndGetHeadOpt = - | {mode: UpdateHeadOpt.GetCanonicialHead} - | {mode: UpdateHeadOpt.GetProposerHead; slot: Slot} - | {mode: UpdateHeadOpt.GetPredictedProposerHead; slot: Slot}; - /** * Provides an implementation of "Ethereum Consensus -- Beacon Chain Fork Choice": * @@ -167,29 +155,6 @@ export class ForkChoice implements IForkChoice { return this.head; } - /** - * - * A multiplexer to wrap around the traditional `updateHead()` according to the scenario - * Scenarios as follow: - * Prepare to propose in the next slot: getHead() -> predictProposerHead() - * Proposing in the current slot: updateHead() -> getProposerHead() - * Others eg. initializing forkchoice, importBlock: updateHead() - */ - updateAndGetHead(opt: UpdateAndGetHeadOpt): ProtoBlock { - const {mode} = opt; - - const canonicialHeadBlock = mode === UpdateHeadOpt.GetPredictedProposerHead ? this.getHead() : this.updateHead(); - switch (mode) { - case UpdateHeadOpt.GetPredictedProposerHead: - return this.predictProposerHead(canonicialHeadBlock, opt.slot); - case UpdateHeadOpt.GetProposerHead: - return this.getProposerHead(canonicialHeadBlock, opt.slot); - case UpdateHeadOpt.GetCanonicialHead: - default: - return canonicialHeadBlock; - } - } - /** * Get the proposer boost root */ @@ -210,7 +175,7 @@ export class ForkChoice implements IForkChoice { */ predictProposerHead(headBlock: ProtoBlock, currentSlot?: Slot): ProtoBlock { // Skip re-org attempt if proposer boost (reorg) are disabled - if (!this.opts?.proposerBoostEnabled || !this.opts?.proposerBoostReorgEnabled) { + if (!this.opts?.proposerBoostEnabled) { this.logger?.verbose("No proposer boot reorg prediction since the related flags are disabled"); return headBlock; } @@ -243,13 +208,13 @@ export class ForkChoice implements IForkChoice { * * This function takes in the canonical head block and determine the proposer head (canonical head block or its parent) * https://github.com/ethereum/consensus-specs/pull/3034 for info about proposer boost reorg - * This function should only be called during block proposal and only be called after `updateHead()` in `updateAndGetHead()` + * This function should only be called during block proposal and only be called after `updateHead()` * * Same as https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#get_proposer_head */ getProposerHead(headBlock: ProtoBlock, slot: Slot): ProtoBlock { // Skip re-org attempt if proposer boost (reorg) are disabled - if (!this.opts?.proposerBoostEnabled || !this.opts?.proposerBoostReorgEnabled) { + if (!this.opts?.proposerBoostEnabled) { this.logger?.verbose("No proposer boot reorg attempt since the related flags are disabled"); return headBlock; } diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index b40e37f7568c..fffbc3e4007f 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -3,7 +3,6 @@ import {CachedBeaconStateAllForks} from "@lodestar/state-transition"; import {Epoch, Slot, ValidatorIndex, phase0, allForks, Root, RootHex} from "@lodestar/types"; import {ProtoBlock, MaybeValidExecutionStatus, LVHExecResponse, ProtoNode} from "../protoArray/interface.js"; import {CheckpointWithHex} from "./store.js"; -import {UpdateAndGetHeadOpt} from "./forkChoice.js"; export type CheckpointHex = { epoch: Epoch; @@ -78,7 +77,6 @@ export interface IForkChoice { getHeadRoot(): RootHex; getHead(): ProtoBlock; updateHead(): ProtoBlock; - updateAndGetHead(mode: UpdateAndGetHeadOpt): ProtoBlock; /** * Retrieves all possible chain heads (leaves of fork choice tree). */ From 5aac9f9d51abda13590281935826beb13d0b1265 Mon Sep 17 00:00:00 2001 From: NC Date: Wed, 27 Mar 2024 18:31:08 +0800 Subject: [PATCH 03/11] isBlockTimely --- packages/fork-choice/src/forkChoice/forkChoice.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 0c746d921ff8..24f83e484b50 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -468,8 +468,7 @@ export class ForkChoice implements IForkChoice { // Assign proposer score boost if the block is timely // before attesting interval = before 1st interval - const isBeforeAttestingInterval = blockDelaySec < this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT; - const isTimely = this.fcStore.currentSlot === slot && isBeforeAttestingInterval; + const isTimely = this.isBlockTimely(block, blockDelaySec); if ( this.opts?.proposerBoostEnabled && isTimely && @@ -1015,6 +1014,15 @@ export class ForkChoice implements IForkChoice { throw Error(`Not found dependent root for block slot ${block.slot}, epoch difference ${epochDifference}`); } + /** + * Return true if the block is timely for the current slot. + * Child class can overwrite this for testing purpose. + */ + protected isBlockTimely(block: allForks.BeaconBlock, blockDelaySec: number): boolean { + const isBeforeAttestingInterval = blockDelaySec < this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT; + return this.fcStore.currentSlot === block.slot && isBeforeAttestingInterval; + } + private getPreMergeExecStatus(executionStatus: MaybeValidExecutionStatus): ExecutionStatus.PreMerge { if (executionStatus !== ExecutionStatus.PreMerge) throw Error(`Invalid pre-merge execution status: expected: ${ExecutionStatus.PreMerge}, got ${executionStatus}`); From b8ad23ffabf22f0c15d485bc24b33e1ec2624391 Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 15:02:31 +0800 Subject: [PATCH 04/11] getProposerHead returns info for metrics --- .../fork-choice/src/forkChoice/forkChoice.ts | 55 ++++++++++++------- .../fork-choice/src/forkChoice/interface.ts | 15 +++++ .../unit/forkChoice/getProposerHead.test.ts | 26 ++++++++- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 24f83e484b50..d71871e590a9 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -40,6 +40,7 @@ import { AncestorResult, AncestorStatus, ForkChoiceMetrics, + NotReorgedReason, } from "./interface.js"; import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalances} from "./store.js"; @@ -189,9 +190,9 @@ export class ForkChoice implements IForkChoice { return headBlock; } - const prelimProposerHeadBlock = this.getPreliminaryProposerHead(headBlock, parentBlock, proposalSlot); + const {prelimProposerHead} = this.getPreliminaryProposerHead(headBlock, parentBlock, proposalSlot); - if (prelimProposerHeadBlock === headBlock) { + if (prelimProposerHead === headBlock) { return headBlock; } @@ -212,37 +213,43 @@ export class ForkChoice implements IForkChoice { * * Same as https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#get_proposer_head */ - getProposerHead(headBlock: ProtoBlock, slot: Slot): ProtoBlock { + getProposerHead( + headBlock: ProtoBlock, + slot: Slot + ): {proposerHead: ProtoBlock; isHeadTimely: boolean; notReorgedReason?: NotReorgedReason} { + const isHeadTimely = headBlock.timeliness; + let proposerHead = headBlock; + // Skip re-org attempt if proposer boost (reorg) are disabled if (!this.opts?.proposerBoostEnabled) { this.logger?.verbose("No proposer boot reorg attempt since the related flags are disabled"); - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ProposerBoostReorgDisabled}; } const parentBlock = this.protoArray.getBlock(headBlock.parentRoot); // No reorg if parentBlock isn't available if (parentBlock === undefined) { - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ParentBlockNotAvailable}; } - const prelimProposerHeadBlock = this.getPreliminaryProposerHead(headBlock, parentBlock, slot); + const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead(headBlock, parentBlock, slot); - if (prelimProposerHeadBlock === headBlock) { - return headBlock; + if (prelimProposerHead === headBlock && prelimNotReorgedReason !== undefined) { + return {proposerHead, isHeadTimely, notReorgedReason: prelimNotReorgedReason}; } // No reorg if attempted reorg is more than a single slot // Half of single_slot_reorg check in the spec is done in getPreliminaryProposerHead() const currentTimeOk = headBlock.slot + 1 === slot; if (!currentTimeOk) { - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ReorgMoreThanOneSlot}; } // No reorg if proposer boost is still in effect const isProposerBoostWornOff = this.proposerBoostRoot !== headBlock.blockRoot; if (!isProposerBoostWornOff) { - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ProposerBoostNotWornOff}; } // No reorg if headBlock is "not weak" ie. headBlock's weight exceeds (REORG_HEAD_WEIGHT_THRESHOLD = 20)% of total attester weight @@ -254,7 +261,7 @@ export class ForkChoice implements IForkChoice { const headNode = this.protoArray.getNode(headBlock.blockRoot); // If headNode is unavailable, give up reorg if (headNode === undefined || headNode.weight >= reorgThreshold) { - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.HeadBlockNotWeak}; } // No reorg if parentBlock is "not strong" ie. parentBlock's weight is less than or equal to (REORG_PARENT_WEIGHT_THRESHOLD = 160)% of total attester weight @@ -266,13 +273,14 @@ export class ForkChoice implements IForkChoice { const parentNode = this.protoArray.getNode(parentBlock.blockRoot); // If parentNode is unavailable, give up reorg if (parentNode === undefined || parentNode.weight <= parentThreshold) { - return headBlock; + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ParentBlockIsStrong}; } // Reorg if all above checks fail this.logger?.info("Will perform single-slot reorg to reorg out current weak head"); + proposerHead = parentBlock; - return parentBlock; + return {proposerHead, isHeadTimely}; } /** @@ -1333,19 +1341,24 @@ export class ForkChoice implements IForkChoice { * No one should be calling this function except these two * */ - private getPreliminaryProposerHead(headBlock: ProtoBlock, parentBlock: ProtoBlock, slot: Slot): ProtoBlock { + private getPreliminaryProposerHead( + headBlock: ProtoBlock, + parentBlock: ProtoBlock, + slot: Slot + ): {prelimProposerHead: ProtoBlock; prelimNotReorgedReason?: NotReorgedReason} { + let prelimProposerHead = headBlock; // No reorg if headBlock is on time // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_head_late const isHeadLate = !headBlock.timeliness; if (!isHeadLate) { - return headBlock; + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.HeadBlockIsTimely}; } // No reorg if we are at epoch boundary where proposer shuffling could change // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_shuffling_stable const isShufflingStable = slot % SLOTS_PER_EPOCH !== 0; if (!isShufflingStable) { - return headBlock; + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.NotShufflingStable}; } // No reorg if headBlock and parentBlock are not ffg competitive @@ -1354,7 +1367,7 @@ export class ForkChoice implements IForkChoice { const {unrealizedJustifiedEpoch: parentBlockCpEpoch, unrealizedJustifiedRoot: parentBlockCpRoot} = parentBlock; const isFFGCompetitive = headBlockCpEpoch === parentBlockCpEpoch && headBlockCpRoot === parentBlockCpRoot; if (!isFFGCompetitive) { - return headBlock; + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.NotFFGCompetitive}; } // No reorg if chain is not finalizing within REORG_MAX_EPOCHS_SINCE_FINALIZATION @@ -1362,7 +1375,7 @@ export class ForkChoice implements IForkChoice { const epochsSinceFinalization = computeEpochAtSlot(slot) - this.getFinalizedCheckpoint().epoch; const isFinalizationOk = epochsSinceFinalization <= this.config.REORG_MAX_EPOCHS_SINCE_FINALIZATION; if (!isFinalizationOk) { - return headBlock; + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.ChainLongUnfinality}; } // -No reorg if we are not proposing on time.- @@ -1372,10 +1385,12 @@ export class ForkChoice implements IForkChoice { // No reorg if this reorg spans more than a single slot const parentSlotOk = parentBlock.slot + 1 === headBlock.slot; if (!parentSlotOk) { - return headBlock; + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.ParentBlockDistanceMoreThanOneSlot}; } - return parentBlock; + prelimProposerHead = parentBlock; + + return {prelimProposerHead}; } } diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index fffbc3e4007f..5016c89c61bd 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -41,6 +41,21 @@ export type AncestorResult = | {code: AncestorStatus.NoCommonAncenstor} | {code: AncestorStatus.BlockUnknown}; +// Reason for not proposer boost reorging +export enum NotReorgedReason { + HeadBlockIsTimely, + ParentBlockNotAvailable, + ProposerBoostReorgDisabled, + NotShufflingStable, + NotFFGCompetitive, + ChainLongUnfinality, + ParentBlockDistanceMoreThanOneSlot, + ReorgMoreThanOneSlot, + ProposerBoostNotWornOff, + HeadBlockNotWeak, + ParentBlockIsStrong, +} + export type ForkChoiceMetrics = { votes: number; queuedAttestations: number; diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts index 40108bd748c3..4d89f78b4cc2 100644 --- a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -5,6 +5,7 @@ import {Slot} from "@lodestar/types"; import {toHex} from "@lodestar/utils"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {ForkChoice, IForkChoiceStore, ProtoArray, ExecutionStatus, ProtoBlock} from "../../../src/index.js"; +import {NotReorgedReason} from "../../../src/forkChoice/interface.js"; import {getBlockRoot, getStateRoot} from "./forkChoice.test.js"; type ProtoBlockWithWeight = ProtoBlock & {weight: number}; // weight of the block itself @@ -119,6 +120,7 @@ describe("Forkchoice / GetProposerHead", function () { headBlock: ProtoBlockWithWeight; expectReorg: boolean; currentSlot?: Slot; + expectedNotReorgedReason?: NotReorgedReason; }[] = [ { id: "Case that meets all conditions to be re-orged", @@ -131,6 +133,7 @@ describe("Forkchoice / GetProposerHead", function () { parentBlock: {...baseParentHeadBlock}, headBlock: {...baseHeadBlock, timeliness: true}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.HeadBlockIsTimely, }, { id: "No reorg when currenSlot is at epoch boundary", @@ -138,18 +141,21 @@ describe("Forkchoice / GetProposerHead", function () { headBlock: {...baseHeadBlock}, expectReorg: false, currentSlot: SLOTS_PER_EPOCH * 2, + expectedNotReorgedReason: NotReorgedReason.NotShufflingStable, }, { id: "No reorg when the blocks are not ffg competitive", parentBlock: {...baseParentHeadBlock}, headBlock: {...baseHeadBlock, unrealizedJustifiedEpoch: 1}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.NotFFGCompetitive, }, { id: "No reorg when the blocks are not ffg competitive 2", parentBlock: {...baseParentHeadBlock}, headBlock: {...baseHeadBlock, unrealizedJustifiedRoot: "-"}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.NotFFGCompetitive, }, { id: "No reorg if long unfinality", @@ -157,12 +163,14 @@ describe("Forkchoice / GetProposerHead", function () { headBlock: {...baseHeadBlock}, expectReorg: false, currentSlot: (genesisEpoch + 2) * SLOTS_PER_EPOCH + 1, + expectedNotReorgedReason: NotReorgedReason.ReorgMoreThanOneSlot, // TODO: To make it such that it returns NotReorgedReason.ChainLongUnfinality }, { id: "No reorg if reorg spans more than a single slot", parentBlock: {...baseParentHeadBlock}, headBlock: {...baseHeadBlock, slot: headSlot + 1}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.ParentBlockDistanceMoreThanOneSlot, }, { id: "No reorg if current slot is more than one slot from head block", @@ -170,18 +178,21 @@ describe("Forkchoice / GetProposerHead", function () { headBlock: {...baseHeadBlock}, expectReorg: false, currentSlot: headSlot + 2, + expectedNotReorgedReason: NotReorgedReason.ReorgMoreThanOneSlot, }, { id: "No reorg if head is strong", parentBlock: {...baseParentHeadBlock}, headBlock: {...baseHeadBlock, weight: 30}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.HeadBlockNotWeak, }, { id: "No reorg if parent is weak", parentBlock: {...baseParentHeadBlock, weight: 211}, headBlock: {...baseHeadBlock}, expectReorg: false, + expectedNotReorgedReason: NotReorgedReason.ParentBlockIsStrong, }, ]; @@ -189,7 +200,14 @@ describe("Forkchoice / GetProposerHead", function () { protoArr = ProtoArray.initialize(genesisBlock, genesisSlot); }); - for (const {id, parentBlock, headBlock, expectReorg, currentSlot: proposalSlot} of testCases) { + for (const { + id, + parentBlock, + headBlock, + expectReorg, + currentSlot: proposalSlot, + expectedNotReorgedReason, + } of testCases) { it(`${id}`, async () => { protoArr.onBlock(parentBlock, parentBlock.slot); protoArr.onBlock(headBlock, headBlock.slot); @@ -207,11 +225,13 @@ describe("Forkchoice / GetProposerHead", function () { const forkChoice = new ForkChoice(config, fcStore, protoArr, undefined, { proposerBoostEnabled: true, - proposerBoostReorgEnabled: true, + // proposerBoostReorgEnabled: true, }); - const proposerHead = forkChoice.getProposerHead(headBlock, currentSlot); + const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead(headBlock, currentSlot); + expect(isHeadTimely).toBe(headBlock.timeliness); + expect(notReorgedReason).toBe(expectedNotReorgedReason); expect(proposerHead.blockRoot).toBe(expectReorg ? parentBlock.blockRoot : headBlock.blockRoot); }); } From 2566732d71de6668adbfa818134dea33b9e3db74 Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 15:13:29 +0800 Subject: [PATCH 05/11] Fix compile issue --- packages/state-transition/src/util/execution.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/state-transition/src/util/execution.ts b/packages/state-transition/src/util/execution.ts index 7ac4da4aeecb..30391e441586 100644 --- a/packages/state-transition/src/util/execution.ts +++ b/packages/state-transition/src/util/execution.ts @@ -114,6 +114,7 @@ export function isExecutionPayload( payload: allForks.FullOrBlindedExecutionPayload ): payload is allForks.ExecutionPayload { return (payload as allForks.ExecutionPayload).transactions !== undefined; + } export function isCapellaPayload( From 24353b639102bbbd8b01bb83201f291a48a5bc1a Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 15:29:36 +0800 Subject: [PATCH 06/11] Fix type check --- packages/beacon-node/src/chain/forkChoice/index.ts | 2 +- packages/fork-choice/src/forkChoice/forkChoice.ts | 4 ++-- .../fork-choice/test/unit/forkChoice/getProposerHead.test.ts | 2 +- packages/state-transition/src/util/execution.ts | 1 - 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/chain/forkChoice/index.ts b/packages/beacon-node/src/chain/forkChoice/index.ts index 047fc0d24977..411b9a267600 100644 --- a/packages/beacon-node/src/chain/forkChoice/index.ts +++ b/packages/beacon-node/src/chain/forkChoice/index.ts @@ -98,7 +98,7 @@ export function initializeForkChoice( }, currentSlot ), + opts, logger, - opts ); } diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index d71871e590a9..f53a26730c14 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -108,8 +108,8 @@ export class ForkChoice implements IForkChoice { private readonly fcStore: IForkChoiceStore, /** The underlying representation of the block DAG. */ private readonly protoArray: ProtoArray, - private readonly logger?: Logger, - private readonly opts?: ForkChoiceOpts + private readonly opts?: ForkChoiceOpts, + private readonly logger?: Logger ) { this.head = this.updateHead(); this.balances = this.fcStore.justified.balances; diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts index 4d89f78b4cc2..9827e8875e7a 100644 --- a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -223,7 +223,7 @@ describe("Forkchoice / GetProposerHead", function () { currentSlot, }); - const forkChoice = new ForkChoice(config, fcStore, protoArr, undefined, { + const forkChoice = new ForkChoice(config, fcStore, protoArr, { proposerBoostEnabled: true, // proposerBoostReorgEnabled: true, }); diff --git a/packages/state-transition/src/util/execution.ts b/packages/state-transition/src/util/execution.ts index 30391e441586..7ac4da4aeecb 100644 --- a/packages/state-transition/src/util/execution.ts +++ b/packages/state-transition/src/util/execution.ts @@ -114,7 +114,6 @@ export function isExecutionPayload( payload: allForks.FullOrBlindedExecutionPayload ): payload is allForks.ExecutionPayload { return (payload as allForks.ExecutionPayload).transactions !== undefined; - } export function isCapellaPayload( From 66b09fe296aba9fd46e4c5c67734cf3b7f5c3eb3 Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 15:59:48 +0800 Subject: [PATCH 07/11] Lint --- packages/beacon-node/src/chain/forkChoice/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/chain/forkChoice/index.ts b/packages/beacon-node/src/chain/forkChoice/index.ts index 411b9a267600..a9e728a0b3a8 100644 --- a/packages/beacon-node/src/chain/forkChoice/index.ts +++ b/packages/beacon-node/src/chain/forkChoice/index.ts @@ -99,6 +99,6 @@ export function initializeForkChoice( currentSlot ), opts, - logger, + logger ); } From f11793ca3ea37aea0092c58c9e72e0bb115b43b7 Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 16:46:43 +0800 Subject: [PATCH 08/11] Add secFromSlot --- packages/fork-choice/src/forkChoice/forkChoice.ts | 13 ++++++++++--- packages/fork-choice/src/forkChoice/interface.ts | 1 + .../test/unit/forkChoice/getProposerHead.test.ts | 15 +++++++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index f53a26730c14..26f1f1484c20 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -174,7 +174,7 @@ export class ForkChoice implements IForkChoice { * * https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/bellatrix/fork-choice.md#should_override_forkchoice_update */ - predictProposerHead(headBlock: ProtoBlock, currentSlot?: Slot): ProtoBlock { + predictProposerHead(headBlock: ProtoBlock, secFromSlot: number, currentSlot?: Slot): ProtoBlock { // Skip re-org attempt if proposer boost (reorg) are disabled if (!this.opts?.proposerBoostEnabled) { this.logger?.verbose("No proposer boot reorg prediction since the related flags are disabled"); @@ -190,7 +190,7 @@ export class ForkChoice implements IForkChoice { return headBlock; } - const {prelimProposerHead} = this.getPreliminaryProposerHead(headBlock, parentBlock, proposalSlot); + const {prelimProposerHead} = this.getPreliminaryProposerHead(headBlock, parentBlock, secFromSlot, proposalSlot); if (prelimProposerHead === headBlock) { return headBlock; @@ -215,6 +215,7 @@ export class ForkChoice implements IForkChoice { */ getProposerHead( headBlock: ProtoBlock, + secFromSlot: number, slot: Slot ): {proposerHead: ProtoBlock; isHeadTimely: boolean; notReorgedReason?: NotReorgedReason} { const isHeadTimely = headBlock.timeliness; @@ -233,7 +234,7 @@ export class ForkChoice implements IForkChoice { return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ParentBlockNotAvailable}; } - const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead(headBlock, parentBlock, slot); + const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead(headBlock, parentBlock, secFromSlot, slot); if (prelimProposerHead === headBlock && prelimNotReorgedReason !== undefined) { return {proposerHead, isHeadTimely, notReorgedReason: prelimNotReorgedReason}; @@ -1344,6 +1345,7 @@ export class ForkChoice implements IForkChoice { private getPreliminaryProposerHead( headBlock: ProtoBlock, parentBlock: ProtoBlock, + secFromSlot: number, slot: Slot ): {prelimProposerHead: ProtoBlock; prelimNotReorgedReason?: NotReorgedReason} { let prelimProposerHead = headBlock; @@ -1381,6 +1383,11 @@ export class ForkChoice implements IForkChoice { // -No reorg if we are not proposing on time.- // Note: Skipping this check as store.time in Lodestar is stored in slot and not unix time // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_proposing_on_time + const proposerReorgCutoff = this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2; + const isProposingOnTime = secFromSlot <= proposerReorgCutoff; + if (!isProposingOnTime) { + return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.NotProposingOnTime}; + } // No reorg if this reorg spans more than a single slot const parentSlotOk = parentBlock.slot + 1 === headBlock.slot; diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index 5016c89c61bd..5086013bd3a1 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -54,6 +54,7 @@ export enum NotReorgedReason { ProposerBoostNotWornOff, HeadBlockNotWeak, ParentBlockIsStrong, + NotProposingOnTime, } export type ForkChoiceMetrics = { diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts index 9827e8875e7a..a5e4ad6db8cf 100644 --- a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -3,7 +3,7 @@ import {fromHexString} from "@chainsafe/ssz"; import {config} from "@lodestar/config/default"; import {Slot} from "@lodestar/types"; import {toHex} from "@lodestar/utils"; -import {SLOTS_PER_EPOCH} from "@lodestar/params"; +import {INTERVALS_PER_SLOT, SLOTS_PER_EPOCH} from "@lodestar/params"; import {ForkChoice, IForkChoiceStore, ProtoArray, ExecutionStatus, ProtoBlock} from "../../../src/index.js"; import {NotReorgedReason} from "../../../src/forkChoice/interface.js"; import {getBlockRoot, getStateRoot} from "./forkChoice.test.js"; @@ -120,6 +120,7 @@ describe("Forkchoice / GetProposerHead", function () { headBlock: ProtoBlockWithWeight; expectReorg: boolean; currentSlot?: Slot; + secFromSlot?: number; expectedNotReorgedReason?: NotReorgedReason; }[] = [ { @@ -194,6 +195,14 @@ describe("Forkchoice / GetProposerHead", function () { expectReorg: false, expectedNotReorgedReason: NotReorgedReason.ParentBlockIsStrong, }, + { + id: "No reorg if not proposing on time", + parentBlock: {...baseParentHeadBlock, weight: 211}, + headBlock: {...baseHeadBlock}, + expectReorg: false, + secFromSlot: (config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2) + 1, + expectedNotReorgedReason: NotReorgedReason.NotProposingOnTime, + } ]; beforeEach(() => { @@ -206,6 +215,7 @@ describe("Forkchoice / GetProposerHead", function () { headBlock, expectReorg, currentSlot: proposalSlot, + secFromSlot, expectedNotReorgedReason, } of testCases) { it(`${id}`, async () => { @@ -213,6 +223,7 @@ describe("Forkchoice / GetProposerHead", function () { protoArr.onBlock(headBlock, headBlock.slot); const currentSlot = proposalSlot ?? headBlock.slot + 1; + const currentSecFromSlot = secFromSlot ?? 0; protoArr.applyScoreChanges({ deltas: [0, parentBlock.weight, headBlock.weight], proposerBoost: null, @@ -228,7 +239,7 @@ describe("Forkchoice / GetProposerHead", function () { // proposerBoostReorgEnabled: true, }); - const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead(headBlock, currentSlot); + const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead(headBlock, currentSecFromSlot, currentSlot); expect(isHeadTimely).toBe(headBlock.timeliness); expect(notReorgedReason).toBe(expectedNotReorgedReason); From abc4787bed80298b697e09b7f6a5627df30ec536 Mon Sep 17 00:00:00 2001 From: NC Date: Thu, 28 Mar 2024 16:52:49 +0800 Subject: [PATCH 09/11] lint --- packages/fork-choice/src/forkChoice/forkChoice.ts | 7 ++++++- .../test/unit/forkChoice/getProposerHead.test.ts | 10 +++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 26f1f1484c20..8cf0181201d4 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -234,7 +234,12 @@ export class ForkChoice implements IForkChoice { return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ParentBlockNotAvailable}; } - const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead(headBlock, parentBlock, secFromSlot, slot); + const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead( + headBlock, + parentBlock, + secFromSlot, + slot + ); if (prelimProposerHead === headBlock && prelimNotReorgedReason !== undefined) { return {proposerHead, isHeadTimely, notReorgedReason: prelimNotReorgedReason}; diff --git a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts index a5e4ad6db8cf..9764b18068bb 100644 --- a/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/getProposerHead.test.ts @@ -200,9 +200,9 @@ describe("Forkchoice / GetProposerHead", function () { parentBlock: {...baseParentHeadBlock, weight: 211}, headBlock: {...baseHeadBlock}, expectReorg: false, - secFromSlot: (config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2) + 1, + secFromSlot: config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2 + 1, expectedNotReorgedReason: NotReorgedReason.NotProposingOnTime, - } + }, ]; beforeEach(() => { @@ -239,7 +239,11 @@ describe("Forkchoice / GetProposerHead", function () { // proposerBoostReorgEnabled: true, }); - const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead(headBlock, currentSecFromSlot, currentSlot); + const {proposerHead, isHeadTimely, notReorgedReason} = forkChoice.getProposerHead( + headBlock, + currentSecFromSlot, + currentSlot + ); expect(isHeadTimely).toBe(headBlock.timeliness); expect(notReorgedReason).toBe(expectedNotReorgedReason); From 582f0ede9c9054d158edb6c0f72c061c86666746 Mon Sep 17 00:00:00 2001 From: NC Date: Tue, 2 Apr 2024 15:54:04 +0800 Subject: [PATCH 10/11] address comment --- .../fork-choice/src/forkChoice/forkChoice.ts | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 8cf0181201d4..07f92ce5aaaf 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -174,7 +174,7 @@ export class ForkChoice implements IForkChoice { * * https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/bellatrix/fork-choice.md#should_override_forkchoice_update */ - predictProposerHead(headBlock: ProtoBlock, secFromSlot: number, currentSlot?: Slot): ProtoBlock { + predictProposerHead(headBlock: ProtoBlock, currentSlot?: Slot): ProtoBlock { // Skip re-org attempt if proposer boost (reorg) are disabled if (!this.opts?.proposerBoostEnabled) { this.logger?.verbose("No proposer boot reorg prediction since the related flags are disabled"); @@ -190,7 +190,7 @@ export class ForkChoice implements IForkChoice { return headBlock; } - const {prelimProposerHead} = this.getPreliminaryProposerHead(headBlock, parentBlock, secFromSlot, proposalSlot); + const {prelimProposerHead} = this.getPreliminaryProposerHead(headBlock, parentBlock, proposalSlot); if (prelimProposerHead === headBlock) { return headBlock; @@ -234,17 +234,21 @@ export class ForkChoice implements IForkChoice { return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.ParentBlockNotAvailable}; } - const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead( - headBlock, - parentBlock, - secFromSlot, - slot - ); + const {prelimProposerHead, prelimNotReorgedReason} = this.getPreliminaryProposerHead(headBlock, parentBlock, slot); if (prelimProposerHead === headBlock && prelimNotReorgedReason !== undefined) { return {proposerHead, isHeadTimely, notReorgedReason: prelimNotReorgedReason}; } + // -No reorg if we are not proposing on time.- + // Note: Skipping this check as store.time in Lodestar is stored in slot and not unix time + // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_proposing_on_time + const proposerReorgCutoff = this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2; + const isProposingOnTime = secFromSlot <= proposerReorgCutoff; + if (!isProposingOnTime) { + return {proposerHead, isHeadTimely, notReorgedReason: NotReorgedReason.NotProposingOnTime}; + } + // No reorg if attempted reorg is more than a single slot // Half of single_slot_reorg check in the spec is done in getPreliminaryProposerHead() const currentTimeOk = headBlock.slot + 1 === slot; @@ -1350,7 +1354,6 @@ export class ForkChoice implements IForkChoice { private getPreliminaryProposerHead( headBlock: ProtoBlock, parentBlock: ProtoBlock, - secFromSlot: number, slot: Slot ): {prelimProposerHead: ProtoBlock; prelimNotReorgedReason?: NotReorgedReason} { let prelimProposerHead = headBlock; @@ -1385,15 +1388,6 @@ export class ForkChoice implements IForkChoice { return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.ChainLongUnfinality}; } - // -No reorg if we are not proposing on time.- - // Note: Skipping this check as store.time in Lodestar is stored in slot and not unix time - // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_proposing_on_time - const proposerReorgCutoff = this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2; - const isProposingOnTime = secFromSlot <= proposerReorgCutoff; - if (!isProposingOnTime) { - return {prelimProposerHead, prelimNotReorgedReason: NotReorgedReason.NotProposingOnTime}; - } - // No reorg if this reorg spans more than a single slot const parentSlotOk = parentBlock.slot + 1 === headBlock.slot; if (!parentSlotOk) { From 79b120ebcd6328ebf06dab5e86fd6f172f3cc206 Mon Sep 17 00:00:00 2001 From: NC Date: Wed, 3 Apr 2024 16:34:03 +0800 Subject: [PATCH 11/11] Remove comment --- packages/fork-choice/src/forkChoice/forkChoice.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 07f92ce5aaaf..7adc2c425816 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -240,8 +240,6 @@ export class ForkChoice implements IForkChoice { return {proposerHead, isHeadTimely, notReorgedReason: prelimNotReorgedReason}; } - // -No reorg if we are not proposing on time.- - // Note: Skipping this check as store.time in Lodestar is stored in slot and not unix time // https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/fork-choice.md#is_proposing_on_time const proposerReorgCutoff = this.config.SECONDS_PER_SLOT / INTERVALS_PER_SLOT / 2; const isProposingOnTime = secFromSlot <= proposerReorgCutoff;