Skip to content

Add permission types package#48

Merged
MoMannn merged 4 commits into
mainfrom
feat/permission-types
Aug 6, 2025
Merged

Add permission types package#48
MoMannn merged 4 commits into
mainfrom
feat/permission-types

Conversation

@MoMannn

@MoMannn MoMannn commented Aug 4, 2025

Copy link
Copy Markdown
Member

📝 Description

Add @metamask/permission-types package that centralizes types for ethereum/ERCs#1098 in a single place. Now this types do not provide only the basic types described in 7715 but also add types for specific permissions used inside Metamask such as:

  • NativeTokenStreamPermission
  • NativeTokenPeriodicPermission
  • Erc20TokenStreamPermission
  • Erc20TokenPeriodicPermission

🚀 Why?

To have all types in a single place and not duplicated across different packages.

🧪 How to Test?

Describe how to test these changes:

  • Manual testing steps:
  • Automated tests added/updated
  • All existing tests pass

⚠️ Breaking Changes

List any breaking changes:

  • No breaking changes
  • Breaking changes (describe below):

📋 Checklist

Check off completed items:

  • Code follows the project's coding standards
  • Self-review completed
  • Documentation updated (if needed)
  • Tests added/updated
  • Changelog updated (if needed)
  • All CI checks pass

🔗 Related Issues

Link to related issues:
Closes #
Related to #

📚 Additional Notes

Any additional information, concerns, or context:

@MoMannn MoMannn self-assigned this Aug 4, 2025
@MoMannn MoMannn marked this pull request as draft August 4, 2025 13:08
cursor[bot]

This comment was marked as outdated.

@MoMannn MoMannn marked this pull request as ready for review August 5, 2025 13:40
@MoMannn MoMannn changed the title [Draft] Add permission types package Add permission types package Aug 5, 2025
jeffsmale90
jeffsmale90 previously approved these changes Aug 6, 2025

@jeffsmale90 jeffsmale90 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good!

Comment thread packages/permission-types/src/types.ts Outdated
Comment on lines +176 to +183
/**
* A custom permission.
* @property data - is a record of the data that is associated with the permission, and the structure is defined by the ERCs.
*/
export type CustomPermission = BasePermission & {
type: 'custom';
data: MetaMaskBasePermissionData & Record<string, unknown>;
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the problem of "custom" permission types is really a problem of "unknown" permission types.

In the Delegation Toolkit, there's a problem where the type hints become worthless if the accepted type is "native-token-stream" | "native-token-periodic" ... | string as the type system will always assign the broadest type, removing the relevant params types for that permission type. Maybe we can support only the known types for now?

In Extension there's a need for unknown permission types, where we want to present these to the user, and have the option to revoke them, even when we don't know how to render the parameters in a user friendly way. Maybe we can add the types for unknown permissions when we come to implement unknown permission type revocation and have a clearer understanding of what types are strictly needed?

If we do add these now, I think CustomPermission should be UnknownPermission or something and the type string (although that might introduce the narrowing problems described above).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I took this from the extension code where we already introduced the custom type. We have a chance to now define and update this type to something else like UnknownPermission. I think the main question is: Do we want to allow general permissions or just the ones that we are currently officially supporting.

I think at some point we should open up to any permission. But that does not need to be with the first version. So I will comment this out for now with a TODO to come to it later.

@MoMannn MoMannn merged commit 4797b61 into main Aug 6, 2025
10 of 13 checks passed
@MoMannn MoMannn deleted the feat/permission-types branch August 6, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants