Fix: SelectedNetworkController should return globally selected networkClient from getProviderAndBlockTracker when domain not in state (#4063)

## Explanation

The SelectedNetworkController currently constructs a proxy for any
domain that has permissions. Other domains have no associated proxy, so
the getProviderAndBlockTracker method would throw an error.

This is problematic because we grab the network client for an origin a
single time when constructing an RPC pipeline for that origin in the
MetaMask extension. We don't re-create the RPC pipeline when permissions
change. That means that the pipeline is setup with the wrong network
client (see here for more details on this bug:
MetaMask/metamask-extension#23509 )

To resolve this problem, we can instead keep network client proxies for
all origins, not just those with permissions. Origins without
permissions still should not have a selected network client ID state,
but they can have proxies that point at the globally selected network
client. That way, when the origin is granted permissions, we can
redirect this proxy to the selected network client and everything works
smoothly, without needing to reconstruct the RPC pipeline.

This PR should be merged shortly after
#4104

## References
Fixes: #4062
See: #4104

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/network-controller`
- **ADDED**: Add `getSelectedNetworkClient()` method that returns the
provider and blockTracker for the currently selected network
- **ADDED**: Add 'NetworkController:getSelectedNetworkClient' action

### `@metamask/selected-network-controller`
- **BREAKING**: `SelectedNetworkController` now requires the
`NetworkController:getSelectedNetworkClient` messenger action
- **CHANGED**: Previously when a domain became no longer permissioned,
it's network client proxy would continue to point at the networkClientId
it was last set to. Now it is set to follow the globally selected
network
- **CHANGED**: `SelectedNetworkController. getProviderAndBlockTracker()`
no longer throws an error if the `useRequestQueue` flag is false
- **CHANGED**: Previously `SelectedNetworkController.
getProviderAndBlockTracker()` threw an error if there was no
networkClientId set for the passed domain. Now it returns a proxy that
follows the globally selected network instead.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

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

Author: 3 people

packages/network-controller/src/NetworkController.ts

@@ -426,6 +426,11 @@ export type NetworkControllerGetNetworkClientByIdAction = {
   handler: NetworkController['getNetworkClientById'];
 };
 
+export type NetworkControllerGetSelectedNetworkClientAction = {
+  type: `NetworkController:getSelectedNetworkClient`;
+  handler: NetworkController['getSelectedNetworkClient'];
+};
+
 export type NetworkControllerGetEIP1559CompatibilityAction = {
   type: `NetworkController:getEIP1559Compatibility`;
   handler: NetworkController['getEIP1559Compatibility'];
@@ -462,6 +467,7 @@ export type NetworkControllerActions =
   | NetworkControllerGetProviderConfigAction
   | NetworkControllerGetEthQueryAction
   | NetworkControllerGetNetworkClientByIdAction
+  | NetworkControllerGetSelectedNetworkClientAction
   | NetworkControllerGetEIP1559CompatibilityAction
   | NetworkControllerFindNetworkClientIdByChainIdAction
   | NetworkControllerSetActiveNetworkAction
@@ -634,13 +640,18 @@ export class NetworkController extends BaseController<
       this.getNetworkConfigurationByNetworkClientId.bind(this),
     );
 
+    this.messagingSystem.registerActionHandler(
+      `${this.name}:getSelectedNetworkClient`,
+      this.getSelectedNetworkClient.bind(this),
+    );
+
     this.#previousProviderConfig = this.state.providerConfig;
   }
 
   /**
    * Accesses the provider and block tracker for the currently selected network.
-   *
    * @returns The proxy and block tracker proxies.
+   * @deprecated This method has been replaced by `getSelectedNetworkClient` (which has a more easily used return type) and will be removed in a future release.
    */
   getProviderAndBlockTracker(): {
     provider: SwappableProxy<ProxyWithAccessibleTarget<Provider>> | undefined;
@@ -654,6 +665,26 @@ export class NetworkController extends BaseController<
     };
   }
 
+  /**
+   * Accesses the provider and block tracker for the currently selected network.
+   *
+   * @returns an object with the provider and block tracker proxies for the currently selected network.
+   */
+  getSelectedNetworkClient():
+    | {
+        provider: SwappableProxy<ProxyWithAccessibleTarget<Provider>>;
+        blockTracker: SwappableProxy<ProxyWithAccessibleTarget<BlockTracker>>;
+      }
+    | undefined {
+    if (this.#providerProxy && this.#blockTrackerProxy) {
+      return {
+        provider: this.#providerProxy,
+        blockTracker: this.#blockTrackerProxy,
+      };
+    }
+    return undefined;
+  }
+
   /**
    * Returns all of the network clients that have been created so far, keyed by
    * their identifier in the network client registry. This collection represents

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

@@ -7426,6 +7426,33 @@ function lookupNetworkTests({
       );
     });
   });
+
+  describe('getSelectedNetworkClient', () => {
+    it('returns the selected network provider and blockTracker proxy when initialized', async () => {
+      await withController(async ({ controller }) => {
+        const fakeProvider = buildFakeProvider();
+        const fakeNetworkClient = buildFakeClient(fakeProvider);
+        mockCreateNetworkClient().mockReturnValue(fakeNetworkClient);
+        await controller.initializeProvider();
+        const defaultNetworkClient = controller.getProviderAndBlockTracker();
+
+        const selectedNetworkClient = controller.getSelectedNetworkClient();
+        expect(defaultNetworkClient.provider).toBe(
+          selectedNetworkClient?.provider,
+        );
+        expect(defaultNetworkClient.blockTracker).toBe(
+          selectedNetworkClient?.blockTracker,
+        );
+      });
+    });
+
+    it('returns undefined when the selected network provider and blockTracker proxy are not initialized', async () => {
+      await withController(async ({ controller }) => {
+        const selectedNetworkClient = controller.getSelectedNetworkClient();
+        expect(selectedNetworkClient).toBeUndefined();
+      });
+    });
+  });
 }
 
 /**

packages/selected-network-controller/src/SelectedNetworkController.ts

@@ -4,6 +4,7 @@ import type {
   BlockTrackerProxy,
   NetworkClientId,
   NetworkControllerGetNetworkClientByIdAction,
+  NetworkControllerGetSelectedNetworkClientAction,
   NetworkControllerGetStateAction,
   NetworkControllerStateChangeEvent,
   ProviderProxy,
@@ -71,6 +72,7 @@ export type SelectedNetworkControllerActions =
 
 export type AllowedActions =
   | NetworkControllerGetNetworkClientByIdAction
+  | NetworkControllerGetSelectedNetworkClientAction
   | NetworkControllerGetStateAction
   | PermissionControllerHasPermissions
   | PermissionControllerGetSubjectsAction;
@@ -171,9 +173,7 @@ export class SelectedNetworkController extends BaseController<
               op === 'remove' &&
               this.state.domains[domain] !== undefined
             ) {
-              this.update(({ domains }) => {
-                delete domains[domain];
-              });
+              this.#unsetNetworkClientIdForDomain(domain);
             }
           }
         });
@@ -222,24 +222,39 @@ export class SelectedNetworkController extends BaseController<
       'NetworkController:getNetworkClientById',
       networkClientId,
     );
-    const networkProxy = this.#domainProxyMap.get(domain);
-    if (networkProxy === undefined) {
-      this.#domainProxyMap.set(domain, {
-        provider: createEventEmitterProxy(networkClient.provider),
-        blockTracker: createEventEmitterProxy(networkClient.blockTracker, {
-          eventFilter: 'skipInternal',
-        }),
-      });
-    } else {
-      networkProxy.provider.setTarget(networkClient.provider);
-      networkProxy.blockTracker.setTarget(networkClient.blockTracker);
-    }
+    const networkProxy = this.getProviderAndBlockTracker(domain);
+    networkProxy.provider.setTarget(networkClient.provider);
+    networkProxy.blockTracker.setTarget(networkClient.blockTracker);
 
     this.update((state) => {
       state.domains[domain] = networkClientId;
     });
   }
 
+  /**
+   * This method is used when a domain is removed from the PermissionsController.
+   * It will remove re-point the network proxy to the globally selected network in the domainProxyMap or, if no globally selected network client is available, delete the proxy.
+   *
+   * @param domain - The domain for which to unset the network client ID.
+   */
+  #unsetNetworkClientIdForDomain(domain: Domain) {
+    const globallySelectedNetworkClient = this.messagingSystem.call(
+      'NetworkController:getSelectedNetworkClient',
+    );
+    const networkProxy = this.#domainProxyMap.get(domain);
+    if (networkProxy && globallySelectedNetworkClient) {
+      networkProxy.provider.setTarget(globallySelectedNetworkClient.provider);
+      networkProxy.blockTracker.setTarget(
+        globallySelectedNetworkClient.blockTracker,
+      );
+    } else if (networkProxy) {
+      this.#domainProxyMap.delete(domain);
+    }
+    this.update((state) => {
+      delete state.domains[domain];
+    });
+  }
+
   #domainHasPermissions(domain: Domain): boolean {
     return this.messagingSystem.call(
       'PermissionController:hasPermissions',
@@ -282,23 +297,23 @@ export class SelectedNetworkController extends BaseController<
    * @returns The proxy and block tracker proxies.
    */
   getProviderAndBlockTracker(domain: Domain): NetworkProxy {
-    if (!this.#getUseRequestQueue()) {
-      throw new Error(
-        'Provider and BlockTracker should be fetched from NetworkController when useRequestQueue is false',
-      );
-    }
     const networkClientId = this.state.domains[domain];
-    if (!networkClientId) {
-      throw new Error(
-        'NetworkClientId has not been set for the requested domain',
-      );
-    }
     let networkProxy = this.#domainProxyMap.get(domain);
     if (networkProxy === undefined) {
-      const networkClient = this.messagingSystem.call(
-        'NetworkController:getNetworkClientById',
-        networkClientId,
-      );
+      let networkClient;
+      if (networkClientId === undefined) {
+        networkClient = this.messagingSystem.call(
+          'NetworkController:getSelectedNetworkClient',
+        );
+        if (networkClient === undefined) {
+          throw new Error('Selected network not initialized');
+        }
+      } else {
+        networkClient = this.messagingSystem.call(
+          'NetworkController:getNetworkClientById',
+          networkClientId,
+        );
+      }
       networkProxy = {
         provider: createEventEmitterProxy(networkClient.provider),
         blockTracker: createEventEmitterProxy(networkClient.blockTracker, {