Update forkchoice using latestValidHash updates from execution engine

packages/beacon-node/src/chain/blocks/index.ts

@@ -68,13 +68,23 @@ export async function processBlocks(
 
     // Fully verify a block to be imported immediately after. Does not produce any side-effects besides adding intermediate
     // states in the state cache through regen.
-    const {postStates, executionStatuses, proposerBalanceDeltas} = await verifyBlocksInEpoch.call(
+    const {postStates, proposerBalanceDeltas, segmentExecStatus} = await verifyBlocksInEpoch.call(
       this,
       parentBlock,
       relevantBlocks,
       opts
     );
 
+    // If segmentExecStatus has lvhForkchoice then, the entire segment should be invalid
+    // and we need to further propagate
+    if (segmentExecStatus.execAborted !== null) {
+      if (segmentExecStatus.invalidSegmentLHV !== undefined) {
+        this.forkChoice.validateLatestHash(segmentExecStatus.invalidSegmentLHV);
+      }
+      throw segmentExecStatus.execAborted.execError;
+    }
+
+    const {executionStatuses} = segmentExecStatus;
     const fullyVerifiedBlocks = relevantBlocks.map(
       (block, i): FullyVerifiedBlock => ({
         block,

packages/beacon-node/src/chain/blocks/types.ts

@@ -1,5 +1,5 @@
 import {CachedBeaconStateAllForks} from "@lodestar/state-transition";
-import {ExecutionStatus} from "@lodestar/fork-choice";
+import {MaybeValidExecutionStatus} from "@lodestar/fork-choice";
 import {allForks, Slot} from "@lodestar/types";
 
 export type ImportBlockOpts = {
@@ -48,9 +48,10 @@ export type FullyVerifiedBlock = {
   parentBlockSlot: Slot;
   proposerBalanceDelta: number;
   /**
-   * If the execution payload couldnt be verified because of EL syncing status, used in optimistic sync or for merge block
+   * If the execution payload couldnt be verified because of EL syncing status,
+   * used in optimistic sync or for merge block
    */
-  executionStatus: ExecutionStatus;
+  executionStatus: MaybeValidExecutionStatus;
   /** Seen timestamp seconds */
   seenTimestampSec: number;
 };

packages/beacon-node/src/chain/blocks/verifyBlock.ts

@@ -1,7 +1,7 @@
 import {CachedBeaconStateAllForks, computeEpochAtSlot} from "@lodestar/state-transition";
 import {allForks, bellatrix} from "@lodestar/types";
 import {toHexString} from "@chainsafe/ssz";
-import {ExecutionStatus, ProtoBlock} from "@lodestar/fork-choice";
+import {ProtoBlock} from "@lodestar/fork-choice";
 import {IChainForkConfig} from "@lodestar/config";
 import {ILogger} from "@lodestar/utils";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
@@ -12,7 +12,7 @@ import {ImportBlockOpts} from "./types.js";
 import {POS_PANDA_MERGE_TRANSITION_BANNER} from "./utils/pandaMergeTransitionBanner.js";
 import {verifyBlocksStateTransitionOnly} from "./verifyBlocksStateTransitionOnly.js";
 import {verifyBlocksSignatures} from "./verifyBlocksSignatures.js";
-import {verifyBlocksExecutionPayload} from "./verifyBlocksExecutionPayloads.js";
+import {verifyBlocksExecutionPayload, SegmentExecStatus} from "./verifyBlocksExecutionPayloads.js";
 
 /**
  * Verifies 1 or more blocks are fully valid; from a linear sequence of blocks.
@@ -32,8 +32,8 @@ export async function verifyBlocksInEpoch(
   opts: BlockProcessOpts & ImportBlockOpts
 ): Promise<{
   postStates: CachedBeaconStateAllForks[];
-  executionStatuses: ExecutionStatus[];
   proposerBalanceDeltas: number[];
+  segmentExecStatus: SegmentExecStatus;
 }> {
   if (blocks.length === 0) {
     throw Error("Empty partiallyVerifiedBlocks");
@@ -64,7 +64,7 @@ export async function verifyBlocksInEpoch(
   const abortController = new AbortController();
 
   try {
-    const [{postStates, proposerBalanceDeltas}, , {executionStatuses, mergeBlockFound}] = await Promise.all([
+    const [{postStates, proposerBalanceDeltas}, , segmentExecStatus] = await Promise.all([
       // Run state transition only
       // TODO: Ensure it yields to allow flushing to workers and engine API
       verifyBlocksStateTransitionOnly(preState0, blocks, this.metrics, abortController.signal, opts),
@@ -76,13 +76,13 @@ export async function verifyBlocksInEpoch(
       verifyBlocksExecutionPayload(this, parentBlock, blocks, preState0, abortController.signal, opts),
     ]);
 
-    if (mergeBlockFound !== null) {
+    if (segmentExecStatus.execAborted === null && segmentExecStatus.mergeBlockFound !== null) {
       // merge block found and is fully valid = state transition + signatures + execution payload.
       // TODO: Will this banner be logged during syncing?
-      logOnPowBlock(this.logger, this.config, mergeBlockFound);
+      logOnPowBlock(this.logger, this.config, segmentExecStatus.mergeBlockFound);
     }
 
-    return {postStates, executionStatuses, proposerBalanceDeltas};
+    return {postStates, proposerBalanceDeltas, segmentExecStatus};
   } finally {
     abortController.abort();
   }

packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts

@@ -7,7 +7,15 @@ import {
 } from "@lodestar/state-transition";
 import {bellatrix, allForks, Slot} from "@lodestar/types";
 import {toHexString} from "@chainsafe/ssz";
-import {IForkChoice, ExecutionStatus, assertValidTerminalPowBlock, ProtoBlock} from "@lodestar/fork-choice";
+import {
+  IForkChoice,
+  assertValidTerminalPowBlock,
+  ProtoBlock,
+  ExecutionStatus,
+  MaybeValidExecutionStatus,
+  LVHValidResponse,
+  LVHInvalidResponse,
+} from "@lodestar/fork-choice";
 import {IChainForkConfig} from "@lodestar/config";
 import {ErrorAborted, ILogger} from "@lodestar/utils";
 import {IExecutionEngine} from "../../execution/engine/index.js";
@@ -26,6 +34,25 @@ export type VerifyBlockExecutionPayloadModules = {
   config: IChainForkConfig;
 };
 
+type ExecAbortType = {blockIndex: number; execError: BlockError};
+export type SegmentExecStatus =
+  | {
+      execAborted: null;
+      executionStatuses: MaybeValidExecutionStatus[];
+      mergeBlockFound: bellatrix.BeaconBlock | null;
+    }
+  | {execAborted: ExecAbortType; invalidSegmentLHV?: LVHInvalidResponse; mergeBlockFound: null};
+
+type VerifyExecutionErrorResponse =
+  | {executionStatus: ExecutionStatus.Invalid; lvhResponse: LVHInvalidResponse; execError: BlockError}
+  | {executionStatus: null; lvhResponse: undefined; execError: BlockError};
+
+type VerifyBlockExecutionResponse =
+  | VerifyExecutionErrorResponse
+  | {executionStatus: ExecutionStatus.Valid; lvhResponse: LVHValidResponse; execError: null}
+  | {executionStatus: ExecutionStatus.Syncing; lvhResponse?: LVHValidResponse; execError: null}
+  | {executionStatus: ExecutionStatus.PreMerge; lvhResponse: undefined; execError: null};
+
 /**
  * Verifies 1 or more execution payloads from a linear sequence of blocks.
  *
@@ -38,8 +65,8 @@ export async function verifyBlocksExecutionPayload(
   preState0: CachedBeaconStateAllForks,
   signal: AbortSignal,
   opts: BlockProcessOpts
-): Promise<{executionStatuses: ExecutionStatus[]; mergeBlockFound: bellatrix.BeaconBlock | null}> {
-  const executionStatuses: ExecutionStatus[] = [];
+): Promise<SegmentExecStatus> {
+  const executionStatuses: MaybeValidExecutionStatus[] = [];
   let mergeBlockFound: bellatrix.BeaconBlock | null = null;
 
   // Error in the same way as verifyBlocksSanityChecks if empty blocks
@@ -114,14 +141,14 @@ export async function verifyBlocksExecutionPayload(
     parentBlock.executionStatus !== ExecutionStatus.PreMerge ||
     lastBlock.message.slot + opts.safeSlotsToImportOptimistically < currentSlot;
 
-  for (const block of blocks) {
+  for (let blockIndex = 0; blockIndex < blocks.length; blockIndex++) {
+    const block = blocks[blockIndex];
     // If blocks are invalid in consensus the main promise could resolve before this loop ends.
     // In that case stop sending blocks to execution engine
     if (signal.aborted) {
       throw new ErrorAborted("verifyBlockExecutionPayloads");
     }
-
-    const {executionStatus} = await verifyBlockExecutionPayload(
+    const verifyResponse = await verifyBlockExecutionPayload(
       chain,
       block,
       preState0,
@@ -130,9 +157,16 @@ export async function verifyBlocksExecutionPayload(
       currentSlot
     );
 
+    // If execError has happened, then we need to extract the segmentExecStatus and return
+    if (verifyResponse.execError !== null) {
+      return getSegmentErrorResponse({verifyResponse, blockIndex}, parentBlock, blocks);
+    }
+
+    // If we are here then its because executionStatus is one of MaybeValidExecutionStatus
+    const {executionStatus} = verifyResponse;
     // It becomes optimistically  safe for following blocks if a post-merge block is deemed fit
-    // for import. If it would not have been safe verifyBlockExecutionPayload would throw error
-    // and we would not be here.
+    // for import. If it would not have been safe verifyBlockExecutionPayload would have
+    // returned execError and loop would have been aborted
     if (executionStatus !== ExecutionStatus.PreMerge) {
       isOptimisticallySafe = true;
     }
@@ -196,14 +230,17 @@ export async function verifyBlocksExecutionPayload(
       }
 
       assertValidTerminalPowBlock(chain.config, mergeBlock, {executionStatus, powBlock, powBlockParent});
-
       // Valid execution payload, but may not be in a valid beacon chain block. Delay printing the POS ACTIVATED banner
       // to the end of the verify block routine, which confirms that this block is fully valid.
       mergeBlockFound = mergeBlock;
     }
   }
 
-  return {executionStatuses, mergeBlockFound};
+  return {
+    execAborted: null,
+    executionStatuses,
+    mergeBlockFound,
+  };
 }
 
 /**
@@ -216,7 +253,7 @@ export async function verifyBlockExecutionPayload(
   opts: BlockProcessOpts,
   isOptimisticallySafe: boolean,
   currentSlot: Slot
-): Promise<{executionStatus: ExecutionStatus}> {
+): Promise<VerifyBlockExecutionResponse> {
   /** Not null if execution is enabled */
   const executionPayloadEnabled =
     isBellatrixStateType(preState0) &&
@@ -232,30 +269,32 @@ export async function verifyBlockExecutionPayload(
 
   if (!executionPayloadEnabled) {
     // isExecutionEnabled() -> false
-    return {executionStatus: ExecutionStatus.PreMerge};
+    return {executionStatus: ExecutionStatus.PreMerge, execError: null} as VerifyBlockExecutionResponse;
   }
 
   // TODO: Handle better notifyNewPayload() returning error is syncing
   const execResult = await chain.executionEngine.notifyNewPayload(executionPayloadEnabled);
 
   switch (execResult.status) {
-    case ExecutePayloadStatus.VALID:
-      chain.forkChoice.validateLatestHash(execResult.latestValidHash, null);
-      return {executionStatus: ExecutionStatus.Valid};
+    case ExecutePayloadStatus.VALID: {
+      const executionStatus: ExecutionStatus.Valid = ExecutionStatus.Valid;
+      const lvhResponse = {executionStatus, latestValidExecHash: execResult.latestValidHash};
+      return {executionStatus, lvhResponse, execError: null};
+    }
 
     case ExecutePayloadStatus.INVALID: {
-      // If the parentRoot is not same as latestValidHash, then the branch from latestValidHash
-      // to parentRoot needs to be invalidated
-      const parentHashHex = toHexString(block.message.parentRoot);
-      chain.forkChoice.validateLatestHash(
-        execResult.latestValidHash,
-        parentHashHex !== execResult.latestValidHash ? parentHashHex : null
-      );
-      throw new BlockError(block, {
+      const executionStatus: ExecutionStatus.Invalid = ExecutionStatus.Invalid;
+      const lvhResponse = {
+        executionStatus,
+        latestValidExecHash: execResult.latestValidHash,
+        invalidateFromBlockHash: toHexString(block.message.parentRoot),
+      };
+      const execError = new BlockError(block, {
         code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
         execStatus: execResult.status,
         errorMessage: execResult.validationError ?? "",
       });
+      return {executionStatus, lvhResponse, execError};
     }
 
     // Accepted and Syncing have the same treatment, as final validation of block is pending
@@ -265,14 +304,15 @@ export async function verifyBlockExecutionPayload(
       // the safeSlotsToImportOptimistically window of current slot, then we can import else
       // we need to throw and not import his block
       if (!isOptimisticallySafe && block.message.slot + opts.safeSlotsToImportOptimistically >= currentSlot) {
-        throw new BlockError(block, {
+        const execError = new BlockError(block, {
           code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
           execStatus: ExecutePayloadStatus.UNSAFE_OPTIMISTIC_STATUS,
           errorMessage: `not safe to import ${execResult.status} payload within ${opts.safeSlotsToImportOptimistically} of currentSlot`,
         });
+        return {executionStatus: null, execError} as VerifyBlockExecutionResponse;
       }
 
-      return {executionStatus: ExecutionStatus.Syncing};
+      return {executionStatus: ExecutionStatus.Syncing, execError: null};
     }
 
     // If the block has is not valid, or it referenced an invalid terminal block then the
@@ -294,11 +334,58 @@ export async function verifyBlockExecutionPayload(
 
     case ExecutePayloadStatus.INVALID_BLOCK_HASH:
     case ExecutePayloadStatus.ELERROR:
-    case ExecutePayloadStatus.UNAVAILABLE:
-      throw new BlockError(block, {
+    case ExecutePayloadStatus.UNAVAILABLE: {
+      const execError = new BlockError(block, {
         code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
         execStatus: execResult.status,
         errorMessage: execResult.validationError,
       });
+      return {executionStatus: null, execError} as VerifyBlockExecutionResponse;
+    }
+  }
+}
+
+function getSegmentErrorResponse(
+  {verifyResponse, blockIndex}: {verifyResponse: VerifyExecutionErrorResponse; blockIndex: number},
+  parentBlock: ProtoBlock,
+  blocks: allForks.SignedBeaconBlock[]
+): SegmentExecStatus {
+  const {executionStatus, lvhResponse, execError} = verifyResponse;
+  let invalidSegmentLHV: LVHInvalidResponse | undefined = undefined;
+
+  if (
+    executionStatus === ExecutionStatus.Invalid &&
+    lvhResponse !== undefined &&
+    lvhResponse.latestValidExecHash !== null
+  ) {
+    let lvhFound = false;
+    for (let mayBeLVHIndex = blockIndex - 1; mayBeLVHIndex >= 0; mayBeLVHIndex--) {
+      const block = blocks[mayBeLVHIndex];
+      if (
+        toHexString((block.message.body as bellatrix.BeaconBlockBody).executionPayload.blockHash) ===
+        lvhResponse.latestValidExecHash
+      ) {
+        lvhFound = true;
+        break;
+      }
+    }
+
+    // If there is no valid in the segment then we have to propagate invalid response
+    // in forkchoice as well if
+    //  - if the parentBlock is also not the lvh
+    //  - and parentBlock is not pre merhe
+    if (
+      !lvhFound &&
+      parentBlock.executionStatus !== ExecutionStatus.PreMerge &&
+      parentBlock.executionPayloadBlockHash !== lvhResponse.latestValidExecHash
+    ) {
+      invalidSegmentLHV = {
+        executionStatus: ExecutionStatus.Invalid,
+        latestValidExecHash: lvhResponse.latestValidExecHash,
+        invalidateFromBlockHash: parentBlock.blockRoot,
+      };
+    }
   }
+  const execAborted = {blockIndex, execError};
+  return {execAborted, invalidSegmentLHV} as SegmentExecStatus;
 }

packages/beacon-node/src/chain/chain.ts

@@ -14,6 +14,8 @@ import {
 import {IBeaconConfig} from "@lodestar/config";
 import {allForks, UintNum64, Root, phase0, Slot, RootHex, Epoch, ValidatorIndex} from "@lodestar/types";
 import {CheckpointWithHex, ExecutionStatus, IForkChoice, ProtoBlock} from "@lodestar/fork-choice";
+import {ProcessShutdownCallback} from "@lodestar/validator";
+
 import {ILogger, toHex} from "@lodestar/utils";
 import {CompositeTypeAny, fromHexString, TreeView, Type} from "@chainsafe/ssz";
 import {SLOTS_PER_EPOCH} from "@lodestar/params";
@@ -117,13 +119,15 @@ export class BeaconChain implements IBeaconChain {
 
   private readonly faultInspectionWindow: number;
   private readonly allowedFaults: number;
+  private processShutdownCallback: ProcessShutdownCallback;
 
   constructor(
     opts: IChainOptions,
     {
       config,
       db,
       logger,
+      processShutdownCallback,
       clock,
       metrics,
       anchorState,
@@ -134,6 +138,7 @@ export class BeaconChain implements IBeaconChain {
       config: IBeaconConfig;
       db: IBeaconDb;
       logger: ILogger;
+      processShutdownCallback: ProcessShutdownCallback;
       /** Used for testing to supply fake clock */
       clock?: IBeaconClock;
       metrics: IMetrics | null;
@@ -147,6 +152,7 @@ export class BeaconChain implements IBeaconChain {
     this.config = config;
     this.db = db;
     this.logger = logger;
+    this.processShutdownCallback = processShutdownCallback;
     this.metrics = metrics;
     this.genesisTime = anchorState.genesisTime;
     this.anchorStateLatestBlockSlot = anchorState.latestBlockHeader.slot;
@@ -523,6 +529,9 @@ export class BeaconChain implements IBeaconChain {
     this.logger.verbose("Clock slot", {slot});
 
     // CRITICAL UPDATE
+    if (this.forkChoice.irrecoverableError) {
+      this.processShutdownCallback(this.forkChoice.irrecoverableError);
+    }
     this.forkChoice.updateTime(slot);
 
     this.metrics?.clockSlot.set(slot);