From aca0d130dad51377b43ec1d842b4a3219e55494e Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Sat, 8 Apr 2023 06:33:30 -0230 Subject: [PATCH 1/9] Ensure that all networkConfiguration object in networkController state have an id --- app/scripts/migrations/084.test.js | 144 +++++++++++++++++++++++++++++ app/scripts/migrations/084.ts | 53 +++++++++++ app/scripts/migrations/index.js | 2 + 3 files changed, 199 insertions(+) create mode 100644 app/scripts/migrations/084.test.js create mode 100644 app/scripts/migrations/084.ts diff --git a/app/scripts/migrations/084.test.js b/app/scripts/migrations/084.test.js new file mode 100644 index 000000000000..333c8c9eb4f2 --- /dev/null +++ b/app/scripts/migrations/084.test.js @@ -0,0 +1,144 @@ +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); + }); +}); diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts new file mode 100644 index 000000000000..edab65ff23dc --- /dev/null +++ b/app/scripts/migrations/084.ts @@ -0,0 +1,53 @@ +import { cloneDeep } from 'lodash'; +import { hasProperty, isObject } from '@metamask/utils'; +import { v4 } from 'uuid'; + +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; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + !isObject(state.NetworkController) + ) { + return state; + } + const NetworkController: any = state.NetworkController; + + const networkConfigurations: Record = NetworkController.networkConfigurations; + + const newNetworkConfigurations: Record = {}; + + for (const networkConfigurationId in networkConfigurations) { + newNetworkConfigurations[networkConfigurationId] = { + ...networkConfigurations[networkConfigurationId], + id: networkConfigurationId, + } + } + + return { + ...state, + NetworkController: { + ...NetworkController, + networkConfigurations: newNetworkConfigurations, + }, + }; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index c3f8e515f610..54a09c2b4e10 100644 --- a/app/scripts/migrations/index.js +++ b/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; From fb3f65057cbb25cd5ae2208dce1da37b13638467 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Sat, 8 Apr 2023 07:33:23 -0230 Subject: [PATCH 2/9] Lint fix --- app/scripts/migrations/084.ts | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index edab65ff23dc..efb722f41094 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -1,11 +1,10 @@ import { cloneDeep } from 'lodash'; -import { hasProperty, isObject } from '@metamask/utils'; -import { v4 } from 'uuid'; +import { isObject } from '@metamask/utils'; export const version = 84; /** - * Ensure that each networkConfigurations object in state.NetworkController.networkConfigurations has an + * 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. @@ -25,21 +24,28 @@ export async function migrate(originalVersionedData: { } function transformState(state: Record) { - if ( - !isObject(state.NetworkController) - ) { + if (!isObject(state.NetworkController)) { return state; } - const NetworkController: any = state.NetworkController; + const { NetworkController }: any = state; - const networkConfigurations: Record = NetworkController.networkConfigurations; + const { + networkConfigurations, + }: { networkConfigurations: Record } = NetworkController; const newNetworkConfigurations: Record = {}; for (const networkConfigurationId in networkConfigurations) { - newNetworkConfigurations[networkConfigurationId] = { - ...networkConfigurations[networkConfigurationId], - id: networkConfigurationId, + if ( + Object.prototype.hasOwnProperty.call( + networkConfigurations, + networkConfigurationId, + ) + ) { + newNetworkConfigurations[networkConfigurationId] = { + ...networkConfigurations[networkConfigurationId], + id: networkConfigurationId, + }; } } From 2a8c2e7b5af06fce00ba7d1a4a3438c5a46331f2 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 10:53:08 -0230 Subject: [PATCH 3/9] Update app/scripts/migrations/084.ts Co-authored-by: Mark Stacey --- app/scripts/migrations/084.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index efb722f41094..6ddf1dfabc96 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -33,7 +33,7 @@ function transformState(state: Record) { networkConfigurations, }: { networkConfigurations: Record } = NetworkController; - const newNetworkConfigurations: Record = {}; + const newNetworkConfigurations: Record> = {}; for (const networkConfigurationId in networkConfigurations) { if ( From 65a8b8fcc787eea316ec8195f47087ef7cc72e0a Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 11:05:38 -0230 Subject: [PATCH 4/9] Add unit tests for error cases --- app/scripts/migrations/084.test.js | 110 +++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/app/scripts/migrations/084.test.js b/app/scripts/migrations/084.test.js index 333c8c9eb4f2..e93b561e5886 100644 --- a/app/scripts/migrations/084.test.js +++ b/app/scripts/migrations/084.test.js @@ -141,4 +141,114 @@ describe('migration #84', () => { }; 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); + }); }); From bb008c474d5900e890ac36baf92439b70d268cfc Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 11:06:06 -0230 Subject: [PATCH 5/9] Simplify code --- app/scripts/migrations/084.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 6ddf1dfabc96..92c359089e1d 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -35,18 +35,11 @@ function transformState(state: Record) { const newNetworkConfigurations: Record> = {}; - for (const networkConfigurationId in networkConfigurations) { - if ( - Object.prototype.hasOwnProperty.call( - networkConfigurations, - networkConfigurationId, - ) - ) { - newNetworkConfigurations[networkConfigurationId] = { - ...networkConfigurations[networkConfigurationId], - id: networkConfigurationId, - }; - } + for (const networkConfigurationId of Object.keys(networkConfigurations)) { + newNetworkConfigurations[networkConfigurationId] = { + ...networkConfigurations[networkConfigurationId], + id: networkConfigurationId, + }; } return { From 8116d9fe46a41a6692a2133091d2be5273903810 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 11:09:01 -0230 Subject: [PATCH 6/9] Remove unnecessary any typing --- app/scripts/migrations/084.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 92c359089e1d..04895ad0899a 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -27,7 +27,7 @@ function transformState(state: Record) { if (!isObject(state.NetworkController)) { return state; } - const { NetworkController }: any = state; + const { NetworkController } = state; const { networkConfigurations, From a601b3409667458b5cea7cbbdfd12a4fc68a12df Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 12:21:13 -0230 Subject: [PATCH 7/9] Fix network controller type checking --- app/scripts/migrations/084.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 04895ad0899a..9cb40f3d7839 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -27,12 +27,17 @@ function transformState(state: Record) { if (!isObject(state.NetworkController)) { return state; } - const { NetworkController } = state; + const { NetworkController }: any = state; + + if (!isObject(NetworkController.networkConfigurations)) { + return state; + } const { networkConfigurations, }: { networkConfigurations: Record } = NetworkController; + const newNetworkConfigurations: Record> = {}; for (const networkConfigurationId of Object.keys(networkConfigurations)) { From 83545e315340497203fa068c4f0d0766ac63eb5b Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 12:31:37 -0230 Subject: [PATCH 8/9] Lint fix --- app/scripts/migrations/084.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 9cb40f3d7839..3529d0402b5d 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -37,7 +37,6 @@ function transformState(state: Record) { networkConfigurations, }: { networkConfigurations: Record } = NetworkController; - const newNetworkConfigurations: Record> = {}; for (const networkConfigurationId of Object.keys(networkConfigurations)) { From affa96040eef3f412f1e0d9738bdb93cbb1f5e99 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 11 Apr 2023 14:54:45 -0230 Subject: [PATCH 9/9] Improve typing --- app/scripts/migrations/084.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 3529d0402b5d..4ae81cdc8680 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -27,21 +27,23 @@ function transformState(state: Record) { if (!isObject(state.NetworkController)) { return state; } - const { NetworkController }: any = state; + const { NetworkController } = state; if (!isObject(NetworkController.networkConfigurations)) { return state; } - const { - networkConfigurations, - }: { networkConfigurations: Record } = NetworkController; + const { networkConfigurations } = NetworkController; const newNetworkConfigurations: Record> = {}; for (const networkConfigurationId of Object.keys(networkConfigurations)) { + const networkConfiguration = networkConfigurations[networkConfigurationId]; + if (!isObject(networkConfiguration)) { + return state; + } newNetworkConfigurations[networkConfigurationId] = { - ...networkConfigurations[networkConfigurationId], + ...networkConfiguration, id: networkConfigurationId, }; }