feat: NetworkController.upsertNetworkConfiguration allow upsert by ne…

…tworkConfigurationId (#4614)

## Explanation

Currently it is not possible to change the rpcUrl for an existing
network configuration without first removing it then re-adding it. This
poses an issue on extension when the rpcUrl for the currently selected
network is changed via the `NetworkForm`. The NetworkForm first removes
the existing networkClient which causes the SelectedNetworkController to
replace any references to the networkClientId being removed with the
selectedNetworkClientId, but the problem is that the
selectedNetworkClientId is no longer valid at that point. Really what we
want in this case is to allow the network configuration to be updated in
place by id, but only if the rpcUrl isn't changing to one that already
exists on a different network configuration.

This PR achieves that by changing
`NetworkController.upsertNetworkConfiguration()` to accept an optional
`id` in the `NetworkConfiguration` param. When provided, it MUST match
to an existing network configuration. Additionally, it must match the id
for any network configuration that may match the target rpcUrl

## References

Related: MetaMask/metamask-extension#26309

## 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**: `NetworkController.upsertNetworkConfiguration()` accepts an
optional `id` property on the `NetworkConfiguration` param. It now
allows a network configuration to have its `rpcUrl` updated in place
when an `id` is specified, but only if that new `rpcUrl` does not
already exist on a different network configuration.

## 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

packages/network-controller/src/NetworkController.ts

@@ -1115,7 +1115,9 @@ export class NetworkController extends BaseController<
    * @returns The ID for the added or updated network configuration.
    */
   async upsertNetworkConfiguration(
-    networkConfiguration: NetworkConfiguration,
+    networkConfiguration: NetworkConfiguration & {
+      id?: NetworkConfigurationId;
+    },
     {
       referrer,
       source,
@@ -1126,11 +1128,17 @@ export class NetworkController extends BaseController<
       setActive?: boolean;
     },
   ): Promise<string> {
-    const sanitizedNetworkConfiguration: NetworkConfiguration = pick(
-      networkConfiguration,
-      ['rpcUrl', 'chainId', 'ticker', 'nickname', 'rpcPrefs'],
-    );
-    const { rpcUrl, chainId, ticker } = sanitizedNetworkConfiguration;
+    const sanitizedNetworkConfiguration: NetworkConfiguration & {
+      id?: NetworkConfigurationId;
+    } = pick(networkConfiguration, [
+      'rpcUrl',
+      'chainId',
+      'ticker',
+      'nickname',
+      'rpcPrefs',
+      'id',
+    ]);
+    const { rpcUrl, chainId, ticker, id } = sanitizedNetworkConfiguration;
 
     assertIsStrictHexString(chainId);
     if (!isSafeChainId(chainId)) {
@@ -1166,12 +1174,33 @@ export class NetworkController extends BaseController<
     const autoManagedNetworkClientRegistry =
       this.#ensureAutoManagedNetworkClientRegistryPopulated();
 
-    const existingNetworkConfiguration = Object.values(
+    const existingNetworkConfigurationWithId = Object.values(
+      this.state.networkConfigurations,
+    ).find((networkConfig) => networkConfig.id === id);
+    if (id && !existingNetworkConfigurationWithId) {
+      throw new Error('No network configuration matches the provided id');
+    }
+
+    const existingNetworkConfigurationWithRpcUrl = Object.values(
       this.state.networkConfigurations,
     ).find(
       (networkConfig) =>
         networkConfig.rpcUrl.toLowerCase() === rpcUrl.toLowerCase(),
     );
+    if (
+      id &&
+      existingNetworkConfigurationWithRpcUrl &&
+      existingNetworkConfigurationWithRpcUrl.id !== id
+    ) {
+      throw new Error(
+        'A different network configuration already exists with the provided rpcUrl',
+      );
+    }
+
+    const existingNetworkConfiguration =
+      existingNetworkConfigurationWithId ??
+      existingNetworkConfigurationWithRpcUrl;
+
     const upsertedNetworkConfigurationId = existingNetworkConfiguration
       ? existingNetworkConfiguration.id
       : random();
@@ -1202,8 +1231,8 @@ export class NetworkController extends BaseController<
 
     this.update((state) => {
       state.networkConfigurations[upsertedNetworkConfigurationId] = {
-        id: upsertedNetworkConfigurationId,
         ...sanitizedNetworkConfiguration,
+        id: upsertedNetworkConfigurationId,
       };
     });