From 48a661ea9b0b453f0efe40632fd02f594edf9854 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 11 Apr 2024 08:48:49 -0700 Subject: [PATCH] Zb/queue flush event (#4139) ## Explanation This PR is a follow up to [this](https://github.com/MetaMask/core/pull/4064/files) - where instead of calling the flush method directly after successfully handling a request, we instead subscribe to SelectedNetworkController state changes and flush the queue for the domain in question when the network changes. One potential issue that may arise is relating to a possible race condition. The question is when will the subscriptions fire - will they happen before processing the next request starts? It's hard to say for certain. ## References ## Changelog ### `@metamask/request-queue-controller` - **ADDED**: A method for flushing or clearing the queue of items for a particular domain - **FIXED**: calling `wallet_switchEthereumChain` will clear all queued transactions from the domain, across all queued 'batches' ## 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 highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/QueuedRequestController.test.ts | 87 ++++++++++++++++++- .../src/QueuedRequestController.ts | 43 ++++++++- 2 files changed, 124 insertions(+), 6 deletions(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.test.ts b/packages/queued-request-controller/src/QueuedRequestController.test.ts index 80bcc9f8cf7..1bfede9c292 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.test.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.test.ts @@ -10,6 +10,7 @@ import { cloneDeep } from 'lodash'; import type { AllowedActions, + AllowedEvents, QueuedRequestControllerActions, QueuedRequestControllerEvents, QueuedRequestControllerMessenger, @@ -801,6 +802,86 @@ describe('QueuedRequestController', () => { expect(request3).toHaveBeenCalled(); }); }); + + it('rejects requests for an origin when the SelectedNetworkController "domains" state for that origin has changed, but preserves requests for other origins', async () => { + const { messenger } = buildControllerMessenger(); + + const options: QueuedRequestControllerOptions = { + messenger: buildQueuedRequestControllerMessenger(messenger), + methodsRequiringNetworkSwitch: ['eth_sendTransaction'], + }; + + const controller = new QueuedRequestController(options); + + const request1 = jest.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + + messenger.publish( + 'SelectedNetworkController:stateChange', + { domains: {} }, + [ + { + op: 'replace', + path: ['domains', 'https://abc.123'], + }, + { + op: 'add', + path: ['domains', 'https://abc.123'], + }, + ], + ); + }); + + const request2 = jest.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + const request3 = jest.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // Enqueue the requests + const promise1 = controller.enqueueRequest( + { + ...buildRequest(), + method: 'wallet_switchEthereumChain', + origin: 'https://abc.123', + }, + request1, + ); + const promise2 = controller.enqueueRequest( + { + ...buildRequest(), + method: 'eth_sendTransaction', + origin: 'https://foo.bar', + }, + request2, + ); + const promise3 = controller.enqueueRequest( + { + ...buildRequest(), + method: 'eth_sendTransaction', + origin: 'https://abc.123', + }, + request3, + ); + + expect( + await Promise.allSettled([promise1, promise2, promise3]), + ).toStrictEqual([ + { status: 'fulfilled', value: undefined }, + { status: 'fulfilled', value: undefined }, + { + status: 'rejected', + reason: new Error( + 'The request has been rejected due to a change in selected network. Please verify the selected network and retry the request.', + ), + }, + ]); + expect(request1).toHaveBeenCalled(); + expect(request2).toHaveBeenCalled(); + expect(request3).not.toHaveBeenCalled(); + }); }); }); @@ -828,7 +909,7 @@ function buildControllerMessenger({ } = {}): { messenger: ControllerMessenger< QueuedRequestControllerActions | AllowedActions, - QueuedRequestControllerEvents + QueuedRequestControllerEvents | AllowedEvents >; mockNetworkControllerGetState: jest.Mocked< NetworkControllerGetStateAction['handler'] @@ -842,7 +923,7 @@ function buildControllerMessenger({ } { const messenger = new ControllerMessenger< QueuedRequestControllerActions | AllowedActions, - QueuedRequestControllerEvents + QueuedRequestControllerEvents | AllowedEvents >(); const mockNetworkControllerGetState = @@ -891,7 +972,7 @@ function buildQueuedRequestControllerMessenger( 'NetworkController:setActiveNetwork', 'SelectedNetworkController:getNetworkClientIdForDomain', ], - allowedEvents: [], + allowedEvents: ['SelectedNetworkController:stateChange'], }); } diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index a371c13ac3c..ce45f94c7c9 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -8,7 +8,11 @@ import type { NetworkControllerGetStateAction, NetworkControllerSetActiveNetworkAction, } from '@metamask/network-controller'; -import type { SelectedNetworkControllerGetNetworkClientIdForDomainAction } from '@metamask/selected-network-controller'; +import type { + SelectedNetworkControllerGetNetworkClientIdForDomainAction, + SelectedNetworkControllerStateChangeEvent, +} from '@metamask/selected-network-controller'; +import { SelectedNetworkControllerEventTypes } from '@metamask/selected-network-controller'; import { createDeferredPromise } from '@metamask/utils'; import type { QueuedRequestMiddlewareJsonRpcRequest } from './types'; @@ -63,12 +67,14 @@ export type AllowedActions = | NetworkControllerSetActiveNetworkAction | SelectedNetworkControllerGetNetworkClientIdForDomainAction; +export type AllowedEvents = SelectedNetworkControllerStateChangeEvent; + export type QueuedRequestControllerMessenger = RestrictedControllerMessenger< typeof controllerName, QueuedRequestControllerActions | AllowedActions, - QueuedRequestControllerEvents, + QueuedRequestControllerEvents | AllowedEvents, AllowedActions['type'], - never + AllowedEvents['type'] >; export type QueuedRequestControllerOptions = { @@ -169,6 +175,37 @@ export class QueuedRequestController extends BaseController< `${controllerName}:enqueueRequest`, this.enqueueRequest.bind(this), ); + + this.messagingSystem.subscribe( + SelectedNetworkControllerEventTypes.stateChange, + (_, patch) => { + patch.forEach(({ op, path }) => { + if ( + (op === 'replace' || op === 'add') && + path.length === 2 && + path[0] === 'domains' && + typeof path[1] === 'string' + ) { + this.#flushQueueForOrigin(path[1]); + } + }); + }, + ); + } + + #flushQueueForOrigin(flushOrigin: string) { + this.#requestQueue + .filter(({ origin }) => origin === flushOrigin) + .forEach(({ processRequest }) => { + processRequest( + new Error( + 'The request has been rejected due to a change in selected network. Please verify the selected network and retry the request.', + ), + ); + }); + this.#requestQueue = this.#requestQueue.filter( + ({ origin }) => origin !== flushOrigin, + ); } /**