From 4126dd3e583118155db98d36a4cd2e5fb34513c8 Mon Sep 17 00:00:00 2001 From: jiexi Date: Thu, 3 Oct 2024 14:41:14 -0700 Subject: [PATCH] Jl/caip multichain/fix connection flow for permitted chains (#27471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Connects the new AmonHenV2 Connection Flow to CAIP Multichain: * Preserves and syncs eth accounts across eip155 scopes when permitted chains are changed * Grants full methods and notifications to scopeObject when a new chain is permitted * `ConnectPage` Approval now uses the caveat values from eth_accounts and endowment:permitted-chains as the default selected account and chains * `wallet_createSession` passes a list of supported eth accounts and eth chainIds based on the supported scopes to be used as the preselected/default values in the ConnectPage Approval * `wallet_createSession` removes supported eip155 scopes that were not approved and adds ones that were not in the original request but were approved [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27471?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** Replace the account addresses below with your own addresses to test the preselected accounts. This request should preselect mainnet and sepolia (which is different from the default which preselects all non testnet networks) ``` const EXTENSION_ID = 'nonfpcflonapegmnfeafnddgdniflbnk'; const extensionPort = chrome.runtime.connect(EXTENSION_ID) extensionPort.onMessage.addListener((msg) => console.log('extensionPort on message', msg)) extensionPort.postMessage({ type: 'caip-x', data: { "jsonrpc": "2.0", method: 'wallet_createSession', params: { requiredScopes: { 'eip155': { references: ['1', '11155111'], methods: [ 'eth_sendTransaction', 'eth_getBalance', 'eth_subscribe' ], notifications: ['eth_subscription'], accounts: ['eip155:1:0x5bA08AF1bc30f17272178bDcACA1C74e94955cF4', 'eip155:1:0xdeadbeef', 'eip155:1:0x398fC6Ec25889e7373310dC4c3491b18575d5d6B'] } }, optionalScopes: { }, sessionProperties: { 'caip154-mandatory': 'true', }, }, } }) ``` replace this with your own address to test preselected accounts. ``` "method": "wallet_requestPermissions", "params": [ { eth_accounts: { caveats: [ { type: 'restrictReturnedAccounts', value: ['0x5bA08AF1bc30f17272178bDcACA1C74e94955cF4'] } ] } } ], }); ``` This one works for preselecting chains ``` "method": "wallet_requestPermissions", "params": [ { 'endowment:permitted-chains': { caveats: [ { type: 'restrictNetworkSwitching', value: ['0x1'] } ] } } ], }); ``` You can also combine the params of these two wallet_requestPermissions examples One for eth_requestAccounts ``` await window.ethereum.request({ "method": "eth_requestAccounts", "params": [], }); ``` And of course you can connect via the wallet UI directly. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. --- .../controllers/permissions/background-api.js | 28 +- .../permissions/background-api.test.js | 110 ++----- ...permission-adapter-permittedChains.test.ts | 9 +- ...caip-permission-adapter-permittedChains.ts | 6 +- .../wallet-createSession/handler.js | 91 ++++-- .../wallet-createSession/handler.test.js | 286 +++++++----------- .../wallet-createSession/helpers.test.ts | 53 +--- .../wallet-createSession/helpers.ts | 16 +- .../wallet-requestPermissions.js | 16 +- .../wallet-requestPermissions.test.js | 42 +-- .../handlers/ethereum-chain-utils.test.js | 8 +- .../handlers/request-accounts.js | 20 +- .../handlers/request-accounts.test.js | 29 +- app/scripts/metamask-controller.js | 8 +- .../permissions-connect-permission-list.js | 7 +- .../edit-accounts-modal.tsx | 9 +- .../site-cell/site-cell.tsx | 5 +- .../connect-page/connect-page.tsx | 29 +- 18 files changed, 278 insertions(+), 494 deletions(-) diff --git a/app/scripts/controllers/permissions/background-api.js b/app/scripts/controllers/permissions/background-api.js index b181eb54ce70..2cc89705f41c 100644 --- a/app/scripts/controllers/permissions/background-api.js +++ b/app/scripts/controllers/permissions/background-api.js @@ -12,16 +12,12 @@ import { getPermittedEthChainIds, setPermittedEthChainIds, } from '../../lib/multichain-api/adapters/caip-permission-adapter-permittedChains'; -import { - CaveatTypes, - RestrictedMethods, -} from '../../../../shared/constants/permissions'; +import { RestrictedMethods } from '../../../../shared/constants/permissions'; import { PermissionNames } from './specifications'; export function getPermissionBackgroundApiMethods({ permissionController, approvalController, - networkController, }) { // To add more than one account when already connected to the dapp const addMoreAccounts = (origin, accounts) => { @@ -75,17 +71,23 @@ export function getPermissionBackgroundApiMethods({ throw new Error('tried to add chains when none have been permissioned'); // TODO: better error } + // get the list of permitted eth accounts before we modify the permitted chains and potentially lose some + const ethAccounts = getEthAccounts(caip25Caveat.value); + const ethChainIds = getPermittedEthChainIds(caip25Caveat.value); const updatedEthChainIds = Array.from( new Set([...ethChainIds, ...chainIds]), ); - const updatedCaveatValue = setPermittedEthChainIds( + let updatedCaveatValue = setPermittedEthChainIds( caip25Caveat.value, updatedEthChainIds, ); + // ensure that the list of permitted eth accounts is intact after permitted chain updates + updatedCaveatValue = setEthAccounts(updatedCaveatValue, ethAccounts); + permissionController.updateCaveat( origin, Caip25EndowmentPermissionName, @@ -95,11 +97,6 @@ export function getPermissionBackgroundApiMethods({ }; const requestAccountsAndChainPermissionsWithId = (origin) => { - const { chainId } = - networkController.getNetworkConfigurationByNetworkClientId( - networkController.state.selectedNetworkClientId, - ); - const id = nanoid(); // NOTE: the eth_accounts/permittedChains approvals will be combined in the future. // Until they are actually combined, when testing, you must request both @@ -115,14 +112,7 @@ export function getPermissionBackgroundApiMethods({ }, permissions: { [RestrictedMethods.eth_accounts]: {}, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: [chainId], - }, - ], - }, + [PermissionNames.permittedChains]: {}, }, }, type: MethodNames.requestPermissions, diff --git a/app/scripts/controllers/permissions/background-api.test.js b/app/scripts/controllers/permissions/background-api.test.js index d2abd4df4e46..3deca2135f68 100644 --- a/app/scripts/controllers/permissions/background-api.test.js +++ b/app/scripts/controllers/permissions/background-api.test.js @@ -1,13 +1,14 @@ import { MethodNames } from '@metamask/permission-controller'; -import { - CaveatTypes, - RestrictedMethods, -} from '../../../../shared/constants/permissions'; +import { RestrictedMethods } from '../../../../shared/constants/permissions'; import { Caip25CaveatType, Caip25EndowmentPermissionName, } from '../../lib/multichain-api/caip25permissions'; import { flushPromises } from '../../../../test/lib/timer-helpers'; +import { + KnownNotifications, + KnownRpcMethods, +} from '../../lib/multichain-api/scope'; import { getPermissionBackgroundApiMethods } from './background-api'; import { PermissionNames } from './specifications'; @@ -469,45 +470,7 @@ describe('permission background API methods', () => { }); describe('requestAccountsAndChainPermissionsWithId', () => { - it('gets the networkConfiguration for the current globally selected network client', () => { - const networkController = { - state: { - selectedNetworkClientId: 'mainnet', - }, - getNetworkConfigurationByNetworkClientId: jest.fn().mockReturnValue({ - chainId: '0x1', - }), - }; - const approvalController = { - addAndShowApprovalRequest: jest.fn().mockResolvedValue({ - approvedChainIds: ['0x1', '0x5'], - approvedAccounts: ['0xdeadbeef'], - }), - }; - const permissionController = { - grantPermissions: jest.fn(), - }; - - getPermissionBackgroundApiMethods({ - networkController, - approvalController, - permissionController, - }).requestAccountsAndChainPermissionsWithId('foo.com'); - - expect( - networkController.getNetworkConfigurationByNetworkClientId, - ).toHaveBeenCalledWith('mainnet'); - }); - it('requests eth_accounts and permittedChains approval and returns the request id', async () => { - const networkController = { - state: { - selectedNetworkClientId: 'mainnet', - }, - getNetworkConfigurationByNetworkClientId: jest.fn().mockReturnValue({ - chainId: '0x1', - }), - }; const approvalController = { addAndShowApprovalRequest: jest.fn().mockResolvedValue({ approvedChainIds: ['0x1', '0x5'], @@ -519,7 +482,6 @@ describe('permission background API methods', () => { }; const result = getPermissionBackgroundApiMethods({ - networkController, approvalController, permissionController, }).requestAccountsAndChainPermissionsWithId('foo.com'); @@ -539,14 +501,7 @@ describe('permission background API methods', () => { }, permissions: { [RestrictedMethods.eth_accounts]: {}, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x1'], - }, - ], - }, + [PermissionNames.permittedChains]: {}, }, }, type: MethodNames.requestPermissions, @@ -555,14 +510,6 @@ describe('permission background API methods', () => { }); it('grants a legacy CAIP-25 permission (isMultichainOrigin: false) with the approved eip155 chainIds and accounts', async () => { - const networkController = { - state: { - selectedNetworkClientId: 'mainnet', - }, - getNetworkConfigurationByNetworkClientId: jest.fn().mockReturnValue({ - chainId: '0x1', - }), - }; const approvalController = { addAndShowApprovalRequest: jest.fn().mockResolvedValue({ approvedChainIds: ['0x1', '0x5'], @@ -574,7 +521,6 @@ describe('permission background API methods', () => { }; getPermissionBackgroundApiMethods({ - networkController, approvalController, permissionController, }).requestAccountsAndChainPermissionsWithId('foo.com'); @@ -594,13 +540,13 @@ describe('permission background API methods', () => { requiredScopes: {}, optionalScopes: { 'eip155:1': { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: ['eip155:1:0xdeadbeef'], }, 'eip155:5': { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: ['eip155:5:0xdeadbeef'], }, }, @@ -649,7 +595,7 @@ describe('permission background API methods', () => { ); }); - it('calls updateCaveat with the chain added', () => { + it('calls updateCaveat with the chain added and all eip155 accounts synced', () => { const permissionController = { getCaveat: jest.fn().mockReturnValue({ value: { @@ -661,7 +607,7 @@ describe('permission background API methods', () => { 'eip155:10': { methods: [], notifications: [], - accounts: ['eip155:10:0x1', 'eip155:10:0x2'], + accounts: ['eip155:10:0x2'], }, }, optionalScopes: { @@ -675,7 +621,7 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], - accounts: ['eip155:1:0x2', 'eip155:1:0x3'], + accounts: ['eip155:1:0x1'], }, }, isMultichainOrigin: true, @@ -698,6 +644,7 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], + accounts: ['eip155:1:0x1', 'eip155:1:0x2'], }, 'eip155:10': { methods: [], @@ -716,12 +663,12 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], - accounts: ['eip155:1:0x2', 'eip155:1:0x3'], + accounts: ['eip155:1:0x1', 'eip155:1:0x2'], }, 'eip155:1337': { - methods: [], - notifications: [], - accounts: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, + accounts: ['eip155:1337:0x1', 'eip155:1337:0x2'], }, }, isMultichainOrigin: true, @@ -765,7 +712,7 @@ describe('permission background API methods', () => { ); }); - it('calls updateCaveat with the chains added', () => { + it('calls updateCaveat with the chains added and all eip155 accounts synced', () => { const permissionController = { getCaveat: jest.fn().mockReturnValue({ value: { @@ -777,7 +724,7 @@ describe('permission background API methods', () => { 'eip155:10': { methods: [], notifications: [], - accounts: ['eip155:10:0x1', 'eip155:10:0x2'], + accounts: ['eip155:10:0x2'], }, }, optionalScopes: { @@ -791,7 +738,7 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], - accounts: ['eip155:1:0x2', 'eip155:1:0x3'], + accounts: ['eip155:1:0x1'], }, }, isMultichainOrigin: true, @@ -814,6 +761,7 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], + accounts: ['eip155:1:0x1', 'eip155:1:0x2'], }, 'eip155:10': { methods: [], @@ -832,17 +780,17 @@ describe('permission background API methods', () => { 'eip155:1': { methods: [], notifications: [], - accounts: ['eip155:1:0x2', 'eip155:1:0x3'], + accounts: ['eip155:1:0x1', 'eip155:1:0x2'], }, 'eip155:4': { - methods: [], - notifications: [], - accounts: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, + accounts: ['eip155:4:0x1', 'eip155:4:0x2'], }, 'eip155:5': { - methods: [], - notifications: [], - accounts: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, + accounts: ['eip155:5:0x1', 'eip155:5:0x2'], }, }, isMultichainOrigin: true, diff --git a/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.test.ts b/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.test.ts index 2df27c39d6e2..aa125193ce95 100644 --- a/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.test.ts +++ b/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.test.ts @@ -1,4 +1,5 @@ import { Caip25CaveatValue } from '../caip25permissions'; +import { KnownNotifications, KnownRpcMethods } from '../scope'; import { addPermittedEthChainId, getPermittedEthChainIds, @@ -89,8 +90,8 @@ describe('CAIP-25 permittedChains adapters', () => { accounts: ['eip155:100:0x100'], }, 'eip155:101': { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: [], }, }, @@ -272,8 +273,8 @@ describe('CAIP-25 permittedChains adapters', () => { accounts: ['eip155:100:0x100'], }, 'eip155:101': { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: [], }, }, diff --git a/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.ts b/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.ts index b1a9ab355b94..8e840c6c327e 100644 --- a/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.ts +++ b/app/scripts/lib/multichain-api/adapters/caip-permission-adapter-permittedChains.ts @@ -2,6 +2,8 @@ import { Hex, KnownCaipNamespace } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; import { Caip25CaveatValue } from '../caip25permissions'; import { + KnownNotifications, + KnownRpcMethods, mergeScopes, parseScopeString, ScopesObject, @@ -44,8 +46,8 @@ export const addPermittedEthChainId = ( optionalScopes: { ...caip25CaveatValue.optionalScopes, [scopeString]: { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: [], }, }, diff --git a/app/scripts/lib/multichain-api/wallet-createSession/handler.js b/app/scripts/lib/multichain-api/wallet-createSession/handler.js index affc03cecef2..3a0ed777d049 100644 --- a/app/scripts/lib/multichain-api/wallet-createSession/handler.js +++ b/app/scripts/lib/multichain-api/wallet-createSession/handler.js @@ -1,5 +1,5 @@ import { EthereumRpcError } from 'eth-rpc-errors'; -import { RestrictedMethods } from '../../../../../shared/constants/permissions'; +import { CaveatTypes } from '../../../../../shared/constants/permissions'; import { mergeScopes, validateAndFlattenScopes, @@ -16,7 +16,16 @@ import { MetaMetricsEventCategory, MetaMetricsEventName, } from '../../../../../shared/constants/metametrics'; -import { assignAccountsToScopes, validateAndAddEip3085 } from './helpers'; +import { PermissionNames } from '../../../controllers/permissions'; +import { + getEthAccounts, + setEthAccounts, +} from '../adapters/caip-permission-adapter-eth-accounts'; +import { + getPermittedEthChainIds, + setPermittedEthChainIds, +} from '../adapters/caip-permission-adapter-permittedChains'; +import { validateAndAddEip3085 } from './helpers'; export async function walletCreateSessionHandler(req, res, _next, end, hooks) { // TODO: Does this handler need a rate limiter/lock like the one in eth_requestAccounts? @@ -91,38 +100,60 @@ export async function walletCreateSessionHandler(req, res, _next, end, hooks) { unsupportableOptionalScopes, }); - // use old account popup for now to get the accounts + // These should be EVM accounts already although the name does not necessary imply that + // These addresses are lowercased already + const existingEvmAddresses = hooks + .listAccounts() + .map((account) => account.address); + const supportedEthAccounts = getEthAccounts({ + requiredScopes: supportedRequiredScopes, + optionalScopes: supportedOptionalScopes, + }) + .map((address) => address.toLowerCase()) + .filter((address) => existingEvmAddresses.includes(address)); + const supportedEthChainIds = getPermittedEthChainIds({ + requiredScopes: supportedRequiredScopes, + optionalScopes: supportedOptionalScopes, + }); + const legacyApproval = await hooks.requestPermissionApprovalForOrigin({ - [RestrictedMethods.eth_accounts]: {}, + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: supportedEthAccounts, + }, + ], + }, + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: supportedEthChainIds, + }, + ], + }, }); - assignAccountsToScopes( - supportedRequiredScopes, - legacyApproval.approvedAccounts, - ); - assignAccountsToScopes( - supportableRequiredScopes, - legacyApproval.approvedAccounts, - ); - assignAccountsToScopes( - supportedOptionalScopes, - legacyApproval.approvedAccounts, + + let caip25CaveatValue = { + requiredScopes: supportedRequiredScopes, + optionalScopes: supportedOptionalScopes, + isMultichainOrigin: true, + // TODO: preserve sessionProperties? + }; + + caip25CaveatValue = setPermittedEthChainIds( + caip25CaveatValue, + legacyApproval.approvedChainIds, ); - assignAccountsToScopes( - supportableOptionalScopes, + caip25CaveatValue = setEthAccounts( + caip25CaveatValue, legacyApproval.approvedAccounts, ); - const grantedRequiredScopes = mergeScopes( - supportedRequiredScopes, - supportableRequiredScopes, - ); - const grantedOptionalScopes = mergeScopes( - supportedOptionalScopes, - supportableOptionalScopes, - ); const sessionScopes = mergeScopes( - grantedRequiredScopes, - grantedOptionalScopes, + caip25CaveatValue.requiredScopes, + caip25CaveatValue.optionalScopes, ); await Promise.all( @@ -153,11 +184,7 @@ export async function walletCreateSessionHandler(req, res, _next, end, hooks) { caveats: [ { type: Caip25CaveatType, - value: { - requiredScopes: grantedRequiredScopes, - optionalScopes: grantedOptionalScopes, - isMultichainOrigin: true, - }, + value: caip25CaveatValue, }, ], }, diff --git a/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js b/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js index ef615b720f47..8df1e48040e3 100644 --- a/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js +++ b/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js @@ -1,18 +1,21 @@ import { EthereumRpcError } from 'eth-rpc-errors'; -import { RestrictedMethods } from '../../../../../shared/constants/permissions'; +import { CaveatTypes } from '../../../../../shared/constants/permissions'; import { validateAndFlattenScopes, processScopedProperties, bucketScopes, assertScopesSupported, + KnownRpcMethods, + KnownNotifications, } from '../scope'; import { Caip25CaveatType, Caip25EndowmentPermissionName, } from '../caip25permissions'; import { shouldEmitDappViewedEvent } from '../../util'; +import { PermissionNames } from '../../../controllers/permissions'; import { walletCreateSessionHandler } from './handler'; -import { assignAccountsToScopes, validateAndAddEip3085 } from './helpers'; +import { validateAndAddEip3085 } from './helpers'; jest.mock('../../util', () => ({ ...jest.requireActual('../../util'), @@ -20,7 +23,13 @@ jest.mock('../../util', () => ({ })); jest.mock('../scope', () => ({ - ...jest.requireActual('../scope'), + ...jest.requireActual('../scope/assert'), + ...jest.requireActual('../scope/authorization'), + ...jest.requireActual('../scope/filter'), + ...jest.requireActual('../scope/scope'), + ...jest.requireActual('../scope/supported'), + ...jest.requireActual('../scope/transform'), + ...jest.requireActual('../scope/validation'), validateAndFlattenScopes: jest.fn(), processScopedProperties: jest.fn(), bucketScopes: jest.fn(), @@ -29,7 +38,6 @@ jest.mock('../scope', () => ({ jest.mock('./helpers', () => ({ ...jest.requireActual('./helpers'), - assignAccountsToScopes: jest.fn(), validateAndAddEip3085: jest.fn(), })); @@ -61,6 +69,7 @@ const createMockedHandler = () => { const end = jest.fn(); const requestPermissionApprovalForOrigin = jest.fn().mockResolvedValue({ approvedAccounts: ['0x1', '0x2', '0x3', '0x4'], + approvedChainIds: ['0x1', '0x5'], }); const grantPermissions = jest.fn().mockResolvedValue(undefined); const findNetworkClientIdByChainId = jest.fn().mockReturnValue('mainnet'); @@ -89,6 +98,7 @@ const createMockedHandler = () => { '0x3': {}, }, }; + const listAccounts = jest.fn().mockReturnValue([]); const response = {}; const handler = (request) => walletCreateSessionHandler(request, response, next, end, { @@ -101,6 +111,7 @@ const createMockedHandler = () => { multichainSubscriptionManager, metamaskState, sendMetrics, + listAccounts, }); return { @@ -116,6 +127,7 @@ const createMockedHandler = () => { multichainSubscriptionManager, metamaskState, sendMetrics, + listAccounts, handler, }; }; @@ -131,7 +143,6 @@ describe('wallet_createSession', () => { supportableScopes: {}, unsupportableScopes: {}, }); - assignAccountsToScopes.mockImplementation((value) => value); }); afterEach(() => { @@ -190,10 +201,10 @@ describe('wallet_createSession', () => { }, }, flattenedOptionalScopes: { - 'eip155:64': { + 'eip155:100': { methods: ['eth_chainId'], notifications: ['accountsChanged', 'chainChanged'], - accounts: ['eip155:64:0x4'], + accounts: ['eip155:100:0x4'], }, }, }); @@ -216,10 +227,10 @@ describe('wallet_createSession', () => { }, }, { - 'eip155:64': { + 'eip155:100': { methods: ['eth_chainId'], notifications: ['accountsChanged', 'chainChanged'], - accounts: ['eip155:64:0x4'], + accounts: ['eip155:100:0x4'], }, }, { foo: 'bar' }, @@ -279,10 +290,10 @@ describe('wallet_createSession', () => { validateAndFlattenScopes.mockReturnValue({ flattenedRequiredScopes: {}, flattenedOptionalScopes: { - 'eip155:64': { + 'eip155:100': { methods: ['eth_chainId'], notifications: ['accountsChanged', 'chainChanged'], - accounts: ['eip155:64:0x4'], + accounts: ['eip155:100:0x4'], }, }, }); @@ -291,10 +302,10 @@ describe('wallet_createSession', () => { expect(bucketScopes).toHaveBeenNthCalledWith( 2, { - 'eip155:64': { + 'eip155:100': { methods: ['eth_chainId'], notifications: ['accountsChanged', 'chainChanged'], - accounts: ['eip155:64:0x4'], + accounts: ['eip155:100:0x4'], }, }, expect.objectContaining({ @@ -311,144 +322,64 @@ describe('wallet_createSession', () => { expect(isChainIdSupportableBody).toContain('validScopedProperties'); }); - it('requests approval for account permission with no args even if there is accounts in the scope', async () => { - const { handler, requestPermissionApprovalForOrigin } = - createMockedHandler(); - bucketScopes - .mockReturnValueOnce({ - supportedScopes: { - 'eip155:1': { - methods: [], - notifications: [], - accounts: ['eip155:1:0x1', 'eip155:1:0x2'], - }, - }, - supportableScopes: { - 'eip155:5': { - methods: [], - notifications: [], - accounts: ['eip155:5:0x2', 'eip155:5:0x3'], - }, - }, - unsupportableScopes: { - 'eip155:64': { - methods: [], - notifications: [], - accounts: ['eip155:64:0x4'], - }, - }, - }) - .mockReturnValueOnce({ - supportedScopes: { - 'eip155:2': { - methods: [], - notifications: [], - accounts: ['eip155:2:0x1', 'eip155:1:0x2'], - }, - }, - supportableScopes: { - 'eip155:6': { - methods: [], - notifications: [], - accounts: ['eip155:6:0x2', 'eip155:6:0x3'], - }, - }, - unsupportableScopes: { - 'eip155:65': { - methods: [], - notifications: [], - accounts: ['eip155:65:0x4'], - }, - }, - }); + it('gets a list of evm accounts in the wallet', async () => { + const { handler, listAccounts } = createMockedHandler(); await handler(baseRequest); - expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ - [RestrictedMethods.eth_accounts]: {}, - }); + expect(listAccounts).toHaveBeenCalled(); }); - it('assigns the permitted accounts to the scopeObjects', async () => { - const { handler } = createMockedHandler(); + it('requests approval for account and permitted chains permission based on the supported eth accounts and eth chains from the supported scopes in the request', async () => { + const { handler, listAccounts, requestPermissionApprovalForOrigin } = + createMockedHandler(); + listAccounts.mockReturnValue([ + { address: '0x1' }, + { address: '0x3' }, + { address: '0x4' }, + ]); bucketScopes .mockReturnValueOnce({ supportedScopes: { - 'eip155:1': { - methods: [], - notifications: [], - }, - }, - supportableScopes: { - 'eip155:5': { - methods: [], - notifications: [], - }, - }, - unsupportableScopes: { - 'eip155:64': { + 'eip155:1337': { methods: [], notifications: [], + accounts: ['eip155:1:0x1', 'eip155:1:0x2'], }, }, + supportableScopes: {}, + unsupportableScopes: {}, }) .mockReturnValueOnce({ supportedScopes: { - 'eip155:2': { - methods: [], - notifications: [], - }, - }, - supportableScopes: { - 'eip155:6': { - methods: [], - notifications: [], - }, - }, - unsupportableScopes: { - 'eip155:65': { + 'eip155:100': { methods: [], notifications: [], + accounts: ['eip155:2:0x1', 'eip155:2:0x3', 'eip155:2:0xdeadbeef'], }, }, + supportableScopes: {}, + unsupportableScopes: {}, }); await handler(baseRequest); - expect(assignAccountsToScopes).toHaveBeenCalledWith( - { - 'eip155:1': { - methods: [], - notifications: [], - }, - }, - ['0x1', '0x2', '0x3', '0x4'], - ); - expect(assignAccountsToScopes).toHaveBeenCalledWith( - { - 'eip155:5': { - methods: [], - notifications: [], - }, - }, - ['0x1', '0x2', '0x3', '0x4'], - ); - expect(assignAccountsToScopes).toHaveBeenCalledWith( - { - 'eip155:2': { - methods: [], - notifications: [], - }, + expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ + [PermissionNames.eth_accounts]: { + caveats: [ + { + type: CaveatTypes.restrictReturnedAccounts, + value: ['0x1', '0x3'], + }, + ], }, - ['0x1', '0x2', '0x3', '0x4'], - ); - expect(assignAccountsToScopes).toHaveBeenCalledWith( - { - 'eip155:6': { - methods: [], - notifications: [], - }, + [PermissionNames.permittedChains]: { + caveats: [ + { + type: CaveatTypes.restrictNetworkSwitching, + value: ['0x539', '0x64'], + }, + ], }, - ['0x1', '0x2', '0x3', '0x4'], - ); + }); }); it('throws an error when requesting account permission approval fails', async () => { @@ -540,41 +471,36 @@ describe('wallet_createSession', () => { expect(validateAndAddEip3085).not.toHaveBeenCalled(); }); - it('grants the CAIP-25 permission for the supported and supportable scopes', async () => { - const { handler, grantPermissions } = createMockedHandler(); + it('grants the CAIP-25 permission for the supported scopes and accounts that were approved', async () => { + const { handler, grantPermissions, requestPermissionApprovalForOrigin } = + createMockedHandler(); bucketScopes .mockReturnValueOnce({ supportedScopes: { - 'eip155:1': { + 'eip155:5': { methods: ['eth_chainId'], notifications: ['accountsChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x2'], - }, - }, - supportableScopes: { - 'eip155:2': { - methods: ['eth_chainId'], - notifications: [], + accounts: [], }, }, + supportableScopes: {}, unsupportableScopes: {}, }) .mockReturnValueOnce({ supportedScopes: { - 'eip155:1': { + 'eip155:100': { methods: ['eth_sendTransaction'], notifications: ['chainChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x3'], - }, - }, - supportableScopes: { - 'eip155:64': { - methods: ['net_version'], - notifications: ['chainChanged'], + accounts: ['eip155:1:0x3'], }, }, + supportableScopes: {}, unsupportableScopes: {}, }); + requestPermissionApprovalForOrigin.mockResolvedValue({ + approvedAccounts: ['0x1', '0x2'], + approvedChainIds: ['0x5', '0x64', '0x539'], // 5, 100, 1337 + }); await handler(baseRequest); expect(grantPermissions).toHaveBeenCalledWith({ @@ -586,25 +512,22 @@ describe('wallet_createSession', () => { type: Caip25CaveatType, value: { requiredScopes: { - 'eip155:1': { + 'eip155:5': { methods: ['eth_chainId'], notifications: ['accountsChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x2'], - }, - 'eip155:2': { - methods: ['eth_chainId'], - notifications: [], + accounts: ['eip155:5:0x1', 'eip155:5:0x2'], }, }, optionalScopes: { - 'eip155:1': { + 'eip155:100': { methods: ['eth_sendTransaction'], notifications: ['chainChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x3'], + accounts: ['eip155:100:0x1', 'eip155:100:0x2'], }, - 'eip155:64': { - methods: ['net_version'], - notifications: ['chainChanged'], + 'eip155:1337': { + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, + accounts: ['eip155:1337:0x1', 'eip155:1337:0x2'], }, }, isMultichainOrigin: true, @@ -652,40 +575,40 @@ describe('wallet_createSession', () => { }); it('returns the session ID, properties, and merged scopes', async () => { - const { handler, response } = createMockedHandler(); + const { handler, requestPermissionApprovalForOrigin, response } = + createMockedHandler(); bucketScopes .mockReturnValueOnce({ supportedScopes: { - 'eip155:1': { + 'eip155:5': { methods: ['eth_chainId'], notifications: ['accountsChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x2'], - }, - }, - supportableScopes: { - 'eip155:2': { - methods: ['eth_chainId'], - notifications: [], + accounts: ['eip155:5:0x1'], }, }, + supportableScopes: {}, unsupportableScopes: {}, }) .mockReturnValueOnce({ supportedScopes: { - 'eip155:1': { - methods: ['eth_sendTransaction'], - notifications: ['chainChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x3'], - }, - }, - supportableScopes: { - 'eip155:64': { + 'eip155:5': { methods: ['net_version'], + notifications: ['chainChanged', 'accountsChanged'], + accounts: [], + }, + 'eip155:100': { + methods: ['eth_sendTransaction'], notifications: ['chainChanged'], + accounts: ['eip155:1:0x3'], }, }, + supportableScopes: {}, unsupportableScopes: {}, }); + requestPermissionApprovalForOrigin.mockResolvedValue({ + approvedAccounts: ['0x1', '0x2'], + approvedChainIds: ['0x5', '0x64'], // 5, 100 + }); await handler(baseRequest); expect(response.result).toStrictEqual({ @@ -694,18 +617,15 @@ describe('wallet_createSession', () => { foo: 'bar', }, sessionScopes: { - 'eip155:1': { - methods: ['eth_chainId', 'eth_sendTransaction'], + 'eip155:5': { + methods: ['eth_chainId', 'net_version'], notifications: ['accountsChanged', 'chainChanged'], - accounts: ['eip155:1:0x1', 'eip155:1:0x2', 'eip155:1:0x3'], - }, - 'eip155:2': { - methods: ['eth_chainId'], - notifications: [], + accounts: ['eip155:5:0x1', 'eip155:5:0x2'], }, - 'eip155:64': { - methods: ['net_version'], + 'eip155:100': { + methods: ['eth_sendTransaction'], notifications: ['chainChanged'], + accounts: ['eip155:100:0x1', 'eip155:100:0x2'], }, }, }); diff --git a/app/scripts/lib/multichain-api/wallet-createSession/helpers.test.ts b/app/scripts/lib/multichain-api/wallet-createSession/helpers.test.ts index ef2e10aabb5d..118f98d569ff 100644 --- a/app/scripts/lib/multichain-api/wallet-createSession/helpers.test.ts +++ b/app/scripts/lib/multichain-api/wallet-createSession/helpers.test.ts @@ -1,7 +1,6 @@ import { RpcEndpointType } from '@metamask/network-controller'; import * as EthereumChainUtils from '../../rpc-method-middleware/handlers/ethereum-chain-utils'; -import { ScopesObject } from '../scope'; -import { assignAccountsToScopes, validateAndAddEip3085 } from './helpers'; +import { validateAndAddEip3085 } from './helpers'; jest.mock('../../rpc-method-middleware/handlers/ethereum-chain-utils', () => ({ validateAddEthereumChainParams: jest.fn(), @@ -13,56 +12,6 @@ describe('wallet_createSession helpers', () => { jest.resetAllMocks(); }); - describe('assignAccountsToScopes', () => { - it('overwrites the accounts property of each scope object with a CAIP-10 id built from the scopeString and passed in accounts', () => { - const scopes: ScopesObject = { - 'eip155:1': { - methods: [], - notifications: [], - accounts: ['will:be:overwitten'], - }, - 'eip155:5': { - methods: [], - notifications: [], - accounts: ['will:be:overwitten'], - }, - }; - - assignAccountsToScopes(scopes, ['0x1', '0x2', '0x3']); - - expect(scopes).toStrictEqual({ - 'eip155:1': { - methods: [], - notifications: [], - accounts: ['eip155:1:0x1', 'eip155:1:0x2', 'eip155:1:0x3'], - }, - 'eip155:5': { - methods: [], - notifications: [], - accounts: ['eip155:5:0x1', 'eip155:5:0x2', 'eip155:5:0x3'], - }, - }); - }); - - it('does not assign accounts for the wallet scope', () => { - const scopes: ScopesObject = { - wallet: { - methods: [], - notifications: [], - }, - }; - - assignAccountsToScopes(scopes, ['0x1', '0x2', '0x3']); - - expect(scopes).toStrictEqual({ - wallet: { - methods: [], - notifications: [], - }, - }); - }); - }); - describe('validateAndAddEip3085', () => { const addNetwork = jest.fn(); const findNetworkClientIdByChainId = jest.fn(); diff --git a/app/scripts/lib/multichain-api/wallet-createSession/helpers.ts b/app/scripts/lib/multichain-api/wallet-createSession/helpers.ts index 12e00aed32aa..2470feeaf25d 100644 --- a/app/scripts/lib/multichain-api/wallet-createSession/helpers.ts +++ b/app/scripts/lib/multichain-api/wallet-createSession/helpers.ts @@ -1,24 +1,10 @@ -import { CaipAccountId, Hex } from '@metamask/utils'; +import { Hex } from '@metamask/utils'; import { NetworkController, RpcEndpointType, } from '@metamask/network-controller'; -import { ScopesObject } from '../scope'; import { validateAddEthereumChainParams } from '../../rpc-method-middleware/handlers/ethereum-chain-utils'; -export const assignAccountsToScopes = ( - scopes: ScopesObject, - accounts: Hex[], -) => { - Object.entries(scopes).forEach(([scopeString, scopeObject]) => { - if (scopeString !== 'wallet') { - scopeObject.accounts = accounts.map( - (account) => `${scopeString}:${account}` as unknown as CaipAccountId, // do we need checks here? - ); - } - }); -}; - export const validateAndAddEip3085 = async ({ eip3085Params, addNetwork, diff --git a/app/scripts/lib/multichain-api/wallet-requestPermissions.js b/app/scripts/lib/multichain-api/wallet-requestPermissions.js index cb10c09f5948..70a63750e65b 100644 --- a/app/scripts/lib/multichain-api/wallet-requestPermissions.js +++ b/app/scripts/lib/multichain-api/wallet-requestPermissions.js @@ -23,7 +23,6 @@ export const requestPermissionsHandler = { grantPermissions: true, requestPermissionApprovalForOrigin: true, getAccounts: true, - getNetworkConfigurationByNetworkClientId: true, }, }; @@ -41,7 +40,6 @@ export const requestPermissionsHandler = { * @param options.grantPermissions * @param options.requestPermissionApprovalForOrigin * @param options.getAccounts - * @param options.getNetworkConfigurationByNetworkClientId * @returns A promise that resolves to nothing */ async function requestPermissionsImplementation( @@ -56,10 +54,9 @@ async function requestPermissionsImplementation( grantPermissions, requestPermissionApprovalForOrigin, getAccounts, - getNetworkConfigurationByNetworkClientId, }, ) { - const { origin, params, networkClientId } = req; + const { origin, params } = req; if (!Array.isArray(params) || !isPlainObject(params[0])) { return end(invalidParams({ data: { request: req } })); @@ -90,16 +87,7 @@ async function requestPermissionsImplementation( } if (!legacyRequestedPermissions[PermissionNames.permittedChains]) { - const { chainId } = - getNetworkConfigurationByNetworkClientId(networkClientId); - legacyRequestedPermissions[PermissionNames.permittedChains] = { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: [chainId], - }, - ], - }; + legacyRequestedPermissions[PermissionNames.permittedChains] = {}; } legacyApproval = await requestPermissionApprovalForOrigin( diff --git a/app/scripts/lib/multichain-api/wallet-requestPermissions.test.js b/app/scripts/lib/multichain-api/wallet-requestPermissions.test.js index 431c682bbcd6..cae18dbbc106 100644 --- a/app/scripts/lib/multichain-api/wallet-requestPermissions.test.js +++ b/app/scripts/lib/multichain-api/wallet-requestPermissions.test.js @@ -94,9 +94,6 @@ const createMockedHandler = () => { }, }), ); - const getNetworkConfigurationByNetworkClientId = jest.fn().mockReturnValue({ - chainId: '0x1', - }); const updateCaveat = jest.fn(); const grantPermissions = jest.fn().mockReturnValue( Object.freeze({ @@ -125,7 +122,6 @@ const createMockedHandler = () => { requestPermissionsHandler.implementation(request, response, next, end, { requestPermissionsForOrigin, getPermissionsForOrigin, - getNetworkConfigurationByNetworkClientId, updateCaveat, grantPermissions, requestPermissionApprovalForOrigin, @@ -139,7 +135,6 @@ const createMockedHandler = () => { end, requestPermissionsForOrigin, getPermissionsForOrigin, - getNetworkConfigurationByNetworkClientId, updateCaveat, grantPermissions, requestPermissionApprovalForOrigin, @@ -175,12 +170,9 @@ describe('requestPermissionsHandler', () => { ); }); - it('requests approval from the ApprovalController for eth_accounts and permittedChains with the chainId for the currently selected networkClientId (either global or dapp selected) when only eth_accounts is specified in params', async () => { - const { - handler, - getNetworkConfigurationByNetworkClientId, - requestPermissionApprovalForOrigin, - } = createMockedHandler(); + it('requests approval from the ApprovalController for eth_accounts and permittedChains when only eth_accounts is specified in params', async () => { + const { handler, requestPermissionApprovalForOrigin } = + createMockedHandler(); await handler({ ...getBaseRequest(), @@ -193,30 +185,17 @@ describe('requestPermissionsHandler', () => { ], }); - expect(getNetworkConfigurationByNetworkClientId).toHaveBeenCalledWith( - 'mainnet', - ); expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: { foo: 'bar', }, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x1'], - }, - ], - }, + [PermissionNames.permittedChains]: {}, }); }); it('requests approval from the ApprovalController for eth_accounts and permittedChains when only permittedChains is specified in params', async () => { - const { - handler, - getNetworkConfigurationByNetworkClientId, - requestPermissionApprovalForOrigin, - } = createMockedHandler(); + const { handler, requestPermissionApprovalForOrigin } = + createMockedHandler(); await handler({ ...getBaseRequest(), @@ -234,7 +213,6 @@ describe('requestPermissionsHandler', () => { ], }); - expect(getNetworkConfigurationByNetworkClientId).not.toHaveBeenCalled(); expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: {}, [PermissionNames.permittedChains]: { @@ -249,11 +227,8 @@ describe('requestPermissionsHandler', () => { }); it('requests approval from the ApprovalController for eth_accounts and permittedChains when both are specified in params', async () => { - const { - handler, - getNetworkConfigurationByNetworkClientId, - requestPermissionApprovalForOrigin, - } = createMockedHandler(); + const { handler, requestPermissionApprovalForOrigin } = + createMockedHandler(); await handler({ ...getBaseRequest(), @@ -274,7 +249,6 @@ describe('requestPermissionsHandler', () => { ], }); - expect(getNetworkConfigurationByNetworkClientId).not.toHaveBeenCalled(); expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: { foo: 'bar', diff --git a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.js b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.js index ec1d14c1d1db..eb5030a51aa5 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.js @@ -6,6 +6,10 @@ import { } from '../../multichain-api/caip25permissions'; import { CaveatTypes } from '../../../../../shared/constants/permissions'; import { PermissionNames } from '../../../controllers/permissions'; +import { + KnownNotifications, + KnownRpcMethods, +} from '../../multichain-api/scope'; import * as EthChainUtils from './ethereum-chain-utils'; describe('Ethereum Chain Utils', () => { @@ -245,8 +249,8 @@ describe('Ethereum Chain Utils', () => { requiredScopes: {}, optionalScopes: { 'eip155:1': { - methods: [], - notifications: [], + methods: KnownRpcMethods.eip155, + notifications: KnownNotifications.eip155, accounts: [], }, }, diff --git a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.js b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.js index df0abab11ac3..e5f962234b29 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.js @@ -9,10 +9,7 @@ import { Caip25CaveatType, Caip25EndowmentPermissionName, } from '../../multichain-api/caip25permissions'; -import { - CaveatTypes, - RestrictedMethods, -} from '../../../../../shared/constants/permissions'; +import { RestrictedMethods } from '../../../../../shared/constants/permissions'; import { setEthAccounts } from '../../multichain-api/adapters/caip-permission-adapter-eth-accounts'; import { PermissionNames } from '../../../controllers/permissions'; import { setPermittedEthChainIds } from '../../multichain-api/adapters/caip-permission-adapter-permittedChains'; @@ -35,7 +32,6 @@ const requestEthereumAccounts = { sendMetrics: true, metamaskState: true, grantPermissions: true, - getNetworkConfigurationByNetworkClientId: true, }, }; export default requestEthereumAccounts; @@ -75,7 +71,6 @@ async function requestEthereumAccountsHandler( sendMetrics, metamaskState, grantPermissions, - getNetworkConfigurationByNetworkClientId, }, ) { const { origin } = req; @@ -104,22 +99,11 @@ async function requestEthereumAccountsHandler( return undefined; } - const { chainId } = getNetworkConfigurationByNetworkClientId( - req.networkClientId, - ); - let legacyApproval; try { legacyApproval = await requestPermissionApprovalForOrigin({ [RestrictedMethods.eth_accounts]: {}, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: [chainId], - }, - ], - }, + [PermissionNames.permittedChains]: {}, }); } catch (err) { res.error = err; diff --git a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.js b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.js index 1ccf1ca27366..d54b468a510d 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.js @@ -4,10 +4,7 @@ import { Caip25CaveatType, Caip25EndowmentPermissionName, } from '../../multichain-api/caip25permissions'; -import { - CaveatTypes, - RestrictedMethods, -} from '../../../../../shared/constants/permissions'; +import { RestrictedMethods } from '../../../../../shared/constants/permissions'; import { PermissionNames } from '../../../controllers/permissions'; import PermittedChainsAdapters from '../../multichain-api/adapters/caip-permission-adapter-permittedChains'; import EthAccountsAdapters from '../../multichain-api/adapters/caip-permission-adapter-eth-accounts'; @@ -66,9 +63,6 @@ const createMockedHandler = () => { }, }; const grantPermissions = jest.fn(); - const getNetworkConfigurationByNetworkClientId = jest.fn().mockReturnValue({ - chainId: '0x1', - }); const response = {}; const handler = (request) => requestEthereumAccounts.implementation(request, response, next, end, { @@ -78,7 +72,6 @@ const createMockedHandler = () => { sendMetrics, metamaskState, grantPermissions, - getNetworkConfigurationByNetworkClientId, }); return { @@ -90,7 +83,6 @@ const createMockedHandler = () => { requestPermissionApprovalForOrigin, sendMetrics, grantPermissions, - getNetworkConfigurationByNetworkClientId, handler, }; }; @@ -159,16 +151,6 @@ describe('requestEthereumAccountsHandler', () => { }); describe('eip155 account permissions do not exist', () => { - it('gets the network configuration for the request networkClientId', async () => { - const { handler, getNetworkConfigurationByNetworkClientId } = - createMockedHandler(); - - await handler(baseRequest); - expect(getNetworkConfigurationByNetworkClientId).toHaveBeenCalledWith( - 'mainnet', - ); - }); - it('requests eth_accounts and permittedChains approval', async () => { const { handler, requestPermissionApprovalForOrigin } = createMockedHandler(); @@ -176,14 +158,7 @@ describe('requestEthereumAccountsHandler', () => { await handler(baseRequest); expect(requestPermissionApprovalForOrigin).toHaveBeenCalledWith({ [RestrictedMethods.eth_accounts]: {}, - [PermissionNames.permittedChains]: { - caveats: [ - { - type: CaveatTypes.restrictNetworkSwitching, - value: ['0x1'], - }, - ], - }, + [PermissionNames.permittedChains]: {}, }); }); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 320f3933170c..4f194d64e52d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3734,7 +3734,6 @@ export default class MetamaskController extends EventEmitter { ...getPermissionBackgroundApiMethods({ permissionController, approvalController, - networkController, }), ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) @@ -5988,10 +5987,6 @@ export default class MetamaskController extends EventEmitter { grantPermissions: this.permissionController.grantPermissions.bind( this.permissionController, ), - getNetworkConfigurationByNetworkClientId: - this.networkController.getNetworkConfigurationByNetworkClientId.bind( - this.networkController, - ), updateCaveat: this.permissionController.updateCaveat.bind( this.permissionController, ), @@ -6164,6 +6159,9 @@ export default class MetamaskController extends EventEmitter { this.networkController.findNetworkClientIdByChainId.bind( this.networkController, ), + listAccounts: this.accountsController.listAccounts.bind( + this.accountsController, + ), addNetwork: this.networkController.addNetwork.bind( this.networkController, ), diff --git a/ui/components/app/permissions-connect-permission-list/permissions-connect-permission-list.js b/ui/components/app/permissions-connect-permission-list/permissions-connect-permission-list.js index da15d384849c..e0bed04e5429 100644 --- a/ui/components/app/permissions-connect-permission-list/permissions-connect-permission-list.js +++ b/ui/components/app/permissions-connect-permission-list/permissions-connect-permission-list.js @@ -7,6 +7,7 @@ import { getSnapsMetadata } from '../../../selectors'; import { getSnapName } from '../../../helpers/utils/util'; import PermissionCell from '../permission-cell'; import { Box } from '../../component-library'; +import { CaveatTypes } from '../../../../shared/constants/permissions'; /** * Get one or more permission descriptions for a permission name. @@ -17,6 +18,10 @@ import { Box } from '../../component-library'; * @returns {JSX.Element} A permission description node. */ function getDescriptionNode(permission, index, accounts) { + const permissionValue = permission?.permissionValue?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value; + return ( ); } diff --git a/ui/components/multichain/edit-accounts-modal/edit-accounts-modal.tsx b/ui/components/multichain/edit-accounts-modal/edit-accounts-modal.tsx index d9303951af2d..4ce30ea396a1 100644 --- a/ui/components/multichain/edit-accounts-modal/edit-accounts-modal.tsx +++ b/ui/components/multichain/edit-accounts-modal/edit-accounts-modal.tsx @@ -30,6 +30,7 @@ import { } from '../../../helpers/constants/design-system'; import { getURLHost } from '../../../helpers/utils/util'; import { MergedInternalAccount } from '../../../selectors/selectors.types'; +import { isEqualCaseInsensitive } from '../../../../shared/modules/string-utils'; type EditAccountsModalProps = { activeTabOrigin: string; @@ -131,8 +132,12 @@ export const EditAccountsModal: React.FC = ({ isPinned={Boolean(account.pinned)} startAccessory={ + isEqualCaseInsensitive( + selectedAccountAddress, + account.address, + ), )} /> } diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx index 4bc42604adf3..309b8ef7b20d 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx @@ -14,6 +14,7 @@ import { } from '../../../../component-library'; import { EditAccountsModal, EditNetworksModal } from '../../..'; import { MergedInternalAccount } from '../../../../../selectors/selectors.types'; +import { isEqualCaseInsensitive } from '../../../../../../shared/modules/string-utils'; import { SiteCellTooltip } from './site-cell-tooltip'; import { SiteCellConnectionListItem } from './site-cell-connection-list-item'; @@ -54,7 +55,9 @@ export const SiteCell: React.FC = ({ const [showEditNetworksModal, setShowEditNetworksModal] = useState(false); const selectedAccounts = accounts.filter(({ address }) => - selectedAccountAddresses.includes(address), + selectedAccountAddresses.some((selectedAccountAddress) => + isEqualCaseInsensitive(selectedAccountAddress, address), + ), ); const selectedNetworks = allNetworks.filter(({ chainId }) => selectedChainIds.includes(chainId), diff --git a/ui/pages/permissions-connect/connect-page/connect-page.tsx b/ui/pages/permissions-connect/connect-page/connect-page.tsx index a30047fbd38a..59399b584a3e 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.tsx @@ -33,6 +33,11 @@ import { import { MergedInternalAccount } from '../../../selectors/selectors.types'; import { mergeAccounts } from '../../../components/multichain/account-list-menu/account-list-menu'; import { TEST_CHAINS } from '../../../../shared/constants/network'; +import { + CaveatTypes, + EndowmentTypes, + RestrictedMethods, +} from '../../../../shared/constants/permissions'; import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer'; export type ConnectPageRequest = { @@ -57,6 +62,20 @@ export const ConnectPage: React.FC = ({ }) => { const t = useI18nContext(); + const ethAccountsPermission = + request?.permissions?.[RestrictedMethods.eth_accounts]; + const requestedAccounts = + ethAccountsPermission?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts, + )?.value || []; + + const permittedChainsPermission = + request?.permissions?.[EndowmentTypes.permittedChains]; + const requestedChainIds = + permittedChainsPermission?.caveats?.find( + (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, + )?.value || []; + const networkConfigurations = useSelector(getNetworkConfigurationsByChainId); const [nonTestNetworks, testNetworks] = useMemo( () => @@ -70,7 +89,10 @@ export const ConnectPage: React.FC = ({ ), [networkConfigurations], ); - const defaultSelectedChainIds = nonTestNetworks.map(({ chainId }) => chainId); + const defaultSelectedChainIds = + requestedChainIds.length > 0 + ? requestedChainIds + : nonTestNetworks.map(({ chainId }) => chainId); const [selectedChainIds, setSelectedChainIds] = useState( defaultSelectedChainIds, ); @@ -84,7 +106,10 @@ export const ConnectPage: React.FC = ({ }, [accounts, internalAccounts]); const currentAccount = useSelector(getSelectedInternalAccount); - const defaultAccountsAddresses = [currentAccount?.address]; + const defaultAccountsAddresses = + requestedAccounts.length > 0 + ? requestedAccounts + : [currentAccount?.address]; const [selectedAccountAddresses, setSelectedAccountAddresses] = useState( defaultAccountsAddresses, );