fix: Add migration 120.4 to delete obsolete currency controller state (…

…#26383)

## **Description**

We are seeing the following sorts of errors in production, as reported
by sentry, in v12.0.2:
`No metadata found for 'conversionDate'`
`No metadata found for 'usdConversionRate'`
`No metadata found for 'nativeCurrency'`
`No metadata found for 'conversionRate'`

Example issue:
https://metamask.sentry.io/issues/5682684113/events/b8006eebb65749f883e907242e52215b/?project=273505&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D+No+metadata+release%3A12.0.1&referrer=previous-event&statsPeriod=14d&stream_index=1

The `CurrencyRateController` stopped using six such properties with
MetaMask/core#1805, which was brought into the
extension with #21549, however, there was not a state migration to
delete those properties at the time

This PR adds migrations to delete those obsolete properties

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26383?quickstart=1)

## **Related issues**

Fixes: #26356

## **Manual testing steps**

To observe the error using steps that mimic users in production
1. Install v11.6.0 and onboard
2. Create a local dev build from the `master` branch
3. Update the v11.6.0 install to the local dev build
4. See the errors in the service worker console

If you repeat those steps, but in step two build from this branch
instead of the `master` branch, the errors will not occur

For a faster manual test of this PR, create a local development build of
this branch and then run this script in the service worker console:
```
window.chrome.storage.local.get(({ data, meta }) => chrome.storage.local.set({ data: { ...data, CurrencyController: { ...data.CurrencyController, conversionDate: 'Jan 1', conversionRate: '2', nativeCurrency: 'test' } }, meta: {...meta, version: 120 } }, () => { chrome.runtime.reload() }))
```
There should be no errors like `No metadata found for 'conversionRate'`
(but if you do the same on develop or master, those errors should be
present)

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Mark Stacey <[email protected]>

app/scripts/migrations/120.4.test.ts

@@ -0,0 +1,98 @@
+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 } },
+      });
+    });
+  });
+});

app/scripts/migrations/120.4.ts

@@ -0,0 +1,72 @@
+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 controller state.
+ *
+ * @param state - The persisted MetaMask state, keyed by controller.
+ */
+function transformState(state: Record<string, unknown>): void {
+  removeObsoleteCurrencyControllerState(state);
+}

app/scripts/migrations/index.js

@@ -134,6 +134,7 @@ const migrations = [
   require('./120.1'),
   require('./120.2'),
   require('./120.3'),
+  require('./120.4'),
   require('./121'),
   require('./122'),
   require('./123'),