fix: fix granted permissions when adding an existing network via wallet_addEthereumChain

app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js

@@ -18,8 +18,6 @@ const addEthereumChain = {
     getNetworkConfigurationByChainId: true,
     setActiveNetwork: true,
     requestUserApproval: true,
-    startApprovalFlow: true,
-    endApprovalFlow: true,
     getCurrentChainIdForDomain: true,
     getCaveat: true,
     requestPermittedChainsPermissionForOrigin: true,
@@ -40,8 +38,6 @@ async function addEthereumChainHandler(
     getNetworkConfigurationByChainId,
     setActiveNetwork,
     requestUserApproval,
-    startApprovalFlow,
-    endApprovalFlow,
     getCurrentChainIdForDomain,
     getCaveat,
     requestPermittedChainsPermissionForOrigin,
@@ -79,7 +75,6 @@ async function addEthereumChainHandler(
     );
   }
 
-  let approvalFlowId;
   let updatedNetwork = existingNetwork;
 
   let rpcIndex = existingNetwork?.rpcEndpoints.findIndex(({ url }) =>
@@ -93,14 +88,14 @@ async function addEthereumChainHandler(
     : undefined;
 
   // If there's something to add or update
-  if (
+
+  const shouldAddOrUpdateNetwork =
     !existingNetwork ||
     rpcIndex !== existingNetwork.defaultRpcEndpointIndex ||
     (firstValidBlockExplorerUrl &&
-      blockExplorerIndex !== existingNetwork.defaultBlockExplorerUrlIndex)
-  ) {
-    ({ id: approvalFlowId } = await startApprovalFlow());
+      blockExplorerIndex !== existingNetwork.defaultBlockExplorerUrlIndex);
 
+  if (shouldAddOrUpdateNetwork) {
     try {
       await requestUserApproval({
         origin,
@@ -183,7 +178,6 @@ async function addEthereumChainHandler(
         });
       }
     } catch (error) {
-      endApprovalFlow({ id: approvalFlowId });
       return end(error);
     }
   }
@@ -193,16 +187,13 @@ async function addEthereumChainHandler(
     const { networkClientId } =
       updatedNetwork.rpcEndpoints[updatedNetwork.defaultRpcEndpointIndex];
 
-    return switchChain(res, end, chainId, networkClientId, approvalFlowId, {
-      isAddFlow: true,
+    return switchChain(res, end, chainId, networkClientId, {
+      autoApprove: shouldAddOrUpdateNetwork,
       setActiveNetwork,
       getCaveat,
-      endApprovalFlow,
       requestPermittedChainsPermissionForOrigin,
       requestPermittedChainsPermissionIncrementalForOrigin,
     });
-  } else if (approvalFlowId) {
-    endApprovalFlow({ id: approvalFlowId });
   }
 
   res.result = null;

app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.test.js

@@ -69,8 +69,6 @@ const createMockedHandler = () => {
     setActiveNetwork: jest.fn(),
     requestUserApproval: jest.fn().mockResolvedValue(123),
     getCaveat: jest.fn(),
-    startApprovalFlow: () => ({ id: 'approvalFlowId' }),
-    endApprovalFlow: jest.fn(),
     addNetwork: jest.fn().mockResolvedValue({
       defaultRpcEndpointIndex: 0,
       rpcEndpoints: [{ networkClientId: 123 }],
@@ -185,10 +183,8 @@ describe('addEthereumChainHandler', () => {
       end,
       NON_INFURA_CHAIN_ID,
       123,
-      'approvalFlowId',
       {
-        isAddFlow: true,
-        endApprovalFlow: mocks.endApprovalFlow,
+        autoApprove: true,
         getCaveat: mocks.getCaveat,
         setActiveNetwork: mocks.setActiveNetwork,
         requestPermittedChainsPermissionForOrigin:
@@ -254,10 +250,8 @@ describe('addEthereumChainHandler', () => {
           end,
           '0x1',
           123,
-          'approvalFlowId',
           {
-            isAddFlow: true,
-            endApprovalFlow: mocks.endApprovalFlow,
+            autoApprove: true,
             getCaveat: mocks.getCaveat,
             setActiveNetwork: mocks.setActiveNetwork,
             requestPermittedChainsPermissionForOrigin:
@@ -270,7 +264,7 @@ describe('addEthereumChainHandler', () => {
     });
 
     describe('if the proposed networkConfiguration does not have a different rpcUrl from the one already in state', () => {
-      it('should only switch to the existing networkConfiguration if one already exists for the given chain id', async () => {
+      it('should only switch to the existing networkConfiguration if one already exists for the given chain id without auto approving the chain permission', async () => {
         const { mocks, end, handler } = createMockedHandler();
         mocks.getCurrentChainIdForDomain.mockReturnValue(CHAIN_IDS.MAINNET);
         mocks.getNetworkConfigurationByChainId.mockReturnValue(
@@ -303,10 +297,8 @@ describe('addEthereumChainHandler', () => {
           end,
           '0xa',
           createMockOptimismConfiguration().rpcEndpoints[0].networkClientId,
-          undefined,
           {
-            isAddFlow: true,
-            endApprovalFlow: mocks.endApprovalFlow,
+            autoApprove: false,
             getCaveat: mocks.getCaveat,
             setActiveNetwork: mocks.setActiveNetwork,
             requestPermittedChainsPermissionForOrigin:

app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js

@@ -1,4 +1,4 @@
-import { errorCodes, rpcErrors } from '@metamask/rpc-errors';
+import { rpcErrors } from '@metamask/rpc-errors';
 import {
   Caip25CaveatType,
   Caip25EndowmentPermissionName,
@@ -164,26 +164,22 @@ export function validateAddEthereumChainParams(params) {
  * @param end - The JSON RPC request's end callback.
  * @param {string} chainId - The chainId being switched to.
  * @param {string} networkClientId - The network client being switched to.
- * @param {string} [approvalFlowId] - The optional approval flow ID to handle.
  * @param {object} hooks - The hooks object.
- * @param {boolean} hooks.isAddFlow - The boolean determining if this call originates from wallet_addEthereumChain.
  * @param {Function} hooks.setActiveNetwork - The callback to change the current network for the origin.
- * @param {Function} hooks.endApprovalFlow - The optional callback to end the approval flow when approvalFlowId is provided.
  * @param {Function} hooks.getCaveat - The callback to get the CAIP-25 caveat for the origin.
  * @param {Function} hooks.requestPermittedChainsPermissionForOrigin - The callback to request a new permittedChains-equivalent CAIP-25 permission.
  * @param {Function} hooks.requestPermittedChainsPermissionIncrementalForOrigin - The callback to add a new chain to the permittedChains-equivalent CAIP-25 permission.
- * @returns a null response on success or an error if user rejects an approval when isAddFlow is false or on unexpected errors.
+ * @param hooks.autoApprove
+ * @returns a null response on success or an error if user rejects an approval when autoApprove is false or on unexpected errors.
  */
 export async function switchChain(
   response,
   end,
   chainId,
   networkClientId,
-  approvalFlowId,
   {
-    isAddFlow,
+    autoApprove,
     setActiveNetwork,
-    endApprovalFlow,
     getCaveat,
     requestPermittedChainsPermissionForOrigin,
     requestPermittedChainsPermissionIncrementalForOrigin,
@@ -201,36 +197,20 @@ export async function switchChain(
       if (!ethChainIds.includes(chainId)) {
         await requestPermittedChainsPermissionIncrementalForOrigin({
           chainId,
-          autoApprove: isAddFlow,
+          autoApprove,
         });
       }
     } else {
       await requestPermittedChainsPermissionForOrigin({
         chainId,
-        autoApprove: isAddFlow,
+        autoApprove,
       });
     }
 
     await setActiveNetwork(networkClientId);
     response.result = null;
   } catch (error) {
-    // We don't want to return an error if user rejects the request
-    // and this is a chained switch request after wallet_addEthereumChain.
-    // approvalFlowId is only defined when this call is of a
-    // wallet_addEthereumChain request so we can use it to determine
-    // if we should return an error
-    if (
-      error.code === errorCodes.provider.userRejectedRequest &&
-      approvalFlowId
-    ) {
-      response.result = null;
-      return end();
-    }
     return end(error);
-  } finally {
-    if (approvalFlowId) {
-      endApprovalFlow({ id: approvalFlowId });
-    }
   }
   return end();
 }