Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: connection Flow to use CAIP25 Permission format #29824

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
0ce5acc
refactor: send transformed caip25 compliant data to ApprovalControlle…
ffmcgee725 Jan 21, 2025
4c7d343
fix: fix payload sent to UI after Jiexi sesh
ffmcgee725 Jan 21, 2025
9b84b2b
refactor: send transformed caip25 compliant data to ApprovalController
ffmcgee725 Jan 22, 2025
8b48878
fix: handle common edge case with empty caveats
ffmcgee725 Jan 22, 2025
f3209cd
docs: document utility UI functions for caip25 connection flow
ffmcgee725 Jan 22, 2025
f793fee
test: unit test for UI utility functions
ffmcgee725 Jan 22, 2025
3c2ea45
Merge branch 'main' into jc/caip25-permission-refactor-main
ffmcgee725 Jan 23, 2025
7c919bc
lint
ffmcgee725 Jan 23, 2025
c88ffae
test: fix metamask controller caip25 unit tests
ffmcgee725 Jan 23, 2025
b244570
fix: address 'undefined' request object in connect-page component whe…
ffmcgee725 Jan 23, 2025
49814b8
docs: minor update
ffmcgee725 Jan 23, 2025
cea8fa8
refactor: address 'background-api.js' requestAccountsAndChainPermissi…
ffmcgee725 Jan 23, 2025
f771a58
Merge branch 'main' into jc/caip25-permission-refactor-main
ffmcgee725 Jan 23, 2025
5040491
docs: remove outdated comments
ffmcgee725 Jan 23, 2025
d885ecf
fix: fix UI reroute
ffmcgee725 Jan 23, 2025
a93dbdb
docs: task for future
ffmcgee725 Jan 23, 2025
9e11d21
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 23, 2025
fa015e3
refactor: 'requestApprovalPermittedChainsPermission' refactored to se…
ffmcgee725 Jan 23, 2025
981ad1e
test: fix 'requestApprovalPermittedChainsPermission' unit test
ffmcgee725 Jan 23, 2025
fe51b63
test: fix connect-page.test.tsx
ffmcgee725 Jan 23, 2025
f644d12
docs: remove commented code
ffmcgee725 Jan 23, 2025
87ff50d
refactor: remove dependency of PermissionNames.permittedChains from p…
ffmcgee725 Jan 23, 2025
1d0b241
docs: remove outdated comment
ffmcgee725 Jan 23, 2025
2efe4c0
test: remove outdated response data from test
ffmcgee725 Jan 23, 2025
9108e47
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 23, 2025
74fc018
refactor: use util functions from @metamask/multichain for chain / ad…
ffmcgee725 Jan 24, 2025
ae8c0f6
refactor: code review improvements
ffmcgee725 Jan 24, 2025
e30e051
fix: minor bug fix
ffmcgee725 Jan 24, 2025
38b6c7f
test: fix background api tets
ffmcgee725 Jan 24, 2025
c0832a4
merge main and fix conflicts
ffmcgee725 Jan 24, 2025
26cc31a
docs: add comment to 'requestAccountsAndChainPermissions'
ffmcgee725 Jan 24, 2025
b10f5ea
docs: add comment to 'requestAccountsAndChainPermissions'
ffmcgee725 Jan 24, 2025
7ca66e9
test: fix unit test as per code review
ffmcgee725 Jan 24, 2025
22b8227
test: minor test refactor
ffmcgee725 Jan 24, 2025
8d5968e
refactor: all redux state passed to container for permission page con…
ffmcgee725 Jan 24, 2025
ecad6ee
refactor: rename 'parseCaip25PermissionsResponse' to 'getCaip25Permis…
ffmcgee725 Jan 24, 2025
262d8f7
Merge branch 'main' into jc/caip25-permission-refactor
adonesky1 Jan 24, 2025
e49c99f
fix: fix issues raised by e2e tests (wip)
ffmcgee725 Jan 25, 2025
6e57d3c
docs: document findings for failing test/e2e/json-rpc/switchEthereumC…
ffmcgee725 Jan 25, 2025
1de4c5e
docs: document problem continuation
ffmcgee725 Jan 25, 2025
7bb7dad
fix: possible solution for permission.js file (wip)
ffmcgee725 Jan 26, 2025
5e3fd1f
fix: fix UI bug for properly rendering chain instead of accounts on w…
ffmcgee725 Jan 26, 2025
ae0bdb4
fix: consistently fetch pendingPermissionsFromOrigin to check for pen…
ffmcgee725 Jan 26, 2025
a1620d4
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 26, 2025
bfccd6e
refactor: code clean up
ffmcgee725 Jan 26, 2025
80611dc
docs: minor adjustment
ffmcgee725 Jan 26, 2025
486d7ea
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 27, 2025
e2a406f
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 24 additions & 38 deletions app/scripts/controllers/permissions/background-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import {
setPermittedEthChainIds,
} from '@metamask/multichain';
import { isSnapId } from '@metamask/snaps-utils';
import { RestrictedMethods } from '../../../../shared/constants/permissions';
import { PermissionNames } from './specifications';

export function getPermissionBackgroundApiMethods({
permissionController,
Expand Down Expand Up @@ -102,14 +100,17 @@ export function getPermissionBackgroundApiMethods({
};

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({
ffmcgee725 marked this conversation as resolved.
Show resolved Hide resolved
/**
* 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 CAIP-25 permission is missing the factory method implementation.
* After the factory method is added, we can move to requesting "endowment:caip25"
* directly from the PermissionController instead.
*/
const {
approvedSessionScopes: { permissions },
} = await approvalController.addAndShowApprovalRequest({
id,
origin,
requestData: {
Expand All @@ -118,41 +119,26 @@ export function getPermissionBackgroundApiMethods({
origin,
},
permissions: {
[RestrictedMethods.eth_accounts]: {},
[PermissionNames.permittedChains]: {},
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: {
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
},
},
],
},
},
},
type: MethodNames.RequestPermissions,
});

const newCaveatValue = {
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
};

const caveatValueWithChains = setPermittedEthChainIds(
newCaveatValue,
legacyApproval.approvedChainIds,
);

const caveatValueWithAccounts = setEthAccounts(
caveatValueWithChains,
legacyApproval.approvedAccounts,
);

permissionController.grantPermissions({
subject: { origin },
approvedPermissions: {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: caveatValueWithAccounts,
},
],
},
},
approvedPermissions: permissions,
});
};

Expand Down
72 changes: 43 additions & 29 deletions app/scripts/controllers/permissions/background-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import {
Caip25CaveatType,
Caip25EndowmentPermissionName,
} from '@metamask/multichain';
import { RestrictedMethods } from '../../../../shared/constants/permissions';
import { flushPromises } from '../../../../test/lib/timer-helpers';
import { getPermissionBackgroundApiMethods } from './background-api';
import { PermissionNames } from './specifications';

describe('permission background API methods', () => {
afterEach(() => {
Expand Down Expand Up @@ -466,13 +464,37 @@ describe('permission background API methods', () => {
});

describe('requestAccountsAndChainPermissionsWithId', () => {
const approvedPermissions = {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdeadbeef'],
},
'eip155:5': {
accounts: ['eip155:5:0xdeadbeef'],
},
},
isMultichainOrigin: false,
},
},
],
},
};

it('requests eth_accounts and permittedChains approval and returns the request id', async () => {
const approvalController = {
addAndShowApprovalRequest: jest.fn().mockResolvedValue({
approvedChainIds: ['0x1', '0x5'],
approvedAccounts: ['0xdeadbeef'],
approvedSessionScopes: {
permissions: approvedPermissions,
jiexi marked this conversation as resolved.
Show resolved Hide resolved
},
}),
};

const permissionController = {
grantPermissions: jest.fn(),
};
Expand All @@ -496,8 +518,18 @@ describe('permission background API methods', () => {
origin: 'foo.com',
},
permissions: {
[RestrictedMethods.eth_accounts]: {},
[PermissionNames.permittedChains]: {},
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: {
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
},
},
],
},
},
},
type: MethodNames.RequestPermissions,
Expand All @@ -508,10 +540,12 @@ describe('permission background API methods', () => {
it('grants a legacy CAIP-25 permission (isMultichainOrigin: false) with the approved eip155 chainIds and accounts', async () => {
const approvalController = {
addAndShowApprovalRequest: jest.fn().mockResolvedValue({
approvedChainIds: ['0x1', '0x5'],
approvedAccounts: ['0xdeadbeef'],
approvedSessionScopes: {
permissions: approvedPermissions,
},
}),
};

const permissionController = {
grantPermissions: jest.fn(),
};
Expand All @@ -527,27 +561,7 @@ describe('permission background API methods', () => {
subject: {
origin: 'foo.com',
},
approvedPermissions: {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdeadbeef'],
},
'eip155:5': {
accounts: ['eip155:5:0xdeadbeef'],
},
},
isMultichainOrigin: false,
},
},
],
},
},
approvedPermissions,
});
});
});
Expand Down
77 changes: 48 additions & 29 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -5416,6 +5416,15 @@ export default class MetamaskController extends EventEmitter {
* @param {Hex} chainId - The chainId to add incrementally.
*/
async requestApprovalPermittedChainsPermission(origin, chainId) {
const caveatValueWithChains = setPermittedEthChainIds(
{
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
},
[chainId],
);

const id = nanoid();
await this.approvalController.addAndShowApprovalRequest({
id,
Expand All @@ -5426,11 +5435,11 @@ export default class MetamaskController extends EventEmitter {
origin,
},
permissions: {
[PermissionNames.permittedChains]: {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: CaveatTypes.restrictNetworkSwitching,
value: [chainId],
type: Caip25CaveatType,
value: caveatValueWithChains,
},
],
},
Expand Down Expand Up @@ -5566,20 +5575,15 @@ export default class MetamaskController extends EventEmitter {
delete permissions[PermissionNames.permittedChains];
}

const id = nanoid();
const legacyApproval =
await this.approvalController.addAndShowApprovalRequest({
id,
origin,
requestData: {
metadata: {
id,
origin,
},
permissions,
},
type: MethodNames.RequestPermissions,
});
const requestedChains =
permissions[PermissionNames.permittedChains]?.caveats?.find(
(caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching,
)?.value ?? [];

const requestedAccounts =
permissions[PermissionNames.eth_accounts]?.caveats?.find(
(caveat) => caveat.type === CaveatTypes.restrictReturnedAccounts,
)?.value ?? [];

const newCaveatValue = {
requiredScopes: {},
Expand All @@ -5593,26 +5597,41 @@ export default class MetamaskController extends EventEmitter {

const caveatValueWithChains = setPermittedEthChainIds(
newCaveatValue,
isSnapId(origin) ? [] : legacyApproval.approvedChainIds,
isSnapId(origin) ? [] : requestedChains,
);

const caveatValueWithAccounts = setEthAccounts(
caveatValueWithChains,
legacyApproval.approvedAccounts,
requestedAccounts,
);

return {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: caveatValueWithAccounts,
const id = nanoid();

const { approvedSessionScopes } =
await this.approvalController.addAndShowApprovalRequest({
id,
origin,
requestData: {
metadata: {
id,
origin,
},
],
},
};
}
permissions: {
[Caip25EndowmentPermissionName]: {
caveats: [
{
type: Caip25CaveatType,
value: caveatValueWithAccounts,
},
],
},
},
},
type: MethodNames.RequestPermissions,
});

return approvedSessionScopes.permissions;
}
// ---------------------------------------------------------------------------
// Identity Management (signature operations)

Expand Down
Loading
Loading