-
-
Notifications
You must be signed in to change notification settings - Fork 252
New Action on the GatorPermissionsController to Allow the Consumer to SubmitRevocation #6713
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
Conversation
c86dca7 to
1833a20
Compare
595f36d to
0a44e3c
Compare
da91f33 to
61b8900
Compare
MoMannn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add changes to CHANGELOG.md under ## [Unreleased].
packages/gator-permissions-controller/src/GatorPermissionsController.ts
Outdated
Show resolved
Hide resolved
…he consumer to submitRevocation
- Add RevocationParams type with delegationHash field - Add PermissionProviderSubmitRevocation RPC method enum - Implement submitRevocation method with proper error handling - Add comprehensive test suite with 100% coverage - Export new types in public API This enables MetaMask clients to submit permission revocations through the gator permissions provider snap using the permissionsProvider_submitRevocation RPC method.
68fe787 to
6999403
Compare
4985f70 to
7b7535f
Compare
jeffsmale90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
packages/gator-permissions-controller/src/GatorPermissionsController.ts
Outdated
Show resolved
Hide resolved
| params, | ||
| }: { | ||
| snapId: SnapId; | ||
| params?: Json; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a more specific type for the params? I guess it's params for a specific method, so we should always know the type, and leaving it as Json is opening us up for bugs.
| */ | ||
| public async fetchAndUpdateGatorPermissions(): Promise<GatorPermissionsMap> { | ||
| public async fetchAndUpdateGatorPermissions( | ||
| params?: Json, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question regarding typing of params
packages/gator-permissions-controller/src/GatorPermissionsController.ts
Outdated
Show resolved
Hide resolved
jeffsmale90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
## Explanation Expose the list of pending revocations in the state to allow MM clients to create custom selectors that filter on `pendingRevocations` to determine whether the revoke CTA should show the "pending" status. - A revocation is added to `pendingRevocations` state on calls to `addPendingRevocation()` - A revocation is removed from `pendingRevocations` state given a `transactionId` when the handler's `cleanup()` function is executed. ## References Forked from #6713 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] 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), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds `pendingRevocations` to controller state with lifecycle management via `addPendingRevocation` and `submitRevocation`, updating metadata, tests, and changelog. > > - **GatorPermissionsController**: > - **State**: Add `pendingRevocations: { txId: string; permissionContext: Hex }[]` with default `[]` and UI exposure in metadata. > - **API**: > - New getter `pendingRevocations`. > - `addPendingRevocation` now appends to state immediately and cleans up on confirm/fail/drop/timeout by `txId`. > - `submitRevocation` submits to Snap and removes entry by `permissionContext` on success. > - **Internals**: Add private helpers to add/remove pending revocations; import `Hex` type. > - **Tests**: Update snapshots, add coverage for pending revocations lifecycle and getter. > - **Docs**: Update `CHANGELOG.md` to note exposed pending revocations and related actions. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 77e676e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Explanation
What is the current state of things and why does it need to change?
The GatorPermissionsController currently provides functionality to fetch, enable, disable, and decode gator permissions, but it lacks the ability to revoke permissions. This missing capability prevents MetaMask clients from providing users with a complete permission management experience where they can not only grant permissions but also revoke them when needed.
What is the solution your changes offer and how does it work?
This PR adds a new
submitRevocationaction to the GatorPermissionsController that allows consumers (MetaMask clients) to submit permission revocations through the gator permissions provider snap. The implementation:RevocationParamstype that accepts adelegationHash(hex string) to identify the permission to revokesubmitRevocationmethod that forwards the revocation request to the gator permissions provider snap using thepermissionsProvider_submitRevocationRPC methodGatorPermissionsNotEnabledErrorandGatorPermissionsProviderError)Are there any changes whose purpose might not obvious to those unfamiliar with the domain?
The
delegationHashparameter is a unique identifier for ERC-7715 delegations/permissions that allows the snap to locate and revoke the specific permission. This hash-based approach is standard in the ERC-7715 delegation framework for permission management.References
[Related to ]
This PR depends on this other PR of the permissions-controller #6713
Checklist
Note: The changelog item should be checked off after updating the CHANGELOG.md file, and the last item can be checked as N/A since this is a new feature addition with no breaking changes.
Note
Adds
submitRevocationandaddPendingRevocationactions, optional fetch filters, and integrates TransactionController events for revocation flow.submitRevocation(submits revocations to snap) andaddPendingRevocation(queues until tx confirmation; cleans up on fail/drop/timeout).TransactionControllerevents:transactionConfirmed,transactionFailed,transactionDropped.fetchAndUpdateGatorPermissionsto accept optional filterparamsforwarded to snap.permissionsProvider_submitRevocation.RevocationParams,PendingRevocationParams; extendStoredGatorPermission*with optionalisRevoked.src/index.ts.submitRevocation, andaddPendingRevocation(including event-driven and error paths).@metamask/transaction-controller; update TS project references to include transaction-controller.Written by Cursor Bugbot for commit 2774c30. This will update automatically on new commits. Configure here.