Skip to content

Conversation

@MoMannn
Copy link
Contributor

@MoMannn MoMannn commented Nov 25, 2025

Explanation

What is the current state of things and why does it need to change?

Currently, when a revocation transaction changes state (confirmed, failed, or dropped), the GatorPermissionsController only submits the revocation (on confirmation) or cleans up listeners. However, it doesn't refresh the local permissions map to reflect the updated state from the permissions provider.

This creates a potential inconsistency where:

  • On confirmation: After successfully submitting a revocation, the local permissions map may still contain stale data until the next manual refresh
  • On failure/drop: When a transaction fails or is dropped, the permission that was pending revocation remains active, but our local state doesn't reflect this

This means the UI could display outdated permission states, leading to confusion or incorrect actions by users.

What is the solution your changes offer and how does it work?

This PR introduces automatic permission refresh after revocation transaction state changes:

  1. Added a refreshPermissions helper function that calls fetchAndUpdateGatorPermissions({ isRevoked: false }) with fire-and-forget semantics, catching and logging any errors to prevent them from blocking cleanup operations.

  2. Integrated refresh into transaction handlers:

    • Confirmed transactions: After submitting the revocation (successfully or not), refresh permissions in a .finally() block to ensure the map reflects the new revoked state
    • Failed transactions: Refresh permissions after cleanup to restore any permissions that were pending revocation but never executed
    • Dropped transactions: Same as failed - refresh to ensure the permissions map is accurate
  3. Error handling: The refresh operation is intentionally fire-and-forget with error logging, ensuring that refresh failures don't interfere with the cleanup flow or cause user-visible errors.

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

Automatically refreshes the gator permissions map after revocation transactions are confirmed, failed, or dropped, with accompanying tests and changelog updates.

  • Controller (src/GatorPermissionsController.ts):
    • Add refreshPermissions helper to call fetchAndUpdateGatorPermissions({ isRevoked: false }) after revocation state changes.
    • On transactionConfirmed: call submitRevocation then refresh in .finally(...).
    • On transactionFailed and transactionDropped: cleanup and refresh permissions.
    • Add error-logged, fire-and-forget refresh flow.
  • Tests (src/GatorPermissionsController.test.ts):
    • Import flushPromises and use it to await async flows.
    • Update tests to assert refresh via permissionsProvider_getGrantedPermissions with { isRevoked: false } after confirmed/failed/dropped states.
    • Add tests for refresh error handling on failed/dropped paths; adjust test names accordingly.
  • Changelog:
    • Note addition: refresh permissions map after revocation state change.

Written by Cursor Bugbot for commit 27160c3. This will update automatically on new commits. Configure here.

@MoMannn MoMannn requested a review from a team as a code owner November 25, 2025 13:34
@MoMannn MoMannn requested a review from a team as a code owner November 25, 2025 13:43
mj-kiwi
mj-kiwi previously approved these changes Nov 26, 2025
Copy link

@mj-kiwi mj-kiwi left a comment

Choose a reason for hiding this comment

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

LGTM

mj-kiwi
mj-kiwi previously approved these changes Nov 26, 2025
jeffsmale90
jeffsmale90 previously approved these changes Nov 27, 2025
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

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

LGTM! One very inconsequential nit

…roller.ts

Co-authored-by: jeffsmale90 <6363749+jeffsmale90@users.noreply.github.com>
@MoMannn MoMannn dismissed stale reviews from jeffsmale90 and mj-kiwi via e256073 November 27, 2025 09:55
jeffsmale90
jeffsmale90 previously approved these changes Nov 27, 2025
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

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

Noice

mj-kiwi
mj-kiwi previously approved these changes Nov 28, 2025
…efresh-gator-permissions-map-after-revocation-state-change
@MoMannn MoMannn dismissed stale reviews from mj-kiwi and jeffsmale90 via 27160c3 November 28, 2025 09:31
@jeffsmale90 jeffsmale90 added the team-delegation MetaMask Delegation Team label Nov 30, 2025
@MoMannn MoMannn added this pull request to the merge queue Dec 1, 2025
Merged via the queue into main with commit 5ed39b4 Dec 1, 2025
282 checks passed
@MoMannn MoMannn deleted the chore--refresh-gator-permissions-map-after-revocation-state-change branch December 1, 2025 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-delegation MetaMask Delegation Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants