refactor: Use permission decoders from `@metamask/7715-permission-types` by jeffsmale90 · Pull Request #9164 · MetaMask/core

jeffsmale90 · 2026-06-17T04:37:56Z

Explanation

MetaMask/smart-accounts-kit#259 adds permission decoder definitions to @metamask/7715-permission-types released in 0.8.0.

This PR updates @metamask/gator-permissions-controller to consume these permission definitions instead of defining the permission decoders directly in the controller.

Note: erc20-token-revocation permission type is no longer supported.

References

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
I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

High Risk
Breaking removal of erc20-token-revocation and a broad swap of delegation permission decoding logic affect how granted permissions are interpreted; incorrect decoding could misrepresent token limits or revocations.

Overview
Moves Gator permission decoding onto @metamask/7715-permission-types ^1.0.0 by building decoders from makePermissionDecoderConfigs and wrapping them with the local makePermissionDecoder, instead of maintaining per-type decoder modules in this package.

Breaking: erc20-token-revocation is removed from default/README supportedPermissionTypes and all related decoder/tests. Decoding now maps chain deployments through toEnforcerAddressesByName / delegationContractsByChainId before invoking shared decoders.

Large local decoder/rule implementations and their integration-style unit tests are deleted; decodePermission.test.ts and makePermissionDecoder.test.ts focus on orchestration (findDecodersWithMatchingCaveatAddresses, selectUniqueDecoderAndDecodedPermission, factory behavior). Controller tests add end-to-end decode coverage for allowance and token-approval-revocation types and expand pending-revocation transaction listener edge cases.

^{Reviewed by Cursor Bugbot for commit 59a1487. Bugbot is set up for automated code reviews on this repo. Configure here.}

socket-security · 2026-07-14T02:32:15Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@metamask/7715-permission-types@0.7.1 ⏵ 1.0.0			⁺⁸	⁺⁶

View full report

mj-kiwi

Solid refactor, just a few minor notes.

mj-kiwi · 2026-07-15T03:41:21Z

@@ -26,7 +26,6 @@ const gatorPermissionsController = new GatorPermissionsController({
      'native-token-periodic',


Minor doc nit — the example supportedPermissionTypes array dropped erc20-token-revocation but doesn't seem to have added token-approval-revocation in its place, even though that type still looks supported. Might just need one line added back so the example stays accurate.

I don't think this example needs to include all implemented permission types - this is showing how the controller may be instantiated with a list of permission types that should be supported.

This list can be incomplete - it's actually highlighting the capability to restrict supported permissions, but also we probably don't want to update this every time we add permission types to the dependency.

mj-kiwi · 2026-07-15T03:42:07Z

@@ -1,11 +1,9 @@
+import type { DeployedContractsByName } from '@metamask/7715-permission-types';


Noticed DeployedContractsByName moved from the precise local alias (typeof DELEGATOR_CONTRACTS)[number][number] to the package's Record<string, Hex>. Not a big deal, but it does mean a typo'd contract name would only surface at runtime (via the Contract not found throw) rather than at compile time like before. Wanted to flag in case that tradeoff wasn't deliberate.

I've improved @metamask/7715-permission-types in MetaMask/smart-accounts-kit#278 to define EnforcerAddressesByName as the API type for contract addresses (keyed contract names, with camelCase). I've updated this PR to assert the type of the @metamask/delegation-deployments contract addresses (exported as Record<string, Hex>) to DelegationDeploymentsEnforcerAddressesByName (keyed contract names, with PascalCase) and converts them to EnforcerAddressesByName in toEnforcerAddressesByName.

mj-kiwi · 2026-07-15T03:43:26Z

@@ -9,12 +9,8 @@ import type {
  ApprovalRevocationTerms,
  Caveat,


While in here — it looks like Erc20TokenAllowancePermission, NativeTokenAllowancePermission, and TokenApprovalRevocationPermission (lines 23, 40, 53 below) are still declared locally with a "not yet defined in the package" comment, but @metamask/7715-permission-types@^0.8.0 seems to export all three now (worth double-checking, since the local TokenApprovalRevocationPermission shape looks slightly different from the package's version). If the package really does cover these now, importing them directly would avoid future drift.

Good call - I've removed the 3 locally defined permission types, along with ExtendedPermissionTypes

mj-kiwi · 2026-07-15T03:44:30Z

-import { makeErc20TokenPeriodicDecoderConfig } from './erc20TokenPeriodic';
-import { makeErc20TokenRevocationDecoderConfig } from './erc20TokenRevocation';
-import { makeErc20TokenStreamDecoderConfig } from './erc20TokenStream';
+import { makePermissionDecoderConfigs } from '@metamask/7715-permission-types';


Just flagging for awareness — with per-type validation logic now living in @metamask/7715-permission-types, this package's tests only exercise a synthetic specified-permission-type rather than the real permission types end-to-end. Might be worth a lightweight integration test here (or confirming coverage exists upstream) so a future bump of the dependency that loosens a validation rule wouldn't slip by unnoticed.

mj-kiwi · 2026-07-15T03:45:22Z

@@ -1,45 +1,22 @@
-import type { Rule } from '@metamask/7715-permission-types';
+import type {


Small one — enforcer addresses look like they get checksummed once for matching, then re-checksummed again per candidate decoder inside validateAndDecodePermission on the ambiguous-type path. Probably not costly today, but could be worth reusing the already-checksummed set if this path gets exercised more.

This is still the case.

My concern here is that it's difficult to ensure that addresses are checksummed (at the API interface) without checksumming them 😓 - we can't type them in such a way to enforce this (unless we switch to byteslike types where case mismatching isn't possible 🤔). So it's a balance between tight coupling, and more checksumming.

Because this only ever happens once per permission, and the number of candidates that pass the caveatAddressesMatch check is normally going to be only 1, I think for now it's a low cost.

I'm going to add a comment explaining it though - let me know if you think that we should actually address this now though.

mj-kiwi · 2026-07-15T03:45:43Z

@@ -9,12 +9,8 @@ import type {
  ApprovalRevocationTerms,


RuleDecoder looks like it's only referenced from a test file now, not from any production decoder — the real shape seems to live in the package's PermissionDecoderConfig. If that's right, might be simpler to move it into the test file (or drop it) so it doesn't read as something production code still relies on.

Moved it into the test file. It does give us a little utility within the tests.

… narrow scope to just test this function

…ensitive

Also fix various linting issues

…eady been checksummed

…ssions

mj-kiwi

LGTM

mj-kiwi · 2026-07-27T04:25:02Z

      throw new Error(`Contracts not found for chainId: ${chainId}`);
    }

+    const contracts = toEnforcerAddressesByName(deploymentContracts);


toEnforcerAddressesByName(deploymentContracts) is now called before the try block in decodePermissionFromPermissionContextForOrigin. This means a "Contract not found" error would escape unwrapped instead of being
caught and rethrown as PermissionDecodingError, which the method's @throws doc promises. Could we move this call back inside the try block, similar to how resolveGrantedPermissionOnChainStatus in permissionOnChainStatus.ts handles it?

jeffsmale90 changed the title ~~Chore/permission decoding externalised permissions~~ refactor: @metamask/gator-permissions-controller consumes Advanced Permission definitions from @metamask/7715-permission-types Jun 17, 2026

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch from 7a4724f to 7eda56a Compare July 14, 2026 02:29

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch from cb23bf0 to c3fbd8e Compare July 14, 2026 02:54

jeffsmale90 marked this pull request as ready for review July 14, 2026 02:54

jeffsmale90 requested review from a team as code owners July 14, 2026 02:54

jeffsmale90 temporarily deployed to default-branch July 14, 2026 02:54 — with GitHub Actions Inactive

jeffsmale90 changed the title ~~refactor: @metamask/gator-permissions-controller consumes Advanced Permission definitions from @metamask/7715-permission-types~~ refactor: Use permission decoders from @metamask/7715-permission-types Jul 14, 2026

jeffsmale90 marked this pull request as draft July 14, 2026 21:25

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch from 2719213 to c436c43 Compare July 15, 2026 01:57

jeffsmale90 marked this pull request as ready for review July 15, 2026 01:57

jeffsmale90 temporarily deployed to default-branch July 15, 2026 01:57 — with GitHub Actions Inactive

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch from c436c43 to 174d64e Compare July 15, 2026 02:02

mj-kiwi reviewed Jul 15, 2026

View reviewed changes

jeffsmale90 added 6 commits July 21, 2026 13:32

Remove permission definitions, rewrite makePermissionDecoder tests to…

bb882e4

… narrow scope to just test this function

Enforcer addresses passed to makePermissionDecoder should be case ins…

164df42

…ensitive

Make decodePermission tests more focussed, and unit test like

aa79559

Add integration tests for all supported permission types

6fc2c63

Also fix various linting issues

Remove types duplicated in @metamask/7715-permission-types

9e4f620

Upgrade @metamask/7715-permission-types to ^0.8.0

7055b57

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch 3 times, most recently from a4ef722 to 7bc7040 Compare July 21, 2026 02:26

jeffsmale90 added 2 commits July 21, 2026 14:46

Upgrade @metamask/7715-permission-types to ^1.0.0

63657c3

Add changelog entry

7cbbc65

jeffsmale90 force-pushed the chore/permission_decoding_externalised_permissions branch from 7bc7040 to 7cbbc65 Compare July 21, 2026 02:46

jeffsmale90 added 2 commits July 24, 2026 10:56

Remove unnecessary toLowerCase when comparing addresses that have alr…

e52cefa

…eady been checksummed

Merge branch 'main' into chore/permission_decoding_externalised_permi…

c14aa60

…ssions

jeffsmale90 added 4 commits July 27, 2026 12:27

Add .js extension to imports

e45b777

Add additional handler test coverage

16b0a35

Lining whitespace

89667d3

Add comment explaining checksumming

59a1487

mj-kiwi approved these changes Jul 27, 2026

View reviewed changes

		@@ -26,7 +26,6 @@ const gatorPermissionsController = new GatorPermissionsController({
		'native-token-periodic',

		@@ -1,11 +1,9 @@
		import type { DeployedContractsByName } from '@metamask/7715-permission-types';

		@@ -9,12 +9,8 @@ import type {
		ApprovalRevocationTerms,
		Caveat,

		@@ -1,45 +1,22 @@
		import type { Rule } from '@metamask/7715-permission-types';
		import type {

Uh oh!

Uh oh!

Conversation

jeffsmale90 commented Jun 17, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

Uh oh!

socket-security Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mj-kiwi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mj-kiwi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeffsmale90 commented Jun 17, 2026 •

edited by cursor Bot

Loading

socket-security Bot commented Jul 14, 2026 •

edited

Loading