fix: get supportedChains to avoid blocking the confirmation process

app/scripts/lib/ppom/ppom-middleware.test.ts

@@ -158,8 +158,12 @@ describe('PPOMMiddleware', () => {
     );
   });
 
-  it('adds loading response to confirmation requests while validation is in progress', async () => {
-    const middlewareFunction = createMiddleware();
+  it('adds checking chain response to confirmation requests while validation is in progress', async () => {
+    const updateSecurityAlertResponse = jest.fn();
+
+    const middlewareFunction = createMiddleware({
+      updateSecurityAlertResponse,
+    });
 
     const req: PPOMMiddlewareRequest<(string | { to: string })[]> = {
       ...REQUEST_MOCK,
@@ -173,9 +177,11 @@ describe('PPOMMiddleware', () => {
       () => undefined,
     );
 
-    expect(req.securityAlertResponse?.reason).toBe(BlockaidReason.inProgress);
+    expect(req.securityAlertResponse?.reason).toBe(
+      BlockaidReason.checkingChain,
+    );
     expect(req.securityAlertResponse?.result_type).toBe(
-      BlockaidResultType.Loading,
+      BlockaidResultType.Benign,
     );
   });
 
@@ -197,50 +203,6 @@ describe('PPOMMiddleware', () => {
     expect(validateRequestWithPPOM).not.toHaveBeenCalled();
   });
 
-  it('does not do validation if unable to get the chainId from the network provider config', async () => {
-    isChainSupportedMock.mockResolvedValue(false);
-    const middlewareFunction = createMiddleware({
-      chainId: null,
-    });
-
-    const req = {
-      ...REQUEST_MOCK,
-      method: 'eth_sendTransaction',
-      securityAlertResponse: undefined,
-    };
-
-    await middlewareFunction(
-      req,
-      { ...JsonRpcResponseStruct.TYPE },
-      () => undefined,
-    );
-
-    expect(req.securityAlertResponse).toBeUndefined();
-    expect(validateRequestWithPPOM).not.toHaveBeenCalled();
-  });
-
-  it('does not do validation if user is not on a supported network', async () => {
-    isChainSupportedMock.mockResolvedValue(false);
-    const middlewareFunction = createMiddleware({
-      chainId: '0x2',
-    });
-
-    const req = {
-      ...REQUEST_MOCK,
-      method: 'eth_sendTransaction',
-      securityAlertResponse: undefined,
-    };
-
-    await middlewareFunction(
-      req,
-      { ...JsonRpcResponseStruct.TYPE },
-      () => undefined,
-    );
-
-    expect(req.securityAlertResponse).toBeUndefined();
-    expect(validateRequestWithPPOM).not.toHaveBeenCalled();
-  });
-
   it('does not do validation when request is not for confirmation method', async () => {
     const middlewareFunction = createMiddleware();
 

app/scripts/lib/ppom/ppom-middleware.ts

@@ -13,14 +13,13 @@ import { MESSAGE_TYPE } from '../../../../shared/constants/app';
 import { SIGNING_METHODS } from '../../../../shared/constants/transaction';
 import { PreferencesController } from '../../controllers/preferences-controller';
 import { AppStateController } from '../../controllers/app-state-controller';
-import { LOADING_SECURITY_ALERT_RESPONSE } from '../../../../shared/constants/security-provider';
+import { SECURITY_ALERT_RESPONSE_CHECKING_CHAIN } from '../../../../shared/constants/security-provider';
 // eslint-disable-next-line import/no-restricted-paths
 import { getProviderConfig } from '../../../../ui/ducks/metamask/metamask';
 import { trace, TraceContext, TraceName } from '../../../../shared/lib/trace';
 import {
   generateSecurityAlertId,
   handlePPOMError,
-  isChainSupported,
   validateRequestWithPPOM,
 } from './ppom-util';
 import { SecurityAlertResponse } from './types';
@@ -88,9 +87,7 @@ export function createPPOMMiddleware<
 
       if (
         !securityAlertsEnabled ||
-        !CONFIRMATION_METHODS.includes(req.method) ||
-        // Do not move this call above this check because it will result in unnecessary calls
-        !(await isChainSupported(chainId))
+        !CONFIRMATION_METHODS.includes(req.method)
       ) {
         return;
       }
@@ -122,6 +119,7 @@ export function createPPOMMiddleware<
             request: req,
             securityAlertId,
             chainId,
+            updateSecurityAlertResponse,
           }).then((securityAlertResponse) => {
             updateSecurityAlertResponse(
               req.method,
@@ -131,18 +129,18 @@ export function createPPOMMiddleware<
           }),
       );
 
-      const loadingSecurityAlertResponse: SecurityAlertResponse = {
-        ...LOADING_SECURITY_ALERT_RESPONSE,
+      const securityAlertResponseCheckingChain: SecurityAlertResponse = {
+        ...SECURITY_ALERT_RESPONSE_CHECKING_CHAIN,
         securityAlertId,
       };
 
       if (SIGNING_METHODS.includes(req.method)) {
         appStateController.addSignatureSecurityAlertResponse(
-          loadingSecurityAlertResponse,
+          securityAlertResponseCheckingChain,
         );
       }
 
-      req.securityAlertResponse = loadingSecurityAlertResponse;
+      req.securityAlertResponse = securityAlertResponseCheckingChain;
     } catch (error) {
       req.securityAlertResponse = handlePPOMError(
         error,

app/scripts/lib/ppom/ppom-util.test.ts

@@ -10,9 +10,12 @@ import {
   SignatureController,
   SignatureRequest,
 } from '@metamask/signature-controller';
+import { Hex } from '@metamask/utils';
 import {
   BlockaidReason,
   BlockaidResultType,
+  LOADING_SECURITY_ALERT_RESPONSE,
+  SECURITY_ALERT_RESPONSE_CHAIN_NOT_SUPPORTED,
   SecurityAlertSource,
 } from '../../../../shared/constants/security-provider';
 import { AppStateController } from '../../controllers/app-state-controller';
@@ -32,7 +35,7 @@ jest.mock('@metamask/transaction-controller', () => ({
 
 const SECURITY_ALERT_ID_MOCK = '1234-5678';
 const TRANSACTION_ID_MOCK = '123';
-const CHAIN_ID_MOCK = '0x1';
+const CHAIN_ID_MOCK = '0x1' as Hex;
 
 const REQUEST_MOCK = {
   method: 'eth_signTypedData_v4',
@@ -110,6 +113,15 @@ describe('PPOM Utils', () => {
   );
   let isSecurityAlertsEnabledMock: jest.SpyInstance;
 
+  const updateSecurityAlertResponseMock = jest.fn();
+
+  const validateRequestWithPPOMOptionsBase = {
+    request: REQUEST_MOCK,
+    securityAlertId: SECURITY_ALERT_ID_MOCK,
+    chainId: CHAIN_ID_MOCK,
+    updateSecurityAlertResponse: updateSecurityAlertResponseMock,
+  };
+
   beforeEach(() => {
     jest.resetAllMocks();
     jest.spyOn(console, 'error').mockImplementation(() => undefined);
@@ -130,10 +142,8 @@ describe('PPOM Utils', () => {
       );
 
       const response = await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
-        request: REQUEST_MOCK,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(response).toStrictEqual({
@@ -145,6 +155,24 @@ describe('PPOM Utils', () => {
       expect(ppom.validateJsonRpc).toHaveBeenCalledWith(REQUEST_MOCK);
     });
 
+    it('updates securityAlertResponse with loading state', async () => {
+      const ppomController = createPPOMControllerMock();
+
+      await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
+        ppomController,
+      });
+
+      expect(updateSecurityAlertResponseMock).toHaveBeenCalledWith(
+        REQUEST_MOCK.method,
+        SECURITY_ALERT_ID_MOCK,
+        {
+          ...LOADING_SECURITY_ALERT_RESPONSE,
+          securityAlertId: SECURITY_ALERT_ID_MOCK,
+        },
+      );
+    });
+
     it('returns error response if validation with PPOM instance throws', async () => {
       const ppom = createPPOMMock();
       const ppomController = createPPOMControllerMock();
@@ -158,10 +186,8 @@ describe('PPOM Utils', () => {
       );
 
       const response = await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
-        request: REQUEST_MOCK,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(response).toStrictEqual({
@@ -177,10 +203,8 @@ describe('PPOM Utils', () => {
       ppomController.usePPOM.mockRejectedValue(createErrorMock());
 
       const response = await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
-        request: REQUEST_MOCK,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(response).toStrictEqual({
@@ -209,10 +233,9 @@ describe('PPOM Utils', () => {
       };
 
       await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
         request,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(ppom.validateJsonRpc).toHaveBeenCalledTimes(1);
@@ -226,6 +249,22 @@ describe('PPOM Utils', () => {
         TRANSACTION_PARAMS_MOCK_1,
       );
     });
+
+    it('returns response indicating chain is not supported', async () => {
+      const ppomController = {} as PPOMController;
+      const CHAIN_ID_UNSUPPORTED_MOCK = '0x2';
+
+      const response = await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
+        ppomController,
+        chainId: CHAIN_ID_UNSUPPORTED_MOCK,
+      });
+
+      expect(response).toStrictEqual({
+        ...SECURITY_ALERT_RESPONSE_CHAIN_NOT_SUPPORTED,
+        securityAlertId: SECURITY_ALERT_ID_MOCK,
+      });
+    });
   });
 
   describe('generateSecurityAlertId', () => {
@@ -318,10 +357,9 @@ describe('PPOM Utils', () => {
       const ppomController = createPPOMControllerMock();
 
       await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
         request,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(ppomController.usePPOM).not.toHaveBeenCalled();
@@ -345,10 +383,9 @@ describe('PPOM Utils', () => {
         .mockRejectedValue(new Error('Test Error'));
 
       await validateRequestWithPPOM({
+        ...validateRequestWithPPOMOptionsBase,
         ppomController,
         request,
-        securityAlertId: SECURITY_ALERT_ID_MOCK,
-        chainId: CHAIN_ID_MOCK,
       });
 
       expect(ppomController.usePPOM).toHaveBeenCalledTimes(1);

app/scripts/lib/ppom/ppom-util.ts

@@ -11,6 +11,8 @@ import { SignatureController } from '@metamask/signature-controller';
 import {
   BlockaidReason,
   BlockaidResultType,
+  LOADING_SECURITY_ALERT_RESPONSE,
+  SECURITY_ALERT_RESPONSE_CHAIN_NOT_SUPPORTED,
   SECURITY_PROVIDER_SUPPORTED_CHAIN_IDS_FALLBACK_LIST,
   SecurityAlertSource,
 } from '../../../../shared/constants/security-provider';
@@ -37,18 +39,37 @@ type PPOMRequest = Omit<JsonRpcRequest, 'method' | 'params'> & {
   method: typeof METHOD_SEND_TRANSACTION;
   params: [TransactionParams];
 };
+
 export async function validateRequestWithPPOM({
   ppomController,
   request,
   securityAlertId,
   chainId,
+  updateSecurityAlertResponse: updateSecurityResponse,
 }: {
   ppomController: PPOMController;
   request: JsonRpcRequest;
   securityAlertId: string;
   chainId: Hex;
+  updateSecurityAlertResponse: (
+    method: string,
+    securityAlertId: string,
+    securityAlertResponse: SecurityAlertResponse,
+  ) => void;
 }): Promise<SecurityAlertResponse> {
   try {
+    if (!(await isChainSupported(chainId))) {
+      return {
+        ...SECURITY_ALERT_RESPONSE_CHAIN_NOT_SUPPORTED,
+        securityAlertId,
+      };
+    }
+
+    updateSecurityResponse(request.method, securityAlertId, {
+      ...LOADING_SECURITY_ALERT_RESPONSE,
+      securityAlertId,
+    });
+
     const normalizedRequest = normalizePPOMRequest(request);
 
     const ppomResponse = isSecurityAlertsAPIEnabled()

app/scripts/lib/transaction/util.test.ts

@@ -425,8 +425,8 @@ describe('Transaction Utils', () => {
         ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
           ...TRANSACTION_OPTIONS_MOCK,
           securityAlertResponse: {
-            reason: BlockaidReason.inProgress,
-            result_type: BlockaidResultType.Loading,
+            reason: BlockaidReason.checkingChain,
+            result_type: BlockaidResultType.Benign,
             securityAlertId: SECURITY_ALERT_ID_MOCK,
           },
         });
@@ -505,29 +505,6 @@ describe('Transaction Utils', () => {
         expect(validateRequestWithPPOMMock).toHaveBeenCalledTimes(0);
       });
 
-      it('unless chain is not supported', async () => {
-        isChainSupportedMock.mockResolvedValue(false);
-
-        await addTransaction({
-          ...request,
-          securityAlertsEnabled: true,
-          chainId: '0xF',
-        });
-
-        expect(
-          request.transactionController.addTransaction,
-        ).toHaveBeenCalledTimes(1);
-
-        expect(
-          request.transactionController.addTransaction,
-        ).toHaveBeenCalledWith(
-          TRANSACTION_PARAMS_MOCK,
-          TRANSACTION_OPTIONS_MOCK,
-        );
-
-        expect(validateRequestWithPPOMMock).toHaveBeenCalledTimes(0);
-      });
-
       it('unless transaction type is swap', async () => {
         const swapRequest = { ...request };
         swapRequest.transactionOptions.type = TransactionType.swap;