Skip to content

refactor: connection Flow to use CAIP25 Permission format #29824

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

Merged
merged 60 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
60 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
e8bb435
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 28, 2025
267046e
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 28, 2025
2fb2e49
test: small adjustment for simplicity
ffmcgee725 Jan 28, 2025
075129b
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 28, 2025
cad7dfe
refactor: permission page container component simplify approved accounts
ffmcgee725 Jan 28, 2025
f3397ef
refactor: code review changes
ffmcgee725 Jan 28, 2025
4c11da7
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 28, 2025
0ed65fe
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 29, 2025
1ed1218
refactor: address code review
ffmcgee725 Jan 29, 2025
fe4cc4e
Merge branch 'main' into jc/caip25-permission-refactor
ffmcgee725 Jan 29, 2025
732b9fa
refactor: address code review
ffmcgee725 Jan 29, 2025
a66eac3
Merge branch 'main' into jc/caip25-permission-refactor
adonesky1 Jan 29, 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
54 changes: 16 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,9 @@ 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({
const {
response: { permissions },
} = await approvalController.addAndShowApprovalRequest({
id,
origin,
requestData: {
Expand All @@ -118,41 +111,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'],
response: {
permissions: approvedPermissions,
},
}),
};

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'],
response: {
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
58 changes: 32 additions & 26 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -5564,20 +5564,11 @@ 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?.[0]?.value ?? [];

const requestedAccounts =
permissions[PermissionNames.eth_accounts]?.caveats?.[0]?.value ?? [];

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

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

const caveatValueWithAccounts = setEthAccounts(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
const caveatValueWithAccounts = setEthAccounts(
const caveatValueWithAccountsAndChains = setEthAccounts(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved in 1ed1218

caveatValueWithChains,
legacyApproval.approvedAccounts,
requestedAccounts,
);

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

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

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

Expand Down
Loading
Loading