From 15c041b174edbd8757c8572455d14006c4ff0d41 Mon Sep 17 00:00:00 2001 From: harkamal Date: Tue, 8 Aug 2023 13:58:03 +0530 Subject: [PATCH 1/2] fix: move and fix execution engine state update checks for online state --- packages/beacon-node/src/execution/engine/http.ts | 8 ++++++++ packages/beacon-node/src/execution/engine/utils.ts | 10 ---------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index a2fc6ebb334d..c8c4a5df28c4 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -421,6 +421,14 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (oldState === newState) return; + // 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; + } + switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); diff --git a/packages/beacon-node/src/execution/engine/utils.ts b/packages/beacon-node/src/execution/engine/utils.ts index 5ce2336f6db3..aa7b5e214be3 100644 --- a/packages/beacon-node/src/execution/engine/utils.ts +++ b/packages/beacon-node/src/execution/engine/utils.ts @@ -94,15 +94,5 @@ export function getExecutionEngineState Date: Tue, 8 Aug 2023 11:06:17 +0200 Subject: [PATCH 2/2] Fix the state change --- .../beacon-node/src/execution/engine/http.ts | 10 +--- .../beacon-node/src/execution/engine/utils.ts | 25 +++++++-- .../test/unit/execution/engine/utils.test.ts | 51 +++++++++++++++++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index c8c4a5df28c4..331e24fa7955 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -131,7 +131,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { protected async fetchWithRetries(payload: RpcPayload

, opts?: ReqOpts): Promise { try { const res = await this.rpc.fetchWithRetries(payload, opts); - this.updateEngineState(ExecutionEngineState.ONLINE); + this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); return res; } catch (err) { this.updateEngineState(getExecutionEngineState({payloadError: err, oldState: this.state})); @@ -421,14 +421,6 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (oldState === newState) return; - // 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; - } - switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); diff --git a/packages/beacon-node/src/execution/engine/utils.ts b/packages/beacon-node/src/execution/engine/utils.ts index aa7b5e214be3..13ad8d855062 100644 --- a/packages/beacon-node/src/execution/engine/utils.ts +++ b/packages/beacon-node/src/execution/engine/utils.ts @@ -85,14 +85,33 @@ function getExecutionEngineStateForPayloadError( export function getExecutionEngineState({ payloadError, payloadStatus, + targetState, oldState, }: - | {payloadStatus: S; payloadError?: never; oldState: ExecutionEngineState} - | {payloadStatus?: never; payloadError: E; oldState: ExecutionEngineState}): ExecutionEngineState { + | {payloadStatus: S; payloadError?: never; targetState?: never; oldState: ExecutionEngineState} + | {payloadStatus?: never; payloadError: E; targetState?: never; oldState: ExecutionEngineState} + | { + payloadStatus?: never; + payloadError?: never; + targetState: ExecutionEngineState; + oldState: ExecutionEngineState; + }): ExecutionEngineState { const newState = - payloadStatus === undefined + targetState !== undefined + ? targetState + : payloadStatus === undefined ? getExecutionEngineStateForPayloadError(payloadError, oldState) : getExecutionEngineStateForPayloadStatus(payloadStatus); + 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; + } + 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 997ec8726bd7..e092d18391cc 100644 --- a/packages/beacon-node/test/unit/execution/engine/utils.test.ts +++ b/packages/beacon-node/test/unit/execution/engine/utils.test.ts @@ -172,6 +172,49 @@ describe("execution / engine / utils", () => { ], ]; + const testCasesTargetState: Record< + ExecutionEngineState, + [oldState: ExecutionEngineState, newState: ExecutionEngineState][] + > = { + [ExecutionEngineState.ONLINE]: [ + [ExecutionEngineState.ONLINE, ExecutionEngineState.ONLINE], + [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.ONLINE], + [ExecutionEngineState.OFFLINE, ExecutionEngineState.ONLINE], + // Once start syncing it should not go back to online state + // Online is an initial state when execution engine is created + [ExecutionEngineState.SYNCED, ExecutionEngineState.SYNCED], + [ExecutionEngineState.SYNCING, ExecutionEngineState.SYNCING], + ], + [ExecutionEngineState.AUTH_FAILED]: [ + [ExecutionEngineState.ONLINE, ExecutionEngineState.AUTH_FAILED], + [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.AUTH_FAILED], + [ExecutionEngineState.OFFLINE, ExecutionEngineState.AUTH_FAILED], + [ExecutionEngineState.SYNCED, ExecutionEngineState.AUTH_FAILED], + [ExecutionEngineState.SYNCING, ExecutionEngineState.AUTH_FAILED], + ], + [ExecutionEngineState.OFFLINE]: [ + [ExecutionEngineState.ONLINE, ExecutionEngineState.OFFLINE], + [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.OFFLINE], + [ExecutionEngineState.OFFLINE, ExecutionEngineState.OFFLINE], + [ExecutionEngineState.SYNCED, ExecutionEngineState.OFFLINE], + [ExecutionEngineState.SYNCING, ExecutionEngineState.OFFLINE], + ], + [ExecutionEngineState.SYNCED]: [ + [ExecutionEngineState.ONLINE, ExecutionEngineState.SYNCED], + [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.SYNCED], + [ExecutionEngineState.OFFLINE, ExecutionEngineState.SYNCED], + [ExecutionEngineState.SYNCED, ExecutionEngineState.SYNCED], + [ExecutionEngineState.SYNCING, ExecutionEngineState.SYNCED], + ], + [ExecutionEngineState.SYNCING]: [ + [ExecutionEngineState.ONLINE, ExecutionEngineState.SYNCING], + [ExecutionEngineState.AUTH_FAILED, ExecutionEngineState.SYNCING], + [ExecutionEngineState.OFFLINE, ExecutionEngineState.SYNCING], + [ExecutionEngineState.SYNCED, ExecutionEngineState.SYNCING], + [ExecutionEngineState.SYNCING, ExecutionEngineState.SYNCING], + ], + }; + for (const payloadStatus of Object.keys(testCasesPayload) as ExecutionPayloadStatus[]) { for (const [oldState, newState] of testCasesPayload[payloadStatus]) { it(`should transition from "${oldState}" to "${newState}" on payload status "${payloadStatus}"`, () => { @@ -188,5 +231,13 @@ describe("execution / engine / utils", () => { }); } } + + for (const targetState of Object.keys(testCasesTargetState) as ExecutionEngineState[]) { + for (const [oldState, newState] of testCasesTargetState[targetState]) { + it(`should transition from "${oldState}" to "${newState}" on when targeting "${targetState}"`, () => { + expect(getExecutionEngineState({targetState, oldState})).to.equal(newState); + }); + } + } }); });