fix: Remove obsolete network controller state properties (#20586)

* fix: remove obsolete network controller state properties

* address comments

* address comments

* address comments

* Add additional precautions for data mutation in migration tests

---------

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

app/scripts/background.js

@@ -360,7 +360,7 @@ async function loadPhishingWarningPage() {
   } catch (error) {
     if (error instanceof PhishingWarningPageTimeoutError) {
       console.warn(
-        'Phishing warning page timeout; page not guaraneteed to work offline.',
+        'Phishing warning page timeout; page not guaranteed to work offline.',
       );
     } else {
       console.error('Failed to initialize phishing warning page', error);

app/scripts/migrations/092.2.test.ts

@@ -0,0 +1,99 @@
+import { NetworkType, toHex } from '@metamask/controller-utils';
+import { NetworkStatus } from '@metamask/network-controller';
+import { cloneDeep } from 'lodash';
+import { version as currentStateVersion, migrate } from './092.2';
+
+const TEST_NETWORK_CONTROLLER_STATE = {
+  networkId: 'network-id',
+  networkStatus: NetworkStatus.Available,
+  providerConfig: {
+    type: NetworkType.rpc,
+    chainId: toHex(42),
+    nickname: 'Funky Town Chain',
+    ticker: 'ETH',
+    id: 'test-network-client-id',
+  },
+  networkDetails: { EIPS: {} },
+  networkConfigurations: {
+    'network-configuration-id-1': {
+      chainId: toHex(42),
+      nickname: 'Localhost 8545',
+      rpcPrefs: {},
+      rpcUrl: 'http://localhost:8545',
+      ticker: 'ETH',
+    },
+  },
+};
+
+const anyPreviousStateVersion = 91;
+
+describe('migration #96', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('should update the state version number in the appropriate metadata field', async () => {
+    const originalVersionedState = {
+      meta: { version: anyPreviousStateVersion },
+      data: {},
+    };
+
+    const newStorage = await migrate(cloneDeep(originalVersionedState));
+
+    expect(newStorage.meta).toStrictEqual({ version: currentStateVersion });
+  });
+
+  it('should return state unaltered if there is no network controller state', async () => {
+    const originalMetaMaskState = {
+      anotherController: 'another-controller-state',
+    };
+    const originalVersionedState = {
+      meta: { version: anyPreviousStateVersion },
+      data: originalMetaMaskState,
+    };
+
+    const updatedVersionedState = await migrate(
+      cloneDeep(originalVersionedState),
+    );
+    expect(updatedVersionedState.data).toStrictEqual(originalMetaMaskState);
+  });
+
+  it('should return unaltered state if there are no obsolete network controller state properties', async () => {
+    const originalMetaMaskState = {
+      anotherController: 'another-controller-state',
+      NetworkController: TEST_NETWORK_CONTROLLER_STATE,
+    };
+    const originalVersionedState = {
+      meta: { version: anyPreviousStateVersion },
+      data: originalMetaMaskState,
+    };
+
+    const updatedVersionedState = await migrate(
+      cloneDeep(originalVersionedState),
+    );
+    expect(updatedVersionedState.data).toStrictEqual(originalMetaMaskState);
+  });
+
+  it('should return updated state without obsolete network controller state properties', async () => {
+    const originalMetaMaskState = {
+      anotherController: 'another-controller-state',
+      NetworkController: {
+        ...TEST_NETWORK_CONTROLLER_STATE,
+        someSortOfRogueObsoleteStateProperty: 'exists',
+      },
+    };
+    const originalVersionedState = {
+      meta: { version: anyPreviousStateVersion },
+      data: originalMetaMaskState,
+    };
+
+    const updatedVersionedState = await migrate(
+      cloneDeep(originalVersionedState),
+    );
+    expect(updatedVersionedState.data).not.toStrictEqual(originalMetaMaskState);
+    expect(updatedVersionedState.data).toStrictEqual({
+      anotherController: 'another-controller-state',
+      NetworkController: TEST_NETWORK_CONTROLLER_STATE,
+    });
+  });
+});

app/scripts/migrations/092.2.ts

@@ -0,0 +1,75 @@
+import { hasProperty } from '@metamask/utils';
+import { captureException } from '@sentry/browser';
+import { cloneDeep, isObject, pick } from 'lodash';
+
+type MetaMaskState = Record<string, unknown>;
+type VersionedState = {
+  meta: { version: number };
+  data: MetaMaskState;
+};
+
+export const version = 92.2;
+
+/**
+ * This migration removes obsolete NetworkController state properties.
+ *
+ * @param originalVersionedState - Versioned MetaMask extension state, exactly what we persist to dist.
+ * @param originalVersionedState.meta - State metadata.
+ * @param originalVersionedState.meta.version - The current state version.
+ * @param originalVersionedState.data - The persisted MetaMask state, keyed by controller.
+ * @returns Updated versioned of MetaMask extension state.
+ */
+export async function migrate(
+  originalVersionedState: VersionedState,
+): Promise<VersionedState> {
+  const updatedVersionedState = cloneDeep(originalVersionedState);
+
+  updatedVersionedState.meta.version = version;
+  updatedVersionedState.data = transformState(updatedVersionedState.data);
+
+  return updatedVersionedState;
+}
+
+function transformState(originalState: MetaMaskState): MetaMaskState {
+  const updatedState =
+    filterOutObsoleteNetworkControllerStateProperties(originalState);
+
+  return updatedState;
+}
+
+function filterOutObsoleteNetworkControllerStateProperties(
+  state: MetaMaskState,
+): MetaMaskState {
+  // https://github.com/MetaMask/core/blob/%40metamask/network-controller%4010.3.1/packages/network-controller/src/NetworkController.ts#L336-L342
+  const CURRENT_NETWORK_CONTROLLER_STATE_PROPS = [
+    'networkId',
+    'networkStatus',
+    'providerConfig',
+    'networkDetails',
+    'networkConfigurations',
+  ];
+
+  if (
+    !hasProperty(state, 'NetworkController') ||
+    !isObject(state.NetworkController)
+  ) {
+    captureException(
+      `Migration ${version}: Invalid NetworkController state: ${typeof state.NetworkController}`,
+    );
+
+    return state;
+  }
+
+  const networkControllerState = state.NetworkController;
+
+  // delete network state properties that are not currently in use
+  const updatedNetworkController = pick(
+    networkControllerState,
+    CURRENT_NETWORK_CONTROLLER_STATE_PROPS,
+  );
+
+  return {
+    ...state,
+    NetworkController: updatedNetworkController,
+  };
+}

app/scripts/migrations/index.js

@@ -97,6 +97,7 @@ import * as m090 from './090';
 import * as m091 from './091';
 import * as m092 from './092';
 import * as m092point1 from './092.1';
+import * as m092point2 from './092.2';
 import * as m092point3 from './092.3';
 import * as m093 from './093';
 import * as m094 from './094';
@@ -196,6 +197,7 @@ const migrations = [
   m091,
   m092,
   m092point1,
+  m092point2,
   m092point3,
   m093,
   m094,