From a876eaba23fa6aa0ff4db05de80f9d06592ff4bc Mon Sep 17 00:00:00 2001
From: Elliot Winkler <elliot.winkler@gmail.com>
Date: Wed, 19 Apr 2023 09:47:33 -0600
Subject: [PATCH] removeNetworkConfiguration validates given ID (#18650)

In the `core` version of NetworkController, `removeNetworkConfiguration`
throws an error if the given network configuration ID doesn't match an
existing network configuration. This commits adds the same check for
consistency. It also makes some minor changes to the implementation and
tests for `removeNetworkConfiguration` for consistency as well.
---
 .../network/network-controller.test.ts        | 49 +++++++++++++------
 .../controllers/network/network-controller.ts |  9 ++--
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/app/scripts/controllers/network/network-controller.test.ts b/app/scripts/controllers/network/network-controller.test.ts
index 508d57fe7..8368decd0 100644
--- a/app/scripts/controllers/network/network-controller.test.ts
+++ b/app/scripts/controllers/network/network-controller.test.ts
@@ -7308,14 +7308,14 @@ describe('NetworkController', () => {
   });
 
   describe('removeNetworkConfigurations', () => {
-    it('should remove a network configuration', async () => {
-      const networkConfigurationId = 'networkConfigurationId';
+    it('removes a network configuration', async () => {
+      const networkConfigurationId = 'testNetworkConfigurationId';
       await withController(
         {
           state: {
             networkConfigurations: {
               [networkConfigurationId]: {
-                id: 'aaaaaa',
+                id: networkConfigurationId,
                 rpcUrl: 'https://test-rpc-url',
                 ticker: 'old_rpc_ticker',
                 nickname: 'old_rpc_chainName',
@@ -7326,25 +7326,44 @@ describe('NetworkController', () => {
           },
         },
         async ({ controller }) => {
-          expect(
-            Object.values(controller.store.getState().networkConfigurations),
-          ).toStrictEqual([
-            {
-              id: 'aaaaaa',
-              rpcUrl: 'https://test-rpc-url',
-              ticker: 'old_rpc_ticker',
-              nickname: 'old_rpc_chainName',
-              rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-              chainId: '0x1',
-            },
-          ]);
           controller.removeNetworkConfiguration(networkConfigurationId);
+
           expect(
             controller.store.getState().networkConfigurations,
           ).toStrictEqual({});
         },
       );
     });
+
+    it('throws if the networkConfigurationId it is passed does not correspond to a network configuration in state', async () => {
+      const testNetworkConfigurationId = 'testNetworkConfigurationId';
+      const invalidNetworkConfigurationId = 'invalidNetworkConfigurationId';
+      await withController(
+        {
+          state: {
+            networkConfigurations: {
+              [testNetworkConfigurationId]: {
+                rpcUrl: 'https://rpc-url.com',
+                ticker: 'old_rpc_ticker',
+                nickname: 'old_rpc_nickname',
+                rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
+                chainId: '0x1',
+                id: testNetworkConfigurationId,
+              },
+            },
+          },
+        },
+        async ({ controller }) => {
+          expect(() =>
+            controller.removeNetworkConfiguration(
+              invalidNetworkConfigurationId,
+            ),
+          ).toThrow(
+            `networkConfigurationId ${invalidNetworkConfigurationId} does not match a configured networkConfiguration`,
+          );
+        },
+      );
+    });
   });
 });
 
diff --git a/app/scripts/controllers/network/network-controller.ts b/app/scripts/controllers/network/network-controller.ts
index 62d89b69f..9b95a8c7c 100644
--- a/app/scripts/controllers/network/network-controller.ts
+++ b/app/scripts/controllers/network/network-controller.ts
@@ -1141,9 +1141,12 @@ export class NetworkController extends EventEmitter {
    * @param networkConfigurationId - The unique id for the network configuration
    * to remove.
    */
-  removeNetworkConfiguration(
-    networkConfigurationId: NetworkConfigurationId,
-  ): void {
+  removeNetworkConfiguration(networkConfigurationId: NetworkConfigurationId) {
+    if (!this.store.getState().networkConfigurations[networkConfigurationId]) {
+      throw new Error(
+        `networkConfigurationId ${networkConfigurationId} does not match a configured networkConfiguration`,
+      );
+    }
     const networkConfigurations = {
       ...this.store.getState().networkConfigurations,
     };