From 3db1130868268370f11999dd87014b5ad49db730 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Fri, 2 Aug 2024 09:36:59 -0230
Subject: [PATCH] fix: Remove obsolete PhishingController state

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

Fixes #26307
---
 app/scripts/migrations/120.2.test.ts | 89 ++++++++++++++++++++++++++++
 app/scripts/migrations/120.2.ts      | 27 +++++++++
 2 files changed, 116 insertions(+)

diff --git a/app/scripts/migrations/120.2.test.ts b/app/scripts/migrations/120.2.test.ts
index 531fe4b0fb8c..b7860a4f03f2 100644
--- a/app/scripts/migrations/120.2.test.ts
+++ b/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: [],
+      });
+    });
+  });
 });
diff --git a/app/scripts/migrations/120.2.ts b/app/scripts/migrations/120.2.ts
index 70452a710747..1db33007bb00 100644
--- a/app/scripts/migrations/120.2.ts
+++ b/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);
 }