revert: use networkClientId to resolve chainId in PPOM Middleware (#2…

…7570)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Reverts [this PPOM
change](#27263) due
to [issue with network configuration for the newly added rpc endpoint
not being available when queried immediately after being
added](#27447)

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27570?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

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

@@ -8,6 +8,7 @@ import {
   BlockaidResultType,
 } from '../../../../shared/constants/security-provider';
 import { flushPromises } from '../../../../test/lib/timer-helpers';
+import { mockNetworkState } from '../../../../test/stub/networks';
 import { createPPOMMiddleware, PPOMMiddlewareRequest } from './ppom-middleware';
 import {
   generateSecurityAlertId,
@@ -36,18 +37,22 @@ const REQUEST_MOCK = {
   params: [],
   id: '',
   jsonrpc: '2.0' as const,
-  origin: 'test.com',
-  networkClientId: 'networkClientId',
 };
 
 const createMiddleware = (
   options: {
-    chainId?: Hex;
+    chainId?: Hex | null;
     error?: Error;
     securityAlertsEnabled?: boolean;
-  } = {},
+    // TODO: Replace `any` with type
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    updateSecurityAlertResponse?: any;
+  } = {
+    updateSecurityAlertResponse: () => undefined,
+  },
 ) => {
-  const { chainId, error, securityAlertsEnabled } = options;
+  const { chainId, error, securityAlertsEnabled, updateSecurityAlertResponse } =
+    options;
 
   const ppomController = {};
 
@@ -66,9 +71,10 @@ const createMiddleware = (
   }
 
   const networkController = {
-    getNetworkConfigurationByNetworkClientId: jest
-      .fn()
-      .mockReturnValue({ chainId: chainId || CHAIN_IDS.MAINNET }),
+    state: {
+      ...mockNetworkState({ chainId: chainId || CHAIN_IDS.MAINNET }),
+      ...(chainId === null ? { providerConfig: {} } : undefined),
+    },
   };
 
   const appStateController = {
@@ -79,9 +85,7 @@ const createMiddleware = (
     listAccounts: () => [{ address: INTERNAL_ACCOUNT_ADDRESS }],
   };
 
-  const updateSecurityAlertResponse = jest.fn();
-
-  const middleware = createPPOMMiddleware(
+  return createPPOMMiddleware(
     // TODO: Replace `any` with type
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     ppomController as any,
@@ -98,16 +102,6 @@ const createMiddleware = (
     accountsController as any,
     updateSecurityAlertResponse,
   );
-
-  return {
-    middleware,
-    ppomController,
-    preferenceController,
-    networkController,
-    appStateController,
-    accountsController,
-    updateSecurityAlertResponse,
-  };
 };
 
 describe('PPOMMiddleware', () => {
@@ -135,37 +129,24 @@ describe('PPOMMiddleware', () => {
     };
   });
 
-  it('gets the network configuration for the request networkClientId', async () => {
-    const { middleware, networkController } = createMiddleware();
-
-    const req = {
-      ...REQUEST_MOCK,
-      method: 'eth_sendTransaction',
-      securityAlertResponse: undefined,
-    };
-
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => undefined);
-
-    await flushPromises();
-
-    expect(
-      networkController.getNetworkConfigurationByNetworkClientId,
-    ).toHaveBeenCalledTimes(1);
-    expect(
-      networkController.getNetworkConfigurationByNetworkClientId,
-    ).toHaveBeenCalledWith('networkClientId');
-  });
-
   it('updates alert response after validating request', async () => {
-    const { middleware, updateSecurityAlertResponse } = createMiddleware();
+    const updateSecurityAlertResponse = jest.fn();
+
+    const middlewareFunction = createMiddleware({
+      updateSecurityAlertResponse,
+    });
 
     const req = {
       ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => undefined);
+    await middlewareFunction(
+      req,
+      { ...JsonRpcResponseStruct.TYPE },
+      () => undefined,
+    );
 
     await flushPromises();
 
@@ -178,15 +159,19 @@ describe('PPOMMiddleware', () => {
   });
 
   it('adds loading response to confirmation requests while validation is in progress', async () => {
-    const { middleware } = createMiddleware();
+    const middlewareFunction = createMiddleware();
 
     const req: PPOMMiddlewareRequest<(string | { to: string })[]> = {
       ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => undefined);
+    await middlewareFunction(
+      req,
+      { ...JsonRpcResponseStruct.TYPE },
+      () => undefined,
+    );
 
     expect(req.securityAlertResponse?.reason).toBe(BlockaidReason.inProgress);
     expect(req.securityAlertResponse?.result_type).toBe(
@@ -195,7 +180,7 @@ describe('PPOMMiddleware', () => {
   });
 
   it('does not do validation if the user has not enabled the preference', async () => {
-    const { middleware } = createMiddleware({
+    const middlewareFunction = createMiddleware({
       securityAlertsEnabled: false,
     });
 
@@ -206,15 +191,37 @@ describe('PPOMMiddleware', () => {
     };
 
     // @ts-expect-error Passing in invalid input for testing purposes
-    await middleware(req, undefined, () => undefined);
+    await middlewareFunction(req, undefined, () => undefined);
+
+    expect(req.securityAlertResponse).toBeUndefined();
+    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 { middleware } = createMiddleware({
+    const middlewareFunction = createMiddleware({
       chainId: '0x2',
     });
 
@@ -224,29 +231,37 @@ describe('PPOMMiddleware', () => {
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => 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 { middleware } = createMiddleware();
+    const middlewareFunction = createMiddleware();
 
     const req = {
       ...REQUEST_MOCK,
       method: 'eth_someRequest',
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => undefined);
+    await middlewareFunction(
+      req,
+      { ...JsonRpcResponseStruct.TYPE },
+      () => undefined,
+    );
 
     expect(req.securityAlertResponse).toBeUndefined();
     expect(validateRequestWithPPOM).not.toHaveBeenCalled();
   });
 
   it('does not do validation when request is send to users own account', async () => {
-    const { middleware } = createMiddleware();
+    const middlewareFunction = createMiddleware();
 
     const req = {
       ...REQUEST_MOCK,
@@ -255,14 +270,18 @@ describe('PPOMMiddleware', () => {
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, () => undefined);
+    await middlewareFunction(
+      req,
+      { ...JsonRpcResponseStruct.TYPE },
+      () => undefined,
+    );
 
     expect(req.securityAlertResponse).toBeUndefined();
     expect(validateRequestWithPPOM).not.toHaveBeenCalled();
   });
 
   it('does not do validation for SIWE signature', async () => {
-    const { middleware } = createMiddleware({
+    const middlewareFunction = createMiddleware({
       securityAlertsEnabled: true,
     });
 
@@ -283,17 +302,17 @@ describe('PPOMMiddleware', () => {
     detectSIWEMock.mockReturnValue({ isSIWEMessage: true } as SIWEMessage);
 
     // @ts-expect-error Passing invalid input for testing purposes
-    await middleware(req, undefined, () => undefined);
+    await middlewareFunction(req, undefined, () => undefined);
 
     expect(req.securityAlertResponse).toBeUndefined();
     expect(validateRequestWithPPOM).not.toHaveBeenCalled();
   });
 
   it('calls next method', async () => {
-    const { middleware } = createMiddleware();
+    const middlewareFunction = createMiddleware();
     const nextMock = jest.fn();
 
-    await middleware(
+    await middlewareFunction(
       { ...REQUEST_MOCK, method: 'eth_sendTransaction' },
       { ...JsonRpcResponseStruct.TYPE },
       nextMock,
@@ -308,15 +327,15 @@ describe('PPOMMiddleware', () => {
 
     const nextMock = jest.fn();
 
-    const { middleware } = createMiddleware({ error });
+    const middlewareFunction = createMiddleware({ error });
 
     const req = {
       ...REQUEST_MOCK,
       method: 'eth_sendTransaction',
       securityAlertResponse: undefined,
     };
 
-    await middleware(req, { ...JsonRpcResponseStruct.TYPE }, nextMock);
+    await middlewareFunction(req, { ...JsonRpcResponseStruct.TYPE }, nextMock);
 
     expect(req.securityAlertResponse).toStrictEqual(
       SECURITY_ALERT_RESPONSE_MOCK,

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

@@ -1,9 +1,6 @@
 import { AccountsController } from '@metamask/accounts-controller';
 import { PPOMController } from '@metamask/ppom-validator';
-import {
-  NetworkClientId,
-  NetworkController,
-} from '@metamask/network-controller';
+import { NetworkController } from '@metamask/network-controller';
 import {
   Json,
   JsonRpcParams,
@@ -17,6 +14,8 @@ import { SIGNING_METHODS } from '../../../../shared/constants/transaction';
 import PreferencesController from '../../controllers/preferences-controller';
 import { AppStateController } from '../../controllers/app-state';
 import { LOADING_SECURITY_ALERT_RESPONSE } 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,
@@ -35,7 +34,6 @@ const CONFIRMATION_METHODS = Object.freeze([
 export type PPOMMiddlewareRequest<
   Params extends JsonRpcParams = JsonRpcParams,
 > = Required<JsonRpcRequest<Params>> & {
-  networkClientId: NetworkClientId;
   securityAlertResponse?: SecurityAlertResponse | undefined;
   traceContext?: TraceContext;
 };
@@ -81,13 +79,14 @@ export function createPPOMMiddleware<
       const securityAlertsEnabled =
         preferencesController.store.getState()?.securityAlertsEnabled;
 
-      // This will always exist as the SelectedNetworkMiddleware
-      // adds networkClientId to the request before this middleware runs
       const { chainId } =
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-        networkController.getNetworkConfigurationByNetworkClientId(
-          req.networkClientId,
-        )!;
+        getProviderConfig({
+          metamask: networkController.state,
+        }) ?? {};
+      if (!chainId) {
+        return;
+      }
+
       const isSupportedChain = await isChainSupported(chainId);
 
       if (

app/scripts/metamask-controller.js

@@ -5624,7 +5624,6 @@ export default class MetamaskController extends EventEmitter {
 
     engine.push(createTracingMiddleware());
 
-    // PPOMMiddleware come after the SelectedNetworkMiddleware
     engine.push(
       createPPOMMiddleware(
         this.ppomController,