fix: move and fix execution engine state update checks for online sta…

…te (#5862) * fix: move and fix execution engine state update checks for online state * Fix the state change --------- Co-authored-by: Nazar Hussain <[email protected]>
ChainSafe · Aug 8, 2023 · a8ad567 · a8ad567
1 parent df2f4d3
commit a8ad567
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 4 deletions.
diff --git 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<R, P = IJson[]>(payload: RpcPayload<P>, opts?: ReqOpts): Promise<R> {
     try {
       const res = await this.rpc.fetchWithRetries<R, P>(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}));

diff --git a/packages/beacon-node/src/execution/engine/utils.ts b/packages/beacon-node/src/execution/engine/utils.ts
@@ -85,12 +85,21 @@ function getExecutionEngineStateForPayloadError(
 export function getExecutionEngineState<S extends ExecutionPayloadStatus | undefined, E extends unknown | undefined>({
   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);
 

diff --git 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);
+        });
+      }
+    }
   });
 });