feat: Migrate eth_accounts and permittedChains to CAIP-25 endowment (#…

…27847)  ## **Description** This PR replaces the replaces the internal `eth_accounts` and `endowment:permittedChains` permission structure with a [CAIP-25](https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-25.md) endowment. It adds adapter logic to translate to and from the new internal CAIP-25 permissions. This change should be transparent to wallet users and to dapps except for ~one~ two cases, see below. This change is required in order to support CAIP-25 and CAIP-27 requests in a follow-up PR that enables the Multichain API. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27847?quickstart=1) ## **Related issues** Related: MetaMask/core#4784 ## **Manual testing steps** There should be no user or dapp facing difference in behavior except: * When calling `wallet_revokePermissions` and specifying either `eth_accounts` or `endowment:permitted-chains`, the entire CAIP-25 permission will be revoked. It will appear to the dapp as if both `eth_accounts` and `endowment:permitted-chains` were revoked. * When calling `wallet_getPermissions` for a permitted dapp when the wallet is **locked**, `eth_accounts` should be returned in addition to `endowment:permitted-chains`. Currently there is a regression on `main` where only `endowment:permitted-chains` gets returned when the wallet is locked. ``` await window.ethereum.request({ "method": "wallet_revokePermissions", "params": [ { eth_accounts: {} } ], }); await window.ethereum.request({ "method": "wallet_revokePermissions", "params": [ { 'endowment:permitted-chains': {} } ], }); await window.ethereum.request({ "method": "wallet_getPermissions", "params": [], }); ``` ### Locked Wallet Behavior with dapp connected Other than the two noted items below, this behavior matches that in `main` - `eth_accounts` returns [] - `wallet_getPermissions` returns permissions incl eth_accounts - `wallet_revokePermissions` works as usual and revokes eth_accounts and revoke permitted-chains together - * Note this fixes a regression in `main` where eth_accounts and permitted-chains aren't revoked as a pair if either is revoked - `eth_requestAccounts` prompts for unlock, after unlock returns accounts if any are permitted, otherwise shows connection prompt - `wallet_requestPermissions` prompts for unlock - signature methods fails with method or accounts not authorized - non-signature methods work as usual - `accountsChanged` empty array on lock. no event after revokePermissions which makes sense since the dapp was told empty array on lock and now it's actually empty array so no changes have occurred as far as the dapp should be concerned. - **CHANGED**: for dapps that were granted chain permissions via the `wallet_addEthereum` or `wallet_switchEthereumChain` flows without account permissions, these permissions will be removed with this migration. We think this ok because: - This is a very uncommon scenario for dapps to request chain switches without account permissions. - These permissions can be regained very trivially with subsequent chain switch requests. ### Testing the migration * Create a dev build from `main` * Install the dev build from the `dist/chrome` directory and proceed through onboarding * Run this command in the background console: ``` chrome.storage.local.get( null, (state) => { state.data.PermissionController = {}; // Replace this line based on instructions below chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * Disable the extension * Switch to `main` and create a dev build * Enable and reload the extension * You should see in the console that migration 139 has failed Repeat the above steps but with the line above replaced with the following for example: * `state.data.NetworkController = {}; ` * `state.data.NetworkController = 'foobar'; ` * `state.data.NetworkController.selectedNetworkClientId = null;` * `state.data.NetworkController.networkConfigurationsByChainId = 'foobar';` ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Alex <[email protected]> Co-authored-by: Elliot Winkler <[email protected]> Co-authored-by: Mark Stacey <[email protected]> Co-authored-by: Erik Marks <[email protected]> Co-authored-by: Frederik Bolding <[email protected]>
MetaMask · Jan 20, 2025 · d5cd7fd · d5cd7fd
1 parent 5ae45ab
commit d5cd7fd
Show file tree

Hide file tree

Showing 71 changed files with 7,958 additions and 2,343 deletions.
diff --git a/.storybook/test-data.js b/.storybook/test-data.js
@@ -1424,17 +1424,27 @@ const state = {
     subjects: {
       'https://app.uniswap.org': {
         permissions: {
-          eth_accounts: {
-            invoker: 'https://app.uniswap.org',
-            parentCapability: 'eth_accounts',
-            id: 'a7342e4b-beae-4525-a36c-c0635fd03359',
-            date: 1620710693178,
+          'endowment:caip25': {
             caveats: [
               {
-                type: 'restrictReturnedAccounts',
-                value: ['0x64a845a5b02460acf8a3d84503b0d68d028b4bb4'],
+                type: 'authorizedScopes',
+                value: {
+                  requiredScopes: {},
+                  optionalScopes: {
+                    'eip155:1': {
+                      accounts: [
+                        'eip155:1:0x64a845a5b02460acf8a3d84503b0d68d028b4bb4',
+                      ],
+                    },
+                  },
+                  isMultichainOrigin: false,
+                },
               },
             ],
+            invoker: 'https://app.uniswap.org',
+            id: 'a7342e4b-beae-4525-a36c-c0635fd03359',
+            date: 1620710693178,
+            parentCapability: 'endowment:caip25',
           },
         },
       },

diff --git a/.yarn/patches/@json-schema-spec-json-pointer-npm-0.1.2-3d06119887.patch b/.yarn/patches/@json-schema-spec-json-pointer-npm-0.1.2-3d06119887.patch
@@ -0,0 +1,13 @@
+diff --git a/lib/index.js b/lib/index.js
+index f5795884311124b221d91f488ed45750eb6e9c80..e030d6f8d8e85e6d1350c565d36ad48bc49af881 100644
+--- a/lib/index.js
++++ b/lib/index.js
+@@ -25,7 +25,7 @@ class Ptr {
+         });
+         return `/${tokens.join("/")}`;
+     }
+-    eval(instance) {
++    shmeval(instance) {
+         for (const token of this.tokens) {
+             if (instance.hasOwnProperty(token)) {
+                 instance = instance[token];
diff --git a/.yarn/patches/@json-schema-tools-reference-resolver-npm-1.2.6-4e1497c16d.patch b/.yarn/patches/@json-schema-tools-reference-resolver-npm-1.2.6-4e1497c16d.patch
@@ -0,0 +1,13 @@
+diff --git a/build/resolve-pointer.js b/build/resolve-pointer.js
+index d5a8ec7486250cd17572eb0e0449725643fc9842..044e74bb51a46e9bf3547f6d7a84763b93260613 100644
+--- a/build/resolve-pointer.js
++++ b/build/resolve-pointer.js
+@@ -27,7 +27,7 @@ exports.default = (function (ref, root) {
+     try {
+         var withoutHash = ref.replace("#", "");
+         var pointer = json_pointer_1.default.parse(withoutHash);
+-        return pointer.eval(root);
++        return pointer.shmeval(root);
+     }
+     catch (e) {
+         throw new InvalidJsonPointerRefError(ref, e.message);
diff --git a/app/scripts/background.js b/app/scripts/background.js
@@ -677,13 +677,9 @@ function emitDappViewedMetricEvent(origin) {
     return;
   }
 
-  const permissions = controller.controllerMessenger.call(
-    'PermissionController:getPermissions',
-    origin,
-  );
   const numberOfConnectedAccounts =
-    permissions?.eth_accounts?.caveats[0]?.value.length;
-  if (!numberOfConnectedAccounts) {
+    controller.getPermittedAccounts(origin).length;
+  if (numberOfConnectedAccounts === 0) {
     return;
   }
 

diff --git a/app/scripts/controllers/permissions/background-api.js b/app/scripts/controllers/permissions/background-api.js
@@ -1,45 +1,177 @@
 import { nanoid } from 'nanoid';
 import {
-  CaveatTypes,
-  RestrictedMethods,
-} from '../../../../shared/constants/permissions';
-import { CaveatFactories, PermissionNames } from './specifications';
+  MethodNames,
+  PermissionDoesNotExistError,
+} from '@metamask/permission-controller';
+import {
+  Caip25CaveatType,
+  Caip25EndowmentPermissionName,
+  getEthAccounts,
+  setEthAccounts,
+  getPermittedEthChainIds,
+  setPermittedEthChainIds,
+} from '@metamask/multichain';
+import { isSnapId } from '@metamask/snaps-utils';
+import { RestrictedMethods } from '../../../../shared/constants/permissions';
+import { PermissionNames } from './specifications';
+
+export function getPermissionBackgroundApiMethods({
+  permissionController,
+  approvalController,
+}) {
+  // Returns the CAIP-25 caveat or undefined if it does not exist
+  const getCaip25Caveat = (origin) => {
+    let caip25Caveat;
+    try {
+      caip25Caveat = permissionController.getCaveat(
+        origin,
+        Caip25EndowmentPermissionName,
+        Caip25CaveatType,
+      );
+    } catch (err) {
+      if (err instanceof PermissionDoesNotExistError) {
+        // suppress expected error in case that the origin
+        // does not have the target permission yet
+      } else {
+        throw err;
+      }
+    }
+    return caip25Caveat;
+  };
 
-export function getPermissionBackgroundApiMethods(permissionController) {
+  // To add more than one account when already connected to the dapp
   const addMoreAccounts = (origin, accounts) => {
-    const caveat = CaveatFactories.restrictReturnedAccounts(accounts);
+    const caip25Caveat = getCaip25Caveat(origin);
+    if (!caip25Caveat) {
+      throw new Error(
+        `Cannot add account permissions for origin "${origin}": no permission currently exists for this origin.`,
+      );
+    }
 
-    permissionController.grantPermissionsIncremental({
-      subject: { origin },
-      approvedPermissions: {
-        [RestrictedMethods.eth_accounts]: { caveats: [caveat] },
-      },
-    });
+    const ethAccounts = getEthAccounts(caip25Caveat.value);
+
+    const updatedEthAccounts = Array.from(
+      new Set([...ethAccounts, ...accounts]),
+    );
+
+    const updatedCaveatValue = setEthAccounts(
+      caip25Caveat.value,
+      updatedEthAccounts,
+    );
+
+    permissionController.updateCaveat(
+      origin,
+      Caip25EndowmentPermissionName,
+      Caip25CaveatType,
+      updatedCaveatValue,
+    );
   };
 
   const addMoreChains = (origin, chainIds) => {
-    const caveat = CaveatFactories.restrictNetworkSwitching(chainIds);
+    const caip25Caveat = getCaip25Caveat(origin);
+    if (!caip25Caveat) {
+      throw new Error(
+        `Cannot add chain permissions for origin "${origin}": no permission currently exists for this origin.`,
+      );
+    }
+
+    const ethChainIds = getPermittedEthChainIds(caip25Caveat.value);
+
+    const updatedEthChainIds = Array.from(
+      new Set([...ethChainIds, ...chainIds]),
+    );
+
+    const caveatValueWithChains = setPermittedEthChainIds(
+      caip25Caveat.value,
+      updatedEthChainIds,
+    );
+
+    // ensure that the list of permitted eth accounts is set for the newly added eth scopes
+    const ethAccounts = getEthAccounts(caveatValueWithChains);
+    const caveatValueWithAccountsSynced = setEthAccounts(
+      caveatValueWithChains,
+      ethAccounts,
+    );
+
+    permissionController.updateCaveat(
+      origin,
+      Caip25EndowmentPermissionName,
+      Caip25CaveatType,
+      caveatValueWithAccountsSynced,
+    );
+  };
+
+  const requestAccountsAndChainPermissions = async (origin, id) => {
+    // Note that we are purposely requesting an approval from the ApprovalController
+    // and then manually forming the permission that is then granted via the
+    // PermissionController rather than calling the PermissionController.requestPermissions()
+    // directly because the Approval UI is still dependent on the notion of there
+    // being separate "eth_accounts" and "endowment:permitted-chains" permissions.
+    // After that depedency is refactored, we can move to requesting "endowment:caip25"
+    // directly from the PermissionController instead.
+    const legacyApproval = await approvalController.addAndShowApprovalRequest({
+      id,
+      origin,
+      requestData: {
+        metadata: {
+          id,
+          origin,
+        },
+        permissions: {
+          [RestrictedMethods.eth_accounts]: {},
+          [PermissionNames.permittedChains]: {},
+        },
+      },
+      type: MethodNames.RequestPermissions,
+    });
+
+    const newCaveatValue = {
+      requiredScopes: {},
+      optionalScopes: {},
+      isMultichainOrigin: false,
+    };
+
+    const caveatValueWithChains = setPermittedEthChainIds(
+      newCaveatValue,
+      legacyApproval.approvedChainIds,
+    );
+
+    const caveatValueWithAccounts = setEthAccounts(
+      caveatValueWithChains,
+      legacyApproval.approvedAccounts,
+    );
 
-    permissionController.grantPermissionsIncremental({
+    permissionController.grantPermissions({
       subject: { origin },
       approvedPermissions: {
-        [PermissionNames.permittedChains]: { caveats: [caveat] },
+        [Caip25EndowmentPermissionName]: {
+          caveats: [
+            {
+              type: Caip25CaveatType,
+              value: caveatValueWithAccounts,
+            },
+          ],
+        },
       },
     });
   };
 
   return {
     addPermittedAccount: (origin, account) =>
       addMoreAccounts(origin, [account]),
+
     addPermittedAccounts: (origin, accounts) =>
       addMoreAccounts(origin, accounts),
 
     removePermittedAccount: (origin, account) => {
-      const { value: existingAccounts } = permissionController.getCaveat(
-        origin,
-        RestrictedMethods.eth_accounts,
-        CaveatTypes.restrictReturnedAccounts,
-      );
+      const caip25Caveat = getCaip25Caveat(origin);
+      if (!caip25Caveat) {
+        throw new Error(
+          `Cannot remove account "${account}": No permissions exist for origin "${origin}".`,
+        );
+      }
+
+      const existingAccounts = getEthAccounts(caip25Caveat.value);
 
       const remainingAccounts = existingAccounts.filter(
         (existingAccount) => existingAccount !== account,
@@ -52,73 +184,66 @@ export function getPermissionBackgroundApiMethods(permissionController) {
       if (remainingAccounts.length === 0) {
         permissionController.revokePermission(
           origin,
-          RestrictedMethods.eth_accounts,
+          Caip25EndowmentPermissionName,
         );
       } else {
+        const updatedCaveatValue = setEthAccounts(
+          caip25Caveat.value,
+          remainingAccounts,
+        );
         permissionController.updateCaveat(
           origin,
-          RestrictedMethods.eth_accounts,
-          CaveatTypes.restrictReturnedAccounts,
-          remainingAccounts,
+          Caip25EndowmentPermissionName,
+          Caip25CaveatType,
+          updatedCaveatValue,
         );
       }
     },
 
     addPermittedChain: (origin, chainId) => addMoreChains(origin, [chainId]),
+
     addPermittedChains: (origin, chainIds) => addMoreChains(origin, chainIds),
 
     removePermittedChain: (origin, chainId) => {
-      const { value: existingChains } = permissionController.getCaveat(
-        origin,
-        PermissionNames.permittedChains,
-        CaveatTypes.restrictNetworkSwitching,
-      );
+      const caip25Caveat = getCaip25Caveat(origin);
+      if (!caip25Caveat) {
+        throw new Error(
+          `Cannot remove permission for chainId "${chainId}": No permissions exist for origin "${origin}".`,
+        );
+      }
 
-      const remainingChains = existingChains.filter(
-        (existingChain) => existingChain !== chainId,
+      const existingEthChainIds = getPermittedEthChainIds(caip25Caveat.value);
+
+      const remainingChainIds = existingEthChainIds.filter(
+        (existingChainId) => existingChainId !== chainId,
       );
 
-      if (remainingChains.length === existingChains.length) {
+      if (remainingChainIds.length === existingEthChainIds.length) {
         return;
       }
 
-      if (remainingChains.length === 0) {
+      if (remainingChainIds.length === 0 && !isSnapId(origin)) {
         permissionController.revokePermission(
           origin,
-          PermissionNames.permittedChains,
+          Caip25EndowmentPermissionName,
         );
       } else {
+        const updatedCaveatValue = setPermittedEthChainIds(
+          caip25Caveat.value,
+          remainingChainIds,
+        );
         permissionController.updateCaveat(
           origin,
-          PermissionNames.permittedChains,
-          CaveatTypes.restrictNetworkSwitching,
-          remainingChains,
+          Caip25EndowmentPermissionName,
+          Caip25CaveatType,
+          updatedCaveatValue,
         );
       }
     },
 
-    requestAccountsAndChainPermissionsWithId: async (origin) => {
+    requestAccountsAndChainPermissionsWithId: (origin) => {
       const id = nanoid();
-      permissionController.requestPermissions(
-        { origin },
-        {
-          [PermissionNames.eth_accounts]: {},
-          [PermissionNames.permittedChains]: {},
-        },
-        { id },
-      );
-      return id;
-    },
-
-    requestAccountsPermissionWithId: async (origin) => {
-      const id = nanoid();
-      permissionController.requestPermissions(
-        { origin },
-        {
-          eth_accounts: {},
-        },
-        { id },
-      );
+      requestAccountsAndChainPermissions(origin, id);
       return id;
     },
   };