address review, changed migration 024 test file for not use the initi…

…al background state, adapt 36 migration to check every state data first and only then modify the data and updated the tests accordingly, updated fixture-builder with networkId property
MetaMask · Mar 19, 2024 · 9454fd2 · 9454fd2
1 parent 7e59132
commit 9454fd2
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 50 deletions.
diff --git a/app/store/migrations/024.test.js b/app/store/migrations/024.test.js
@@ -1,7 +1,6 @@
 import migrate from './024';
 import { merge } from 'lodash';
 import initialRootState from '../../util/test/initial-root-state';
-import initialBackgroundState from '../../util/test/initial-background-state.json';
 import { captureException } from '@sentry/react-native';
 
 jest.mock('@sentry/react-native', () => ({
@@ -54,64 +53,38 @@ describe('Migration #24', () => {
   it('should migrate loading network state', () => {
     const state = {
       engine: {
-        backgroundState: merge({}, initialBackgroundState, {
+        backgroundState: {
           NetworkController: {
             network: 'loading',
           },
-        }),
+        },
       },
     };
 
     const newState = migrate(state);
 
     expect(newState.engine.backgroundState.NetworkController).toStrictEqual({
-      networkConfigurations: {},
       networkId: null,
       networkStatus: 'unknown',
-      providerConfig: {
-        chainId: '0x1',
-        ticker: 'ETH',
-        type: 'mainnet',
-      },
-      networksMetadata: {
-        mainnet: {
-          EIPS: {},
-          status: 'unknown',
-        },
-      },
-      selectedNetworkClientId: 'mainnet',
     });
   });
 
   it('should migrate non-loading network state', () => {
     const state = {
       engine: {
-        backgroundState: merge({}, initialBackgroundState, {
+        backgroundState: {
           NetworkController: {
             network: '1',
           },
-        }),
+        },
       },
     };
 
     const newState = migrate(state);
 
     expect(newState.engine.backgroundState.NetworkController).toStrictEqual({
-      networkConfigurations: {},
       networkId: '1',
       networkStatus: 'available',
-      providerConfig: {
-        chainId: '0x1',
-        type: 'mainnet',
-        ticker: 'ETH',
-      },
-      networksMetadata: {
-        mainnet: {
-          EIPS: {},
-          status: 'unknown',
-        },
-      },
-      selectedNetworkClientId: 'mainnet',
     });
   });
 });
diff --git a/app/store/migrations/036.test.ts b/app/store/migrations/036.test.ts
@@ -8,6 +8,33 @@ const expectedState = {
   engine: {
     backgroundState: {
       NetworkController: {
+        networkConfigurations: {},
+        networksMetadata: {
+          goerli: {
+            EIPS: {},
+            status: 'unknown',
+          },
+          'linea-goerli': {
+            EIPS: {},
+            status: 'unknown',
+          },
+          'linea-mainnet': {
+            EIPS: {},
+            status: 'unknown',
+          },
+          'linea-sepolia': {
+            EIPS: {},
+            status: 'unknown',
+          },
+          mainnet: {
+            EIPS: {},
+            status: 'unknown',
+          },
+          sepolia: {
+            EIPS: {},
+            status: 'unknown',
+          },
+        },
         selectedNetworkClientId: 'mainnet',
         providerConfig: {
           type: 'mainnet',
@@ -119,6 +146,7 @@ describe('Migration #36', () => {
       engine: {
         backgroundState: {
           NetworkController: {
+            networkConfigurations: {},
             networkDetails: {},
             networkStatus: 'test',
             providerConfig: {
@@ -140,6 +168,7 @@ describe('Migration #36', () => {
       engine: {
         backgroundState: {
           NetworkController: {
+            networkConfigurations: {},
             networkDetails: {},
             networkStatus: 'test',
             providerConfig: {
@@ -158,6 +187,33 @@ describe('Migration #36', () => {
       engine: {
         backgroundState: {
           NetworkController: {
+            networkConfigurations: {},
+            networksMetadata: {
+              goerli: {
+                EIPS: {},
+                status: 'unknown',
+              },
+              'linea-goerli': {
+                EIPS: {},
+                status: 'unknown',
+              },
+              'linea-mainnet': {
+                EIPS: {},
+                status: 'unknown',
+              },
+              'linea-sepolia': {
+                EIPS: {},
+                status: 'unknown',
+              },
+              mainnet: {
+                EIPS: {},
+                status: 'unknown',
+              },
+              sepolia: {
+                EIPS: {},
+                status: 'unknown',
+              },
+            },
             providerConfig: {
               type: 'rpc',
               chainId: '0x1',
@@ -180,6 +236,7 @@ describe('Migration #36', () => {
           NetworkController: {
             networkDetails: {},
             networkStatus: 'test',
+            networkConfigurations: {},
             providerConfig: {
               type: 'mainnet',
               chainId: '0x1',
@@ -204,7 +261,7 @@ describe('Migration #36', () => {
                 chainId: '0xa86a',
                 id: '52b62bd0-296c-41d0-9772-2f418c9e81ef',
                 nickname: 'Avalanche Mainnet C-Chain',
-                rpcPrefs: ['Object'],
+                rpcPrefs: [''],
                 rpcUrl: 'https://api.avax.network/ext/bc/C/rpc',
                 ticker: 'AVAX',
               },
@@ -232,7 +289,7 @@ describe('Migration #36', () => {
                 chainId: '0xa86a',
                 id: '52b62bd0-296c-41d0-9772-2f418c9e81ef',
                 nickname: 'Avalanche Mainnet C-Chain',
-                rpcPrefs: ['Object'],
+                rpcPrefs: [''],
                 rpcUrl: 'https://api.avax.network/ext/bc/C/rpc',
                 ticker: 'AVAX',
               },

diff --git a/app/store/migrations/036.ts b/app/store/migrations/036.ts
@@ -5,7 +5,8 @@ import { InfuraNetworkType } from '@metamask/controller-utils';
 
 /**
  * This migration removes networkDetails and networkStatus property
- * This migration add a new property `networkMetadata` to the NetworkController (Still under investigation if it's needed)
+ * This migration add a new property `networkMetadata` to the NetworkController
+ * This migrations adds a new property called `selectedNetworkClientId` to the NetworkController
  * @param {unknown} stateAsync - Promise Redux state.
  * @returns Migrated Redux state.
  */
@@ -62,11 +63,16 @@ export default async function migrate(stateAsync: unknown) {
     return state;
   }
 
-  if (networkControllerState.networkDetails) {
-    delete networkControllerState.networkDetails;
-  }
-  if (networkControllerState.networkStatus) {
-    delete networkControllerState.networkStatus;
+  if (
+    !isObject(networkControllerState.networkConfigurations) ||
+    !hasProperty(networkControllerState, 'networkConfigurations')
+  ) {
+    captureException(
+      new Error(
+        `Migration 36: Invalid NetworkController networkConfigurations state: '${typeof networkControllerState.networkConfigurations}'`,
+      ),
+    );
+    return state;
   }
 
   if (
@@ -81,6 +87,13 @@ export default async function migrate(stateAsync: unknown) {
     return state;
   }
 
+  if (networkControllerState.networkDetails) {
+    delete networkControllerState.networkDetails;
+  }
+  if (networkControllerState.networkStatus) {
+    delete networkControllerState.networkStatus;
+  }
+
   newNetworkControllerState.selectedNetworkClientId =
     (networkControllerState.providerConfig.id as string | undefined) ??
     (networkControllerState.providerConfig.type as string);
@@ -95,17 +108,6 @@ export default async function migrate(stateAsync: unknown) {
     };
   });
 
-  if (
-    !isObject(networkControllerState.networkConfigurations) ||
-    !hasProperty(networkControllerState, 'networkConfigurations')
-  ) {
-    captureException(
-      new Error(
-        `Migration 36: Invalid NetworkController networkConfigurations state: '${typeof networkControllerState.networkConfigurations}'`,
-      ),
-    );
-    return state;
-  }
   Object.keys(networkControllerState.networkConfigurations).forEach(
     (networkConfigurationId) => {
       customNetworksMetadata = {

diff --git a/e2e/fixtures/fixture-builder.js b/e2e/fixtures/fixture-builder.js
@@ -136,6 +136,7 @@ class FixtureBuilder {
             },
             NetworkController: {
               selectedNetworkClientId: 'mainnet',
+              networkId: '1',
               providerConfig: {
                 type: 'mainnet',
                 chainId: '0x1',