fix: dapp receiving two accountsChanged and wallet_sessionChanged events when adding and removing accounts/chainIds in the same edit action

app/scripts/controllers/permissions/background-api.js

@@ -43,38 +43,28 @@ export function getPermissionBackgroundApiMethods({
     return caip25Caveat;
   };
 
-  // To add more than one account when already connected to the dapp
-  const addMoreAccounts = (origin, addresses) => {
+  const setPermittedAccounts = (origin, caipAccountIds) => {
     const caip25Caveat = getCaip25Caveat(origin);
     if (!caip25Caveat) {
       throw new Error(
-        `Cannot add account permissions for origin "${origin}": no permission currently exists for this origin.`,
+        `Cannot set account permissions "${caipAccountIds.join(
+          ', ',
+        )}" for origin "${origin}": no permission currently exists for this origin.`,
       );
     }
 
-    const internalAccounts = addresses.map((address) => {
-      return accountsController.getAccountByAddress(address);
-    });
-
-    // Only the first scope in the scopes array is needed because
-    // setNonSCACaipAccountIdsInCaip25CaveatValue currently sets accounts on all matching
-    // namespaces, not just the exact CaipChainId.
-    const caipAccountIds = internalAccounts.map((internalAccount) => {
-      return `${internalAccount.scopes[0]}:${internalAccount.address}`;
-    });
-
-    const existingPermittedAccountIds = getCaipAccountIdsFromCaip25CaveatValue(
-      caip25Caveat.value,
-    );
+    if (caipAccountIds.length === 0) {
+      permissionController.revokePermission(
+        origin,
+        Caip25EndowmentPermissionName,
+      );
+      return;
+    }
 
     const existingPermittedChainIds = getAllScopesFromCaip25CaveatValue(
       caip25Caveat.value,
     );
 
-    const updatedAccountIds = Array.from(
-      new Set([...existingPermittedAccountIds, ...caipAccountIds]),
-    );
-
     let updatedPermittedChainIds = [...existingPermittedChainIds];
 
     const allNetworksList = Object.keys(
@@ -88,7 +78,7 @@ export function getPermissionBackgroundApiMethods({
       }),
     );
 
-    updatedAccountIds.forEach((caipAccountAddress) => {
+    caipAccountIds.forEach((caipAccountAddress) => {
       const {
         chain: { namespace: accountNamespace },
       } = parseCaipAccountId(caipAccountAddress);
@@ -129,7 +119,7 @@ export function getPermissionBackgroundApiMethods({
     const updatedCaveatValueWithAccountIds =
       setNonSCACaipAccountIdsInCaip25CaveatValue(
         updatedCaveatValueWithChainIds,
-        updatedAccountIds,
+        caipAccountIds,
       );
 
     permissionController.updateCaveat(
@@ -140,43 +130,92 @@ export function getPermissionBackgroundApiMethods({
     );
   };
 
-  const addMoreChains = (origin, chainIds) => {
+  const setPermittedChains = (origin, chainIds) => {
     const caip25Caveat = getCaip25Caveat(origin);
     if (!caip25Caveat) {
       throw new Error(
-        `Cannot add chain permissions for origin "${origin}": no permission currently exists for this origin.`,
+        `Cannot set permission for chainIds "${chainIds.join(
+          ', ',
+        )}": No permissions exist for origin "${origin}".`,
       );
     }
 
-    const updatedChainIds = Array.from(
-      new Set([
-        ...getAllScopesFromCaip25CaveatValue(caip25Caveat.value),
-        ...chainIds,
-      ]),
-    );
+    if (chainIds.length === 0 && !isSnapId(origin)) {
+      permissionController.revokePermission(
+        origin,
+        Caip25EndowmentPermissionName,
+      );
+    } else {
+      const updatedCaveatValueWithChainIds = setChainIdsInCaip25CaveatValue(
+        caip25Caveat.value,
+        chainIds,
+      );
+
+      const existingPermittedAccountIds =
+        getCaipAccountIdsFromCaip25CaveatValue(caip25Caveat.value);
+
+      const updatedCaveatValueWithAccountIds =
+        setNonSCACaipAccountIdsInCaip25CaveatValue(
+          updatedCaveatValueWithChainIds,
+          existingPermittedAccountIds,
+        );
+
+      permissionController.updateCaveat(
+        origin,
+        Caip25EndowmentPermissionName,
+        Caip25CaveatType,
+        updatedCaveatValueWithAccountIds,
+      );
+    }
+  };
+
+  const addMoreAccounts = (origin, addresses) => {
+    const caip25Caveat = getCaip25Caveat(origin);
+    if (!caip25Caveat) {
+      throw new Error(
+        `Cannot add account permissions for origin "${origin}": no permission currently exists for this origin.`,
+      );
+    }
 
-    const caveatValueWithChainIds = setChainIdsInCaip25CaveatValue(
+    const internalAccounts = addresses.map((address) => {
+      return accountsController.getAccountByAddress(address);
+    });
+
+    // Only the first scope in the scopes array is needed because
+    // setPermittedAccounts currently sets accounts on all matching
+    // namespaces, not just the exact CaipChainId.
+    const caipAccountIds = internalAccounts.map((internalAccount) => {
+      return `${internalAccount.scopes[0]}:${internalAccount.address}`;
+    });
+
+    const existingPermittedAccountIds = getCaipAccountIdsFromCaip25CaveatValue(
       caip25Caveat.value,
-      updatedChainIds,
     );
 
-    const permittedAccountIds = getCaipAccountIdsFromCaip25CaveatValue(
-      caip25Caveat.value,
+    const updatedAccountIds = Array.from(
+      new Set([...existingPermittedAccountIds, ...caipAccountIds]),
     );
 
-    // ensure that the list of permitted accounts is set for the newly added scopes
-    const caveatValueWithAccountsSynced =
-      setNonSCACaipAccountIdsInCaip25CaveatValue(
-        caveatValueWithChainIds,
-        permittedAccountIds,
+    setPermittedAccounts(origin, updatedAccountIds);
+  };
+
+  const addMoreChains = (origin, chainIds) => {
+    const caip25Caveat = getCaip25Caveat(origin);
+    if (!caip25Caveat) {
+      throw new Error(
+        `Cannot add chain permissions for origin "${origin}": no permission currently exists for this origin.`,
       );
+    }
 
-    permissionController.updateCaveat(
-      origin,
-      Caip25EndowmentPermissionName,
-      Caip25CaveatType,
-      caveatValueWithAccountsSynced,
+    const existingPermittedChainIds = getAllScopesFromCaip25CaveatValue(
+      caip25Caveat.value,
+    );
+
+    const updatedChainIds = Array.from(
+      new Set([...existingPermittedChainIds, ...chainIds]),
     );
+
+    setPermittedChains(origin, updatedChainIds);
   };
 
   const requestAccountsAndChainPermissions = async (origin, id) => {
@@ -221,11 +260,11 @@ export function getPermissionBackgroundApiMethods({
   };
 
   return {
-    addPermittedAccount: (origin, account) =>
-      addMoreAccounts(origin, [account]),
+    addPermittedAccount: (origin, address) =>
+      addMoreAccounts(origin, [address]),
 
-    addPermittedAccounts: (origin, accounts) =>
-      addMoreAccounts(origin, accounts),
+    addPermittedAccounts: (origin, addresses) =>
+      addMoreAccounts(origin, addresses),
 
     removePermittedAccount: (origin, address) => {
       const caip25Caveat = getCaip25Caveat(origin);
@@ -249,29 +288,15 @@ export function getPermissionBackgroundApiMethods({
         },
       );
 
-      if (remainingAccountIds.length === existingAccountIds.length) {
+      if (existingAccountIds.length === remainingAccountIds.length) {
         return;
       }
 
-      if (remainingAccountIds.length === 0) {
-        permissionController.revokePermission(
-          origin,
-          Caip25EndowmentPermissionName,
-        );
-      } else {
-        const updatedCaveatValue = setNonSCACaipAccountIdsInCaip25CaveatValue(
-          caip25Caveat.value,
-          remainingAccountIds,
-        );
-        permissionController.updateCaveat(
-          origin,
-          Caip25EndowmentPermissionName,
-          Caip25CaveatType,
-          updatedCaveatValue,
-        );
-      }
+      setPermittedAccounts(origin, remainingAccountIds);
     },
 
+    setPermittedAccounts,
+
     addPermittedChain: (origin, chainId) => addMoreChains(origin, [chainId]),
 
     addPermittedChains: (origin, chainIds) => addMoreChains(origin, chainIds),
@@ -292,29 +317,15 @@ export function getPermissionBackgroundApiMethods({
         (existingChainId) => existingChainId !== chainId,
       );
 
-      if (remainingChainIds.length === existingChainIds.length) {
+      if (existingChainIds.length === remainingChainIds.length) {
         return;
       }
 
-      if (remainingChainIds.length === 0 && !isSnapId(origin)) {
-        permissionController.revokePermission(
-          origin,
-          Caip25EndowmentPermissionName,
-        );
-      } else {
-        const updatedCaveatValue = setChainIdsInCaip25CaveatValue(
-          caip25Caveat.value,
-          remainingChainIds,
-        );
-        permissionController.updateCaveat(
-          origin,
-          Caip25EndowmentPermissionName,
-          Caip25CaveatType,
-          updatedCaveatValue,
-        );
-      }
+      setPermittedChains(origin, remainingChainIds);
     },
 
+    setPermittedChains,
+
     requestAccountsAndChainPermissionsWithId: (origin) => {
       const id = nanoid();
       requestAccountsAndChainPermissions(origin, id);