diff --git a/app/scripts/migrations/120.2.test.ts b/app/scripts/migrations/120.2.test.ts index b7860a4f03f2..9cd64275580b 100644 --- a/app/scripts/migrations/120.2.test.ts +++ b/app/scripts/migrations/120.2.test.ts @@ -250,9 +250,105 @@ describe('migration #120.2', () => { ); }); - it('does nothing if obsolete properties are not set', async () => { + it('captures an error and leaves state unchanged if providerConfig state is corrupted', async () => { + const oldState = { + NetworkController: { + providerConfig: 'invalid', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error( + `Migration ${version}: Invalid NetworkController providerConfig state of type 'string'`, + ), + ); + }); + + it('captures an error and leaves state unchanged if networkConfigurations state is corrupted', async () => { + const oldState = { + NetworkController: { + networkConfigurations: 'invalid', + providerConfig: {}, + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error( + `Migration ${version}: Invalid NetworkController networkConfigurations state of type 'string'`, + ), + ); + }); + + it('does nothing if obsolete properties and providerConfig are not set', async () => { + const oldState = { + NetworkController: { + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('does nothing if obsolete properties are not set and providerConfig is set to undefined', async () => { + const oldState = { + NetworkController: { + // This should be impossible because `undefined` cannot be returned from persisted state, + // it's not valid JSON. But a bug in migration 14 ends up setting this to `undefined`. + providerConfig: undefined, + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('does nothing if obsolete properties and providerConfig id are not set', async () => { + const oldState = { + NetworkController: { + providerConfig: {}, + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data).toEqual(oldState); + }); + + it('does not remove a valid providerConfig id', async () => { const oldState = { NetworkController: { + networkConfigurations: { + 'valid-id': {}, + }, + providerConfig: { + id: 'valid-id', + }, selectedNetworkClientId: 'example', }, }; @@ -289,6 +385,81 @@ describe('migration #120.2', () => { }); }); + it('removes providerConfig id if network configuration is missing', async () => { + const oldState = { + NetworkController: { + providerConfig: { + id: 'invalid-id', + }, + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data.NetworkController).toEqual({ + providerConfig: {}, + selectedNetworkClientId: 'example', + }); + }); + + it('removes providerConfig id that does not match any network configuration', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + 'valid-id': {}, + }, + providerConfig: { + id: 'invalid-id', + }, + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data.NetworkController).toEqual({ + networkConfigurations: { + 'valid-id': {}, + }, + providerConfig: {}, + selectedNetworkClientId: 'example', + }); + }); + + it('removes providerConfig id with an invalid type', async () => { + const oldState = { + NetworkController: { + networkConfigurations: { + '123': {}, + }, + providerConfig: { + id: 123, + }, + selectedNetworkClientId: 'example', + }, + }; + + const transformedState = await migrate({ + meta: { version: oldVersion }, + data: cloneDeep(oldState), + }); + + expect(transformedState.data.NetworkController).toEqual({ + networkConfigurations: { + '123': {}, + }, + providerConfig: {}, + selectedNetworkClientId: 'example', + }); + }); + it('still migrates NetworkController state if other controllers have invalid state', async () => { const oldState = { NetworkController: { diff --git a/app/scripts/migrations/120.2.ts b/app/scripts/migrations/120.2.ts index 1db33007bb00..06e6cc091b9e 100644 --- a/app/scripts/migrations/120.2.ts +++ b/app/scripts/migrations/120.2.ts @@ -1,5 +1,6 @@ import { cloneDeep } from 'lodash'; import { hasProperty, isObject } from '@metamask/utils'; +import log from 'loglevel'; type VersionedData = { meta: { version: number }; @@ -40,7 +41,7 @@ function removeObsoleteSnapControllerState( if (!hasProperty(state, 'SnapController')) { return; } else if (!isObject(state.SnapController)) { - global.sentry.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: Invalid SnapController state of type '${typeof state.SnapController}'`, ), @@ -93,7 +94,7 @@ function removeObsoleteNetworkControllerState( if (!hasProperty(state, 'NetworkController')) { return; } else if (!isObject(state.NetworkController)) { - global.sentry.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: Invalid NetworkController state of type '${typeof state.NetworkController}'`, ), @@ -103,6 +104,53 @@ function removeObsoleteNetworkControllerState( const networkControllerState = state.NetworkController; + // Check for invalid `providerConfig.id`, and remove if found + if ( + hasProperty(networkControllerState, 'providerConfig') && + // This should be impossible because `undefined` cannot be returned from persisted state, + // it's not valid JSON. But a bug in migration 14 ends up setting this to `undefined`. + networkControllerState.providerConfig !== undefined + ) { + if (!isObject(networkControllerState.providerConfig)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid NetworkController providerConfig state of type '${typeof state + .NetworkController.providerConfig}'`, + ), + ); + return; + } + const { providerConfig } = networkControllerState; + + const validNetworkConfigurationIds = []; + if (hasProperty(networkControllerState, 'networkConfigurations')) { + if (!isObject(networkControllerState.networkConfigurations)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid NetworkController networkConfigurations state of type '${typeof networkControllerState.networkConfigurations}'`, + ), + ); + return; + } + + validNetworkConfigurationIds.push( + ...Object.keys(networkControllerState.networkConfigurations), + ); + } + + if (hasProperty(providerConfig, 'id')) { + if ( + typeof providerConfig.id !== 'string' || + !validNetworkConfigurationIds.includes(providerConfig.id) + ) { + log.warn( + `Migration ${version}: Removing invalid provider id ${providerConfig.id}`, + ); + delete providerConfig.id; + } + } + } + delete networkControllerState.networkDetails; delete networkControllerState.networkId; delete networkControllerState.networkStatus;