Warn users when they attempt to add a network that is already configured (#11179)

* warn users when they attempt to add a network that is already configured

* clean up validation logic

* fixing up e2e tests

* Update test/e2e/helpers.js

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

app/_locales/en/messages.json

@@ -289,6 +289,9 @@
   "chainIdDefinition": {
     "message": "The chain ID used to sign transactions for this network."
   },
+  "chainIdExistsErrorMsg": {
+    "message": "This Chain ID is currently used by the $1 network."
+  },
   "chromeRequiredForHardwareWallets": {
     "message": "You need to use MetaMask on Google Chrome in order to connect to your Hardware Wallet."
   },
@@ -2349,7 +2352,7 @@
     "message": "URLs require the appropriate HTTP/HTTPS prefix."
   },
   "urlExistsErrorMsg": {
-    "message": "URL is already present in existing list of networks"
+    "message": "This URL is currently used by the $1 network."
   },
   "usePhishingDetection": {
     "message": "Use Phishing Detection"

test/e2e/helpers.js

@@ -27,6 +27,7 @@ async function withFixtures(options, testSuite) {
   } = options;
   const fixtureServer = new FixtureServer();
   const ganacheServer = new Ganache();
+  let secondaryGanacheServer;
   let dappServer;
   let segmentServer;
   let segmentStub;
@@ -34,6 +35,16 @@ async function withFixtures(options, testSuite) {
   let webDriver;
   try {
     await ganacheServer.start(ganacheOptions);
+    if (ganacheOptions?.concurrent) {
+      const { port, chainId } = ganacheOptions.concurrent;
+      secondaryGanacheServer = new Ganache();
+      await secondaryGanacheServer.start({
+        blockTime: 2,
+        _chainIdRpc: chainId,
+        port,
+        vmErrorsOnRPCResponse: false,
+      });
+    }
     await fixtureServer.start();
     await fixtureServer.loadState(path.join(__dirname, 'fixtures', fixtures));
     if (dapp) {
@@ -103,6 +114,9 @@ async function withFixtures(options, testSuite) {
   } finally {
     await fixtureServer.stop();
     await ganacheServer.quit();
+    if (ganacheOptions?.concurrent) {
+      await secondaryGanacheServer.quit();
+    }
     if (webDriver) {
       await webDriver.quit();
     }

test/e2e/tests/custom-rpc-history.spec.js

@@ -12,6 +12,49 @@ describe('Stores custom RPC history', function () {
     ],
   };
   it(`creates first custom RPC entry`, async function () {
+    const port = 8546;
+    const chainId = 1338;
+    await withFixtures(
+      {
+        fixtures: 'imported-account',
+        ganacheOptions: { ...ganacheOptions, concurrent: { port, chainId } },
+        title: this.test.title,
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+
+        const rpcUrl = `http://127.0.0.1:${port}`;
+        const networkName = 'Secondary Ganache Testnet';
+
+        await driver.clickElement('.network-display');
+
+        await driver.clickElement({ text: 'Custom RPC', tag: 'span' });
+
+        await driver.findElement('.settings-page__sub-header-text');
+
+        const customRpcInputs = await driver.findElements('input[type="text"]');
+        const networkNameInput = customRpcInputs[0];
+        const rpcUrlInput = customRpcInputs[1];
+        const chainIdInput = customRpcInputs[2];
+
+        await networkNameInput.clear();
+        await networkNameInput.sendKeys(networkName);
+
+        await rpcUrlInput.clear();
+        await rpcUrlInput.sendKeys(rpcUrl);
+
+        await chainIdInput.clear();
+        await chainIdInput.sendKeys(chainId.toString());
+
+        await driver.clickElement('.network-form__footer .btn-secondary');
+        await driver.findElement({ text: networkName, tag: 'div' });
+      },
+    );
+  });
+
+  it('warns user when they enter url or chainId for an already configured network', async function () {
     await withFixtures(
       {
         fixtures: 'imported-account',
@@ -23,8 +66,9 @@ describe('Stores custom RPC history', function () {
         await driver.fill('#password', 'correct horse battery staple');
         await driver.press('#password', driver.Key.ENTER);
 
-        const rpcUrl = 'http://127.0.0.1:8545/1';
-        const chainId = '0x539'; // Ganache default, decimal 1337
+        // duplicate network
+        const duplicateRpcUrl = 'http://localhost:8545';
+        const duplicateChainId = '0x539';
 
         await driver.clickElement('.network-display');
 
@@ -37,13 +81,19 @@ describe('Stores custom RPC history', function () {
         const chainIdInput = customRpcInputs[2];
 
         await rpcUrlInput.clear();
-        await rpcUrlInput.sendKeys(rpcUrl);
+        await rpcUrlInput.sendKeys(duplicateRpcUrl);
+        await driver.findElement({
+          text: 'This URL is currently used by the Localhost 8545 network.',
+          tag: 'p',
+        });
 
         await chainIdInput.clear();
-        await chainIdInput.sendKeys(chainId);
-
-        await driver.clickElement('.network-form__footer .btn-secondary');
-        await driver.findElement({ text: rpcUrl, tag: 'div' });
+        await chainIdInput.sendKeys(duplicateChainId);
+        await driver.findElement({
+          text:
+            'This Chain ID is currently used by the Localhost 8545 network.',
+          tag: 'p',
+        });
       },
     );
   });

ui/pages/settings/networks-tab/network-form/network-form.component.js

@@ -10,6 +10,7 @@ import {
   isSafeChainId,
 } from '../../../../../shared/modules/network.utils';
 import { jsonRpcRequest } from '../../../../../shared/modules/rpc.utils';
+import { decimalToHex } from '../../../../helpers/utils/conversions.util';
 
 const FORM_STATE_KEYS = [
   'rpcUrl',
@@ -39,7 +40,7 @@ export default class NetworkForm extends PureComponent {
     isCurrentRpcTarget: PropTypes.bool,
     blockExplorerUrl: PropTypes.string,
     rpcPrefs: PropTypes.object,
-    rpcUrls: PropTypes.array,
+    networksToRender: PropTypes.array,
     isFullScreen: PropTypes.bool,
   };
 
@@ -332,11 +333,25 @@ export default class NetworkForm extends PureComponent {
   };
 
   validateChainIdOnChange = (chainIdArg = '') => {
+    const { networksToRender } = this.props;
     const chainId = chainIdArg.trim();
     let errorMessage = '';
     let radix = 10;
+    const hexChainId = chainId.startsWith('0x')
+      ? chainId
+      : `0x${decimalToHex(chainId)}`;
+    const [matchingChainId] = networksToRender.filter(
+      (e) => e.chainId === hexChainId,
+    );
 
-    if (chainId.startsWith('0x')) {
+    if (chainId === '') {
+      this.setErrorTo('chainId', '');
+      return;
+    } else if (matchingChainId) {
+      errorMessage = this.context.t('chainIdExistsErrorMsg', [
+        matchingChainId.label ?? matchingChainId.labelKey,
+      ]);
+    } else if (chainId.startsWith('0x')) {
       radix = 16;
       if (!/^0x[0-9a-f]+$/iu.test(chainId)) {
         errorMessage = this.context.t('invalidHexNumber');
@@ -433,23 +448,29 @@ export default class NetworkForm extends PureComponent {
 
   validateUrlRpcUrl = (url, stateKey) => {
     const { t } = this.context;
-    const { rpcUrls } = this.props;
+    const { networksToRender } = this.props;
     const { chainId: stateChainId } = this.state;
-    const isValidUrl = validUrl.isWebUri(url) && url !== '';
+    const isValidUrl = validUrl.isWebUri(url);
     const chainIdFetchFailed = this.hasError(
       'chainId',
       t('failedToFetchChainId'),
     );
+    const [matchingRPCUrl] = networksToRender.filter((e) => e.rpcUrl === url);
 
-    if (!isValidUrl) {
+    if (!isValidUrl && url !== '') {
       this.setErrorTo(
         stateKey,
         this.context.t(
           this.isValidWhenAppended(url) ? 'urlErrorMsg' : 'invalidRPC',
         ),
       );
-    } else if (rpcUrls.includes(url)) {
-      this.setErrorTo(stateKey, this.context.t('urlExistsErrorMsg'));
+    } else if (matchingRPCUrl) {
+      this.setErrorTo(
+        stateKey,
+        this.context.t('urlExistsErrorMsg', [
+          matchingRPCUrl.label ?? matchingRPCUrl.labelKey,
+        ]),
+      );
     } else {
       this.setErrorTo(stateKey, '');
     }

ui/pages/settings/networks-tab/networks-tab.component.js

@@ -202,12 +202,12 @@ export default class NetworksTab extends PureComponent {
         {this.renderNetworksList()}
         {shouldRenderNetworkForm ? (
           <NetworkForm
-            rpcUrls={networksToRender.map((network) => network.rpcUrl)}
             setRpcTarget={setRpcTarget}
             editRpc={editRpc}
             networkName={label || (labelKey && t(labelKey)) || ''}
             rpcUrl={rpcUrl}
             chainId={chainId}
+            networksToRender={networksToRender}
             ticker={ticker}
             onClear={(shouldUpdateHistory = true) => {
               setNetworksTabAddMode(false);