Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cherry-pick): Add migration 120.4 to delete obsolete currency, phishing and network controller state #26390

Merged
merged 2 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 231 additions & 0 deletions app/scripts/migrations/120.4.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
import { cloneDeep } from 'lodash';
import { migrate, version } from './120.4';

const sentryCaptureExceptionMock = jest.fn();

global.sentry = {
captureException: sentryCaptureExceptionMock,
};

const oldVersion = 120.3;

describe('migration #120.4', () => {
afterEach(() => {
jest.resetAllMocks();
});

it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.meta).toStrictEqual({ version });
});

describe('CurrencyController', () => {
it('does nothing if CurrencyController state is not set', async () => {
const oldState = {
OtherController: {},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('captures an error and leaves state unchanged if CurrencyController state is corrupted', async () => {
const oldState = {
CurrencyController: 'invalid',
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
new Error(
`Migration ${version}: Invalid CurrencyController state of type 'string'`,
),
);
});

it('deletes obsolete properties from the CurrencyController state', async () => {
const oldState = {
CurrencyController: {
conversionDate: 'test',
conversionRate: 'test',
nativeCurrency: 'test',
pendingCurrentCurrency: 'test',
pendingNativeCurrency: 'test',
usdConversionRate: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({ CurrencyController: {} });
});

it('does not delete non-obsolete properties from the CurrencyController state', async () => {
const oldState = {
CurrencyController: {
currencyRates: { test: 123 },
conversionRate: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
CurrencyController: { currencyRates: { test: 123 } },
});
});
});

describe('PhishingController', () => {
it('does nothing if PhishingController state is not set', async () => {
const oldState = {
OtherController: {},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('captures an error and leaves state unchanged if PhishingController state is corrupted', async () => {
const oldState = {
PhishingController: 'invalid',
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
new Error(
`Migration ${version}: Invalid PhishingController state of type 'string'`,
),
);
});

it('deletes obsolete properties from the PhishingController state', async () => {
const oldState = {
PhishingController: {
phishing: 'test',
lastFetched: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({ PhishingController: {} });
});

it('does not delete non-obsolete properties from the PhishingController state', async () => {
const oldState = {
PhishingController: {
phishing: 'test',
phishingLists: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
PhishingController: { phishingLists: 'test' },
});
});
});

describe('NetworkController', () => {
it('does nothing if NetworkController state is not set', async () => {
const oldState = {
OtherController: {},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('captures an error and leaves state unchanged if NetworkController state is corrupted', async () => {
const oldState = {
NetworkController: '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 state of type 'string'`,
),
);
});

it('deletes obsolete properties from the NetworkController state', async () => {
const oldState = {
NetworkController: {
network: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({ NetworkController: {} });
});

it('does not delete non-obsolete properties from the NetworkController state', async () => {
const oldState = {
NetworkController: {
network: 'test',
selectedNetworkClientId: 'test',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: { selectedNetworkClientId: 'test' },
});
});
});
});
119 changes: 119 additions & 0 deletions app/scripts/migrations/120.4.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { hasProperty, isObject } from '@metamask/utils';
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 120.4;

/**
* This migration removes properties from the CurrencyController state that
* are no longer used. There presence in state causes "No metadata found" errors
*
* @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: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

/**
* Remove obsolete CurrencyController state
*
* The six properties deleted here were no longer used as of
* assets-controllers v18.0.0
*
* See https://github.com/MetaMask/core/pull/1805 for the removal of these
* properties from the controller.
*
* @param state - The persisted MetaMask state, keyed by controller.
*/
function removeObsoleteCurrencyControllerState(
state: Record<string, unknown>,
): void {
if (!hasProperty(state, 'CurrencyController')) {
return;
} else if (!isObject(state.CurrencyController)) {
global.sentry?.captureException?.(
new Error(
`Migration ${version}: Invalid CurrencyController state of type '${typeof state.CurrencyController}'`,
),
);
return;
}

delete state.CurrencyController.conversionDate;
delete state.CurrencyController.conversionRate;
delete state.CurrencyController.nativeCurrency;
delete state.CurrencyController.pendingCurrentCurrency;
delete state.CurrencyController.pendingNativeCurrency;
delete state.CurrencyController.usdConversionRate;
}

/**
* Remove obsolete PhishingController state
*
* @param state - The persisted MetaMask state, keyed by controller.
*/
function removeObsoletePhishingControllerState(
state: Record<string, unknown>,
): void {
if (!hasProperty(state, 'PhishingController')) {
return;
} else if (!isObject(state.PhishingController)) {
global.sentry?.captureException?.(
new Error(
`Migration ${version}: Invalid PhishingController state of type '${typeof state.PhishingController}'`,
),
);
return;
}

delete state.PhishingController.phishing;
delete state.PhishingController.lastFetched;
}

/**
* Remove obsolete NetworkController state
*
* @param state - The persisted MetaMask state, keyed by controller.
*/
function removeObsoleteNetworkControllerState(
state: Record<string, unknown>,
): void {
if (!hasProperty(state, 'NetworkController')) {
return;
} else if (!isObject(state.NetworkController)) {
global.sentry?.captureException?.(
new Error(
`Migration ${version}: Invalid NetworkController state of type '${typeof state.NetworkController}'`,
),
);
return;
}

delete state.NetworkController.network;
}

/**
* Remove obsolete controller state.
*
* @param state - The persisted MetaMask state, keyed by controller.
*/
function transformState(state: Record<string, unknown>): void {
removeObsoleteCurrencyControllerState(state);
removeObsoletePhishingControllerState(state);
removeObsoleteNetworkControllerState(state);
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ const migrations = [
require('./120.1'),
require('./120.2'),
require('./120.3'),
require('./120.4'),
];

export default migrations;