fix: Remove obsolete PhishingController state (#26308)

## **Description**

Sentry reports showed cases of obsolete state in the PhishingController
state. This obsolete state has been removed.

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

## **Related issues**

Fixes #26307

## **Manual testing steps**

N/A

## **Screenshots/Recordings**

N/A

## **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
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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.

app/scripts/migrations/120.2.test.ts

@@ -313,4 +313,93 @@ describe('migration #120.2', () => {
       });
     });
   });
+
+  describe('PhishingController', () => {
+    it('does nothing if PhishingController state is not set', async () => {
+      const oldState = {
+        PreferencesController: {},
+      };
+
+      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('does nothing if obsolete properties are not set', async () => {
+      const oldState = {
+        PhishingController: {
+          phishingLists: [],
+        },
+      };
+
+      const transformedState = await migrate({
+        meta: { version: oldVersion },
+        data: cloneDeep(oldState),
+      });
+
+      expect(transformedState.data).toEqual(oldState);
+    });
+
+    it('removes all obsolete properties', async () => {
+      const oldState = {
+        PhishingController: {
+          listState: {},
+          phishingLists: [],
+        },
+      };
+
+      const transformedState = await migrate({
+        meta: { version: oldVersion },
+        data: cloneDeep(oldState),
+      });
+
+      expect(transformedState.data).toEqual({
+        PhishingController: {
+          phishingLists: [],
+        },
+      });
+    });
+
+    it('still migrates PhishingController state if other controllers have invalid state', async () => {
+      const oldState = {
+        NetworkController: 'invalid',
+        PhishingController: {
+          listState: {},
+          phishingLists: [],
+        },
+        SelectedNetworkController: 'invalid',
+        SnapController: 'invalid',
+      };
+
+      const transformedState = await migrate({
+        meta: { version: oldVersion },
+        data: cloneDeep(oldState),
+      });
+
+      expect(transformedState.data.PhishingController).toEqual({
+        phishingLists: [],
+      });
+    });
+  });
 });

app/scripts/migrations/120.2.ts

@@ -110,6 +110,32 @@ function removeObsoleteNetworkControllerState(
   delete networkControllerState.provider;
 }
 
+/**
+ * Remove obsolete `listState` property from PhishingController state.
+ *
+ * We don't know exactly why yet, but we see from Sentry that some users have this property still
+ * in state. It is no longer used.
+ *
+ * @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;
+  }
+  if (hasProperty(state.PhishingController, 'listState')) {
+    delete state.PhishingController.listState;
+  }
+}
+
 /**
  * Remove obsolete controller state.
  *
@@ -119,4 +145,5 @@ function transformState(state: Record<string, unknown>): void {
   removeObsoleteSnapControllerState(state);
   removeObsoleteSelectedNetworkControllerState(state);
   removeObsoleteNetworkControllerState(state);
+  removeObsoletePhishingControllerState(state);
 }