Allowing custom rpc form submission when chainId is a duplicate (#11363)

3 years ago · a171ac7b34
parent e10ddbe3a3
commit a171ac7b34
2 changed files with 122 additions and 42 deletions
--- a/test/e2e/tests/custom-rpc-history.spec.js
+++ b/test/e2e/tests/custom-rpc-history.spec.js
@ -54,7 +54,7 @@ describe('Stores custom RPC history', function () {
    );
  });

-  it('warns user when they enter url or chainId for an already configured network', async function () {
+  it('warns user when they enter url for an already configured network', async function () {
    await withFixtures(
      {
        fixtures: 'imported-account',
@ -68,7 +68,6 @@ describe('Stores custom RPC history', function () {

        // duplicate network
        const duplicateRpcUrl = 'http://localhost:8545';
-        const duplicateChainId = '0x539';

        await driver.clickElement('.network-display');

@ -78,7 +77,6 @@ describe('Stores custom RPC history', function () {

        const customRpcInputs = await driver.findElements('input[type="text"]');
        const rpcUrlInput = customRpcInputs[1];
-        const chainIdInput = customRpcInputs[2];

        await rpcUrlInput.clear();
        await rpcUrlInput.sendKeys(duplicateRpcUrl);
@ -86,6 +84,38 @@ describe('Stores custom RPC history', function () {
          text: 'This URL is currently used by the Localhost 8545 network.',
          tag: 'p',
        });
+      },
+    );
+  });
+
+  it('warns user when they enter chainId for an already configured network', async function () {
+    await withFixtures(
+      {
+        fixtures: 'imported-account',
+        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);
+
+        // duplicate network
+        const newRpcUrl = 'http://localhost:8544';
+        const duplicateChainId = '0x539';
+
+        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 rpcUrlInput = customRpcInputs[1];
+        const chainIdInput = customRpcInputs[2];
+
+        await rpcUrlInput.clear();
+        await rpcUrlInput.sendKeys(newRpcUrl);

        await chainIdInput.clear();
        await chainIdInput.sendKeys(duplicateChainId);
--- a/ui/pages/settings/networks-tab/network-form/network-form.component.js
+++ b/ui/pages/settings/networks-tab/network-form/network-form.component.js
@ -280,6 +280,7 @@ export default class NetworkForm extends PureComponent {
  }) {
    const { errors } = this.state;
    const { viewOnly } = this.props;
+    const errorMessage = errors[fieldKey]?.msg || '';

    return (
      <div className="networks-tab__network-form-row">
@ -305,7 +306,7 @@ export default class NetworkForm extends PureComponent {
          margin="dense"
          value={value}
          disabled={viewOnly}
-          error={errors[fieldKey]}
+          error={errorMessage}
          autoFocus={autoFocus}
        />
      </div>
@ -328,45 +329,78 @@ export default class NetworkForm extends PureComponent {
    });
  };

-  hasError = (errorKey, errorVal) => {
-    return this.state.errors[errorKey] === errorVal;
+  setErrorEmpty = (errorKey) => {
+    this.setState({
+      errors: {
+        ...this.state.errors,
+        [errorKey]: {
+          msg: '',
+          key: '',
+        },
+      },
+    });
+  };
+
+  hasError = (errorKey, errorKeyVal) => {
+    return this.state.errors[errorKey]?.key === errorKeyVal;
  };

-  validateChainIdOnChange = (chainIdArg = '') => {
+  hasErrors = () => {
+    const { errors } = this.state;
+    return Object.keys(errors).some((key) => {
+      const error = errors[key];
+      // Do not factor in duplicate chain id error for submission disabling
+      if (key === 'chainId' && error.key === 'chainIdExistsErrorMsg') {
+        return false;
+      }
+      return error.key && error.msg;
+    });
+  };
+
+  validateChainIdOnChange = (selfRpcUrl, chainIdArg = '') => {
    const { networksToRender } = this.props;
    const chainId = chainIdArg.trim();
+    let errorKey = '';
    let errorMessage = '';
    let radix = 10;
    const hexChainId = chainId.startsWith('0x')
      ? chainId
      : `0x${decimalToHex(chainId)}`;
    const [matchingChainId] = networksToRender.filter(
-      (e) => e.chainId === hexChainId,
+      (e) => e.chainId === hexChainId && e.rpcUrl !== selfRpcUrl,
    );

    if (chainId === '') {
-      this.setErrorTo('chainId', '');
+      this.setErrorEmpty('chainId');
      return;
    } else if (matchingChainId) {
+      errorKey = 'chainIdExistsErrorMsg';
      errorMessage = this.context.t('chainIdExistsErrorMsg', [
        matchingChainId.label ?? matchingChainId.labelKey,
      ]);
    } else if (chainId.startsWith('0x')) {
      radix = 16;
      if (!/^0x[0-9a-f]+$/iu.test(chainId)) {
+        errorKey = 'invalidHexNumber';
        errorMessage = this.context.t('invalidHexNumber');
      } else if (!isPrefixedFormattedHexString(chainId)) {
        errorMessage = this.context.t('invalidHexNumberLeadingZeros');
      }
    } else if (!/^[0-9]+$/u.test(chainId)) {
+      errorKey = 'invalidNumber';
      errorMessage = this.context.t('invalidNumber');
    } else if (chainId.startsWith('0')) {
+      errorKey = 'invalidNumberLeadingZeros';
      errorMessage = this.context.t('invalidNumberLeadingZeros');
    } else if (!isSafeChainId(parseInt(chainId, radix))) {
+      errorKey = 'invalidChainIdTooBig';
      errorMessage = this.context.t('invalidChainIdTooBig');
    }

-    this.setErrorTo('chainId', errorMessage);
+    this.setErrorTo('chainId', {
+      key: errorKey,
+      msg: errorMessage,
+    });
  };

  /**
@ -381,6 +415,7 @@ export default class NetworkForm extends PureComponent {
   */
  validateChainIdOnSubmit = async (formChainId, parsedChainId, rpcUrl) => {
    const { t } = this.context;
+    let errorKey;
    let errorMessage;
    let endpointChainId;
    let providerError;
@ -393,6 +428,7 @@ export default class NetworkForm extends PureComponent {
    }

    if (providerError || typeof endpointChainId !== 'string') {
+      errorKey = 'failedToFetchChainId';
      errorMessage = t('failedToFetchChainId');
    } else if (parsedChainId !== endpointChainId) {
      // Here, we are in an error state. The endpoint should always return a
@ -410,6 +446,7 @@ export default class NetworkForm extends PureComponent {
        }
      }

+      errorKey = 'endpointReturnedDifferentChainId';
      errorMessage = t('endpointReturnedDifferentChainId', [
        endpointChainId.length <= 12
          ? endpointChainId
@ -417,12 +454,15 @@ export default class NetworkForm extends PureComponent {
      ]);
    }

-    if (errorMessage) {
-      this.setErrorTo('chainId', errorMessage);
+    if (errorKey) {
+      this.setErrorTo('chainId', {
+        key: errorKey,
+        msg: errorMessage,
+      });
      return false;
    }

-    this.setErrorTo('chainId', '');
+    this.setErrorEmpty('chainId');
    return true;
  };

@ -432,17 +472,25 @@ export default class NetworkForm extends PureComponent {
  };

  validateBlockExplorerURL = (url, stateKey) => {
+    const { t } = this.context;
    if (!validUrl.isWebUri(url) && url !== '') {
-      this.setErrorTo(
-        stateKey,
-        this.context.t(
-          this.isValidWhenAppended(url)
-            ? 'urlErrorMsg'
-            : 'invalidBlockExplorerURL',
-        ),
-      );
+      let errorKey;
+      let errorMessage;
+
+      if (this.isValidWhenAppended(url)) {
+        errorKey = 'urlErrorMsg';
+        errorMessage = t('urlErrorMsg');
+      } else {
+        errorKey = 'invalidBlockExplorerURL';
+        errorMessage = t('invalidBlockExplorerURL');
+      }
+
+      this.setErrorTo(stateKey, {
+        key: errorKey,
+        msg: errorMessage,
+      });
    } else {
-      this.setErrorTo(stateKey, '');
+      this.setErrorEmpty(stateKey);
    }
  };

@ -451,28 +499,32 @@ export default class NetworkForm extends PureComponent {
    const { networksToRender } = this.props;
    const { chainId: stateChainId } = this.state;
    const isValidUrl = validUrl.isWebUri(url);
-    const chainIdFetchFailed = this.hasError(
-      'chainId',
-      t('failedToFetchChainId'),
-    );
+    const chainIdFetchFailed = this.hasError('chainId', 'failedToFetchChainId');
    const [matchingRPCUrl] = networksToRender.filter((e) => e.rpcUrl === url);

    if (!isValidUrl && url !== '') {
-      this.setErrorTo(
-        stateKey,
-        this.context.t(
-          this.isValidWhenAppended(url) ? 'urlErrorMsg' : 'invalidRPC',
-        ),
-      );
+      let errorKey;
+      let errorMessage;
+      if (this.isValidWhenAppended(url)) {
+        errorKey = 'urlErrorMsg';
+        errorMessage = t('urlErrorMsg');
+      } else {
+        errorKey = 'invalidRPC';
+        errorMessage = t('invalidRPC');
+      }
+      this.setErrorTo(stateKey, {
+        key: errorKey,
+        msg: errorMessage,
+      });
    } else if (matchingRPCUrl) {
-      this.setErrorTo(
-        stateKey,
-        this.context.t('urlExistsErrorMsg', [
+      this.setErrorTo(stateKey, {
+        key: 'urlExistsErrorMsg',
+        msg: t('urlExistsErrorMsg', [
          matchingRPCUrl.label ?? matchingRPCUrl.labelKey,
        ]),
-      );
+      });
    } else {
-      this.setErrorTo(stateKey, '');
+      this.setErrorEmpty(stateKey);
    }

    // Re-validate the chain id if it could not be found with previous rpc url
@ -501,18 +553,16 @@ export default class NetworkForm extends PureComponent {
      chainId = '',
      ticker,
      blockExplorerUrl,
-      errors,
    } = this.state;

    const deletable =
      !networksTabIsInAddMode && !isCurrentRpcTarget && !viewOnly;
-
    const isSubmitDisabled =
+      this.hasErrors() ||
      this.isSubmitting() ||
      this.stateIsUnchanged() ||
      !rpcUrl ||
-      !chainId ||
-      Object.values(errors).some((x) => x);
+      !chainId;

    return (
      <div className="networks-tab__network-form">
@ -535,7 +585,7 @@ export default class NetworkForm extends PureComponent {
          textFieldId: 'chainId',
          onChange: this.setStateWithValue(
            'chainId',
-            this.validateChainIdOnChange,
+            this.validateChainIdOnChange.bind(this, rpcUrl),
          ),
          value: chainId,
          tooltipText: viewOnly ? null : t('networkSettingsChainIdDescription'),