rebasing optmistic, refac of the execution management in protoblock

packages/fork-choice/src/forkChoice/forkChoice.ts

@@ -11,7 +11,7 @@ import {
 import {IChainConfig, IChainForkConfig} from "@chainsafe/lodestar-config";
 
 import {computeDeltas} from "../protoArray/computeDeltas";
-import {HEX_ZERO_HASH, IVoteTracker, IProtoBlock} from "../protoArray/interface";
+import {HEX_ZERO_HASH, IVoteTracker, IProtoBlock, ExecutionStatus} from "../protoArray/interface";
 import {ProtoArray} from "../protoArray/protoArray";
 
 import {IForkChoiceMetrics} from "../metrics";
@@ -359,6 +359,12 @@ export class ForkChoice implements IForkChoice {
     const blockRoot = this.config.getForkTypes(slot).BeaconBlock.hashTreeRoot(block);
     const targetRoot = slot === targetSlot ? blockRoot : state.blockRoots[targetSlot % SLOTS_PER_HISTORICAL_ROOT];
 
+    // let executionPayload={executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge};
+    // if(merge.isMergeBlockBodyType(block.body)){
+    //   if(executionStatus===ExecutionStatus.PreMerge) throw Error("Expecting post-merge executionStatus");
+    //   executionPayload={executionPayloadBlockHash: toHexString(block.body.executionPayload.blockHash), executionStatus}
+    // }
+
     // This does not apply a vote to the block, it just makes fork choice aware of the block so
     // it can still be identified as the head even if it doesn't have any votes.
     this.protoArray.onBlock({
@@ -367,17 +373,18 @@ export class ForkChoice implements IForkChoice {
       parentRoot: parentRootHex,
       targetRoot: toHexString(targetRoot),
       stateRoot: toHexString(block.stateRoot),
-      executionPayloadBlockHash: merge.isMergeBlockBodyType(block.body)
-        ? toHexString(block.body.executionPayload.blockHash)
-        : null,
+
       justifiedEpoch: stateJustifiedEpoch,
       justifiedRoot: toHexString(state.currentJustifiedCheckpoint.root),
       finalizedEpoch: finalizedCheckpoint.epoch,
       finalizedRoot: toHexString(state.finalizedCheckpoint.root),
 
-      // Optimistic sync based status to verify/prune later
-      payloadStatusUnknown: preCachedData?.payloadStatusUnknown,
-      isMergeBlock: preCachedData?.isMergeBlock,
+      ...(merge.isMergeBlockBodyType(block.body)
+        ? {
+            executionPayloadBlockHash: toHexString(block.body.executionPayload.blockHash),
+            executionStatus: this.getPostMergeExecStatus(preCachedData),
+          }
+        : {executionPayloadBlockHash: null, executionStatus: this.getPreMergeExecStatus(preCachedData)}),
     });
   }
 
@@ -643,6 +650,20 @@ export class ForkChoice implements IForkChoice {
     // Silently ignore for now if all calls were valid
   }
 
+  private getPreMergeExecStatus(preCachedData?: OnBlockPrecachedData): ExecutionStatus.PreMerge {
+    const executionStatus = preCachedData?.executionStatus || ExecutionStatus.PreMerge;
+    if (executionStatus !== ExecutionStatus.PreMerge) throw Error("Invalid pre-merge execution status");
+    return executionStatus;
+  }
+
+  private getPostMergeExecStatus(
+    preCachedData?: OnBlockPrecachedData
+  ): ExecutionStatus.Valid | ExecutionStatus.Syncing {
+    const executionStatus = preCachedData?.executionStatus || ExecutionStatus.Syncing;
+    if (executionStatus === ExecutionStatus.PreMerge) throw Error("Invalid post-merge execution status");
+    return executionStatus;
+  }
+
   private updateJustified(justifiedCheckpoint: CheckpointWithHex, justifiedBalances: number[]): void {
     this.synced = false;
     this.justifiedBalances = justifiedBalances;
@@ -938,7 +959,7 @@ function assertValidTerminalPowBlock(
     // If no TERMINAL_BLOCK_HASH override, check ttd
 
     // Delay powBlock checks if the payload execution status is unknown because of syncing response in executePayload call while verifying
-    if (preCachedData?.payloadStatusUnknown) return;
+    if (preCachedData?.executionStatus === ExecutionStatus.Syncing) return;
 
     const {powBlock, powBlockParent} = preCachedData || {};
     if (!powBlock) throw Error("onBlock preCachedData must include powBlock");

packages/fork-choice/src/forkChoice/interface.ts

@@ -1,5 +1,5 @@
 import {Epoch, Slot, ValidatorIndex, phase0, allForks, Root, RootHex} from "@chainsafe/lodestar-types";
-import {IProtoBlock} from "../protoArray/interface";
+import {IProtoBlock, ExecutionStatus} from "../protoArray/interface";
 import {CheckpointWithHex} from "./store";
 
 export type CheckpointHex = {
@@ -166,7 +166,7 @@ export type OnBlockPrecachedData = {
    * Optimistic sync fields
    */
   isMergeBlock?: boolean;
-  payloadStatusUnknown?: boolean;
+  executionStatus?: ExecutionStatus;
 };
 
 export interface ILatestMessage {

packages/fork-choice/src/index.ts

@@ -1,5 +1,5 @@
 export {ProtoArray} from "./protoArray/protoArray";
-export {IProtoBlock, IProtoNode} from "./protoArray/interface";
+export {IProtoBlock, IProtoNode, ExecutionStatus} from "./protoArray/interface";
 
 export {ForkChoice} from "./forkChoice/forkChoice";
 export {

packages/fork-choice/src/protoArray/interface.ts

@@ -13,12 +13,22 @@ export interface IVoteTracker {
   nextEpoch: Epoch;
 }
 
+export enum ExecutionStatus {
+  Valid,
+  Syncing,
+  PreMerge,
+}
+
+type BlockExecution =
+  | {executionPayloadBlockHash: RootHex; executionStatus: ExecutionStatus.Valid | ExecutionStatus.Syncing}
+  | {executionPayloadBlockHash: null; executionStatus: ExecutionStatus.PreMerge};
 /**
  * A block that is to be applied to the fork choice
  *
  * A simplified version of BeaconBlock
  */
-export interface IProtoBlock {
+
+export type IProtoBlock = BlockExecution & {
   /**
    * The slot is not necessary for ProtoArray,
    * it just exists so external components can easily query the block slot.
@@ -39,30 +49,20 @@ export interface IProtoBlock {
    * it also just exists for upstream components (namely attestation verification)
    */
   targetRoot: RootHex;
-  /**
-   * `executionPayloadBlockHash` is not necessary for ProtoArray either.
-   * Here to do ExecutionEngine.notify_forkchoice_updated() easier.
-   * TODO: Check with other teams if this is the optimal strategy
-   */
-  executionPayloadBlockHash: RootHex | null;
+
   justifiedEpoch: Epoch;
   justifiedRoot: RootHex;
   finalizedEpoch: Epoch;
   finalizedRoot: RootHex;
-  /**
-   * Optimistic sync based statuses
-   */
-  payloadStatusUnknown?: boolean;
-  isMergeBlock?: boolean;
-}
+};
 
 /**
  * A block root with additional metadata required to form a DAG
  * with vote weights and best blocks stored as metadata
  */
-export interface IProtoNode extends IProtoBlock {
+export type IProtoNode = IProtoBlock & {
   parent?: number;
   weight: number;
   bestChild?: number;
   bestDescendant?: number;
-}
+};

packages/fork-choice/src/protoArray/protoArray.ts

@@ -50,7 +50,7 @@ export class ProtoArray {
       ...block,
       // We are using the blockROot as the targetRoot, since it always lies on an epoch boundary
       targetRoot: block.blockRoot,
-    });
+    } as IProtoBlock);
     return protoArray;
   }
 

packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts

@@ -1,4 +1,4 @@
-import {ForkChoice, IForkChoiceStore, IProtoBlock, ProtoArray} from "../../../src";
+import {ForkChoice, IForkChoiceStore, IProtoBlock, ProtoArray, ExecutionStatus} from "../../../src";
 import {config} from "@chainsafe/lodestar-config/default";
 import {expect} from "chai";
 import {fromHexString} from "@chainsafe/ssz";
@@ -18,12 +18,15 @@ describe("Forkchoice", function () {
     stateRoot,
     parentRoot,
     blockRoot: finalizedRoot,
-    executionPayloadBlockHash: null,
+
     justifiedEpoch: genesisEpoch,
     justifiedRoot: genesisRoot,
     finalizedEpoch: genesisEpoch,
     finalizedRoot: genesisRoot,
-  });
+
+    executionPayloadBlockHash: null,
+    executionStatus: ExecutionStatus.PreMerge,
+  } as Omit<IProtoBlock, "targetRoot">);
 
   // Add block that is a finalized descendant.
   const block: IProtoBlock = {
@@ -32,11 +35,14 @@ describe("Forkchoice", function () {
     parentRoot: finalizedRoot,
     stateRoot,
     targetRoot: finalizedRoot,
-    executionPayloadBlockHash: null,
+
     justifiedEpoch: genesisEpoch,
     justifiedRoot: genesisRoot,
     finalizedEpoch: genesisEpoch,
     finalizedRoot: genesisRoot,
+
+    executionPayloadBlockHash: null,
+    executionStatus: ExecutionStatus.PreMerge,
   };
 
   const fcStore: IForkChoiceStore = {

packages/fork-choice/test/unit/protoArray/getCommonAncestor.test.ts

@@ -1,5 +1,5 @@
 import {expect} from "chai";
-import {ProtoArray} from "../../../src";
+import {ProtoArray, ExecutionStatus} from "../../../src";
 
 describe("getCommonAncestor", () => {
   const blocks: {slot: number; root: string; parent: string}[] = [
@@ -29,11 +29,13 @@ describe("getCommonAncestor", () => {
     stateRoot: "-",
     parentRoot: "-",
     blockRoot: "0",
-    executionPayloadBlockHash: null,
+
     justifiedEpoch: 0,
     justifiedRoot: "-",
     finalizedEpoch: 0,
     finalizedRoot: "-",
+
+    ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
   });
 
   for (const block of blocks) {
@@ -43,11 +45,13 @@ describe("getCommonAncestor", () => {
       parentRoot: block.parent,
       stateRoot: "-",
       targetRoot: "-",
-      executionPayloadBlockHash: null,
+
       justifiedEpoch: 0,
       justifiedRoot: "-",
       finalizedEpoch: 0,
       finalizedRoot: "-",
+
+      ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
     });
   }
 

packages/fork-choice/test/unit/protoArray/protoArray.test.ts

@@ -1,7 +1,7 @@
 import {RootHex} from "@chainsafe/lodestar-types";
 import {expect} from "chai";
 
-import {ProtoArray} from "../../../src";
+import {ProtoArray, ExecutionStatus} from "../../../src";
 
 describe("ProtoArray", () => {
   it("finalized descendant", () => {
@@ -19,11 +19,13 @@ describe("ProtoArray", () => {
       stateRoot,
       parentRoot,
       blockRoot: finalizedRoot,
-      executionPayloadBlockHash: null,
+
       justifiedEpoch: genesisEpoch,
       justifiedRoot: stateRoot,
       finalizedEpoch: genesisEpoch,
       finalizedRoot: stateRoot,
+
+      ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
     });
 
     // Add block that is a finalized descendant.
@@ -33,11 +35,13 @@ describe("ProtoArray", () => {
       parentRoot: finalizedRoot,
       stateRoot,
       targetRoot: finalizedRoot,
-      executionPayloadBlockHash: null,
+
       justifiedEpoch: genesisEpoch,
       justifiedRoot: stateRoot,
       finalizedEpoch: genesisEpoch,
       finalizedRoot: stateRoot,
+
+      ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
     });
 
     // Add block that is *not* a finalized descendant.
@@ -47,11 +51,13 @@ describe("ProtoArray", () => {
       parentRoot: unknown,
       stateRoot,
       targetRoot: finalizedRoot,
-      executionPayloadBlockHash: null,
+
       justifiedEpoch: genesisEpoch,
       justifiedRoot: stateRoot,
       finalizedEpoch: genesisEpoch,
       finalizedRoot: stateRoot,
+
+      ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
     });
 
     // ancestorRoot, descendantRoot, isDescendant

packages/lodestar/src/chain/blocks/importBlock.ts

@@ -9,7 +9,7 @@ import {
   altair,
   computeEpochAtSlot,
 } from "@chainsafe/lodestar-beacon-state-transition";
-import {IForkChoice, OnBlockPrecachedData} from "@chainsafe/lodestar-fork-choice";
+import {IForkChoice, OnBlockPrecachedData, ExecutionStatus} from "@chainsafe/lodestar-fork-choice";
 import {ILogger} from "@chainsafe/lodestar-utils";
 import {IChainForkConfig} from "@chainsafe/lodestar-config";
 import {IMetrics} from "../../metrics";
@@ -59,7 +59,7 @@ export type ImportBlockModules = {
  * - Send events after everything is done
  */
 export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock: FullyVerifiedBlock): Promise<void> {
-  const {block, postState, parentBlock, skipImportingAttestations, payloadStatusUnknown} = fullyVerifiedBlock;
+  const {block, postState, parentBlock, skipImportingAttestations, executionStatus} = fullyVerifiedBlock;
   const pendingEvents = new PendingEvents(chain.emitter);
 
   // - Observe attestations
@@ -79,7 +79,7 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
   // current justified checkpoint should be prev epoch or current epoch if it's just updated
   // it should always have epochBalances there bc it's a checkpoint state, ie got through processEpoch
   const justifiedCheckpoint = postState.currentJustifiedCheckpoint;
-  const onBlockPrecachedData: OnBlockPrecachedData = {payloadStatusUnknown};
+  const onBlockPrecachedData: OnBlockPrecachedData = {executionStatus};
   if (justifiedCheckpoint.epoch > chain.forkChoice.getJustifiedCheckpoint().epoch) {
     const state = getStateForJustifiedBalances(chain, postState, block);
     onBlockPrecachedData.justifiedBalances = getEffectiveBalances(state);
@@ -93,7 +93,8 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
     // pow_block = get_pow_block(block.body.execution_payload.parent_hash)
     const powBlockRootHex = toHexString(block.message.body.executionPayload.parentHash);
     const powBlock = await chain.eth1.getPowBlock(powBlockRootHex);
-    if (!powBlock && !payloadStatusUnknown) throw Error(`merge block parent POW block not found ${powBlockRootHex}`);
+    if (!powBlock && executionStatus !== ExecutionStatus.Syncing)
+      throw Error(`merge block parent POW block not found ${powBlockRootHex}`);
     // pow_parent = get_pow_block(pow_block.parent_hash)
     const powBlockParent = powBlock && (await chain.eth1.getPowBlock(powBlock.parentHash));
     if (powBlock && !powBlockParent)

packages/lodestar/src/chain/blocks/types.ts

@@ -1,5 +1,5 @@
 import {CachedBeaconState} from "@chainsafe/lodestar-beacon-state-transition";
-import {IProtoBlock} from "@chainsafe/lodestar-fork-choice";
+import {IProtoBlock, ExecutionStatus} from "@chainsafe/lodestar-fork-choice";
 import {allForks} from "@chainsafe/lodestar-types";
 
 export type FullyVerifiedBlockFlags = {
@@ -22,7 +22,7 @@ export type FullyVerifiedBlockFlags = {
   /**
    * If the execution payload couldnt be verified because of EL syncing status, used in optimistic sync or for merge block
    */
-  payloadStatusUnknown?: boolean;
+  executionStatus?: ExecutionStatus;
 };
 
 export type PartiallyVerifiedBlockFlags = FullyVerifiedBlockFlags & {