Sync NetworkController upsertNetworkConfiguration w/ core (#19426)

This makes it easier to visually compare differences in the
NetworkController unit tests between core and this repo.

app/scripts/controllers/network/network-controller.test.ts

@@ -15,7 +15,6 @@ import {
   NetworkStatus,
   NETWORK_TYPES,
 } from '../../../../shared/constants/network';
-import { MetaMetricsNetworkEventSource } from '../../../../shared/constants/metametrics';
 import {
   NetworkController,
   NetworkControllerAction,
@@ -3880,13 +3879,92 @@ describe('NetworkController', () => {
   });
 
   describe('upsertNetworkConfiguration', () => {
+    it('adds the given network configuration when its rpcURL does not match an existing configuration', async () => {
+      uuidV4Mock.mockImplementationOnce(() => 'network-configuration-id-1');
+
+      await withController(async ({ controller }) => {
+        const rpcUrlNetwork = {
+          chainId: toHex(9999),
+          rpcUrl: 'https://test-rpc.com',
+          ticker: 'RPC',
+        };
+
+        expect(controller.store.getState().networkConfigurations).toStrictEqual(
+          {},
+        );
+
+        await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
+          referrer: 'https://test-dapp.com',
+          source: 'dapp',
+        });
+
+        expect(
+          Object.values(controller.store.getState().networkConfigurations),
+        ).toStrictEqual(
+          expect.arrayContaining([
+            {
+              ...rpcUrlNetwork,
+              nickname: undefined,
+              rpcPrefs: undefined,
+              id: 'network-configuration-id-1',
+            },
+          ]),
+        );
+      });
+    });
+
+    it('update a network configuration when the configuration being added has an rpcURL that matches an existing configuration', async () => {
+      await withController(
+        {
+          state: {
+            networkConfigurations: {
+              testNetworkConfigurationId: {
+                rpcUrl: 'https://rpc-url.com',
+                ticker: 'old_rpc_ticker',
+                nickname: 'old_rpc_nickname',
+                rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
+                chainId: toHex(1),
+                id: 'testNetworkConfigurationId',
+              },
+            },
+          },
+        },
+        async ({ controller }) => {
+          await controller.upsertNetworkConfiguration(
+            {
+              rpcUrl: 'https://rpc-url.com',
+              ticker: 'new_rpc_ticker',
+              nickname: 'new_rpc_nickname',
+              rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
+              chainId: toHex(1),
+            },
+            { referrer: 'https://test-dapp.com', source: 'dapp' },
+          );
+          expect(
+            Object.values(controller.store.getState().networkConfigurations),
+          ).toStrictEqual(
+            expect.arrayContaining([
+              {
+                rpcUrl: 'https://rpc-url.com',
+                nickname: 'new_rpc_nickname',
+                ticker: 'new_rpc_ticker',
+                rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
+                chainId: toHex(1),
+                id: 'testNetworkConfigurationId',
+              },
+            ]),
+          );
+        },
+      );
+    });
+
     it('throws if the given chain ID is not a 0x-prefixed hex number', async () => {
       const invalidChainId = '1';
       await withController(async ({ controller }) => {
-        await expect(() =>
+        await expect(async () =>
           controller.upsertNetworkConfiguration(
             {
-              /* @ts-expect-error We are intentionally passing bad input. */
+              // @ts-expect-error Intentionally invalid
               chainId: invalidChainId,
               nickname: 'RPC',
               rpcPrefs: { blockExplorerUrl: 'test-block-explorer.com' },
@@ -3895,7 +3973,7 @@ describe('NetworkController', () => {
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           ),
         ).rejects.toThrow(
@@ -3908,7 +3986,7 @@ describe('NetworkController', () => {
 
     it('throws if the given chain ID is greater than the maximum allowed ID', async () => {
       await withController(async ({ controller }) => {
-        await expect(() =>
+        await expect(async () =>
           controller.upsertNetworkConfiguration(
             {
               chainId: '0xFFFFFFFFFFFFFFFF',
@@ -3919,7 +3997,7 @@ describe('NetworkController', () => {
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           ),
         ).rejects.toThrow(
@@ -3936,14 +4014,14 @@ describe('NetworkController', () => {
           controller.upsertNetworkConfiguration(
             /* @ts-expect-error We are intentionally passing bad input. */
             {
-              chainId: '0x9999',
+              chainId: toHex(9999),
               nickname: 'RPC',
               rpcPrefs: { blockExplorerUrl: 'test-block-explorer.com' },
               ticker: 'RPC',
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           ),
         ).rejects.toThrow(
@@ -3956,10 +4034,10 @@ describe('NetworkController', () => {
 
     it('throws if rpcUrl passed is not a valid Url', async () => {
       await withController(async ({ controller }) => {
-        await expect(() =>
+        await expect(async () =>
           controller.upsertNetworkConfiguration(
             {
-              chainId: '0x9999',
+              chainId: toHex(9999),
               nickname: 'RPC',
               rpcPrefs: { blockExplorerUrl: 'test-block-explorer.com' },
               ticker: 'RPC',
@@ -3967,7 +4045,7 @@ describe('NetworkController', () => {
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           ),
         ).rejects.toThrow(new Error('rpcUrl must be a valid URL'));
@@ -3976,18 +4054,18 @@ describe('NetworkController', () => {
 
     it('throws if the no (or a falsy) ticker is passed', async () => {
       await withController(async ({ controller }) => {
-        await expect(() =>
+        await expect(async () =>
           controller.upsertNetworkConfiguration(
-            /* @ts-expect-error We are intentionally passing bad input. */
+            // @ts-expect-error - we want to test the case where no ticker is present.
             {
-              chainId: '0x5',
+              chainId: toHex(5),
               nickname: 'RPC',
               rpcPrefs: { blockExplorerUrl: 'test-block-explorer.com' },
               rpcUrl: 'https://mock-rpc-url',
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           ),
         ).rejects.toThrow(
@@ -4000,22 +4078,63 @@ describe('NetworkController', () => {
 
     it('throws if an options object is not passed as a second argument', async () => {
       await withController(async ({ controller }) => {
-        await expect(() =>
+        await expect(async () =>
-          /* @ts-expect-error We are intentionally passing bad input. */
+          // @ts-expect-error - we want to test the case where no second arg is passed.
           controller.upsertNetworkConfiguration({
-            chainId: '0x5',
+            chainId: toHex(5),
             nickname: 'RPC',
             rpcPrefs: { blockExplorerUrl: 'test-block-explorer.com' },
             rpcUrl: 'https://mock-rpc-url',
           }),
-        ).rejects.toThrow(
+        ).rejects.toThrow('Cannot read properties of undefined');
-          new Error(
-            "Cannot read properties of undefined (reading 'setActive')",
-          ),
-        );
       });
     });
 
+    it('throws if referrer and source arguments are not passed', async () => {
+      uuidV4Mock.mockImplementationOnce(() => 'networkConfigurationId');
+      const trackEventSpy = jest.fn();
+      await withController(
+        {
+          state: {
+            providerConfig: {
+              type: NETWORK_TYPES.RPC,
+              rpcUrl: 'https://mock-rpc-url',
+              chainId: toHex(111),
+              ticker: 'TEST',
+              id: 'testNetworkConfigurationId',
+            },
+            networkConfigurations: {
+              testNetworkConfigurationId: {
+                rpcUrl: 'https://mock-rpc-url',
+                chainId: toHex(111),
+                ticker: 'TEST',
+                id: 'testNetworkConfigurationId',
+                nickname: undefined,
+                rpcPrefs: undefined,
+              },
+            },
+          },
+          trackMetaMetricsEvent: trackEventSpy,
+        },
+        async ({ controller }) => {
+          const newNetworkConfiguration = {
+            rpcUrl: 'https://new-chain-rpc-url',
+            chainId: toHex(222),
+            ticker: 'NEW',
+            nickname: 'new-chain',
+            rpcPrefs: { blockExplorerUrl: 'https://block-explorer' },
+          };
+
+          await expect(async () =>
+            // @ts-expect-error - we want to test the case where the options object is empty.
+            controller.upsertNetworkConfiguration(newNetworkConfiguration, {}),
+          ).rejects.toThrow(
+            'referrer and source are required arguments for adding or updating a network configuration',
+          );
+        },
+      );
+    });
+
     it('should add the given network if all required properties are present but nither rpcPrefs nor nickname properties are passed', async () => {
       uuidV4Mock.mockImplementationOnce(() => 'networkConfigurationId');
       await withController(
@@ -4026,14 +4145,14 @@ describe('NetworkController', () => {
         },
         async ({ controller }) => {
           const rpcUrlNetwork = {
-            chainId: '0x1' as const,
+            chainId: toHex(1),
             rpcUrl: 'https://test-rpc-url',
             ticker: 'test_ticker',
           };
 
           await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(
@@ -4062,7 +4181,7 @@ describe('NetworkController', () => {
         },
         async ({ controller }) => {
           const rpcUrlNetwork = {
-            chainId: '0x1' as const,
+            chainId: toHex(1),
             rpcUrl: 'https://test-rpc-url',
             ticker: 'test_ticker',
             invalidKey: 'new-chain',
@@ -4071,7 +4190,7 @@ describe('NetworkController', () => {
 
           await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(
@@ -4079,7 +4198,7 @@ describe('NetworkController', () => {
           ).toStrictEqual(
             expect.arrayContaining([
               {
-                chainId: '0x1',
+                chainId: toHex(1),
                 rpcUrl: 'https://test-rpc-url',
                 ticker: 'test_ticker',
                 nickname: undefined,
@@ -4103,7 +4222,7 @@ describe('NetworkController', () => {
                 ticker: 'ticker',
                 nickname: 'nickname',
                 rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-                chainId: '0x1',
+                chainId: toHex(1),
                 id: 'networkConfigurationId',
               },
             },
@@ -4111,7 +4230,7 @@ describe('NetworkController', () => {
         },
         async ({ controller }) => {
           const rpcUrlNetwork = {
-            chainId: '0x1' as const,
+            chainId: toHex(1),
             nickname: 'RPC',
             rpcPrefs: undefined,
             rpcUrl: 'https://test-rpc-url-2',
@@ -4120,7 +4239,7 @@ describe('NetworkController', () => {
 
           await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(
@@ -4132,7 +4251,7 @@ describe('NetworkController', () => {
                 ticker: 'ticker',
                 nickname: 'nickname',
                 rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-                chainId: '0x1',
+                chainId: toHex(1),
                 id: 'networkConfigurationId',
               },
               { ...rpcUrlNetwork, id: 'networkConfigurationId2' },
@@ -4152,7 +4271,7 @@ describe('NetworkController', () => {
                 ticker: 'old_rpc_ticker',
                 nickname: 'old_rpc_chainName',
                 rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-                chainId: '0x1',
+                chainId: toHex(1),
                 id: 'networkConfigurationId',
               },
             },
@@ -4165,11 +4284,11 @@ describe('NetworkController', () => {
             ticker: 'new_rpc_ticker',
             nickname: 'new_rpc_chainName',
             rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
-            chainId: '0x1' as const,
+            chainId: toHex(1),
           };
           await controller.upsertNetworkConfiguration(updatedConfiguration, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
           expect(
             Object.values(controller.store.getState().networkConfigurations),
@@ -4179,7 +4298,7 @@ describe('NetworkController', () => {
               nickname: 'new_rpc_chainName',
               ticker: 'new_rpc_ticker',
               rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
-              chainId: '0x1',
+              chainId: toHex(1),
               id: 'networkConfigurationId',
             },
           ]);
@@ -4197,7 +4316,7 @@ describe('NetworkController', () => {
                 ticker: 'ticker',
                 nickname: 'nickname',
                 rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-                chainId: '0x1',
+                chainId: toHex(1),
                 id: 'networkConfigurationId',
               },
               networkConfigurationId2: {
@@ -4205,7 +4324,7 @@ describe('NetworkController', () => {
                 ticker: 'ticker-2',
                 nickname: 'nickname-2',
                 rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-                chainId: '0x9999',
+                chainId: toHex(9999),
                 id: 'networkConfigurationId2',
               },
             },
@@ -4218,11 +4337,11 @@ describe('NetworkController', () => {
               ticker: 'new-ticker',
               nickname: 'new-nickname',
               rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
-              chainId: '0x1',
+              chainId: toHex(1),
             },
             {
               referrer: 'https://test-dapp.com',
-              source: MetaMetricsNetworkEventSource.Dapp,
+              source: 'dapp',
             },
           );
 
@@ -4234,7 +4353,7 @@ describe('NetworkController', () => {
               ticker: 'new-ticker',
               nickname: 'new-nickname',
               rpcPrefs: { blockExplorerUrl: 'alternativetestchainscan.io' },
-              chainId: '0x1',
+              chainId: toHex(1),
               id: 'networkConfigurationId',
             },
             {
@@ -4242,7 +4361,7 @@ describe('NetworkController', () => {
               ticker: 'ticker-2',
               nickname: 'nickname-2',
               rpcPrefs: { blockExplorerUrl: 'testchainscan.io' },
-              chainId: '0x9999',
+              chainId: toHex(9999),
               id: 'networkConfigurationId2',
             },
           ]);
@@ -4252,40 +4371,43 @@ describe('NetworkController', () => {
 
     it('should add the given network and not set it to active if the setActive option is not passed (or a falsy value is passed)', async () => {
       uuidV4Mock.mockImplementationOnce(() => 'networkConfigurationId');
-      const originalProviderConfig = {
+      const originalProvider = {
         type: NETWORK_TYPES.RPC,
         rpcUrl: 'https://mock-rpc-url',
-        chainId: '0xtest' as const,
+        chainId: toHex(111),
         ticker: 'TEST',
+        id: 'testNetworkConfigurationId',
       };
       await withController(
         {
           state: {
-            providerConfig: originalProviderConfig,
+            providerConfig: originalProvider,
             networkConfigurations: {
               testNetworkConfigurationId: {
                 rpcUrl: 'https://mock-rpc-url',
-                chainId: '0xtest',
+                chainId: toHex(111),
                 ticker: 'TEST',
                 id: 'testNetworkConfigurationId',
+                nickname: undefined,
+                rpcPrefs: undefined,
               },
             },
           },
         },
         async ({ controller }) => {
           const rpcUrlNetwork = {
-            chainId: '0x1' as const,
+            chainId: toHex(222),
             rpcUrl: 'https://test-rpc-url',
             ticker: 'test_ticker',
           };
 
           await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(controller.store.getState().providerConfig).toStrictEqual(
-            originalProviderConfig,
+            originalProvider,
           );
         },
       );
@@ -4297,17 +4419,22 @@ describe('NetworkController', () => {
         {
           state: {
             providerConfig: {
-              type: 'rpc',
+              type: NETWORK_TYPES.RPC,
               rpcUrl: 'https://mock-rpc-url',
-              chainId: '0xtest',
+              chainId: toHex(111),
               ticker: 'TEST',
+              id: 'testNetworkConfigurationId',
+              nickname: undefined,
+              rpcPrefs: undefined,
             },
             networkConfigurations: {
               testNetworkConfigurationId: {
                 rpcUrl: 'https://mock-rpc-url',
-                chainId: '0xtest',
+                chainId: toHex(111),
                 ticker: 'TEST',
                 id: 'testNetworkConfigurationId',
+                nickname: undefined,
+                rpcPrefs: undefined,
               },
             },
           },
@@ -4315,25 +4442,27 @@ describe('NetworkController', () => {
         async ({ controller }) => {
           const fakeProvider = buildFakeProvider();
           const fakeNetworkClient = buildFakeClient(fakeProvider);
-          mockCreateNetworkClient().mockReturnValue(fakeNetworkClient);
+          createNetworkClientMock.mockReturnValue(fakeNetworkClient);
           const rpcUrlNetwork = {
-            chainId: '0x1' as const,
             rpcUrl: 'https://test-rpc-url',
+            chainId: toHex(222),
             ticker: 'test_ticker',
           };
 
           await controller.upsertNetworkConfiguration(rpcUrlNetwork, {
             setActive: true,
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(controller.store.getState().providerConfig).toStrictEqual({
-            ...rpcUrlNetwork,
+            type: 'rpc',
+            rpcUrl: 'https://test-rpc-url',
+            chainId: toHex(222),
+            ticker: 'test_ticker',
+            id: 'networkConfigurationId',
             nickname: undefined,
             rpcPrefs: undefined,
-            type: 'rpc',
-            id: 'networkConfigurationId',
           });
         },
       );
@@ -4346,17 +4475,22 @@ describe('NetworkController', () => {
         {
           state: {
             providerConfig: {
-              type: 'rpc',
+              type: NETWORK_TYPES.RPC,
               rpcUrl: 'https://mock-rpc-url',
-              chainId: '0xtest',
+              chainId: toHex(111),
               ticker: 'TEST',
+              id: 'testNetworkConfigurationId',
+              nickname: undefined,
+              rpcPrefs: undefined,
             },
             networkConfigurations: {
               testNetworkConfigurationId: {
                 rpcUrl: 'https://mock-rpc-url',
-                chainId: '0xtest',
+                chainId: toHex(111),
                 ticker: 'TEST',
                 id: 'testNetworkConfigurationId',
+                nickname: undefined,
+                rpcPrefs: undefined,
               },
             },
           },
@@ -4365,7 +4499,7 @@ describe('NetworkController', () => {
         async ({ controller }) => {
           const newNetworkConfiguration = {
             rpcUrl: 'https://new-chain-rpc-url',
-            chainId: '0x9999' as const,
+            chainId: toHex(222),
             ticker: 'NEW',
             nickname: 'new-chain',
             rpcPrefs: { blockExplorerUrl: 'https://block-explorer' },
@@ -4373,7 +4507,7 @@ describe('NetworkController', () => {
 
           await controller.upsertNetworkConfiguration(newNetworkConfiguration, {
             referrer: 'https://test-dapp.com',
-            source: MetaMetricsNetworkEventSource.Dapp,
+            source: 'dapp',
           });
 
           expect(
@@ -4381,9 +4515,11 @@ describe('NetworkController', () => {
           ).toStrictEqual([
             {
               rpcUrl: 'https://mock-rpc-url',
-              chainId: '0xtest',
+              chainId: toHex(111),
               ticker: 'TEST',
               id: 'testNetworkConfigurationId',
+              nickname: undefined,
+              rpcPrefs: undefined,
             },
             {
               ...newNetworkConfiguration,
@@ -4397,7 +4533,7 @@ describe('NetworkController', () => {
               url: 'https://test-dapp.com',
             },
             properties: {
-              chain_id: '0x9999',
+              chain_id: toHex(222),
               symbol: 'NEW',
               source: 'dapp',
             },
@@ -4405,48 +4541,6 @@ describe('NetworkController', () => {
         },
       );
     });
-
-    it('throws if referrer and source arguments are not passed', async () => {
-      uuidV4Mock.mockImplementationOnce(() => 'networkConfigurationId');
-      const trackEventSpy = jest.fn();
-      await withController(
-        {
-          state: {
-            providerConfig: {
-              type: 'rpc',
-              rpcUrl: 'https://mock-rpc-url',
-              chainId: '0xtest',
-              ticker: 'TEST',
-            },
-            networkConfigurations: {
-              testNetworkConfigurationId: {
-                rpcUrl: 'https://mock-rpc-url',
-                chainId: '0xtest',
-                ticker: 'TEST',
-                id: 'testNetworkConfigurationId',
-              },
-            },
-          },
-          trackMetaMetricsEvent: trackEventSpy,
-        },
-        async ({ controller }) => {
-          const newNetworkConfiguration = {
-            rpcUrl: 'https://new-chain-rpc-url',
-            chainId: '0x9999' as const,
-            ticker: 'NEW',
-            nickname: 'new-chain',
-            rpcPrefs: { blockExplorerUrl: 'https://block-explorer' },
-          };
-
-          await expect(() =>
-            /* @ts-expect-error We are intentionally passing bad input. */
-            controller.upsertNetworkConfiguration(newNetworkConfiguration, {}),
-          ).rejects.toThrow(
-            'referrer and source are required arguments for adding or updating a network configuration',
-          );
-        },
-      );
-    });
   });
 
   describe('removeNetworkConfigurations', () => {