Ensure that all networkConfiguration object in networkController state have an id (#18513)

* Ensure that all networkConfiguration object in networkController state have an id

* Lint fix

* Update app/scripts/migrations/084.ts

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

* Add unit tests for error cases

* Simplify code

* Remove unnecessary any typing

* Fix network controller type checking

* Lint fix

* Improve typing

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>

app/scripts/migrations/084.test.js

@@ -0,0 +1,254 @@
+import { v4 } from 'uuid';
+import { migrate, version } from './084';
+
+jest.mock('uuid', () => {
+  const actual = jest.requireActual('uuid');
+
+  return {
+    ...actual,
+    v4: jest.fn(),
+  };
+});
+
+describe('migration #84', () => {
+  beforeEach(() => {
+    v4.mockImplementationOnce(() => 'network-configuration-id-1')
+      .mockImplementationOnce(() => 'network-configuration-id-2')
+      .mockImplementationOnce(() => 'network-configuration-id-3')
+      .mockImplementationOnce(() => 'network-configuration-id-4');
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+  it('should update the version metadata', async () => {
+    const oldStorage = {
+      meta: {
+        version: 83,
+      },
+      data: {},
+    };
+
+    const newStorage = await migrate(oldStorage);
+    expect(newStorage.meta).toStrictEqual({
+      version,
+    });
+  });
+
+  it('should use the key of the networkConfigurations object to set the id of each network configuration', async () => {
+    const oldStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          networkConfigurations: {
+            'network-configuration-id-1': {
+              chainId: '0x539',
+              nickname: 'Localhost 8545',
+              rpcPrefs: {},
+              rpcUrl: 'http://localhost:8545',
+              ticker: 'ETH',
+            },
+            'network-configuration-id-2': {
+              chainId: '0xa4b1',
+              nickname: 'Arbitrum One',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://explorer.arbitrum.io',
+              },
+              rpcUrl:
+                'https://arbitrum-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748',
+              ticker: 'ETH',
+            },
+            'network-configuration-id-3': {
+              chainId: '0x4e454152',
+              nickname: 'Aurora Mainnet',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://aurorascan.dev/',
+              },
+              rpcUrl:
+                'https://aurora-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748',
+              ticker: 'Aurora ETH',
+            },
+            'network-configuration-id-4': {
+              chainId: '0x38',
+              nickname:
+                'BNB Smart Chain (previously Binance Smart Chain Mainnet)',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://bscscan.com/',
+              },
+              rpcUrl: 'https://bsc-dataseed.binance.org/',
+              ticker: 'BNB',
+            },
+          },
+        },
+      },
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const expectedNewStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          networkConfigurations: {
+            'network-configuration-id-1': {
+              chainId: '0x539',
+              nickname: 'Localhost 8545',
+              rpcPrefs: {},
+              rpcUrl: 'http://localhost:8545',
+              ticker: 'ETH',
+              id: 'network-configuration-id-1',
+            },
+            'network-configuration-id-2': {
+              chainId: '0xa4b1',
+              nickname: 'Arbitrum One',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://explorer.arbitrum.io',
+              },
+              rpcUrl:
+                'https://arbitrum-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748',
+              ticker: 'ETH',
+              id: 'network-configuration-id-2',
+            },
+            'network-configuration-id-3': {
+              chainId: '0x4e454152',
+              nickname: 'Aurora Mainnet',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://aurorascan.dev/',
+              },
+              rpcUrl:
+                'https://aurora-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748',
+              ticker: 'Aurora ETH',
+              id: 'network-configuration-id-3',
+            },
+            'network-configuration-id-4': {
+              chainId: '0x38',
+              nickname:
+                'BNB Smart Chain (previously Binance Smart Chain Mainnet)',
+              rpcPrefs: {
+                blockExplorerUrl: 'https://bscscan.com/',
+              },
+              rpcUrl: 'https://bsc-dataseed.binance.org/',
+              ticker: 'BNB',
+              id: 'network-configuration-id-4',
+            },
+          },
+        },
+      },
+    };
+    expect(newStorage).toStrictEqual(expectedNewStorage);
+  });
+
+  it('should not modify state if state.NetworkController is undefined', async () => {
+    const oldStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        testProperty: 'testValue',
+      },
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const expectedNewStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        testProperty: 'testValue',
+      },
+    };
+    expect(newStorage).toStrictEqual(expectedNewStorage);
+  });
+
+  it('should not modify state if state.NetworkController is not an object', async () => {
+    const oldStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: false,
+        testProperty: 'testValue',
+      },
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const expectedNewStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: false,
+        testProperty: 'testValue',
+      },
+    };
+    expect(newStorage).toStrictEqual(expectedNewStorage);
+  });
+
+  it('should not modify state if state.NetworkController.networkConfigurations is undefined', async () => {
+    const oldStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          testNetworkControllerProperty: 'testNetworkControllerValue',
+          networkConfigurations: undefined,
+        },
+        testProperty: 'testValue',
+      },
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const expectedNewStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          testNetworkControllerProperty: 'testNetworkControllerValue',
+          networkConfigurations: undefined,
+        },
+        testProperty: 'testValue',
+      },
+    };
+    expect(newStorage).toStrictEqual(expectedNewStorage);
+  });
+
+  it('should not modify state if state.NetworkController.networkConfigurations is an empty object', async () => {
+    const oldStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          testNetworkControllerProperty: 'testNetworkControllerValue',
+          networkConfigurations: {},
+        },
+        testProperty: 'testValue',
+      },
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const expectedNewStorage = {
+      meta: {
+        version,
+      },
+      data: {
+        NetworkController: {
+          testNetworkControllerProperty: 'testNetworkControllerValue',
+          networkConfigurations: {},
+        },
+        testProperty: 'testValue',
+      },
+    };
+    expect(newStorage).toStrictEqual(expectedNewStorage);
+  });
+});

app/scripts/migrations/084.ts

@@ -0,0 +1,58 @@
+import { cloneDeep } from 'lodash';
+import { isObject } from '@metamask/utils';
+
+export const version = 84;
+
+/**
+ * Ensure that each networkConfigurations object in state.NetworkController.networkConfigurations has an
+ * `id` property which matches the key pointing that object
+ *
+ * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
+ * @param originalVersionedData.meta - State metadata.
+ * @param originalVersionedData.meta.version - The current state version.
+ * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
+ * @returns Updated versioned MetaMask extension state.
+ */
+export async function migrate(originalVersionedData: {
+  meta: { version: number };
+  data: Record<string, unknown>;
+}) {
+  const versionedData = cloneDeep(originalVersionedData);
+  versionedData.meta.version = version;
+  versionedData.data = transformState(versionedData.data);
+  return versionedData;
+}
+
+function transformState(state: Record<string, unknown>) {
+  if (!isObject(state.NetworkController)) {
+    return state;
+  }
+  const { NetworkController } = state;
+
+  if (!isObject(NetworkController.networkConfigurations)) {
+    return state;
+  }
+
+  const { networkConfigurations } = NetworkController;
+
+  const newNetworkConfigurations: Record<string, Record<string, unknown>> = {};
+
+  for (const networkConfigurationId of Object.keys(networkConfigurations)) {
+    const networkConfiguration = networkConfigurations[networkConfigurationId];
+    if (!isObject(networkConfiguration)) {
+      return state;
+    }
+    newNetworkConfigurations[networkConfigurationId] = {
+      ...networkConfiguration,
+      id: networkConfigurationId,
+    };
+  }
+
+  return {
+    ...state,
+    NetworkController: {
+      ...NetworkController,
+      networkConfigurations: newNetworkConfigurations,
+    },
+  };
+}

app/scripts/migrations/index.js

@@ -87,6 +87,7 @@ import m080 from './080';
 import * as m081 from './081';
 import * as m082 from './082';
 import * as m083 from './083';
+import * as m084 from './084';
 
 const migrations = [
   m002,
@@ -171,6 +172,7 @@ const migrations = [
   m081,
   m082,
   m083,
+  m084,
 ];
 
 export default migrations;