Replace findAttesterDependentRoot with getDependantRoot (#4555)

* Add getDependantRoot

* Replace findAttesterDependentRoot with getDependantRoot

* Remove findAttesterDependentRoot

* Update tests

* Rename to dependent

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 {

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;
   });
 });
 

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,
   };
 }
 

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 {

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 */

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,

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.

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"
       );