feat: change queued-request-controller methods array params to call…

…backs (#4423)

## Explanation

The existing `methodsWithConfirmation` array param is not sufficient for
determining if a method will cause a confirmation to be presented to the
user or not because some methods such as `eth_requestAccounts` may only
cause a prompt in some scenarios but not others.

This PR replaces the array params used in the QueuedRequestController
and its middleware to be callbacks that accept the entire request object
instead.

## References

See: MetaMask/metamask-extension#25310

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/queued-request-controller`

- **BREAKING**: `QueuedRequestController` constructor no longer accepts
the `methodsRequiringNetworkSwitch` array param. It's now replaced with
the `shouldRequestSwitchNetwork` function param which should return true
when a request requires the globally selected network to match that of
the dapp.
- **BREAKING**: `createQueuedRequestMiddleware` no longer accepts the
`methodsWithConfirmation` array param. It's now replaced with the
`shouldEnqueueRequest` function param which should return true when a
request should be handled by the QueuedRequestController


## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

packages/queued-request-controller/src/QueuedRequestController.test.ts

@@ -26,7 +26,7 @@ describe('QueuedRequestController', () => {
   it('can be instantiated with default values', () => {
     const options: QueuedRequestControllerOptions = {
       messenger: buildQueuedRequestControllerMessenger(),
-      methodsRequiringNetworkSwitch: [],
+      shouldRequestSwitchNetwork: () => false,
       clearPendingConfirmations: jest.fn(),
     };
 
@@ -62,7 +62,7 @@ describe('QueuedRequestController', () => {
       await firstRequest;
     });
 
-    it('switches network if a request comes in for a different network client and the method is in the methodsRequiringNetworkSwitch param', async () => {
+    it('switches network if a request comes in for a different network client and shouldRequestSwitchNetwork returns true', async () => {
       const mockSetActiveNetwork = jest.fn();
       const { messenger } = buildControllerMessenger({
         networkControllerGetState: jest.fn().mockReturnValue({
@@ -81,7 +81,8 @@ describe('QueuedRequestController', () => {
       );
       const controller = buildQueuedRequestController({
         messenger: buildQueuedRequestControllerMessenger(messenger),
-        methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+        shouldRequestSwitchNetwork: ({ method }) =>
+          method === 'method_requiring_network_switch',
         clearPendingConfirmations: jest.fn(),
       });
 
@@ -98,7 +99,7 @@ describe('QueuedRequestController', () => {
       );
     });
 
-    it('does not switch networks if the method is not in the methodsRequiringNetworkSwitch param', async () => {
+    it('does not switch networks if shouldRequestSwitchNetwork returns false', async () => {
       const mockSetActiveNetwork = jest.fn();
       const { messenger } = buildControllerMessenger({
         networkControllerGetState: jest.fn().mockReturnValue({
@@ -117,11 +118,12 @@ describe('QueuedRequestController', () => {
       );
       const controller = buildQueuedRequestController({
         messenger: buildQueuedRequestControllerMessenger(messenger),
-        methodsRequiringNetworkSwitch: [],
+        shouldRequestSwitchNetwork: ({ method }) =>
+          method === 'method_requiring_network_switch',
       });
 
       await controller.enqueueRequest(
-        { ...buildRequest(), method: 'not_in_methodsRequiringNetworkSwitch' },
+        { ...buildRequest(), method: 'not_requiring_network_switch' },
         () => new Promise((resolve) => setTimeout(resolve, 10)),
       );
 
@@ -537,7 +539,8 @@ describe('QueuedRequestController', () => {
         });
         const controller = buildQueuedRequestController({
           messenger: buildQueuedRequestControllerMessenger(messenger),
-          methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+          shouldRequestSwitchNetwork: ({ method }) =>
+            method === 'method_requiring_network_switch',
         });
 
         await expect(() =>
@@ -572,7 +575,8 @@ describe('QueuedRequestController', () => {
         });
         const controller = buildQueuedRequestController({
           messenger: buildQueuedRequestControllerMessenger(messenger),
-          methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+          shouldRequestSwitchNetwork: ({ method }) =>
+            method === 'method_requiring_network_switch',
         });
         const firstRequest = controller.enqueueRequest(
           {
@@ -626,7 +630,8 @@ describe('QueuedRequestController', () => {
         });
         const controller = buildQueuedRequestController({
           messenger: buildQueuedRequestControllerMessenger(messenger),
-          methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+          shouldRequestSwitchNetwork: ({ method }) =>
+            method === 'method_requiring_network_switch',
         });
         const firstRequest = controller.enqueueRequest(
           {
@@ -679,7 +684,8 @@ describe('QueuedRequestController', () => {
         });
         const controller = buildQueuedRequestController({
           messenger: buildQueuedRequestControllerMessenger(messenger),
-          methodsRequiringNetworkSwitch: ['method_requiring_network_switch'],
+          shouldRequestSwitchNetwork: ({ method }) =>
+            method === 'method_requiring_network_switch',
         });
         const firstRequest = controller.enqueueRequest(
           {
@@ -810,7 +816,8 @@ describe('QueuedRequestController', () => {
 
       const options: QueuedRequestControllerOptions = {
         messenger: buildQueuedRequestControllerMessenger(messenger),
-        methodsRequiringNetworkSwitch: ['eth_sendTransaction'],
+        shouldRequestSwitchNetwork: ({ method }) =>
+          method === 'eth_sendTransaction',
         clearPendingConfirmations: jest.fn(),
       };
 
@@ -891,7 +898,8 @@ describe('QueuedRequestController', () => {
 
       const options: QueuedRequestControllerOptions = {
         messenger: buildQueuedRequestControllerMessenger(messenger),
-        methodsRequiringNetworkSwitch: ['eth_sendTransaction'],
+        shouldRequestSwitchNetwork: ({ method }) =>
+          method === 'eth_sendTransaction',
         clearPendingConfirmations: jest.fn(),
       };
 
@@ -1027,7 +1035,7 @@ function buildQueuedRequestController(
 ): QueuedRequestController {
   const options: QueuedRequestControllerOptions = {
     messenger: buildQueuedRequestControllerMessenger(),
-    methodsRequiringNetworkSwitch: [],
+    shouldRequestSwitchNetwork: () => false,
     clearPendingConfirmations: jest.fn(),
     ...overrideOptions,
   };

packages/queued-request-controller/src/QueuedRequestController.ts

@@ -79,7 +79,9 @@ export type QueuedRequestControllerMessenger = RestrictedControllerMessenger<
 
 export type QueuedRequestControllerOptions = {
   messenger: QueuedRequestControllerMessenger;
-  methodsRequiringNetworkSwitch: string[];
+  shouldRequestSwitchNetwork: (
+    request: QueuedRequestMiddlewareJsonRpcRequest,
+  ) => boolean;
   clearPendingConfirmations: () => void;
 };
 
@@ -136,14 +138,16 @@ export class QueuedRequestController extends BaseController<
   #processingRequestCount = 0;
 
   /**
-   * This is a list of methods that require the globally selected network
-   * to match the dapp selected network before being processed. These can
+   * This is a function that returns true if a request requires the globally selected
+   * network to match the dapp selected network before being processed. These can
    * be for UI/UX reasons where the currently selected network is displayed
    * in the confirmation even though it will be submitted on the correct
    * network for the dapp. It could also be that a method expects the
    * globally selected network to match some value in the request params itself.
    */
-  readonly #methodsRequiringNetworkSwitch: string[];
+  readonly #shouldRequestSwitchNetwork: (
+    request: QueuedRequestMiddlewareJsonRpcRequest,
+  ) => boolean;
 
   #clearPendingConfirmations: () => void;
 
@@ -152,12 +156,12 @@ export class QueuedRequestController extends BaseController<
    *
    * @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.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.
    */
   constructor({
     messenger,
-    methodsRequiringNetworkSwitch,
+    shouldRequestSwitchNetwork,
     clearPendingConfirmations,
   }: QueuedRequestControllerOptions) {
     super({
@@ -171,7 +175,7 @@ export class QueuedRequestController extends BaseController<
       messenger,
       state: { queuedRequestCount: 0 },
     });
-    this.#methodsRequiringNetworkSwitch = methodsRequiringNetworkSwitch;
+    this.#shouldRequestSwitchNetwork = shouldRequestSwitchNetwork;
     this.#clearPendingConfirmations = clearPendingConfirmations;
     this.#registerMessageHandlers();
   }
@@ -346,7 +350,7 @@ export class QueuedRequestController extends BaseController<
         this.#updateQueuedRequestCount();
 
         await waitForDequeue;
-      } else if (this.#methodsRequiringNetworkSwitch.includes(request.method)) {
+      } else if (this.#shouldRequestSwitchNetwork(request)) {
         // Process request immediately
         // Requires switching network now if necessary
         await this.#switchNetworkIfNecessary();

packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts

@@ -101,12 +101,13 @@ describe('createQueuedRequestMiddleware', () => {
     expect(mockEnqueueRequest).not.toHaveBeenCalled();
   });
 
-  it('enqueues the request if the method is in the methodsWithConfirmation param', async () => {
+  it('enqueues the request if shouldEnqueueRest returns true', async () => {
     const mockEnqueueRequest = getMockEnqueueRequest();
     const middleware = buildQueuedRequestMiddleware({
       enqueueRequest: mockEnqueueRequest,
       useRequestQueue: () => true,
-      methodsWithConfirmation: ['method_with_confirmation'],
+      shouldEnqueueRequest: ({ method }) =>
+        method === 'method_with_confirmation',
     });
     const request = {
       ...getRequestDefaults(),
@@ -167,7 +168,7 @@ describe('createQueuedRequestMiddleware', () => {
           .fn()
           .mockRejectedValue(new Error('enqueuing error')),
         useRequestQueue: () => true,
-        methodsWithConfirmation: ['method_should_be_enqueued'],
+        shouldEnqueueRequest: () => true,
       });
       const request = {
         ...getRequestDefaults(),
@@ -191,7 +192,7 @@ describe('createQueuedRequestMiddleware', () => {
           .fn()
           .mockRejectedValue(new Error('enqueuing error')),
         useRequestQueue: () => true,
-        methodsWithConfirmation: ['method_should_be_enqueued'],
+        shouldEnqueueRequest: () => true,
       });
       const request = {
         ...getRequestDefaults(),
@@ -271,7 +272,7 @@ function buildQueuedRequestMiddleware(
   const options = {
     enqueueRequest: getMockEnqueueRequest(),
     useRequestQueue: () => false,
-    methodsWithConfirmation: [],
+    shouldEnqueueRequest: () => false,
     ...overrideOptions,
   };
 

packages/queued-request-controller/src/QueuedRequestMiddleware.ts

@@ -39,24 +39,26 @@ function hasRequiredMetadata(
  * @param options - Configuration options.
  * @param options.enqueueRequest - A method for enqueueing a request.
  * @param options.useRequestQueue - A function that determines if the request queue feature is enabled.
- * @param options.methodsWithConfirmation - A list of methods that can cause a confirmation to be presented to the user.
+ * @param options.shouldEnqueueRequest - A function that returns if a request should be handled by the QueuedRequestController.
  * @returns The JSON-RPC middleware that manages queued requests.
  */
 export const createQueuedRequestMiddleware = ({
   enqueueRequest,
   useRequestQueue,
-  methodsWithConfirmation,
+  shouldEnqueueRequest,
 }: {
   enqueueRequest: QueuedRequestController['enqueueRequest'];
   useRequestQueue: () => boolean;
-  methodsWithConfirmation: string[];
+  shouldEnqueueRequest: (
+    request: QueuedRequestMiddlewareJsonRpcRequest,
+  ) => boolean;
 }): JsonRpcMiddleware<JsonRpcParams, Json> => {
   return createAsyncMiddleware(async (req: JsonRpcRequest, res, next) => {
     hasRequiredMetadata(req);
 
     // if the request queue feature is turned off, or this method is not a confirmation method
     // bypass the queue completely
-    if (!useRequestQueue() || !methodsWithConfirmation.includes(req.method)) {
+    if (!useRequestQueue() || !shouldEnqueueRequest(req)) {
       return await next();
     }