Fix findNetworkClientIdByChainId to consider default endpoints

Currently, the `findNetworkClientIdByChainId` method in
`NetworkController`, which is used in various places, mostly commonly
TransactionController and the wallet middleware, returns the network
client ID corresponding to the first listed RPC endpoint for the given
chain. This may not match users' expectations if they have chosen
another RPC endpoint as the default. This commit fixes the
implementation of `findNetworkClientIdByChainId` to match this
expectation.

packages/network-controller/CHANGELOG.md

@@ -37,6 +37,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Bump `@metamask/eth-json-rpc-infura` to `^10.1.0`
   - Bump `@metamask/eth-json-rpc-middleware` to `^15.1.0`
 
+### Fixed
+
+- Fix `findNetworkClientIdByChainId` to return the network client ID for the chain's configured default RPC endpoint instead of its first listed RPC endpoint ([#5344](https://github.com/MetaMask/core/pull/5344))
+
 ## [22.2.1]
 
 ### Changed

packages/network-controller/src/NetworkController.ts

@@ -2100,20 +2100,27 @@ export class NetworkController extends BaseController<
   }
 
   /**
-   * Searches for a network configuration ID with the given ChainID and returns it.
+   * Searches for the default RPC endpoint configured for the given chain and
+   * returns its network client ID. This can then be passed to
+   * {@link getNetworkClientById} to retrieve the network client.
    *
-   * @param chainId - ChainId to search for
-   * @returns networkClientId of the network configuration with the given chainId
+   * @param chainId - Chain ID to search for.
+   * @returns The ID of the network client created for the chain's default RPC
+   * endpoint.
    */
   findNetworkClientIdByChainId(chainId: Hex): NetworkClientId {
-    const networkClients = this.getNetworkClientRegistry();
-    const networkClientEntry = Object.entries(networkClients).find(
-      ([_, networkClient]) => networkClient.configuration.chainId === chainId,
-    );
-    if (networkClientEntry === undefined) {
-      throw new Error("Couldn't find networkClientId for chainId");
+    const networkConfiguration =
+      this.state.networkConfigurationsByChainId[chainId];
+
+    if (!networkConfiguration) {
+      throw new Error(`Invalid chain ID "${chainId}"`);
     }
-    return networkClientEntry[0];
+
+    const { networkClientId } =
+      networkConfiguration.rpcEndpoints[
+        networkConfiguration.defaultRpcEndpointIndex
+      ];
+    return networkClientId;
   }
 
   /**

packages/network-controller/tests/NetworkController.test.ts

@@ -969,48 +969,81 @@ describe('NetworkController', () => {
     });
   });
 
-  describe('findNetworkConfigurationByChainId', () => {
-    it('returns the network configuration for the given chainId', async () => {
+  describe.each([
+    [
+      'findNetworkClientIdByChainId',
+      (
+        {
+          controller,
+        }: {
+          controller: NetworkController;
+        },
+        args: Parameters<NetworkController['findNetworkClientIdByChainId']>,
+      ): ReturnType<NetworkController['findNetworkClientIdByChainId']> =>
+        controller.findNetworkClientIdByChainId(...args),
+    ],
+    [
+      'NetworkController:findNetworkClientIdByChainId',
+      (
+        {
+          messenger,
+        }: {
+          messenger: Messenger<
+            NetworkControllerActions,
+            NetworkControllerEvents
+          >;
+        },
+        args: Parameters<NetworkController['findNetworkClientIdByChainId']>,
+      ): ReturnType<NetworkController['findNetworkClientIdByChainId']> =>
+        messenger.call(
+          'NetworkController:findNetworkClientIdByChainId',
+          ...args,
+        ),
+    ],
+  ])('%s', (_desc, findNetworkClientIdByChainId) => {
+    it('returns the ID of the network client corresponding to the default RPC endpoint for the given chain', async () => {
       await withController(
-        { infuraProjectId: 'some-infura-project-id' },
-        async ({ controller }) => {
-          const fakeNetworkClient = buildFakeClient();
-          mockCreateNetworkClient().mockReturnValue(fakeNetworkClient);
-
-          const networkClientId =
-            controller.findNetworkClientIdByChainId('0x1');
-          expect(networkClientId).toBe('mainnet');
+        {
+          state: buildNetworkControllerStateWithDefaultSelectedNetworkClientId({
+            networkConfigurationsByChainId: {
+              '0x1337': buildCustomNetworkConfiguration({
+                chainId: '0x1337' as const,
+                defaultRpcEndpointIndex: 1,
+                rpcEndpoints: [
+                  {
+                    name: 'Test Endpoint 1',
+                    networkClientId: 'AAAA-AAAA-AAAA-AAAA',
+                    url: 'https://test.network/1',
+                    type: RpcEndpointType.Custom,
+                  },
+                  {
+                    name: 'Test Endpoint 2',
+                    networkClientId: 'BBBB-BBBB-BBBB-BBBB',
+                    url: 'https://test.network/2',
+                    type: RpcEndpointType.Custom,
+                  },
+                ],
+              }),
+            },
+          }),
         },
-      );
-    });
+        ({ controller, messenger }) => {
+          const networkClientId = findNetworkClientIdByChainId(
+            { controller, messenger },
+            ['0x1337'],
+          );
 
-    it('throws if the chainId doesnt exist in the configuration', async () => {
-      await withController(
-        { infuraProjectId: 'some-infura-project-id' },
-        async ({ controller }) => {
-          const fakeNetworkClient = buildFakeClient();
-          mockCreateNetworkClient().mockReturnValue(fakeNetworkClient);
-          expect(() =>
-            controller.findNetworkClientIdByChainId('0xdeadbeef'),
-          ).toThrow("Couldn't find networkClientId for chainId");
+          expect(networkClientId).toBe('BBBB-BBBB-BBBB-BBBB');
         },
       );
     });
 
-    it('is callable from the controller messenger', async () => {
-      await withController(
-        { infuraProjectId: 'some-infura-project-id' },
-        async ({ messenger }) => {
-          const fakeNetworkClient = buildFakeClient();
-          mockCreateNetworkClient().mockReturnValue(fakeNetworkClient);
-
-          const networkClientId = messenger.call(
-            'NetworkController:findNetworkClientIdByChainId',
-            '0x1',
-          );
-          expect(networkClientId).toBe('mainnet');
-        },
-      );
+    it('throws if there are no network clients registered for the given chain', async () => {
+      await withController(({ controller, messenger }) => {
+        expect(() =>
+          findNetworkClientIdByChainId({ controller, messenger }, ['0x999999']),
+        ).toThrow('Invalid chain ID "0x999999"');
+      });
     });
   });