Skip to content

Commit 096b7ee

Browse files
jeffsmale90jeffsmale90
authored
feat:add erc20-token-revocation permission to controller state (#7318)
Plus minor refactor to permission map generation. ## Explanation We recently added `erc20-token-revocation` to the permission granting flow. This change adds the type to the permission controller state, so that these permissions become differentiated in the All Permission section (rather than relegated to "other"). Also includes a minor refactor of parsing a list of stored permissions into a multi-dimensional map keyed by `permissionType | chainId`, which previously required changes to this function for every new type added. Now simply adding the permission type to the `createEmptyGatorPermissionsMap` function will be required for new permissions. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds a dedicated `erc20-token-revocation` bucket to controller state and refactors permission categorization to be extensible via a centralized empty-map factory. > > - **gator-permissions-controller** > - **State/Defaults**: Introduce `createEmptyGatorPermissionsMap()` and include `erc20-token-revocation` in default/cleared state; update getters/disable flow to use new factory. > - **Logic**: Rewrite `#categorizePermissionsDataByTypeAndChainId` to dynamically bucket by known permission types (fallback to `other`) instead of switch-case. > - **Types**: Extend `GatorPermissionsMap` to include `erc20-token-revocation` (imports `Erc20TokenRevocationPermission`). > - **Tests**: Update expectations for default/empty maps; add test ensuring `erc20-token-revocation` is categorized separately; preserve sanitization checks; tweak utils serialization-failure test. > - **Changelog**: Note differentiation of `erc20-token-revocation` from `other`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e4e996d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2b2cf96 commit 096b7ee

File tree

5 files changed

+107
-69
lines changed

5 files changed

+107
-69
lines changed

‎packages/gator-permissions-controller/CHANGELOG.md‎

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

1212
- **BREAKING:** Permission decoding now rejects `TimestampEnforcer` caveats with zero `timestampBeforeThreshold` values ([#7195](https://github.com/MetaMask/core/pull/7195))
1313
- Permission decoding logic for `erc20-token-revocation` permission type ([#7299](https://github.com/MetaMask/core/pull/7299))
14+
- Differentiate `erc20-token-revocation` permissions from `other` in controller state ([#7318](https://github.com/MetaMask/core/pull/7318))
1415
- Validation errors for `TimestampEnforcer` include: invalid terms length, non-zero `timestampAfterThreshold`, and zero `timestampBeforeThreshold` ([#7195](https://github.com/MetaMask/core/pull/7195))
1516
- Bump `@metamask/transaction-controller` from `^62.3.1` to `^62.5.0` ([#7289](https://github.com/MetaMask/core/pull/7289), [#7325](https://github.com/MetaMask/core/pull/7325))
1617

‎packages/gator-permissions-controller/src/GatorPermissionsController.test.ts‎

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ describe('GatorPermissionsController', () => {
9595
expect(controller.state.isGatorPermissionsEnabled).toBe(false);
9696
expect(controller.state.gatorPermissionsMapSerialized).toStrictEqual(
9797
JSON.stringify({
98+
'erc20-token-revocation': {},
9899
'native-token-stream': {},
99100
'native-token-periodic': {},
100101
'erc20-token-stream': {},
@@ -171,6 +172,7 @@ describe('GatorPermissionsController', () => {
171172
expect(controller.state.isGatorPermissionsEnabled).toBe(false);
172173
expect(controller.state.gatorPermissionsMapSerialized).toBe(
173174
JSON.stringify({
175+
'erc20-token-revocation': {},
174176
'native-token-stream': {},
175177
'native-token-periodic': {},
176178
'erc20-token-stream': {},
@@ -222,6 +224,7 @@ describe('GatorPermissionsController', () => {
222224
const result = await controller.fetchAndUpdateGatorPermissions();
223225

224226
expect(result).toStrictEqual({
227+
'erc20-token-revocation': expect.any(Object),
225228
'native-token-stream': expect.any(Object),
226229
'native-token-periodic': expect.any(Object),
227230
'erc20-token-stream': expect.any(Object),
@@ -257,6 +260,7 @@ describe('GatorPermissionsController', () => {
257260
sanitizedCheck('native-token-periodic');
258261
sanitizedCheck('erc20-token-stream');
259262
sanitizedCheck('erc20-token-periodic');
263+
sanitizedCheck('erc20-token-revocation');
260264
sanitizedCheck('other');
261265

262266
// Specifically verify that the entry with rules has rules preserved
@@ -278,6 +282,47 @@ describe('GatorPermissionsController', () => {
278282
]);
279283
});
280284

285+
it('categorizes erc20-token-revocation permissions into its own bucket', async () => {
286+
const chainId = '0x1' as Hex;
287+
// Create a minimal revocation permission entry and cast to satisfy types
288+
const revocationEntry = {
289+
permissionResponse: {
290+
chainId,
291+
address: '0x0000000000000000000000000000000000000001',
292+
signer: {
293+
type: 'account',
294+
data: { address: '0x0000000000000000000000000000000000000002' },
295+
},
296+
permission: {
297+
type: 'erc20-token-revocation',
298+
isAdjustmentAllowed: false,
299+
// Data shape is enforced by external types; not relevant for categorization
300+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
301+
data: {} as any,
302+
},
303+
context: '0xdeadbeef',
304+
dependencyInfo: [],
305+
signerMeta: {
306+
delegationManager: '0x0000000000000000000000000000000000000003',
307+
},
308+
},
309+
siteOrigin: 'https://example.org',
310+
} as unknown;
311+
const rootMessenger = getRootMessenger({
312+
snapControllerHandleRequestActionHandler: async () =>
313+
[revocationEntry] as unknown,
314+
});
315+
const controller = new GatorPermissionsController({
316+
messenger: getGatorPermissionsControllerMessenger(rootMessenger),
317+
});
318+
319+
await controller.enableGatorPermissions();
320+
const result = await controller.fetchAndUpdateGatorPermissions();
321+
322+
expect(result['erc20-token-revocation']).toBeDefined();
323+
expect(result['erc20-token-revocation'][chainId]).toHaveLength(1);
324+
});
325+
281326
it('throws error when gator permissions are not enabled', async () => {
282327
const controller = new GatorPermissionsController({
283328
messenger: getGatorPermissionsControllerMessenger(),
@@ -304,6 +349,7 @@ describe('GatorPermissionsController', () => {
304349
const result = await controller.fetchAndUpdateGatorPermissions();
305350

306351
expect(result).toStrictEqual({
352+
'erc20-token-revocation': {},
307353
'native-token-stream': {},
308354
'native-token-periodic': {},
309355
'erc20-token-stream': {},
@@ -326,6 +372,7 @@ describe('GatorPermissionsController', () => {
326372
const result = await controller.fetchAndUpdateGatorPermissions();
327373

328374
expect(result).toStrictEqual({
375+
'erc20-token-revocation': {},
329376
'native-token-stream': {},
330377
'native-token-periodic': {},
331378
'erc20-token-stream': {},
@@ -390,6 +437,7 @@ describe('GatorPermissionsController', () => {
390437
const { gatorPermissionsMap } = controller;
391438

392439
expect(gatorPermissionsMap).toStrictEqual({
440+
'erc20-token-revocation': {},
393441
'native-token-stream': {},
394442
'native-token-periodic': {},
395443
'erc20-token-stream': {},
@@ -501,7 +549,7 @@ describe('GatorPermissionsController', () => {
501549
),
502550
).toMatchInlineSnapshot(`
503551
Object {
504-
"gatorPermissionsMapSerialized": "{\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
552+
"gatorPermissionsMapSerialized": "{\\"erc20-token-revocation\\":{},\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
505553
"gatorPermissionsProviderSnapId": "npm:@metamask/gator-permissions-snap",
506554
"isFetchingGatorPermissions": false,
507555
"isGatorPermissionsEnabled": false,
@@ -523,7 +571,7 @@ describe('GatorPermissionsController', () => {
523571
),
524572
).toMatchInlineSnapshot(`
525573
Object {
526-
"gatorPermissionsMapSerialized": "{\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
574+
"gatorPermissionsMapSerialized": "{\\"erc20-token-revocation\\":{},\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
527575
"isGatorPermissionsEnabled": false,
528576
}
529577
`);
@@ -542,7 +590,7 @@ describe('GatorPermissionsController', () => {
542590
),
543591
).toMatchInlineSnapshot(`
544592
Object {
545-
"gatorPermissionsMapSerialized": "{\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
593+
"gatorPermissionsMapSerialized": "{\\"erc20-token-revocation\\":{},\\"native-token-stream\\":{},\\"native-token-periodic\\":{},\\"erc20-token-stream\\":{},\\"erc20-token-periodic\\":{},\\"other\\":{}}",
546594
"pendingRevocations": Array [],
547595
}
548596
`);

‎packages/gator-permissions-controller/src/GatorPermissionsController.ts‎

Lines changed: 41 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,15 @@ const controllerName = 'GatorPermissionsController';
5858
const defaultGatorPermissionsProviderSnapId =
5959
'npm:@metamask/gator-permissions-snap' as SnapId;
6060

61-
const defaultGatorPermissionsMap: GatorPermissionsMap = {
62-
'native-token-stream': {},
63-
'native-token-periodic': {},
64-
'erc20-token-stream': {},
65-
'erc20-token-periodic': {},
66-
other: {},
61+
const createEmptyGatorPermissionsMap: () => GatorPermissionsMap = () => {
62+
return {
63+
'erc20-token-revocation': {},
64+
'native-token-stream': {},
65+
'native-token-periodic': {},
66+
'erc20-token-stream': {},
67+
'erc20-token-periodic': {},
68+
other: {},
69+
};
6770
};
6871

6972
/**
@@ -157,7 +160,7 @@ export function getDefaultGatorPermissionsControllerState(): GatorPermissionsCon
157160
return {
158161
isGatorPermissionsEnabled: false,
159162
gatorPermissionsMapSerialized: serializeGatorPermissionsMap(
160-
defaultGatorPermissionsMap,
163+
createEmptyGatorPermissionsMap(),
161164
),
162165
isFetchingGatorPermissions: false,
163166
gatorPermissionsProviderSnapId: defaultGatorPermissionsProviderSnapId,
@@ -509,63 +512,39 @@ export default class GatorPermissionsController extends BaseController<
509512
| StoredGatorPermission<Signer, PermissionTypesWithCustom>[]
510513
| null,
511514
): GatorPermissionsMap {
515+
const gatorPermissionsMap = createEmptyGatorPermissionsMap();
516+
512517
if (!storedGatorPermissions) {
513-
return defaultGatorPermissionsMap;
518+
return gatorPermissionsMap;
514519
}
515520

516-
return storedGatorPermissions.reduce<GatorPermissionsMap>(
517-
(gatorPermissionsMap, storedGatorPermission) => {
518-
const { permissionResponse } = storedGatorPermission;
519-
const permissionType = permissionResponse.permission.type;
520-
const { chainId } = permissionResponse;
521-
522-
const sanitizedStoredGatorPermission =
523-
this.#sanitizeStoredGatorPermission(storedGatorPermission);
524-
525-
switch (permissionType) {
526-
case 'native-token-stream':
527-
case 'native-token-periodic':
528-
case 'erc20-token-stream':
529-
case 'erc20-token-periodic':
530-
if (!gatorPermissionsMap[permissionType][chainId]) {
531-
gatorPermissionsMap[permissionType][chainId] = [];
532-
}
533-
534-
(
535-
gatorPermissionsMap[permissionType][
536-
chainId
537-
] as StoredGatorPermissionSanitized<
538-
Signer,
539-
PermissionTypesWithCustom
540-
>[]
541-
).push(sanitizedStoredGatorPermission);
542-
break;
543-
default:
544-
if (!gatorPermissionsMap.other[chainId]) {
545-
gatorPermissionsMap.other[chainId] = [];
546-
}
547-
548-
(
549-
gatorPermissionsMap.other[
550-
chainId
551-
] as StoredGatorPermissionSanitized<
552-
Signer,
553-
PermissionTypesWithCustom
554-
>[]
555-
).push(sanitizedStoredGatorPermission);
556-
break;
557-
}
558-
559-
return gatorPermissionsMap;
560-
},
561-
{
562-
'native-token-stream': {},
563-
'native-token-periodic': {},
564-
'erc20-token-stream': {},
565-
'erc20-token-periodic': {},
566-
other: {},
567-
},
568-
);
521+
for (const storedGatorPermission of storedGatorPermissions) {
522+
const {
523+
permissionResponse: {
524+
permission: { type: permissionType },
525+
chainId,
526+
},
527+
} = storedGatorPermission;
528+
529+
const isPermissionTypeKnown = Object.prototype.hasOwnProperty.call(
530+
gatorPermissionsMap,
531+
permissionType,
532+
);
533+
534+
const permissionTypeKey = isPermissionTypeKnown
535+
? (permissionType as keyof GatorPermissionsMap)
536+
: 'other';
537+
538+
type PermissionsMapElementArray =
539+
GatorPermissionsMap[typeof permissionTypeKey][typeof chainId];
540+
541+
gatorPermissionsMap[permissionTypeKey][chainId] = [
542+
...(gatorPermissionsMap[permissionTypeKey][chainId] || []),
543+
this.#sanitizeStoredGatorPermission(storedGatorPermission),
544+
] as PermissionsMapElementArray;
545+
}
546+
547+
return gatorPermissionsMap;
569548
}
570549

571550
/**
@@ -602,7 +581,7 @@ export default class GatorPermissionsController extends BaseController<
602581
this.update((state) => {
603582
state.isGatorPermissionsEnabled = false;
604583
state.gatorPermissionsMapSerialized = serializeGatorPermissionsMap(
605-
defaultGatorPermissionsMap,
584+
createEmptyGatorPermissionsMap(),
606585
);
607586
});
608587
}

‎packages/gator-permissions-controller/src/types.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
Erc20TokenPeriodicPermission,
99
Rule,
1010
MetaMaskBasePermissionData,
11+
Erc20TokenRevocationPermission,
1112
} from '@metamask/7715-permission-types';
1213
import type { Delegation } from '@metamask/delegation-core';
1314
import type { Hex } from '@metamask/utils';
@@ -174,6 +175,12 @@ export type StoredGatorPermissionSanitized<
174175
* Represents a map of gator permissions by chainId and permission type.
175176
*/
176177
export type GatorPermissionsMap = {
178+
'erc20-token-revocation': {
179+
[chainId: Hex]: StoredGatorPermissionSanitized<
180+
Signer,
181+
Erc20TokenRevocationPermission
182+
>[];
183+
};
177184
'native-token-stream': {
178185
[chainId: Hex]: StoredGatorPermissionSanitized<
179186
Signer,

‎packages/gator-permissions-controller/src/utils.test.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from './utils';
66

77
const defaultGatorPermissionsMap: GatorPermissionsMap = {
8+
'erc20-token-revocation': {},
89
'native-token-stream': {},
910
'native-token-periodic': {},
1011
'erc20-token-stream': {},
@@ -24,18 +25,20 @@ describe('utils - serializeGatorPermissionsMap() tests', () => {
2425
});
2526

2627
it('throws an error when serialization fails', () => {
27-
// Create a valid GatorPermissionsMap structure but with circular reference
2828
const gatorPermissionsMap = {
29+
'erc20-token-revocation': {},
2930
'native-token-stream': {},
3031
'native-token-periodic': {},
3132
'erc20-token-stream': {},
3233
'erc20-token-periodic': {},
3334
other: {},
3435
};
3536

36-
// Add circular reference to cause JSON.stringify to fail
37-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
38-
(gatorPermissionsMap as any).circular = gatorPermissionsMap;
37+
// explicitly cause serialization to fail
38+
(gatorPermissionsMap as unknown as { toJSON: () => void }).toJSON =
39+
(): void => {
40+
throw new Error('Failed serialization');
41+
};
3942

4043
expect(() => {
4144
serializeGatorPermissionsMap(gatorPermissionsMap);

0 commit comments

Comments
 (0)