fix: clear pending confirmations when handling revokePerms

MetaMask · Apr 15, 2024 · 59774e4 · 59774e4
1 parent f55298f
commit 59774e4
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 2 deletions.
diff --git a/packages/queued-request-controller/src/QueuedRequestController.test.ts b/packages/queued-request-controller/src/QueuedRequestController.test.ts
@@ -27,6 +27,7 @@ describe('QueuedRequestController', () => {
     const options: QueuedRequestControllerOptions = {
       messenger: buildQueuedRequestControllerMessenger(),
       methodsRequiringNetworkSwitch: [],
+      clearPendingConfirmations: jest.fn(),
     };
 
     const controller = new QueuedRequestController(options);
@@ -81,6 +82,7 @@ describe('QueuedRequestController', () => {
       const controller = buildQueuedRequestController({
         messenger: buildQueuedRequestControllerMessenger(messenger),
         methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+        clearPendingConfirmations: jest.fn(),
       });
 
       await controller.enqueueRequest(
@@ -809,6 +811,7 @@ describe('QueuedRequestController', () => {
       const options: QueuedRequestControllerOptions = {
         messenger: buildQueuedRequestControllerMessenger(messenger),
         methodsRequiringNetworkSwitch: ['eth_sendTransaction'],
+        clearPendingConfirmations: jest.fn(),
       };
 
       const controller = new QueuedRequestController(options);
@@ -882,6 +885,43 @@ describe('QueuedRequestController', () => {
       expect(request2).toHaveBeenCalled();
       expect(request3).not.toHaveBeenCalled();
     });
+
+    it('calls clearPendingConfirmations when the SelectedNetworkController "domains" state for that origin has been removed', async () => {
+      const { messenger } = buildControllerMessenger();
+
+      const options: QueuedRequestControllerOptions = {
+        messenger: buildQueuedRequestControllerMessenger(messenger),
+        methodsRequiringNetworkSwitch: ['eth_sendTransaction'],
+        clearPendingConfirmations: jest.fn(),
+      };
+
+      const controller = new QueuedRequestController(options);
+
+      const request1 = jest.fn(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+
+        messenger.publish(
+          'SelectedNetworkController:stateChange',
+          { domains: {} },
+          [
+            {
+              op: 'remove',
+              path: ['domains', 'https://abc.123'],
+            },
+          ],
+        );
+      });
+
+      await controller.enqueueRequest(
+        {
+          ...buildRequest(),
+          method: 'wallet_revokePermissions',
+          origin: 'https://abc.123',
+        },
+        request1,
+      );
+      expect(options.clearPendingConfirmations).toHaveBeenCalledTimes(1);
+    });
   });
 });
 
@@ -988,6 +1028,7 @@ function buildQueuedRequestController(
   const options: QueuedRequestControllerOptions = {
     messenger: buildQueuedRequestControllerMessenger(),
     methodsRequiringNetworkSwitch: [],
+    clearPendingConfirmations: jest.fn(),
     ...overrideOptions,
   };
 

diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts
@@ -80,6 +80,7 @@ export type QueuedRequestControllerMessenger = RestrictedControllerMessenger<
 export type QueuedRequestControllerOptions = {
   messenger: QueuedRequestControllerMessenger;
   methodsRequiringNetworkSwitch: string[];
+  clearPendingConfirmations: () => void;
 };
 
 /**
@@ -144,16 +145,20 @@ export class QueuedRequestController extends BaseController<
    */
   readonly #methodsRequiringNetworkSwitch: string[];
 
+  #clearPendingConfirmations: () => void;
+
   /**
    * Construct a QueuedRequestController.
    *
    * @param options - Controller options.
    * @param options.messenger - The restricted controller messenger that facilitates communication with other controllers.
    * @param options.methodsRequiringNetworkSwitch - A list of methods that require the globally selected network to match the dapp selected network.
+   * @param options.clearPendingConfirmations - A function that will clear all the pending confirmations.
    */
   constructor({
     messenger,
     methodsRequiringNetworkSwitch,
+    clearPendingConfirmations,
   }: QueuedRequestControllerOptions) {
     super({
       name: controllerName,
@@ -167,6 +172,7 @@ export class QueuedRequestController extends BaseController<
       state: { queuedRequestCount: 0 },
     });
     this.#methodsRequiringNetworkSwitch = methodsRequiringNetworkSwitch;
+    this.#clearPendingConfirmations = clearPendingConfirmations;
     this.#registerMessageHandlers();
   }
 
@@ -181,12 +187,17 @@ export class QueuedRequestController extends BaseController<
       (_, patch) => {
         patch.forEach(({ op, path }) => {
           if (
-            (op === 'replace' || op === 'add' || op === 'remove') &&
             path.length === 2 &&
             path[0] === 'domains' &&
             typeof path[1] === 'string'
           ) {
-            this.#flushQueueForOrigin(path[1]);
+            const origin = path[1];
+            this.#flushQueueForOrigin(origin);
+            // When a domain is removed from SelectedNetworkController, its because of revoke permissions.
+            // Rather than subscribe to the permissions controller event in addition to the selectedNetworkController ones, we simplify it and just handle remove on this event alone.
+            if (op === 'remove' && origin === this.#originOfCurrentBatch) {
+              this.#clearPendingConfirmations();
+            }
           }
         });
       },