delay chain validation (#17413)

app/_locales/en/messages.json

@@ -263,6 +263,9 @@
   "addToken": {
     "message": "Add token"
   },
+  "addingCustomNetwork": {
+    "message": "Adding Network"
+  },
   "address": {
     "message": "Address"
   },
@@ -1308,6 +1311,9 @@
     "message": "Stack:",
     "description": "Title for error stack, which is displayed for debugging purposes"
   },
+  "errorWhileConnectingToRPC": {
+    "message": "Error while connecting to the custom network."
+  },
   "ethGasPriceFetchWarning": {
     "message": "Backup gas price is provided as the main gas estimation service is unavailable right now."
   },
@@ -2033,6 +2039,9 @@
   "mismatchedNetworkSymbol": {
     "message": "The submitted currency symbol does not match what we expect for this chain ID."
   },
+  "mismatchedRpcChainId": {
+    "message": "Chain ID returned by the custom network does not match the submitted chain ID."
+  },
   "mismatchedRpcUrl": {
     "message": "According to our records the submitted RPC URL value does not match a known provider for this chain ID."
   },

app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js

@@ -10,7 +10,6 @@ import {
   isPrefixedFormattedHexString,
   isSafeChainId,
 } from '../../../../../shared/modules/network.utils';
-import { jsonRpcRequest } from '../../../../../shared/modules/rpc.utils';
 
 const addEthereumChain = {
   methodNames: [MESSAGE_TYPE.ADD_ETHEREUM_CHAIN],
@@ -155,6 +154,7 @@ async function addEthereumChainHandler(
     if (currentChainId === _chainId && currentRpcUrl === firstValidRPCUrl) {
       return end();
     }
+
     // If this network is already added with but is not the currently selected network
     // Ask the user to switch the network
     try {
@@ -182,28 +182,6 @@ async function addEthereumChainHandler(
     return end();
   }
 
-  let endpointChainId;
-
-  try {
-    endpointChainId = await jsonRpcRequest(firstValidRPCUrl, 'eth_chainId');
-  } catch (err) {
-    return end(
-      ethErrors.rpc.internal({
-        message: `Request for method 'eth_chainId on ${firstValidRPCUrl} failed`,
-        data: { networkErr: err },
-      }),
-    );
-  }
-
-  if (_chainId !== endpointChainId) {
-    return end(
-      ethErrors.rpc.invalidParams({
-        message: `Chain ID returned by RPC URL ${firstValidRPCUrl} does not match ${_chainId}`,
-        data: { chainId: endpointChainId },
-      }),
-    );
-  }
-
   if (typeof chainName !== 'string' || !chainName) {
     return end(
       ethErrors.rpc.invalidParams({
@@ -266,8 +244,7 @@ async function addEthereumChainHandler(
   }
 
   try {
-    await addCustomRpc(
+    const customRpc = await requestUserApproval({
-      await requestUserApproval({
       origin,
       type: MESSAGE_TYPE.ADD_ETHEREUM_CHAIN,
       requestData: {
@@ -277,9 +254,8 @@ async function addEthereumChainHandler(
         rpcUrl: firstValidRPCUrl,
         ticker,
       },
-      }),
+    });
-    );
+    await addCustomRpc(customRpc);
-
     sendMetrics({
       event: 'Custom Network Added',
       category: EVENT.CATEGORIES.NETWORK,

coverage-targets.js

@@ -6,10 +6,10 @@
 // subset of files to check against these targets.
 module.exports = {
   global: {
-    lines: 64,
+    lines: 64.5,
-    branches: 52.5,
+    branches: 53,
-    statements: 63.1,
+    statements: 63,
-    functions: 56.1,
+    functions: 56.5,
   },
   transforms: {
     branches: 100,

test/e2e/mock-e2e.js

@@ -32,6 +32,20 @@ async function setupMocking(server, testSpecificMock) {
       return {};
     },
   });
+  await server
+    .forPost(
+      'https://arbitrum-mainnet.infura.io/v3/00000000000000000000000000000000',
+    )
+    .thenCallback(() => {
+      return {
+        statusCode: 200,
+        json: {
+          jsonrpc: '2.0',
+          id: '1675864782845',
+          result: '0xa4b1',
+        },
+      };
+    });
 
   await server.forPost('https://api.segment.io/v1/batch').thenCallback(() => {
     return {
@@ -372,6 +386,19 @@ async function setupMocking(server, testSpecificMock) {
       json: emptyHotlist,
     };
   });
+
+  await server
+    .forPost('https://customnetwork.com/api/customRPC')
+    .thenCallback(() => {
+      return {
+        statusCode: 200,
+        json: {
+          jsonrpc: '2.0',
+          id: '1675864782845',
+          result: '0x122',
+        },
+      };
+    });
 }
 
 module.exports = { setupMocking };

test/e2e/tests/add-custom-network.spec.js

@@ -17,6 +17,213 @@ describe('Custom network', function () {
       },
     ],
   };
+
+  it('should show warning when adding chainId 0x1(ethereum) and be followed by an wrong chainId error', async function () {
+    await withFixtures(
+      {
+        dapp: true,
+        fixtures: new FixtureBuilder()
+          .withPermissionControllerConnectedToTestDapp()
+          .build(),
+        ganacheOptions,
+        title: this.test.title,
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+
+        await driver.openNewPage('http://127.0.0.1:8080/');
+        await driver.executeScript(`
+          var params = [{
+            chainId: "0x1",
+            chainName: "Fake Ethereum Network",
+            nativeCurrency: {
+              name: "",
+              symbol: "ETH",
+              decimals: 18
+            },
+            rpcUrls: ["https://customnetwork.com/api/customRPC"],
+            blockExplorerUrls: [ "http://localhost:8080/api/customRPC" ]
+          }]
+          window.ethereum.request({
+            method: 'wallet_addEthereumChain',
+            params
+          })
+        `);
+        const windowHandles = await driver.waitUntilXWindowHandles(3);
+
+        await driver.switchToWindowWithTitle(
+          'MetaMask Notification',
+          windowHandles,
+        );
+
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        const warningTxt =
+          'You are adding a new RPC provider for Ethereum Mainnet';
+
+        await driver.findElement({
+          tag: 'h4',
+          text: warningTxt,
+        });
+
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        const errMsg =
+          'Chain ID returned by the custom network does not match the submitted chain ID.';
+        await driver.findElement({
+          tag: 'span',
+          text: errMsg,
+        });
+
+        const approveBtn = await driver.findElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        assert.equal(await approveBtn.isEnabled(), false);
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Cancel',
+        });
+      },
+    );
+  });
+  it("don't add bad rpc custom network", async function () {
+    await withFixtures(
+      {
+        dapp: true,
+        fixtures: new FixtureBuilder()
+          .withPermissionControllerConnectedToTestDapp()
+          .build(),
+        ganacheOptions,
+        title: this.test.title,
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+
+        await driver.openNewPage('http://127.0.0.1:8080/');
+        await driver.executeScript(`
+          var params = [{
+            chainId: "0x123",
+            chainName: "Antani",
+            nativeCurrency: {
+              name: "",
+              symbol: "ANTANI",
+              decimals: 18
+            },
+            rpcUrls: ["https://customnetwork.com/api/customRPC"],
+            blockExplorerUrls: [ "http://localhost:8080/api/customRPC" ]
+          }]
+          window.ethereum.request({
+            method: 'wallet_addEthereumChain',
+            params
+          })
+        `);
+        const windowHandles = await driver.waitUntilXWindowHandles(3);
+
+        await driver.switchToWindowWithTitle(
+          'MetaMask Notification',
+          windowHandles,
+        );
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        const errMsg =
+          'Chain ID returned by the custom network does not match the submitted chain ID.';
+        await driver.findElement({
+          tag: 'span',
+          text: errMsg,
+        });
+
+        const approveBtn = await driver.findElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        assert.equal(await approveBtn.isEnabled(), false);
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Cancel',
+        });
+      },
+    );
+  });
+
+  it("don't add unreachable custom network", async function () {
+    await withFixtures(
+      {
+        dapp: true,
+        fixtures: new FixtureBuilder()
+          .withPermissionControllerConnectedToTestDapp()
+          .build(),
+        ganacheOptions,
+        title: this.test.title,
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+
+        await driver.openNewPage('http://127.0.0.1:8080/');
+        await driver.executeScript(`
+          var params = [{
+            chainId: "0x123",
+            chainName: "Antani",
+            nativeCurrency: {
+              name: "",
+              symbol: "ANTANI",
+              decimals: 18
+            },
+            rpcUrls: ["https://doesntexist.abc/customRPC"],
+            blockExplorerUrls: [ "http://localhost:8080/api/customRPC" ]
+          }]
+          window.ethereum.request({
+            method: 'wallet_addEthereumChain',
+            params
+          })
+        `);
+        const windowHandles = await driver.waitUntilXWindowHandles(3);
+
+        await driver.switchToWindowWithTitle(
+          'MetaMask Notification',
+          windowHandles,
+        );
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        await driver.findElement({
+          tag: 'span',
+          text: 'Error while connecting to the custom network.',
+        });
+
+        const approveBtn = await driver.findElement({
+          tag: 'button',
+          text: 'Approve',
+        });
+
+        assert.equal(await approveBtn.isEnabled(), false);
+        await driver.clickElement({
+          tag: 'button',
+          text: 'Cancel',
+        });
+      },
+    );
+  });
+
   it('add custom network and switch the network', async function () {
     await withFixtures(
       {
@@ -85,7 +292,6 @@ describe('Custom network', function () {
 
         await driver.clickElement({ tag: 'button', text: 'Close' });
         await driver.clickElement({ tag: 'button', text: 'Approve' });
-
         await driver.clickElement({
           tag: 'h6',
           text: 'Switch to Arbitrum One',

ui/pages/confirmation/components/confirmation-footer/confirmation-footer.js

@@ -8,11 +8,15 @@ export default function ConfirmationFooter({
   onCancel,
   submitText,
   cancelText,
+  loadingText,
   alerts,
+  loading,
+  submitAlerts,
 }) {
   return (
     <div className="confirmation-footer">
       {alerts}
+      {submitAlerts}
       <div className="confirmation-footer__actions">
         {onCancel ? (
           <Button type="secondary" onClick={onCancel}>
@@ -20,13 +24,14 @@ export default function ConfirmationFooter({
           </Button>
         ) : null}
         <Button
+          disabled={Boolean(loading)}
           type="primary"
           onClick={onSubmit}
           className={classnames({
             centered: !onCancel,
           })}
         >
-          {submitText}
+          {loading ? loadingText : submitText}
         </Button>
       </div>
     </div>
@@ -39,4 +44,7 @@ ConfirmationFooter.propTypes = {
   cancelText: PropTypes.string,
   onSubmit: PropTypes.func.isRequired,
   submitText: PropTypes.string.isRequired,
+  loadingText: PropTypes.string,
+  loading: PropTypes.bool,
+  submitAlerts: PropTypes.node,
 };

ui/pages/confirmation/confirmation.js

@@ -177,6 +177,10 @@ export default function ConfirmationPage({
   const setInputState = (key, value) => {
     setInputStates((currentState) => ({ ...currentState, [key]: value }));
   };
+  const [loading, setLoading] = useState(false);
+  const [loadingText, setLoadingText] = useState();
+
+  const [submitAlerts, setSubmitAlerts] = useState([]);
 
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   const snap = useSelector((state) =>
@@ -258,14 +262,28 @@ export default function ConfirmationPage({
     return INPUT_STATE_CONFIRMATIONS.includes(type);
   };
 
-  const handleSubmit = () =>
+  const handleSubmitResult = (submitResult) => {
-    templateState[pendingConfirmation.id]?.useWarningModal
+    if (submitResult?.length > 0) {
-      ? setShowWarningModal(true)
+      setLoadingText(templatedValues.submitText);
-      : templatedValues.onSubmit(
+      setSubmitAlerts(submitResult);
-          hasInputState(pendingConfirmation.type)
+      setLoading(true);
+    } else {
+      setLoading(false);
+    }
+  };
+  const handleSubmit = async () => {
+    setLoading(true);
+    if (templateState[pendingConfirmation.id]?.useWarningModal) {
+      setShowWarningModal(true);
+    } else {
+      const inputState = hasInputState(pendingConfirmation.type)
         ? inputStates[MESSAGE_TYPE.SNAP_DIALOG_PROMPT]
-            : null,
+        : null;
-        );
+      // submit result is an array of errors or empty on success
+      const submitResult = await templatedValues.onSubmit(inputState);
+      handleSubmitResult(submitResult);
+    }
+  };
 
   return (
     <div className="confirmation-page">
@@ -332,7 +350,8 @@ export default function ConfirmationPage({
         {showWarningModal && (
           <ConfirmationWarningModal
             onSubmit={async () => {
-              await templatedValues.onSubmit();
+              const res = await templatedValues.onSubmit();
+              await handleSubmitResult(res);
               setShowWarningModal(false);
             }}
             onCancel={templatedValues.onCancel}
@@ -362,6 +381,13 @@ export default function ConfirmationPage({
         onCancel={templatedValues.onCancel}
         submitText={templatedValues.submitText}
         cancelText={templatedValues.cancelText}
+        loadingText={loadingText || templatedValues.loadingText}
+        loading={loading}
+        submitAlerts={submitAlerts.map((alert, idx) => (
+          <Callout key={alert.id} severity={alert.severity} isFirst={idx === 0}>
+            <MetaMaskTemplateRenderer sections={alert.content} />
+          </Callout>
+        ))}
       />
     </div>
   );

ui/pages/confirmation/templates/add-ethereum-chain.js

@@ -14,6 +14,7 @@ import {
 import { DEFAULT_ROUTE } from '../../../helpers/constants/routes';
 import ZENDESK_URLS from '../../../helpers/constants/zendesk-url';
 import fetchWithCache from '../../../../shared/lib/fetch-with-cache';
+import { jsonRpcRequest } from '../../../../shared/modules/rpc.utils';
 
 const UNRECOGNIZED_CHAIN = {
   id: 'UNRECOGNIZED_CHAIN',
@@ -101,6 +102,34 @@ const MISMATCHED_NETWORK_RPC = {
   },
 };
 
+const MISMATCHED_NETWORK_RPC_CHAIN_ID = {
+  id: 'MISMATCHED_NETWORK_RPC_CHAIN_ID',
+  severity: SEVERITIES.DANGER,
+  content: {
+    element: 'span',
+    children: {
+      element: 'MetaMaskTranslation',
+      props: {
+        translationKey: 'mismatchedRpcChainId',
+      },
+    },
+  },
+};
+
+const ERROR_CONNECTING_TO_RPC = {
+  id: 'ERROR_CONNECTING_TO_RPC',
+  severity: SEVERITIES.DANGER,
+  content: {
+    element: 'span',
+    children: {
+      element: 'MetaMaskTranslation',
+      props: {
+        translationKey: 'errorWhileConnectingToRPC',
+      },
+    },
+  },
+};
+
 async function getAlerts(pendingApproval) {
   const alerts = [];
   const safeChainsList =
@@ -154,7 +183,7 @@ function getState(pendingApproval) {
 
 function getValues(pendingApproval, t, actions, history) {
   const originIsMetaMask = pendingApproval.origin === 'metamask';
-
+  const customRpcUrl = pendingApproval.requestData.rpcUrl;
   return {
     content: [
       {
@@ -180,6 +209,7 @@ function getValues(pendingApproval, t, actions, history) {
           },
         ],
       },
+
       {
         element: 'Typography',
         key: 'title',
@@ -331,7 +361,25 @@ function getValues(pendingApproval, t, actions, history) {
     ],
     cancelText: t('cancel'),
     submitText: t('approveButtonText'),
+    loadingText: t('addingCustomNetwork'),
     onSubmit: async () => {
+      let endpointChainId;
+      try {
+        endpointChainId = await jsonRpcRequest(customRpcUrl, 'eth_chainId');
+      } catch (err) {
+        console.error(
+          `Request for method 'eth_chainId on ${customRpcUrl} failed`,
+        );
+        return [ERROR_CONNECTING_TO_RPC];
+      }
+
+      if (pendingApproval.requestData.chainId !== endpointChainId) {
+        console.error(
+          `Chain ID returned by RPC URL ${customRpcUrl} does not match ${endpointChainId}`,
+        );
+        return [MISMATCHED_NETWORK_RPC_CHAIN_ID];
+      }
+
       await actions.resolvePendingApproval(
         pendingApproval.id,
         pendingApproval.requestData,
@@ -340,6 +388,7 @@ function getValues(pendingApproval, t, actions, history) {
         actions.addCustomNetwork(pendingApproval.requestData);
         history.push(DEFAULT_ROUTE);
       }
+      return [];
     },
     onCancel: () =>
       actions.rejectPendingApproval(

ui/pages/confirmation/templates/index.js

@@ -33,6 +33,7 @@ const ALLOWED_TEMPLATE_KEYS = [
   'onSubmit',
   'networkDisplay',
   'submitText',
+  'loadingText',
 ];
 
 /**