Update code with feedback

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

@@ -25,7 +25,7 @@ import {IExecutionEngine} from "../../execution/engine/interface.js";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
 import {IClock} from "../../util/clock.js";
 import {BlockProcessOpts} from "../options.js";
-import {ExecutePayloadStatus} from "../../execution/engine/interface.js";
+import {ExecutionPayloadStatus} from "../../execution/engine/interface.js";
 import {IEth1ForBlockProduction} from "../../eth1/index.js";
 import {Metrics} from "../../metrics/metrics.js";
 import {ImportBlockOpts} from "./types.js";
@@ -304,13 +304,13 @@ export async function verifyBlockExecutionPayload(
   chain.metrics?.engineNotifyNewPayloadResult.inc({result: execResult.status});
 
   switch (execResult.status) {
-    case ExecutePayloadStatus.VALID: {
+    case ExecutionPayloadStatus.VALID: {
       const executionStatus: ExecutionStatus.Valid = ExecutionStatus.Valid;
       const lvhResponse = {executionStatus, latestValidExecHash: execResult.latestValidHash};
       return {executionStatus, lvhResponse, execError: null};
     }
 
-    case ExecutePayloadStatus.INVALID: {
+    case ExecutionPayloadStatus.INVALID: {
       const executionStatus: ExecutionStatus.Invalid = ExecutionStatus.Invalid;
       const lvhResponse = {
         executionStatus,
@@ -326,15 +326,15 @@ export async function verifyBlockExecutionPayload(
     }
 
     // Accepted and Syncing have the same treatment, as final validation of block is pending
-    case ExecutePayloadStatus.ACCEPTED:
-    case ExecutePayloadStatus.SYNCING: {
+    case ExecutionPayloadStatus.ACCEPTED:
+    case ExecutionPayloadStatus.SYNCING: {
       // Check if the entire segment was deemed safe or, this block specifically itself if not in
       // 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) {
         const execError = new BlockError(block, {
           code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
-          execStatus: ExecutePayloadStatus.UNSAFE_OPTIMISTIC_STATUS,
+          execStatus: ExecutionPayloadStatus.UNSAFE_OPTIMISTIC_STATUS,
           errorMessage: `not safe to import ${execResult.status} payload within ${opts.safeSlotsToImportOptimistically} of currentSlot`,
         });
         return {executionStatus: null, execError} as VerifyBlockExecutionResponse;
@@ -360,9 +360,9 @@ export async function verifyBlockExecutionPayload(
     // back. But for now, lets assume other mechanisms like unknown parent block of a future
     // child block will cause it to replay
 
-    case ExecutePayloadStatus.INVALID_BLOCK_HASH:
-    case ExecutePayloadStatus.ELERROR:
-    case ExecutePayloadStatus.UNAVAILABLE: {
+    case ExecutionPayloadStatus.INVALID_BLOCK_HASH:
+    case ExecutionPayloadStatus.ELERROR:
+    case ExecutionPayloadStatus.UNAVAILABLE: {
       const execError = new BlockError(block, {
         code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
         execStatus: execResult.status,

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

@@ -2,7 +2,7 @@ import {toHexString} from "@chainsafe/ssz";
 import {allForks, RootHex, Slot, ValidatorIndex} from "@lodestar/types";
 import {LodestarError} from "@lodestar/utils";
 import {CachedBeaconStateAllForks} from "@lodestar/state-transition";
-import {ExecutePayloadStatus} from "../../execution/engine/interface.js";
+import {ExecutionPayloadStatus} from "../../execution/engine/interface.js";
 import {QueueErrorCode} from "../../util/queue/index.js";
 import {GossipActionError} from "./gossipValidation.js";
 
@@ -66,8 +66,8 @@ export enum BlockErrorCode {
 }
 
 type ExecutionErrorStatus = Exclude<
-  ExecutePayloadStatus,
-  ExecutePayloadStatus.VALID | ExecutePayloadStatus.ACCEPTED | ExecutePayloadStatus.SYNCING
+  ExecutionPayloadStatus,
+  ExecutionPayloadStatus.VALID | ExecutionPayloadStatus.ACCEPTED | ExecutionPayloadStatus.SYNCING
 >;
 
 export type BlockErrorType =

packages/beacon-node/src/execution/engine/http.ts

@@ -1,14 +1,19 @@
 import {Root, RootHex, allForks, Wei} from "@lodestar/types";
 import {SLOTS_PER_EPOCH, ForkName, ForkSeq} from "@lodestar/params";
 import {Logger} from "@lodestar/logger";
-import {IJsonRpcHttpClient, ReqOpts} from "../../eth1/provider/jsonRpcHttpClient.js";
+import {
+  ErrorJsonRpcResponse,
+  HttpRpcError,
+  IJsonRpcHttpClient,
+  ReqOpts,
+} from "../../eth1/provider/jsonRpcHttpClient.js";
 import {Metrics} from "../../metrics/index.js";
 import {JobItemQueue} from "../../util/queue/index.js";
 import {EPOCHS_PER_BATCH} from "../../sync/constants.js";
 import {numToQuantity} from "../../eth1/provider/utils.js";
 import {IJson, RpcPayload} from "../../eth1/interface.js";
 import {
-  ExecutePayloadStatus,
+  ExecutionPayloadStatus,
   ExecutePayloadResponse,
   IExecutionEngine,
   PayloadId,
@@ -206,30 +211,36 @@ export class ExecutionEngineHttp implements IExecutionEngine {
       };
     }
 
-    const {status, latestValidHash, validationError} = await (this.rpcFetchQueue.push(engineRequest) as Promise<
-      EngineApiRpcReturnTypes[typeof method]
-    >);
+    const {status, latestValidHash, validationError} = await (
+      this.rpcFetchQueue.push(engineRequest) as Promise<EngineApiRpcReturnTypes[typeof method]>
+    ).catch((e: Error) => {
+      if (e instanceof HttpRpcError || e instanceof ErrorJsonRpcResponse) {
+        return {status: ExecutionPayloadStatus.ELERROR, latestValidHash: null, validationError: e.message};
+      } else {
+        return {status: ExecutionPayloadStatus.UNAVAILABLE, latestValidHash: null, validationError: e.message};
+      }
+    });
 
     this.updateEngineState(getExecutionEngineState({payloadStatus: status, oldState: this.state}));
 
     switch (status) {
-      case ExecutePayloadStatus.VALID:
+      case ExecutionPayloadStatus.VALID:
         return {status, latestValidHash: latestValidHash ?? "0x0", validationError: null};
 
-      case ExecutePayloadStatus.INVALID:
+      case ExecutionPayloadStatus.INVALID:
         // As per latest specs if latestValidHash can be null and it would mean only
         // invalidate this block
         return {status, latestValidHash, validationError};
 
-      case ExecutePayloadStatus.SYNCING:
-      case ExecutePayloadStatus.ACCEPTED:
+      case ExecutionPayloadStatus.SYNCING:
+      case ExecutionPayloadStatus.ACCEPTED:
         return {status, latestValidHash: null, validationError: null};
 
-      case ExecutePayloadStatus.INVALID_BLOCK_HASH:
+      case ExecutionPayloadStatus.INVALID_BLOCK_HASH:
         return {status, latestValidHash: null, validationError: validationError ?? "Malformed block"};
 
-      case ExecutePayloadStatus.UNAVAILABLE:
-      case ExecutePayloadStatus.ELERROR:
+      case ExecutionPayloadStatus.UNAVAILABLE:
+      case ExecutionPayloadStatus.ELERROR:
         return {
           status,
           latestValidHash: null,
@@ -238,7 +249,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
 
       default:
         return {
-          status: ExecutePayloadStatus.ELERROR,
+          status: ExecutionPayloadStatus.ELERROR,
           latestValidHash: null,
           validationError: `Invalid EL status on executePayload: ${status}`,
         };
@@ -310,7 +321,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
     this.updateEngineState(getExecutionEngineState({payloadStatus: status, oldState: this.state}));
 
     switch (status) {
-      case ExecutePayloadStatus.VALID:
+      case ExecutionPayloadStatus.VALID:
         // if payloadAttributes are provided, a valid payloadId is expected
         if (payloadAttributesRpc) {
           if (!payloadId || payloadId === "0x") {
@@ -322,15 +333,15 @@ export class ExecutionEngineHttp implements IExecutionEngine {
         }
         return payloadId !== "0x" ? payloadId : null;
 
-      case ExecutePayloadStatus.SYNCING:
+      case ExecutionPayloadStatus.SYNCING:
         // Throw error on syncing if requested to produce a block, else silently ignore
         if (payloadAttributes) {
           throw Error("Execution Layer Syncing");
         } else {
           return null;
         }
 
-      case ExecutePayloadStatus.INVALID:
+      case ExecutionPayloadStatus.INVALID:
         throw Error(
           `Invalid ${payloadAttributes ? "prepare payload" : "forkchoice request"}, validationError=${
             validationError ?? ""

packages/beacon-node/src/execution/engine/interface.ts

@@ -8,7 +8,7 @@ import {ExecutionPayloadBody} from "./types.js";
 
 export {PayloadIdCache, PayloadId, WithdrawalV1};
 
-export enum ExecutePayloadStatus {
+export enum ExecutionPayloadStatus {
   /** given payload is valid */
   VALID = "VALID",
   /** given payload is invalid */
@@ -39,19 +39,26 @@ export enum ExecutionEngineState {
 }
 
 export type ExecutePayloadResponse =
-  | {status: ExecutePayloadStatus.SYNCING | ExecutePayloadStatus.ACCEPTED; latestValidHash: null; validationError: null}
-  | {status: ExecutePayloadStatus.VALID; latestValidHash: RootHex; validationError: null}
-  | {status: ExecutePayloadStatus.INVALID; latestValidHash: RootHex | null; validationError: string | null}
   | {
-      status: ExecutePayloadStatus.INVALID_BLOCK_HASH | ExecutePayloadStatus.ELERROR | ExecutePayloadStatus.UNAVAILABLE;
+      status: ExecutionPayloadStatus.SYNCING | ExecutionPayloadStatus.ACCEPTED;
+      latestValidHash: null;
+      validationError: null;
+    }
+  | {status: ExecutionPayloadStatus.VALID; latestValidHash: RootHex; validationError: null}
+  | {status: ExecutionPayloadStatus.INVALID; latestValidHash: RootHex | null; validationError: string | null}
+  | {
+      status:
+        | ExecutionPayloadStatus.INVALID_BLOCK_HASH
+        | ExecutionPayloadStatus.ELERROR
+        | ExecutionPayloadStatus.UNAVAILABLE;
       latestValidHash: null;
       validationError: string;
     };
 
 export type ForkChoiceUpdateStatus =
-  | ExecutePayloadStatus.VALID
-  | ExecutePayloadStatus.INVALID
-  | ExecutePayloadStatus.SYNCING;
+  | ExecutionPayloadStatus.VALID
+  | ExecutionPayloadStatus.INVALID
+  | ExecutionPayloadStatus.SYNCING;
 
 export type PayloadAttributes = {
   timestamp: number;

packages/beacon-node/src/execution/engine/mock.ts

@@ -24,7 +24,7 @@ import {
   BlobsBundleRpc,
   ExecutionPayloadBodyRpc,
 } from "./types.js";
-import {ExecutePayloadStatus, PayloadIdCache} from "./interface.js";
+import {ExecutionPayloadStatus, PayloadIdCache} from "./interface.js";
 import {JsonRpcBackend} from "./utils.js";
 
 const INTEROP_GAS_LIMIT = 30e6;
@@ -171,7 +171,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
     if (!this.validBlocks.has(parentHash)) {
       // if requisite data for the payload's acceptance or validation is missing
       // return {status: SYNCING, latestValidHash: null, validationError: null}
-      return {status: ExecutePayloadStatus.SYNCING, latestValidHash: null, validationError: null};
+      return {status: ExecutionPayloadStatus.SYNCING, latestValidHash: null, validationError: null};
     }
 
     // 4. Client software MAY NOT validate the payload if the payload doesn't belong to the canonical chain.
@@ -190,7 +190,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
     // IF the payload has been fully validated while processing the call
     // RETURN payload status from the Payload validation process
     // If validation succeeds, the response MUST contain {status: VALID, latestValidHash: payload.blockHash}
-    return {status: ExecutePayloadStatus.VALID, latestValidHash: blockHash, validationError: null};
+    return {status: ExecutionPayloadStatus.VALID, latestValidHash: blockHash, validationError: null};
   }
 
   /**
@@ -246,7 +246,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
       //
       // > TODO: Implement
       return {
-        payloadStatus: {status: ExecutePayloadStatus.SYNCING, latestValidHash: null, validationError: null},
+        payloadStatus: {status: ExecutionPayloadStatus.SYNCING, latestValidHash: null, validationError: null},
         payloadId: null,
       };
     }
@@ -337,7 +337,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
       // IF the payload is deemed VALID and the build process has begun
       // {payloadStatus: {status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null}, payloadId: buildProcessId}
       return {
-        payloadStatus: {status: ExecutePayloadStatus.VALID, latestValidHash: null, validationError: null},
+        payloadStatus: {status: ExecutionPayloadStatus.VALID, latestValidHash: null, validationError: null},
         payloadId: String(payloadId as number),
       };
     }
@@ -347,7 +347,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
       // IF the payload is deemed VALID and a build process hasn't been started
       // {payloadStatus: {status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null}, payloadId: null}
       return {
-        payloadStatus: {status: ExecutePayloadStatus.VALID, latestValidHash: null, validationError: null},
+        payloadStatus: {status: ExecutionPayloadStatus.VALID, latestValidHash: null, validationError: null},
         payloadId: null,
       };
     }

packages/beacon-node/src/execution/engine/types.ts

@@ -16,7 +16,7 @@ import {
   QUANTITY,
   quantityToBigint,
 } from "../../eth1/provider/utils.js";
-import {ExecutePayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes} from "./interface.js";
+import {ExecutionPayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes} from "./interface.js";
 import {WithdrawalV1} from "./payloadIdCache.js";
 
 /* eslint-disable @typescript-eslint/naming-convention */
@@ -65,7 +65,7 @@ export type EngineApiRpcParamTypes = {
 };
 
 export type PayloadStatus = {
-  status: ExecutePayloadStatus;
+  status: ExecutionPayloadStatus;
   latestValidHash: DATA | null;
   validationError: string | null;
 };