diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 02f8f35919ff..602e5a6cb931 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -16,7 +16,8 @@ import { getBlockRootAtSlot, } from "@lodestar/state-transition"; import {toHexString} from "@chainsafe/ssz"; -import {IForkChoice} from "@lodestar/fork-choice"; +import {IForkChoice, EpochDifference} from "@lodestar/fork-choice"; +import {toHex} from "@lodestar/utils"; import {MapDef} from "../../util/map.js"; import {intersectUint8Arrays, IntersectResult} from "../../util/bitArray.js"; import {pruneBySlot} from "./utils.js"; @@ -433,14 +434,20 @@ export function isValidAttestationData( // Otherwise the shuffling is determined by the block at the end of the target epoch // minus the shuffling lookahead (usually 2). We call this the "pivot". const pivotSlot = computeStartSlotAtEpoch(targetEpoch - 1) - 1; - const statePivotBlockRoot = toHexString(getBlockRootAtSlot(state, pivotSlot)); + const stateDependentRoot = toHexString(getBlockRootAtSlot(state, pivotSlot)); // Use fork choice's view of the block DAG to quickly evaluate whether the attestation's // pivot block is the same as the current state's pivot block. If it is, then the // attestation's shuffling is the same as the current state's. // To account for skipped slots, find the first block at *or before* the pivot slot. - const pivotBlockRoot = forkChoice.findAttesterDependentRoot(data.beaconBlockRoot); - return pivotBlockRoot === statePivotBlockRoot; + const beaconBlockRootHex = toHex(data.beaconBlockRoot); + const beaconBlock = forkChoice.getBlockHex(beaconBlockRootHex); + if (!beaconBlock) { + throw Error(`Attestation data.beaconBlockRoot ${beaconBlockRootHex} not found in forkchoice`); + } + + const attestationDependantRoot = forkChoice.getDependentRoot(beaconBlock, EpochDifference.previous); + return attestationDependantRoot === stateDependentRoot; } function flagIsTimelySource(flag: number): boolean { diff --git a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index dd722be9a840..2a3e2292b3ff 100644 --- a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -21,6 +21,7 @@ import {generateAttestation, generateEmptyAttestation} from "../../../utils/atte import {generateCachedState} from "../../../utils/state.js"; import {renderBitArray} from "../../../utils/render.js"; import {ZERO_HASH_HEX} from "../../../../src/constants/constants.js"; +import {generateEmptyProtoBlock} from "../../../utils/block.js"; /** Valid signature of random data to prevent BLS errors */ const validSignature = fromHexString( @@ -71,15 +72,15 @@ describe("AggregatedAttestationPool", function () { it(name, function () { const aggregationBits = new BitArray(new Uint8Array(attestingBits), 8); pool.add({...attestation, aggregationBits}, aggregationBits.getTrueBitIndexes().length, committee); - forkchoiceStub.findAttesterDependentRoot.returns(ZERO_HASH_HEX); + forkchoiceStub.getBlockHex.returns(generateEmptyProtoBlock()); + forkchoiceStub.getDependentRoot.returns(ZERO_HASH_HEX); expect(pool.getAttestationsForBlock(forkchoiceStub, altairState).length > 0).to.equal( isReturned, "Wrong attestation isReturned" ); - expect( - forkchoiceStub.findAttesterDependentRoot.calledOnce, - "forkchoice should be called to check pivot block" - ).to.equal(true); + expect(forkchoiceStub.getDependentRoot.calledOnce, "forkchoice should be called to check pivot block").to.equal( + true + ); }); } @@ -99,13 +100,13 @@ describe("AggregatedAttestationPool", function () { // all attesters are not seen const attestingIndices = [2, 3]; pool.add(attestation, attestingIndices.length, committee); - forkchoiceStub.findAttesterDependentRoot.returns("0xWeird"); + forkchoiceStub.getBlockHex.returns(generateEmptyProtoBlock()); + forkchoiceStub.getDependentRoot.returns("0xWeird"); expect(pool.getAttestationsForBlock(forkchoiceStub, altairState)).to.be.deep.equal( [], "no attestation since incorrect pivot block root" ); - expect(forkchoiceStub.findAttesterDependentRoot.calledOnce, "forkchoice should be called to check pivot block").to - .be.true; + expect(forkchoiceStub.getDependentRoot.calledOnce, "forkchoice should be called to check pivot block").to.be.true; }); }); diff --git a/packages/beacon-node/test/utils/mocks/chain/chain.ts b/packages/beacon-node/test/utils/mocks/chain/chain.ts index 8c8f886a2c79..afb3430b5a2b 100644 --- a/packages/beacon-node/test/utils/mocks/chain/chain.ts +++ b/packages/beacon-node/test/utils/mocks/chain/chain.ts @@ -286,7 +286,7 @@ function mockForkChoice(): IForkChoice { getBlockSummariesAtSlot: () => [block], getCommonAncestorDistance: () => null, validateLatestHash: () => {}, - findAttesterDependentRoot: () => block.blockRoot, + getDependentRoot: () => rootHex, }; } diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index f69f39767948..4d3344c53a75 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -34,7 +34,7 @@ import {ProtoArray} from "../protoArray/protoArray.js"; import {ProtoArrayError, ProtoArrayErrorCode} from "../protoArray/errors.js"; import {ForkChoiceError, ForkChoiceErrorCode, InvalidBlockCode, InvalidAttestationCode} from "./errors.js"; -import {IForkChoice, LatestMessage, QueuedAttestation, PowBlockHex} from "./interface.js"; +import {IForkChoice, LatestMessage, QueuedAttestation, PowBlockHex, EpochDifference} from "./interface.js"; import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalances} from "./store.js"; /* eslint-disable max-len */ @@ -773,40 +773,56 @@ export class ForkChoice implements IForkChoice { } /** - * Find dependent root of a head block - * epoch - 2 | epoch - 1 | epoch - * pivotSlot | - | blockSlot + * A dependent root is the block root of the last block before the state transition that decided a specific shuffling + * + * For proposer shuffling with 0 epochs of lookahead = previous immediate epoch transition + * For attester shuffling with 1 epochs of lookahead = last epoch's epoch transition + * + * ``` + * epoch: 0 1 2 3 4 + * |-------|-------|=======|-------| + * dependent root A -------------^ + * dependent root B -----^ + * ``` + * - proposer shuffling for a block in epoch 2: dependent root A (EpochDifference = 0) + * - attester shuffling for a block in epoch 2: dependent root B (EpochDifference = 1) */ - findAttesterDependentRoot(headBlockHash: Root): RootHex | null { - let block = this.getBlock(headBlockHash); - if (!block) return null; - const {slot} = block; - // The shuffling is determined by the block at the end of the target epoch - // minus the shuffling lookahead (usually 2). We call this the "pivot". - const pivotSlot = computeStartSlotAtEpoch(computeEpochAtSlot(slot) - 1) - 1; - - // 1st hop: target block - block = this.getBlockHex(block.targetRoot); - if (!block) return null; - if (block.slot <= pivotSlot) return block.blockRoot; - // or parent of target block - block = this.getBlockHex(block.parentRoot); - if (!block) return null; - if (block.slot <= pivotSlot) return block.blockRoot; - - // 2nd hop: go back 1 more epoch, target block - block = this.getBlockHex(block.targetRoot); - if (!block) return null; - if (block.slot <= pivotSlot) return block.blockRoot; - // or parent of target block - block = this.getBlockHex(block.parentRoot); - if (!block) return null; - // most of the time, in a stable network, it reaches here - if (block.slot <= pivotSlot) return block.blockRoot; - - throw Error( - `Not able to find attester dependent root for head ${headBlockHash}, slot ${slot}, pivotSlot ${pivotSlot}, last tried slot ${block.slot}` - ); + getDependentRoot(block: ProtoBlock, epochDifference: EpochDifference): RootHex { + // The navigation at the end of the while loop will always progress backwards, + // jumping to a block with a strictly less slot number. So the condition `blockEpoch < atEpoch` + // is guaranteed to happen. Given the use of target blocks for faster navigation, it will take + // at most `2 * (blockEpoch - atEpoch + 1)` iterations to find the dependant root. + + const beforeSlot = block.slot - (block.slot % SLOTS_PER_EPOCH) - epochDifference * SLOTS_PER_EPOCH; + + // Special case close to genesis block, return the genesis block root + if (beforeSlot <= 0) { + const genesisBlock = this.protoArray.nodes[0]; + if (genesisBlock === undefined || genesisBlock.slot !== 0) { + throw Error("Genesis block not available"); + } + return genesisBlock.blockRoot; + } + + // eslint-disable-next-line no-constant-condition + while (true) { + // Dependant root must be in epoch less than `beforeSlot` + if (block.slot < beforeSlot) { + return block.blockRoot; + } + + // Skip one last jump if there's no skipped slot at first slot of the epoch + if (block.slot === beforeSlot) { + return block.parentRoot; + } + + block = + block.blockRoot === block.targetRoot + ? // For the first slot of the epoch, a block is it's own target + this.protoArray.getBlockReadonly(block.parentRoot) + : // else we can navigate much faster jumping to the target block + this.protoArray.getBlockReadonly(block.targetRoot); + } } private getPreMergeExecStatus(executionStatus: MaybeValidExecutionStatus): ExecutionStatus.PreMerge { diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index 15f1e0feb86e..ee129b9e6a9f 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -19,6 +19,11 @@ export type CheckpointHexWithBalance = { balances: EffectiveBalanceIncrements; }; +export enum EpochDifference { + current = 0, + previous = 1, +} + export interface IForkChoice { irrecoverableError?: Error; /** @@ -157,9 +162,11 @@ export interface IForkChoice { * Optimistic sync validate till validated latest hash, invalidate any decendant branch if invalidated branch decendant provided */ validateLatestHash(execResponse: LVHExecResponse): void; - /** Find attester dependent root of a block */ - findAttesterDependentRoot(headBlockHash: Root): RootHex | null; - /** Get critical error from forkChoice */ + + /** + * A dependent root is the block root of the last block before the state transition that decided a specific shuffling + */ + getDependentRoot(block: ProtoBlock, atEpochDiff: EpochDifference): RootHex; } /** Same to the PowBlock but we want RootHex to work with forkchoice conveniently */ diff --git a/packages/fork-choice/src/index.ts b/packages/fork-choice/src/index.ts index 95ee2cd5b9d3..c8af2b5d6f4b 100644 --- a/packages/fork-choice/src/index.ts +++ b/packages/fork-choice/src/index.ts @@ -10,7 +10,7 @@ export { } from "./protoArray/interface.js"; export {ForkChoice, ForkChoiceOpts, assertValidTerminalPowBlock} from "./forkChoice/forkChoice.js"; -export {IForkChoice, PowBlockHex} from "./forkChoice/interface.js"; +export {IForkChoice, PowBlockHex, EpochDifference} from "./forkChoice/interface.js"; export {ForkChoiceStore, IForkChoiceStore, CheckpointWithHex, JustifiedBalancesGetter} from "./forkChoice/store.js"; export { InvalidAttestation, diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 6476dd3c3841..a71a3b85bb7c 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -867,6 +867,7 @@ export class ProtoArray { return this.getNodeByIndex(blockIndex); } + /** Return MUTABLE ProtoBlock for blockRoot (spreads properties) */ getBlock(blockRoot: RootHex): ProtoBlock | undefined { const node = this.getNode(blockRoot); if (!node) { @@ -877,6 +878,15 @@ export class ProtoArray { }; } + /** Return NON-MUTABLE ProtoBlock for blockRoot (does not spread properties) */ + getBlockReadonly(blockRoot: RootHex): ProtoBlock { + const node = this.getNode(blockRoot); + if (!node) { + throw Error(`No block for root ${blockRoot}`); + } + return node; + } + /** * Returns `true` if the `descendantRoot` has an ancestor with `ancestorRoot`. * Always returns `false` if either input roots are unknown. diff --git a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts index 2309b94bc3d6..9834bad2ebd9 100644 --- a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts @@ -4,7 +4,14 @@ import {fromHexString} from "@chainsafe/ssz"; import {RootHex} from "@lodestar/types"; import {computeEpochAtSlot} from "@lodestar/state-transition"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; -import {ForkChoice, IForkChoiceStore, ProtoBlock, ProtoArray, ExecutionStatus} from "../../../src/index.js"; +import { + ForkChoice, + IForkChoiceStore, + ProtoBlock, + ProtoArray, + ExecutionStatus, + EpochDifference, +} from "../../../src/index.js"; describe("Forkchoice", function () { const genesisSlot = 0; @@ -139,13 +146,20 @@ describe("Forkchoice", function () { ]; for (const {name, skippedSlots, pivotSlot} of dependentRootTestCases) { - it(`findAttesterDependentRoot - ${name}`, () => { + it(`getDependentRoot (EpochDifference.previous) - ${name}`, () => { const slot = 2 * 32 + 5; populateProtoArray(slot, skippedSlots); const forkchoice = new ForkChoice(config, fcStore, protoArr); + + // Initial point const blockRoot = getBlockRoot(slot); + const block = forkchoice.getBlockHex(blockRoot); + if (!block) throw Error(`No block for blockRoot ${blockRoot}`); + + // Expected const pivotRoot = getBlockRoot(pivotSlot); - expect(forkchoice.findAttesterDependentRoot(fromHexString(blockRoot))).to.be.equal( + + expect(forkchoice.getDependentRoot(block, EpochDifference.previous)).to.be.equal( pivotRoot, "incorrect attester dependent root" );