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({ 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], ], ], ];