Skip to content

Conversation

@hanzel98
Copy link

@hanzel98 hanzel98 commented Oct 26, 2025

Overview

This PR adds a complete revocation flow for gator permissions, including delegation status checking and transaction handling.

Key Changes

1. Enhanced Gator Permissions Controller

  • Added delegation status checking: New isDelegationDisabled function to check if a delegation is already disabled on-chain
  • Improved error handling: Better error messages and logging for debugging
  • Enhanced API: Updated fetchAndUpdateGatorPermissions to accept optional parameters for filtering

2. Background API Integration

  • Added call method: New API method in metamask-controller.js to handle eth_call requests
  • Network client integration: Proper routing of RPC calls through the network controller
  • Messenger configuration: Added TransactionController:transactionConfirmed event to allowed events

3. Delegation Utilities

  • New delegation functions: Added encodeDisabledDelegationsCheck and decodeDisabledDelegationsResult for on-chain delegation status checking
  • Enhanced delegation handling: Better support for checking delegation states

4. Revocation Flow Improvements

  • Smart revocation logic: Automatically detects if delegation is already disabled and handles accordingly
  • Transaction management: Proper handling of disable transactions and pending revocations
  • Better user experience: More informative logging and error handling

Technical Details

Files Modified:

  • app/scripts/metamask-controller.js: Added call method for RPC requests
  • app/scripts/controller-init/messengers/gator-permissions/gator-permissions-controller-messenger.ts: Added transaction confirmed event
  • ui/store/controller-actions/gator-permissions-controller.ts: Enhanced with delegation checking and improved API
  • ui/hooks/gator-permissions/useRevokeGatorPermissions.ts: Added delegation status checking
  • shared/lib/delegation/delegation.ts: New delegation utility functions

Key Features:

  1. On-chain delegation checking: Verifies if delegation is already disabled before creating transactions
  2. Improved error handling: Better debugging and user feedback
  3. Enhanced API flexibility: Support for parameterized permission fetching
  4. Transaction flow optimization: Smarter handling of revocation scenarios

Testing

  • All existing functionality preserved
  • New delegation checking logic tested
  • Error handling improved with better logging

Note

Implements gator permission revocation with on-chain disabled checks, wires background eth_call, and updates messaging/events and UI hooks to manage pending/submitted revocations.

  • Gator Permissions (background/UI wiring):
    • Expose addPendingRevocation and submitRevocation via controller init and UI controller actions (ui/store/controller-actions/gator-permissions-controller.ts).
    • Update fetchAndUpdateGatorPermissions to accept optional params; UI hook passes { isRevoked: false }.
    • Add isDelegationDisabled UI action: performs background eth_call to disabledDelegations(bytes32) and decodes result.
  • Revocation Hook (ui/hooks/gator-permissions/useRevokeGatorPermissions.ts):
    • Compute delegation hash (getDelegationHashOffchain) and skip tx if already disabled; call submitRevocation and return a mock confirmed TransactionMeta.
    • Otherwise create disable tx and mark with addPendingRevocation.
  • Delegation utils (shared/lib/delegation/delegation.ts):
    • Add encodeDisabledDelegationsCheck and decodeDisabledDelegationsResult helpers.
  • Background API (app/scripts/metamask-controller.js):
    • Add call(method, params, networkClientId) export; special-case eth_call via the correct network client, otherwise delegate to controllerMessenger.
  • Messenger (app/scripts/controller-init/messengers/gator-permissions/...):
    • Allow TransactionController events: transactionConfirmed, transactionFailed, transactionDropped.
  • Tests: Update useRevokeGatorPermissions tests to cover disabled checks, pending/submitted revocations, and navigation behavior.

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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-delegation MetaMask Delegation Team label Oct 26, 2025
cursor[bot]

This comment was marked as outdated.

@hanzel98
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@hanzel98 hanzel98 changed the title feat: Add gator permissions revocation flow Add Gator Permissions Revocation Flow Oct 26, 2025
@hanzel98
Copy link
Author

Right now the actions is failing because the gator-permissions-controller needs to be updated
image

There are 2 dependencies

return {
// etc
getState: this.getState.bind(this),
call: async (method, params, networkClientId) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure this is not ok. This changes the flow for: eth_call for all calls. Better to change isDelegationDisabled to include direct call then to change this.

const callData = encodeDisabledDelegationsCheck({ delegationHash });

// Make eth_call request through the network controller
const result = await submitRequestToBackground<Hex>('call', [
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather make this here:

 const networkClient = this.networkController.getNetworkClientById(networkClientId);
  const result = await networkClient.provider.request({
    method: 'eth_call',
    params: [{
      to: delegationManagerAddress,
      data: callData,
    }, 'latest'],
  });

Something like that. Instead of overriding how eth_call is handled.

if (isDisabled) {
await submitRevocation(permissionContext);
// Return a mock transaction meta since no actual transaction is needed
const mockTransactionMeta = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather return null, or a different object (like a new object type) instead of a mock object.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you return null you can then probably remove chainId from dependency array

);
};
export const fetchAndUpdateGatorPermissions = async (
params?: Json,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe define params type? Probably define in core and import here so its not just Json?

api: {
fetchAndUpdateGatorPermissions:
controller.fetchAndUpdateGatorPermissions.bind(controller),
addPendingRevocation: controller.addPendingRevocation.bind(controller),
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably change the PR to draft until core is released and you can update package.json with higher version of core so that this functions are available.

const delegation =
extractDelegationFromGatorPermissionContext(permissionContext);

const delegationHash = getDelegationHashOffchain(delegation);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit (and not connected to this PR) but maybe a better name would be generateDelegationHash ?

Base automatically changed from feat/add-review-gator-permissions-page to main October 28, 2025 17:49
@hanzel98 hanzel98 requested a review from a team as a code owner October 28, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size-M team-delegation MetaMask Delegation Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants