From 9439173aa6158d0a7faba3e3cf21a448652411a0 Mon Sep 17 00:00:00 2001
From: Nazar Hussain <nazarhussain@gmail.com>
Date: Tue, 1 Aug 2023 14:34:10 +0200
Subject: [PATCH] Update the error test cases

---
 .../beacon-node/src/execution/engine/http.ts  |  5 +----
 .../beacon-node/src/execution/engine/utils.ts | 22 +++++++------------
 .../test/unit/execution/engine/utils.test.ts  | 18 ++++++---------
 3 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts
index b7c9e0a8d059..6f86f330744b 100644
--- a/packages/beacon-node/src/execution/engine/http.ts
+++ b/packages/beacon-node/src/execution/engine/http.ts
@@ -129,10 +129,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
       this.updateEngineState(ExecutionEngineState.ONLINE);
       return res;
     } catch (err) {
-      const newState = getExecutionEngineState({payloadError: err, oldState: this.state});
-      if (newState !== undefined) {
-        this.updateEngineState(newState);
-      }
+      this.updateEngineState(getExecutionEngineState({payloadError: err, oldState: this.state}));
 
       // TODO: This is case where we can't determine the nature of error.
       // We should throw only for such cases not for the which we knew the status
diff --git a/packages/beacon-node/src/execution/engine/utils.ts b/packages/beacon-node/src/execution/engine/utils.ts
index a8735b4af30b..670fc9fd153c 100644
--- a/packages/beacon-node/src/execution/engine/utils.ts
+++ b/packages/beacon-node/src/execution/engine/utils.ts
@@ -51,9 +51,8 @@ function getExecutionEngineStateForPayloadStatus(payloadStatus: ExecutePayloadSt
     case ExecutePayloadStatus.UNAVAILABLE:
       return ExecutionEngineState.OFFLINE;
 
-    // In case we can't determine the state, we assume it's online
-    // This assumption is better than considering offline, because the offline state may trigger some notifications
     default:
+      // In case we can't determine the state, we assume it stays in old state
       return ExecutionEngineState.ONLINE;
   }
 }
@@ -61,7 +60,7 @@ function getExecutionEngineStateForPayloadStatus(payloadStatus: ExecutePayloadSt
 function getExecutionEngineStateForPayloadError(
   payloadError: unknown,
   oldState: ExecutionEngineState
-): ExecutionEngineState | undefined {
+): ExecutionEngineState {
   if (isErrorAborted(payloadError) || isQueueErrorAborted(payloadError)) {
     return oldState;
   }
@@ -79,35 +78,30 @@ function getExecutionEngineStateForPayloadError(
     return ExecutionEngineState.AUTH_FAILED;
   }
 
-  return undefined;
+  return oldState;
 }
 
-export function getExecutionEngineState<
-  S extends ExecutePayloadStatus | undefined,
-  E extends unknown | undefined,
-  R = S extends undefined ? ExecutionEngineState | undefined : ExecutionEngineState,
->({
+export function getExecutionEngineState<S extends ExecutePayloadStatus | undefined, E extends unknown | undefined>({
   payloadError,
   payloadStatus,
   oldState,
 }:
   | {payloadStatus: S; payloadError?: never; oldState: ExecutionEngineState}
-  | {payloadStatus?: never; payloadError: E; oldState: ExecutionEngineState}): R {
+  | {payloadStatus?: never; payloadError: E; oldState: ExecutionEngineState}): ExecutionEngineState {
   const newState =
     payloadStatus === undefined
       ? getExecutionEngineStateForPayloadError(payloadError, oldState)
       : getExecutionEngineStateForPayloadStatus(payloadStatus);
 
-  // The `payloadError` is something based on which we can't determine the state of execution engine
-  if (newState === undefined) return undefined as R;
+  if (newState === oldState) return oldState;
 
   // The ONLINE is initial state and can reached from offline or auth failed error
   if (
     newState === ExecutionEngineState.ONLINE &&
     !(oldState === ExecutionEngineState.OFFLINE || oldState === ExecutionEngineState.AUTH_FAILED)
   ) {
-    return oldState as R;
+    return oldState;
   }
 
-  return newState as R;
+  return newState;
 }
diff --git a/packages/beacon-node/test/unit/execution/engine/utils.test.ts b/packages/beacon-node/test/unit/execution/engine/utils.test.ts
index e7fd6ce87add..94827d57e7eb 100644
--- a/packages/beacon-node/test/unit/execution/engine/utils.test.ts
+++ b/packages/beacon-node/test/unit/execution/engine/utils.test.ts
@@ -10,7 +10,7 @@ import {
 import {QueueError, QueueErrorCode} from "../../../../src/util/queue/errors.js";
 import {ErrorJsonRpcResponse, HttpRpcError} from "../../../../src/eth1/provider/jsonRpcHttpClient.js";
 
-describe("execution/engine/utils", () => {
+describe("execution / engine / utils", () => {
   describe("getExecutionEngineState", () => {
     const testCasesPayload: Record<
       ExecutePayloadStatus,
@@ -81,11 +81,7 @@ describe("execution/engine/utils", () => {
       ],
     };
 
-    type ErrorTestCase = [
-      string,
-      Error,
-      [oldState: ExecutionEngineState, newState: ExecutionEngineState | undefined][],
-    ];
+    type ErrorTestCase = [string, Error, [oldState: ExecutionEngineState, newState: ExecutionEngineState][]];
     const testCasesError: ErrorTestCase[] = [
       [
         "abort error",
@@ -167,11 +163,11 @@ describe("execution/engine/utils", () => {
         "unknown error",
         new Error("unknown error"),
         [
-          [ExecutionEngineState.ONLINE, undefined],
-          [ExecutionEngineState.AUTH_FAILED, undefined],
-          [ExecutionEngineState.OFFLINE, undefined],
-          [ExecutionEngineState.SYNCED, undefined],
-          [ExecutionEngineState.SYNCING, undefined],
+          [ExecutionEngineState.ONLINE, ExecutionEngineState.ONLINE],
+          [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.AUTH_FAILED],
+          [ExecutionEngineState.OFFLINE, ExecutionEngineState.OFFLINE],
+          [ExecutionEngineState.SYNCED, ExecutionEngineState.SYNCED],
+          [ExecutionEngineState.SYNCING, ExecutionEngineState.SYNCING],
         ],
       ],
     ];