Skip to content

Commit 8244d35

Browse files
Merge branch 'main' into tuna/revert-575.0.0
2 parents 3bdf458 + 09d848c commit 8244d35

19 files changed

+793
-54
lines changed

eslint-warning-thresholds.json

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -349,21 +349,10 @@
349349
"jest/no-conditional-in-test": 1
350350
},
351351
"packages/signature-controller/src/SignatureController.ts": {
352-
"@typescript-eslint/no-unsafe-enum-comparison": 4
353-
},
354-
"packages/signature-controller/src/utils/decoding-api.test.ts": {
355-
"import-x/order": 1,
356-
"jsdoc/tag-lines": 1
357-
},
358-
"packages/signature-controller/src/utils/decoding-api.ts": {
359-
"import-x/order": 1
360-
},
361-
"packages/signature-controller/src/utils/normalize.test.ts": {
362-
"import-x/order": 1
352+
"@typescript-eslint/no-unsafe-enum-comparison": 3
363353
},
364354
"packages/signature-controller/src/utils/normalize.ts": {
365-
"@typescript-eslint/no-unused-vars": 1,
366-
"jsdoc/tag-lines": 2
355+
"@typescript-eslint/no-unused-vars": 1
367356
},
368357
"packages/signature-controller/src/utils/validation.ts": {
369358
"@typescript-eslint/no-base-to-string": 1,

packages/account-tree-controller/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fix group naming for PK/Hardware accounts ([#6677](https://github.com/MetaMask/core/pull/6677))
13+
- Previously, the first PK/Hardware account would start as `Account 2` as opposed to `Account 1` and thus subsequent group names were off as well.
14+
1015
## [1.0.0]
1116

1217
### Changed

packages/account-tree-controller/src/AccountTreeController.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ describe('AccountTreeController', () => {
587587
type: AccountGroupType.SingleAccount,
588588
accounts: [MOCK_SNAP_ACCOUNT_2.id],
589589
metadata: {
590-
name: 'Account 2', // Updated: per-wallet numbering (different wallet)
590+
name: 'Account 1', // Updated: per-wallet numbering (different wallet)
591591
pinned: false,
592592
hidden: false,
593593
},
@@ -610,7 +610,7 @@ describe('AccountTreeController', () => {
610610
type: AccountGroupType.SingleAccount,
611611
accounts: [MOCK_HARDWARE_ACCOUNT_1.id],
612612
metadata: {
613-
name: 'Account 2', // Updated: per-wallet numbering (different wallet)
613+
name: 'Account 1', // Updated: per-wallet numbering (different wallet)
614614
pinned: false,
615615
hidden: false,
616616
},
@@ -652,13 +652,13 @@ describe('AccountTreeController', () => {
652652
},
653653
[expectedKeyringWalletIdGroup]: {
654654
name: {
655-
value: 'Account 2', // Updated: per-wallet numbering (different wallet)
655+
value: 'Account 1', // Updated: per-wallet numbering (different wallet)
656656
lastUpdatedAt: expect.any(Number),
657657
},
658658
},
659659
[expectedSnapWalletIdGroup]: {
660660
name: {
661-
value: 'Account 2', // Updated: per-wallet numbering (different wallet)
661+
value: 'Account 1', // Updated: per-wallet numbering (different wallet)
662662
lastUpdatedAt: expect.any(Number),
663663
},
664664
},
@@ -2430,8 +2430,8 @@ describe('AccountTreeController', () => {
24302430

24312431
// Groups should use consistent default naming regardless of import time
24322432
// Updated expectations based on per-wallet sequential naming logic
2433-
expect(group1?.metadata.name).toBe('Account 3'); // Updated: reflects actual naming logic
2434-
expect(group2?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
2433+
expect(group1?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
2434+
expect(group2?.metadata.name).toBe('Account 1'); // Updated: reflects actual naming logic
24352435
});
24362436

24372437
it('uses fallback naming when rule-based naming returns empty string', () => {
@@ -2902,9 +2902,9 @@ describe('AccountTreeController', () => {
29022902
// Critical assertion: should have 2 unique names (no duplicates)
29032903
expect(uniqueNames.size).toBe(2);
29042904

2905-
// Due to optimization, names start at wallet.length, so we get "Account 3" and "Account 4"
2906-
expect(allNames).toContain('Account 3');
2907-
expect(allNames).toContain('Account 4');
2905+
// Due to optimization, names start at wallet.length, so we get "Account 1" and "Account 2"
2906+
expect(allNames).toContain('Account 1');
2907+
expect(allNames).toContain('Account 2');
29082908

29092909
// Verify they're actually different
29102910
expect(group1.metadata.name).not.toBe(group2.metadata.name);

packages/account-tree-controller/src/AccountTreeController.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,11 @@ export class AccountTreeController extends BaseController<
458458
} else {
459459
// For other wallet types, start with the number of existing groups
460460
// This gives us the next logical sequential number
461-
proposedNameIndex = Object.keys(wallet.groups).length;
461+
proposedNameIndex = Object.keys(wallet.groups).length - 1;
462462
}
463463

464464
// Use the higher of the two: highest parsed index or computed index
465-
proposedNameIndex = Math.max(highestAccountNameIndex, proposedNameIndex);
465+
proposedNameIndex = Math.min(highestAccountNameIndex, proposedNameIndex);
466466

467467
// Find a unique name by checking for conflicts and incrementing if needed
468468
let nameExists: boolean;

packages/signature-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6473](https://github.com/MetaMask/core/pull/6473))
13+
- Decode delegation permissions using `@metamask/gator-permissions-controller` when calling `newUnsignedTypedMessage` ([#6619](https://github.com/MetaMask/core/pull/6619))
1314

1415
### Changed
1516

packages/signature-controller/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"@metamask/accounts-controller": "^33.1.0",
6060
"@metamask/approval-controller": "^7.1.3",
6161
"@metamask/auto-changelog": "^3.4.4",
62+
"@metamask/gator-permissions-controller": "^0.2.0",
6263
"@metamask/keyring-controller": "^23.1.0",
6364
"@metamask/logging-controller": "^6.0.4",
6465
"@metamask/network-controller": "^24.2.0",
@@ -73,6 +74,7 @@
7374
"peerDependencies": {
7475
"@metamask/accounts-controller": "^33.0.0",
7576
"@metamask/approval-controller": "^7.0.0",
77+
"@metamask/gator-permissions-controller": "^0.2.0",
7678
"@metamask/keyring-controller": "^23.0.0",
7779
"@metamask/logging-controller": "^6.0.0",
7880
"@metamask/network-controller": "^24.0.0"

packages/signature-controller/src/SignatureController.test.ts

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,27 @@ const PERMIT_REQUEST_MOCK = {
8989
traceContext: null,
9090
};
9191

92+
const DELEGATION_PARAMS_MOCK = {
93+
data: '{"types":{"EIP712Domain":[{"name":"chainId","type":"uint256"}],"Delegation":[{"name":"delegate","type":"address"},{"name":"delegator","type":"address"},{"name":"authority","type":"bytes"},{"name":"caveats","type":"bytes"}]},"primaryType":"Delegation","domain":{"chainId":1},"message":{"delegate":"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","delegator":"0x975e73efb9ff52e23bac7f7e043a1ecd06d05477","authority":"0x1234abcd","caveats":[]},"metadata":{"origin":"https://metamask.github.io","justification":"Testing delegation"}}',
94+
from: '0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
95+
version: 'V4',
96+
signatureMethod: 'eth_signTypedData_v4',
97+
};
98+
99+
const DELEGATION_REQUEST_MOCK = {
100+
method: 'eth_signTypedData_v4',
101+
params: [
102+
'0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
103+
DELEGATION_PARAMS_MOCK.data,
104+
],
105+
jsonrpc: '2.0',
106+
id: 1680528591,
107+
origin: 'npm:@metamask/gator-permissions-snap',
108+
networkClientId: 'mainnet',
109+
tabId: 1048807182,
110+
traceContext: null,
111+
};
112+
92113
/**
93114
* Create a mock messenger instance.
94115
*
@@ -101,6 +122,7 @@ function createMessengerMock() {
101122
const keyringControllerSignTypedMessageMock = jest.fn();
102123
const loggingControllerAddMock = jest.fn();
103124
const networkControllerGetNetworkClientByIdMock = jest.fn();
125+
const decodePermissionFromPermissionContextForOriginMock = jest.fn();
104126

105127
// eslint-disable-next-line @typescript-eslint/no-explicit-any
106128
const callMock = (method: string, ...args: any[]) => {
@@ -117,6 +139,8 @@ function createMessengerMock() {
117139
return loggingControllerAddMock(...args);
118140
case 'NetworkController:getNetworkClientById':
119141
return networkControllerGetNetworkClientByIdMock(...args);
142+
case 'GatorPermissionsController:decodePermissionFromPermissionContextForOrigin':
143+
return decodePermissionFromPermissionContextForOriginMock(...args);
120144
default:
121145
throw new Error(`Messenger method not recognised: ${method}`);
122146
}
@@ -144,12 +168,17 @@ function createMessengerMock() {
144168
},
145169
});
146170

171+
decodePermissionFromPermissionContextForOriginMock.mockReturnValue({
172+
kind: 'decoded-permission',
173+
});
174+
147175
return {
148176
accountsControllerGetStateMock,
149177
approvalControllerAddRequestMock,
150178
keyringControllerSignPersonalMessageMock,
151179
keyringControllerSignTypedMessageMock,
152180
loggingControllerAddMock,
181+
decodePermissionFromPermissionContextForOriginMock,
153182
messenger,
154183
};
155184
}
@@ -933,6 +962,201 @@ describe('SignatureController', () => {
933962
).toBe(SignTypedDataVersion.V3);
934963
});
935964

965+
describe('delegations', () => {
966+
it('invokes decodePermissionFromRequest to get execution permission', async () => {
967+
const {
968+
controller,
969+
decodePermissionFromPermissionContextForOriginMock,
970+
} = createController();
971+
972+
await controller.newUnsignedTypedMessage(
973+
DELEGATION_PARAMS_MOCK,
974+
DELEGATION_REQUEST_MOCK,
975+
SignTypedDataVersion.V4,
976+
{ parseJsonData: false },
977+
);
978+
979+
expect(
980+
decodePermissionFromPermissionContextForOriginMock,
981+
).toHaveBeenCalledWith({
982+
origin: 'npm:@metamask/gator-permissions-snap',
983+
chainId: 1,
984+
delegation: {
985+
delegate: '0x5B38Da6a701c568545dCfcB03FcB875f56beddC4',
986+
delegator: '0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
987+
authority: '0x1234abcd',
988+
caveats: [],
989+
},
990+
metadata: {
991+
origin: 'https://metamask.github.io',
992+
justification: 'Testing delegation',
993+
},
994+
});
995+
});
996+
997+
it('does not invoke decodePermissionFromRequest if version is not V4', async () => {
998+
const {
999+
controller,
1000+
decodePermissionFromPermissionContextForOriginMock,
1001+
} = createController();
1002+
1003+
await controller.newUnsignedTypedMessage(
1004+
DELEGATION_PARAMS_MOCK,
1005+
DELEGATION_REQUEST_MOCK,
1006+
SignTypedDataVersion.V3,
1007+
{ parseJsonData: false },
1008+
);
1009+
1010+
expect(
1011+
decodePermissionFromPermissionContextForOriginMock,
1012+
).not.toHaveBeenCalled();
1013+
});
1014+
1015+
it('sets decodedPermission on the message state', async () => {
1016+
const { controller } = createController();
1017+
1018+
await controller.newUnsignedTypedMessage(
1019+
DELEGATION_PARAMS_MOCK,
1020+
DELEGATION_REQUEST_MOCK,
1021+
SignTypedDataVersion.V4,
1022+
{ parseJsonData: false },
1023+
);
1024+
1025+
const { decodedPermission } =
1026+
controller.state.signatureRequests[ID_MOCK];
1027+
1028+
expect(decodedPermission).toStrictEqual({
1029+
kind: 'decoded-permission',
1030+
});
1031+
});
1032+
1033+
it('does not invoke decodePermissionFromRequest if data is not a delegation request', async () => {
1034+
const {
1035+
controller,
1036+
decodePermissionFromPermissionContextForOriginMock,
1037+
} = createController();
1038+
1039+
await controller.newUnsignedTypedMessage(
1040+
{
1041+
...DELEGATION_PARAMS_MOCK,
1042+
data: '{primaryType:"not-a-delegation"}',
1043+
},
1044+
DELEGATION_REQUEST_MOCK,
1045+
SignTypedDataVersion.V4,
1046+
{ parseJsonData: false },
1047+
);
1048+
1049+
expect(
1050+
decodePermissionFromPermissionContextForOriginMock,
1051+
).not.toHaveBeenCalled();
1052+
});
1053+
1054+
it('does not set decodedPermission if data is not a delegation request', async () => {
1055+
const { controller } = createController();
1056+
1057+
await controller.newUnsignedTypedMessage(
1058+
{
1059+
...DELEGATION_PARAMS_MOCK,
1060+
data: '{primaryType:"not-a-delegation"}',
1061+
},
1062+
DELEGATION_REQUEST_MOCK,
1063+
SignTypedDataVersion.V4,
1064+
{ parseJsonData: false },
1065+
);
1066+
1067+
const { decodedPermission } =
1068+
controller.state.signatureRequests[ID_MOCK];
1069+
1070+
expect(decodedPermission).toBeUndefined();
1071+
});
1072+
1073+
it('does not set decodedPermission if decoding throws an error', async () => {
1074+
const {
1075+
controller,
1076+
decodePermissionFromPermissionContextForOriginMock,
1077+
} = createController();
1078+
1079+
decodePermissionFromPermissionContextForOriginMock.mockImplementation(
1080+
() => {
1081+
throw new Error('An error occurred');
1082+
},
1083+
);
1084+
1085+
await controller.newUnsignedTypedMessage(
1086+
DELEGATION_PARAMS_MOCK,
1087+
DELEGATION_REQUEST_MOCK,
1088+
SignTypedDataVersion.V4,
1089+
{ parseJsonData: false },
1090+
);
1091+
1092+
const { decodedPermission } =
1093+
controller.state.signatureRequests[ID_MOCK];
1094+
1095+
expect(decodedPermission).toBeUndefined();
1096+
});
1097+
1098+
it('does not set decodedPermission if decoding returns undefined', async () => {
1099+
const {
1100+
controller,
1101+
decodePermissionFromPermissionContextForOriginMock,
1102+
} = createController();
1103+
1104+
decodePermissionFromPermissionContextForOriginMock.mockReturnValue(
1105+
undefined,
1106+
);
1107+
1108+
await controller.newUnsignedTypedMessage(
1109+
DELEGATION_PARAMS_MOCK,
1110+
DELEGATION_REQUEST_MOCK,
1111+
SignTypedDataVersion.V4,
1112+
{ parseJsonData: false },
1113+
);
1114+
1115+
const { decodedPermission } =
1116+
controller.state.signatureRequests[ID_MOCK];
1117+
1118+
expect(decodedPermission).toBeUndefined();
1119+
});
1120+
1121+
it('does not set decodedPermission if metadata is invalid', async () => {
1122+
const { controller } = createController();
1123+
1124+
const delegationParamsMock = {
1125+
...DELEGATION_PARAMS_MOCK,
1126+
data: {
1127+
...JSON.parse(DELEGATION_PARAMS_MOCK.data),
1128+
metadata: {},
1129+
},
1130+
};
1131+
1132+
const delegationRequestMock = {
1133+
method: 'eth_signTypedData_v4',
1134+
params: [
1135+
'0x975e73efb9ff52e23bac7f7e043a1ecd06d05477',
1136+
delegationParamsMock.data,
1137+
],
1138+
jsonrpc: '2.0',
1139+
id: 1680528591,
1140+
origin: 'npm:@metamask/gator-permissions-snap',
1141+
networkClientId: 'mainnet',
1142+
tabId: 1048807182,
1143+
traceContext: null,
1144+
};
1145+
1146+
await controller.newUnsignedTypedMessage(
1147+
delegationParamsMock,
1148+
delegationRequestMock,
1149+
SignTypedDataVersion.V4,
1150+
{ parseJsonData: false },
1151+
);
1152+
1153+
const { decodedPermission } =
1154+
controller.state.signatureRequests[ID_MOCK];
1155+
1156+
expect(decodedPermission).toBeUndefined();
1157+
});
1158+
});
1159+
9361160
describe('decodeSignature', () => {
9371161
it('invoke decodeSignature to get decoding data', async () => {
9381162
const MOCK_STATE_CHANGES = {

0 commit comments

Comments
 (0)