Skip to content

Commit

Permalink
Jl/caip multichain/fix connection flow for permitted chains (#27471)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

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

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
jiexi authored Oct 3, 2024
1 parent 68db523 commit 4126dd3
Show file tree
Hide file tree
Showing 18 changed files with 278 additions and 494 deletions.
28 changes: 9 additions & 19 deletions app/scripts/controllers/permissions/background-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -115,14 +112,7 @@ export function getPermissionBackgroundApiMethods({
},
permissions: {
[RestrictedMethods.eth_accounts]: {},
[PermissionNames.permittedChains]: {
caveats: [
{
type: CaveatTypes.restrictNetworkSwitching,
value: [chainId],
},
],
},
[PermissionNames.permittedChains]: {},
},
},
type: MethodNames.requestPermissions,
Expand Down
110 changes: 29 additions & 81 deletions app/scripts/controllers/permissions/background-api.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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'],
Expand All @@ -519,7 +482,6 @@ describe('permission background API methods', () => {
};

const result = getPermissionBackgroundApiMethods({
networkController,
approvalController,
permissionController,
}).requestAccountsAndChainPermissionsWithId('foo.com');
Expand All @@ -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,
Expand All @@ -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'],
Expand All @@ -574,7 +521,6 @@ describe('permission background API methods', () => {
};

getPermissionBackgroundApiMethods({
networkController,
approvalController,
permissionController,
}).requestAccountsAndChainPermissionsWithId('foo.com');
Expand All @@ -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'],
},
},
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand All @@ -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,
Expand All @@ -698,6 +644,7 @@ describe('permission background API methods', () => {
'eip155:1': {
methods: [],
notifications: [],
accounts: ['eip155:1:0x1', 'eip155:1:0x2'],
},
'eip155:10': {
methods: [],
Expand All @@ -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,
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand All @@ -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,
Expand All @@ -814,6 +761,7 @@ describe('permission background API methods', () => {
'eip155:1': {
methods: [],
notifications: [],
accounts: ['eip155:1:0x1', 'eip155:1:0x2'],
},
'eip155:10': {
methods: [],
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Caip25CaveatValue } from '../caip25permissions';
import { KnownNotifications, KnownRpcMethods } from '../scope';
import {
addPermittedEthChainId,
getPermittedEthChainIds,
Expand Down Expand Up @@ -89,8 +90,8 @@ describe('CAIP-25 permittedChains adapters', () => {
accounts: ['eip155:100:0x100'],
},
'eip155:101': {
methods: [],
notifications: [],
methods: KnownRpcMethods.eip155,
notifications: KnownNotifications.eip155,
accounts: [],
},
},
Expand Down Expand Up @@ -272,8 +273,8 @@ describe('CAIP-25 permittedChains adapters', () => {
accounts: ['eip155:100:0x100'],
},
'eip155:101': {
methods: [],
notifications: [],
methods: KnownRpcMethods.eip155,
notifications: KnownNotifications.eip155,
accounts: [],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -44,8 +46,8 @@ export const addPermittedEthChainId = (
optionalScopes: {
...caip25CaveatValue.optionalScopes,
[scopeString]: {
methods: [],
notifications: [],
methods: KnownRpcMethods.eip155,
notifications: KnownNotifications.eip155,
accounts: [],
},
},
Expand Down
Loading

0 comments on commit 4126dd3

Please sign in to comment.