feat: Store metadata when revoking a permission

[V00D00-child]

Description

Currently, during permission revocation submission, we store a boolean revoked flag to track the permission revocation status in profile sync. This PR extends the permission revocation submission process to allow storing additional metadata on the revocation transaction.

References

Required by(Core): Attach metadata when submitting a revocation to the permission provider snap

Pre-merge author checklist

• I've followed MetaMask 7715 Permissions Snaps Contributor Docs and MetaMask 7715 Permissions Snaps Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

---

Note

Introduces on-chain validation and metadata storage for revocations.

• New BlockchainClient: Adds checkDelegationDisabledOnChain and checkTransactionReceipt with retry logic and ensureChain usage
• Refactor: Moves delegation-disabled check out of BlockchainTokenMetadataClient into BlockchainClient
• Profile Sync: Extends StoredGrantedPermission with optional revocationMetadata (currently txHash); updates updatePermissionRevocationStatus(permissionContext, isRevoked, revocationMetadata) to persist/clear metadata
• RPC submitRevocation: Validates revocationMetadata, ensures delegation is disabled on-chain, optionally verifies txHash success via receipt, then updates profile sync; handler now depends on blockchainClient
• Validation/Types: Adds zTransactionReceipt and validateTransactionReceipt; updates revocation params schema; exposes TransactionReceipt type
• Utils: Adds getTransactionReceipt with retries in utils/blockchain.ts
• Wiring/Tests: Wires BlockchainClient in index.ts; adds/updates comprehensive tests for client, RPC, and profile sync

^{Written by Cursor Bugbot for commit 5dbb6e6. This will update automatically on new commits. Configure here.}

[jeffsmale90]

A couple of general comments, and, I think a more architectural concern:

I think we should remove isRevoked from the stored permission, and use the existence of revocationMetadata to determine whether the permission is revoked to help improve data integrity.

[jeffsmale90]

As per my previous comment - I think we should remove isRevoked and make the revocationMetadata optional - and remove it's default value.

[jeffsmale90]

One other thought - we should ensure that a permission without revocationMetadata, but with isRevoked: true is parsed appropriately with a revocationMetadata (a legacy revoked permission)

[V00D00-child]

We can make revocationMetadata optional, but per the previous comment that you already acknowledged, we cannot remove isRevoked due to an edge case. Let us not continue revisiting the discussion about removing isRevoked, as it has been resolved.

[V00D00-child]

Updated revocationMetadata to optional on the StoredGrantedPermission type

[jeffsmale90]

nit: can we use _ prefixed named properties here? I know this is an unconfigured instance, but it would be nice to see from this signature what the properties are

[jeffsmale90]

as per other comments - we should drop isRevoked, and just make revocationMetadata optional - actually, is there ever a scenario where we want to unrevoke the stored permission?

Maybe this should just be a function markPermissionRevoked that requires the revocationMetadata

[V00D00-child]

We are making revocationMetadata required on the interface on the corresponding GatorPermissionsController PR, so that it seems odd to make the update function take an optional revocationMetadata.

The DelegationManager contract allows enabling delegation after it is disabled, so the permission can be unrevoked.

[cursor]

Optional txHash allows skipping transaction verification

Medium Severity

The txHash field in zRevocationParams is marked as optional, which means callers can submit revocations with revocationMetadata: {} (empty object). When txHash is missing, the transaction receipt verification in submitRevocation is entirely skipped due to the if (txHash) guard. While the on-chain delegation disabled check still runs, this bypasses verification that the specific revocation transaction was successful. Based on reviewer feedback indicating "metadata should be required," this appears to be an unintended security gap in the revocation verification flow.

Additional Locations (1)

• packages/gator-permissions-snap/src/rpc/rpcHandler.ts#L259-L273

 

[V00D00-child]

This bypassing verification is expected behavior, as some submit revocations do not have an associated txHash when the RPC method is called from the MM client. In this edge case, revocations are submitted as revocationMetadata: {} (empty object).

This is highlighted in previous comments: #228 (comment)