From 567ce0d4f9344e60266edb130db8a62d6f03233f Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 24 Jun 2024 12:52:20 -0700 Subject: [PATCH 1/4] WIP --- .../src/QueuedRequestController.ts | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index 291210a812..d11cf59f04 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -83,6 +83,7 @@ export type QueuedRequestControllerOptions = { request: QueuedRequestMiddlewareJsonRpcRequest, ) => boolean; clearPendingConfirmations: () => void; + showApprovalRequest: () => Promise; }; /** @@ -151,6 +152,8 @@ export class QueuedRequestController extends BaseController< #clearPendingConfirmations: () => void; + #showApprovalRequest: () => Promise; + /** * Construct a QueuedRequestController. * @@ -163,6 +166,7 @@ export class QueuedRequestController extends BaseController< messenger, shouldRequestSwitchNetwork, clearPendingConfirmations, + showApprovalRequest, }: QueuedRequestControllerOptions) { super({ name: controllerName, @@ -175,8 +179,10 @@ export class QueuedRequestController extends BaseController< messenger, state: { queuedRequestCount: 0 }, }); + this.#shouldRequestSwitchNetwork = shouldRequestSwitchNetwork; this.#clearPendingConfirmations = clearPendingConfirmations; + this.#showApprovalRequest = showApprovalRequest; this.#registerMessageHandlers(); } @@ -301,6 +307,32 @@ export class QueuedRequestController extends BaseController< }); } + async #waitForDequeue( + origin: string + ): Promise { + this.#showApprovalRequest() + const { + promise, + reject, + resolve, + } = createDeferredPromise({ + suppressUnhandledRejection: true, + }); + this.#requestQueue.push({ + origin: origin, + processRequest: (error: unknown) => { + if (error) { + reject(error); + } else { + resolve(); + } + }, + }); + this.#updateQueuedRequestCount(); + + return promise; + } + /** * Enqueue a request to be processed in a batch with other requests from the same origin. * @@ -330,26 +362,7 @@ export class QueuedRequestController extends BaseController< this.state.queuedRequestCount > 0 || this.#originOfCurrentBatch !== request.origin ) { - const { - promise: waitForDequeue, - reject, - resolve, - } = createDeferredPromise({ - suppressUnhandledRejection: true, - }); - this.#requestQueue.push({ - origin: request.origin, - processRequest: (error: unknown) => { - if (error) { - reject(error); - } else { - resolve(); - } - }, - }); - this.#updateQueuedRequestCount(); - - await waitForDequeue; + await this.#waitForDequeue(request.origin) } else if (this.#shouldRequestSwitchNetwork(request)) { // Process request immediately // Requires switching network now if necessary From 34ee5f59dbd426fe752d696b753d5df2fe1ff3cc Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 24 Jun 2024 13:10:21 -0700 Subject: [PATCH 2/4] move show into enqueueRequest --- .../queued-request-controller/src/QueuedRequestController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index d11cf59f04..799ea97e57 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -310,7 +310,6 @@ export class QueuedRequestController extends BaseController< async #waitForDequeue( origin: string ): Promise { - this.#showApprovalRequest() const { promise, reject, @@ -362,6 +361,7 @@ export class QueuedRequestController extends BaseController< this.state.queuedRequestCount > 0 || this.#originOfCurrentBatch !== request.origin ) { + this.#showApprovalRequest() await this.#waitForDequeue(request.origin) } else if (this.#shouldRequestSwitchNetwork(request)) { // Process request immediately From 470319c92c2085aad89c53598880989350697ccc Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 24 Jun 2024 13:32:35 -0700 Subject: [PATCH 3/4] add spec --- .../src/QueuedRequestController.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/queued-request-controller/src/QueuedRequestController.test.ts b/packages/queued-request-controller/src/QueuedRequestController.test.ts index 6b1f517013..aa52b7f7e4 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.test.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.test.ts @@ -28,6 +28,7 @@ describe('QueuedRequestController', () => { messenger: buildQueuedRequestControllerMessenger(), shouldRequestSwitchNetwork: () => false, clearPendingConfirmations: jest.fn(), + showApprovalRequest: jest.fn(), }; const controller = new QueuedRequestController(options); @@ -184,6 +185,32 @@ describe('QueuedRequestController', () => { await secondRequest; }); + it('focuses the existing approval request UI if a request from another origin is being processed', async () => { + const mockShowApprovalRequest = jest.fn(); + const controller = buildQueuedRequestController({ + showApprovalRequest: mockShowApprovalRequest, + }); + // Trigger first request + const firstRequest = controller.enqueueRequest( + { ...buildRequest(), origin: 'https://exampleorigin1.metamask.io' }, + () => new Promise((resolve) => setTimeout(resolve, 10)), + ); + + const secondRequestNext = jest.fn(); + const secondRequest = controller.enqueueRequest( + { ...buildRequest(), origin: 'https://exampleorigin2.metamask.io' }, + secondRequestNext, + ); + + // should focus the existing approval immediately after being queued + expect(mockShowApprovalRequest).toHaveBeenCalledTimes(1); + + await firstRequest; + await secondRequest; + + expect(mockShowApprovalRequest).toHaveBeenCalledTimes(1); + }); + it('drains batch from queue when current batch finishes', async () => { const controller = buildQueuedRequestController(); // Trigger first batch @@ -819,6 +846,7 @@ describe('QueuedRequestController', () => { shouldRequestSwitchNetwork: ({ method }) => method === 'eth_sendTransaction', clearPendingConfirmations: jest.fn(), + showApprovalRequest: jest.fn(), }; const controller = new QueuedRequestController(options); @@ -901,6 +929,7 @@ describe('QueuedRequestController', () => { shouldRequestSwitchNetwork: ({ method }) => method === 'eth_sendTransaction', clearPendingConfirmations: jest.fn(), + showApprovalRequest: jest.fn(), }; const controller = new QueuedRequestController(options); @@ -1037,6 +1066,7 @@ function buildQueuedRequestController( messenger: buildQueuedRequestControllerMessenger(), shouldRequestSwitchNetwork: () => false, clearPendingConfirmations: jest.fn(), + showApprovalRequest: jest.fn(), ...overrideOptions, }; From 33d09d7a03597fa4d05dc92e3ce34beb04fc0fb0 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 24 Jun 2024 13:32:40 -0700 Subject: [PATCH 4/4] lint --- .../src/QueuedRequestController.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index e86b643d87..712caa5b32 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -83,7 +83,7 @@ export type QueuedRequestControllerOptions = { request: QueuedRequestMiddlewareJsonRpcRequest, ) => boolean; clearPendingConfirmations: () => void; - showApprovalRequest: () => Promise; + showApprovalRequest: () => void; }; /** @@ -150,9 +150,17 @@ export class QueuedRequestController extends BaseController< request: QueuedRequestMiddlewareJsonRpcRequest, ) => boolean; + /** + * This is a function that clears all pending confirmations across + * several controllers that may handle them. + */ #clearPendingConfirmations: () => void; - #showApprovalRequest: () => Promise; + /** + * This is a function that makes the confirmation notification view + * become visible and focused to the user + */ + #showApprovalRequest: () => void; /** * Construct a QueuedRequestController. @@ -161,6 +169,8 @@ export class QueuedRequestController extends BaseController< * @param options.messenger - The restricted controller messenger that facilitates communication with other controllers. * @param options.shouldRequestSwitchNetwork - A function that returns if a request requires the globally selected network to match the dapp selected network. * @param options.clearPendingConfirmations - A function that will clear all the pending confirmations. + * @param options.showApprovalRequest - A function for opening the UI such that + * the existing request can be displayed to the user. */ constructor({ messenger, @@ -307,18 +317,12 @@ export class QueuedRequestController extends BaseController< }); } - async #waitForDequeue( - origin: string - ): Promise { - const { - promise, - reject, - resolve, - } = createDeferredPromise({ + async #waitForDequeue(origin: string): Promise { + const { promise, reject, resolve } = createDeferredPromise({ suppressUnhandledRejection: true, }); this.#requestQueue.push({ - origin: origin, + origin, processRequest: (error: unknown) => { if (error) { reject(error); @@ -361,8 +365,8 @@ export class QueuedRequestController extends BaseController< this.state.queuedRequestCount > 0 || this.#originOfCurrentBatch !== request.origin ) { - this.#showApprovalRequest() - await this.#waitForDequeue(request.origin) + this.#showApprovalRequest(); + await this.#waitForDequeue(request.origin); } else if (this.#shouldRequestSwitchNetwork(request)) { // Process request immediately // Requires switching network now if necessary