Increase friction to bypass estimated revert (#12576)

* If a transaction would revert/fail, 1. hide the gas estimate info. 2. Disable the confirm button. 3. All user to enable the confirm button anyways. 4. Do not show the default Transaction error message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Always return a value for hasSimulationError Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Use primary button of action message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Remove hasSimulationError from getErrorKey Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Move confirm anyways logic to base component. Change message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Disable edit if there's simulation error Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * hide confirm anyways button once clicked. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Move Actionable Primary Action to flex-end Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Fix unit tests Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Fix nested ternary lint issues Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * add !confirmAnyways to conditions to show GasDetails. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * ConfirmAnyways should be read from state Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Rename tryAnywayOption Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Remove await tick Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint issue fix Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes after rebase Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * description should show that it's content being tested. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * If a transaction would revert/fail, 1. hide the gas estimate info. 2. Disable the confirm button. 3. All user to enable the confirm button anyways. 4. Do not show the default Transaction error message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Always return a value for hasSimulationError Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Use primary button of action message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Remove hasSimulationError from getErrorKey Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Move confirm anyways logic to base component. Change message Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Disable edit if there's simulation error Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * hide confirm anyways button once clicked. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Move Actionable Primary Action to flex-end Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Fix unit tests Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Fix nested ternary lint issues Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * add !confirmAnyways to conditions to show GasDetails. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * ConfirmAnyways should be read from state Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Rename tryAnywayOption Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Remove await tick Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint issue fix Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Lint fixes after rebase Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * description should show that it's content being tested. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * Move simulation fails message inline with gas details component (#12705) * Move simulation fails message inline with gas details component * Remove old unit tests Co-authored-by: Dan Miller <danjm.com@gmail.com> * lint fix * use an XOR operation. Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> * The changes in this file are no longer needed because we hide the edit button now Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com> Co-authored-by: Dan Miller <danjm.com@gmail.com>
3 years ago · 143a5c4a53
parent 6838a3d074
commit 143a5c4a53
6 changed files with 363 additions and 150 deletions
--- a/app/_locales/en/messages.json
+++ b/app/_locales/en/messages.json
@ -2314,6 +2314,9 @@
  "signed": {
    "message": "Signed"
  },
+  "simulationErrorMessage": {
+    "message": "This transaction is expected to fail. Trying to execute it is expected to be expensive but fail, and is not recommended."
+  },
  "skip": {
    "message": "Skip"
  },
@ -2977,6 +2980,9 @@
  "tryAgain": {
    "message": "Try again"
  },
+  "tryAnywayOption": {
+    "message": "I will try anyway"
+  },
  "turnOnTokenDetection": {
    "message": "Turn on enhanced token detection"
  },
--- a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js
+++ b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js
@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import classnames from 'classnames';
 import { Tabs, Tab } from '../../../ui/tabs';
 import ErrorMessage from '../../../ui/error-message';
+import ActionableMessage from '../../../ui/actionable-message/actionable-message';
 import { PageContainerFooter } from '../../../ui/page-container';
 import { ConfirmPageContainerSummary, ConfirmPageContainerWarning } from '.';

@ -17,6 +18,7 @@ export default class ConfirmPageContainerContent extends Component {
    detailsComponent: PropTypes.node,
    errorKey: PropTypes.string,
    errorMessage: PropTypes.string,
+    hasSimulationError: PropTypes.bool,
    hideSubtitle: PropTypes.bool,
    identiconAddress: PropTypes.string,
    nonce: PropTypes.string,
@ -31,8 +33,10 @@ export default class ConfirmPageContainerContent extends Component {
    onCancel: PropTypes.func,
    cancelText: PropTypes.string,
    onSubmit: PropTypes.func,
+    onConfirmAnyways: PropTypes.func,
    submitText: PropTypes.string,
    disabled: PropTypes.bool,
+    hideConfirmAnyways: PropTypes.bool,
    unapprovedTxCount: PropTypes.number,
    rejectNText: PropTypes.string,
    hideTitle: PropTypes.boolean,
@ -71,6 +75,7 @@ export default class ConfirmPageContainerContent extends Component {
      action,
      errorKey,
      errorMessage,
+      hasSimulationError,
      title,
      titleComponent,
      subtitleComponent,
@ -91,14 +96,32 @@ export default class ConfirmPageContainerContent extends Component {
      origin,
      ethGasPriceWarning,
      hideTitle,
+      onConfirmAnyways,
+      hideConfirmAnyways,
    } = this.props;

+    const primaryAction = hideConfirmAnyways
+      ? null
+      : {
+          label: this.context.t('tryAnywayOption'),
+          onClick: onConfirmAnyways,
+        };
+
    return (
      <div className="confirm-page-container-content">
        {warning ? <ConfirmPageContainerWarning warning={warning} /> : null}
        {ethGasPriceWarning && (
          <ConfirmPageContainerWarning warning={ethGasPriceWarning} />
        )}
+        {hasSimulationError && (
+          <div className="confirm-page-container-content__error-container">
+            <ActionableMessage
+              type="danger"
+              primaryAction={primaryAction}
+              message={this.context.t('simulationErrorMessage')}
+            />
+          </div>
+        )}
        <ConfirmPageContainerSummary
          className={classnames({
            'confirm-page-container-summary--border':
@ -115,7 +138,7 @@ export default class ConfirmPageContainerContent extends Component {
          hideTitle={hideTitle}
        />
        {this.renderContent()}
-        {(errorKey || errorMessage) && (
+        {(errorKey || errorMessage) && !hasSimulationError && (
          <div className="confirm-page-container-content__error-container">
            <ErrorMessage errorMessage={errorMessage} errorKey={errorKey} />
          </div>
--- a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.test.js
+++ b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.test.js
@ -0,0 +1,124 @@
+import { fireEvent } from '@testing-library/react';
+import React from 'react';
+import configureMockStore from 'redux-mock-store';
+import { renderWithProvider } from '../../../../../test/lib/render-helpers';
+import { TRANSACTION_ERROR_KEY } from '../../../../helpers/constants/error-keys';
+import ConfirmPageContainerContent from './confirm-page-container-content.component';
+
+describe('Confirm Page Container Content', () => {
+  const mockStore = {
+    metamask: {
+      provider: {
+        type: 'test',
+      },
+    },
+  };
+
+  const store = configureMockStore()(mockStore);
+
+  let props = {};
+
+  beforeEach(() => {
+    const mockOnCancel = jest.fn();
+    const mockOnCancelAll = jest.fn();
+    const mockOnSubmit = jest.fn();
+    const mockOnConfirmAnyways = jest.fn();
+    props = {
+      action: ' Withdraw Stake',
+      errorMessage: null,
+      errorKey: null,
+      hasSimulationError: true,
+      onCancelAll: mockOnCancelAll,
+      onCancel: mockOnCancel,
+      cancelText: 'Reject',
+      onSubmit: mockOnSubmit,
+      onConfirmAnyways: mockOnConfirmAnyways,
+      submitText: 'Confirm',
+      disabled: true,
+      origin: 'http://localhost:4200',
+      hideTitle: false,
+    };
+  });
+
+  it('render ConfirmPageContainer component with simulation error', async () => {
+    const { queryByText, getByText } = renderWithProvider(
+      <ConfirmPageContainerContent {...props} />,
+      store,
+    );
+
+    expect(
+      queryByText('Transaction Error. Exception thrown in contract code.'),
+    ).not.toBeInTheDocument();
+    expect(
+      queryByText(
+        'This transaction is expected to fail. Trying to execute it is expected to be expensive but fail, and is not recommended.',
+      ),
+    ).toBeInTheDocument();
+    expect(queryByText('I will try anyway')).toBeInTheDocument();
+
+    const confirmButton = getByText('Confirm');
+    expect(getByText('Confirm').closest('button')).toBeDisabled();
+    fireEvent.click(confirmButton);
+    expect(props.onSubmit).toHaveBeenCalledTimes(0);
+
+    const iWillTryButton = getByText('I will try anyway');
+    fireEvent.click(iWillTryButton);
+    expect(props.onConfirmAnyways).toHaveBeenCalledTimes(1);
+
+    const cancelButton = getByText('Reject');
+    fireEvent.click(cancelButton);
+    expect(props.onCancel).toHaveBeenCalledTimes(1);
+  });
+
+  it('render ConfirmPageContainer component with another error', async () => {
+    props.hasSimulationError = false;
+    props.disabled = true;
+    props.errorKey = TRANSACTION_ERROR_KEY;
+    const { queryByText, getByText } = renderWithProvider(
+      <ConfirmPageContainerContent {...props} />,
+      store,
+    );
+
+    expect(
+      queryByText(
+        'This transaction is expected to fail. Trying to execute it is expected to be expensive but fail, and is not recommended.',
+      ),
+    ).not.toBeInTheDocument();
+    expect(queryByText('I will try anyway')).not.toBeInTheDocument();
+    expect(getByText('Confirm').closest('button')).toBeDisabled();
+    expect(
+      getByText('Transaction Error. Exception thrown in contract code.'),
+    ).toBeInTheDocument();
+
+    const cancelButton = getByText('Reject');
+    fireEvent.click(cancelButton);
+    expect(props.onCancel).toHaveBeenCalledTimes(1);
+  });
+
+  it('render ConfirmPageContainer component with no errors', async () => {
+    props.hasSimulationError = false;
+    props.disabled = false;
+    const { queryByText, getByText } = renderWithProvider(
+      <ConfirmPageContainerContent {...props} />,
+      store,
+    );
+
+    expect(
+      queryByText(
+        'This transaction is expected to fail. Trying to execute it is expected to be expensive but fail, and is not recommended.',
+      ),
+    ).not.toBeInTheDocument();
+    expect(
+      queryByText('Transaction Error. Exception thrown in contract code.'),
+    ).not.toBeInTheDocument();
+    expect(queryByText('I will try anyway')).not.toBeInTheDocument();
+
+    const confirmButton = getByText('Confirm');
+    fireEvent.click(confirmButton);
+    expect(props.onSubmit).toHaveBeenCalledTimes(1);
+
+    const cancelButton = getByText('Reject');
+    fireEvent.click(cancelButton);
+    expect(props.onCancel).toHaveBeenCalledTimes(1);
+  });
+});
--- a/ui/components/ui/actionable-message/actionable-message.js
+++ b/ui/components/ui/actionable-message/actionable-message.js
@ -26,6 +26,7 @@ export default function ActionableMessage({
  type = 'default',
  useIcon = false,
  iconFillColor = '',
+  roundedButtons,
 }) {
  const actionableMessageClassName = classnames(
    'actionable-message',
@ -35,6 +36,9 @@ export default function ActionableMessage({
    { 'actionable-message--with-icon': useIcon },
  );

+  const onlyOneAction =
+    (primaryAction && !secondaryAction) || (secondaryAction && !primaryAction);
+
  return (
    <div className={actionableMessageClassName}>
      {useIcon ? <InfoTooltipIcon fillColor={iconFillColor} /> : null}
@ -47,12 +51,19 @@ export default function ActionableMessage({
      )}
      <div className="actionable-message__message">{message}</div>
      {(primaryAction || secondaryAction) && (
-        <div className="actionable-message__actions">
+        <div
+          className={classnames('actionable-message__actions', {
+            'actionable-message__actions--single': onlyOneAction,
+          })}
+        >
          {primaryAction && (
            <button
              className={classnames(
                'actionable-message__action',
                'actionable-message__action--primary',
+                {
+                  'actionable-message__action--rounded': roundedButtons,
+                },
              )}
              onClick={primaryAction.onClick}
            >
@ -64,6 +75,9 @@ export default function ActionableMessage({
              className={classnames(
                'actionable-message__action',
                'actionable-message__action--secondary',
+                {
+                  'actionable-message__action--rounded': roundedButtons,
+                },
              )}
              onClick={secondaryAction.onClick}
            >
@ -92,4 +106,5 @@ ActionableMessage.propTypes = {
  infoTooltipText: PropTypes.string,
  useIcon: PropTypes.bool,
  iconFillColor: PropTypes.string,
+  roundedButtons: PropTypes.bool,
 };
--- a/ui/components/ui/actionable-message/index.scss
+++ b/ui/components/ui/actionable-message/index.scss
@ -38,14 +38,22 @@
  &__actions {
    display: flex;
    width: 80%;
-    justify-content: space-evenly;
+    justify-content: flex-end;
    align-items: center;
    margin-top: 10px;
    color: $Blue-600;
+
+    &--single {
+      width: 100%;
+    }
  }

  &__action {
    font-weight: bold;
+
+    &--rounded {
+      border-radius: 8px;
+    }
  }

  &__info-tooltip-wrapper {
--- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js
+++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js
@ -13,7 +13,6 @@ import {
 } from '../../helpers/constants/routes';
 import {
  INSUFFICIENT_FUNDS_ERROR_KEY,
-  TRANSACTION_ERROR_KEY,
  GAS_LIMIT_TOO_LOW_ERROR_KEY,
  ETH_GAS_PRICE_FETCH_WARNING_KEY,
  GAS_PRICE_FETCH_FAILURE_ERROR_KEY,
@ -21,6 +20,7 @@ import {
 import UserPreferencedCurrencyDisplay from '../../components/app/user-preferenced-currency-display';
 import { PRIMARY, SECONDARY } from '../../helpers/constants/common';
 import TextField from '../../components/ui/text-field';
+import ActionableMessage from '../../components/ui/actionable-message';
 import {
  TRANSACTION_TYPES,
  TRANSACTION_STATUSES,
@ -143,6 +143,7 @@ export default class ConfirmTransactionBase extends Component {
    submitWarning: '',
    ethGasPriceWarning: '',
    editingGas: false,
+    confirmAnyways: false,
  };

  componentDidUpdate(prevProps) {
@ -216,7 +217,7 @@ export default class ConfirmTransactionBase extends Component {
      balance,
      conversionRate,
      hexMaximumTransactionFee,
-      txData: { simulationFails, txParams: { value: amount } = {} } = {},
+      txData: { txParams: { value: amount } = {} } = {},
      customGas,
      noGasPrice,
      gasFeeIsCustom,
@ -245,15 +246,6 @@ export default class ConfirmTransactionBase extends Component {
      };
    }

-    if (simulationFails) {
-      return {
-        valid: true,
-        errorKey: simulationFails.errorKey
-          ? simulationFails.errorKey
-          : TRANSACTION_ERROR_KEY,
-      };
-    }
-
    if (noGasPrice && !gasFeeIsCustom) {
      return {
        valid: false,
@ -296,6 +288,10 @@ export default class ConfirmTransactionBase extends Component {
    this.setState({ editingGas: false });
  }

+  handleConfirmAnyways() {
+    this.setState({ confirmAnyways: true });
+  }
+
  renderDetails() {
    const {
      primaryTotalTextOverride,
@ -321,6 +317,15 @@ export default class ConfirmTransactionBase extends Component {
    } = this.props;
    const { t } = this.context;

+    const { valid } = this.getErrorKey();
+    const isDisabled = () => {
+      return this.state.confirmAnyways ? false : !valid;
+    };
+
+    const hasSimulationError = Boolean(txData.simulationFails);
+    const renderSimulationFailureWarning =
+      hasSimulationError && !this.state.confirmAnyways;
+
    const renderTotalMaxAmount = () => {
      if (
        primaryTotalTextOverrideMaxAmount === undefined &&
@ -411,13 +416,8 @@ export default class ConfirmTransactionBase extends Component {
      </div>
    ) : null;

-    return (
-      <div className="confirm-page-container-content__details">
-        {EIP_1559_V2 && <LowPriorityMessage />}
-        <TransactionDetail
-          onEdit={() => this.handleEditGas()}
-          rows={[
-            EIP_1559_V2 ? (
+    const renderGasDetailsItem = () => {
+      return EIP_1559_V2 ? (
        <GasDetailsItem
          key="gas_details"
          hexMaximumTransactionFee={hexMaximumTransactionFee}
@ -506,9 +506,7 @@ export default class ConfirmTransactionBase extends Component {
          subText={
            !isMultiLayerFeeNetwork &&
            t('editGasSubTextFee', [
-                    <b key="editGasSubTextFeeLabel">
-                      {t('editGasSubTextFeeLabel')}
-                    </b>,
+              <b key="editGasSubTextFeeLabel">{t('editGasSubTextFeeLabel')}</b>,
              <div
                key="editGasSubTextFeeValue"
                className="confirm-page-container-content__currency-container"
@ -550,8 +548,35 @@ export default class ConfirmTransactionBase extends Component {
            </>
          }
        />
-            ),
-            isMultiLayerFeeNetwork && (
+      );
+    };
+
+    const simulationFailureWarning = () => (
+      <div className="confirm-page-container-content__error-container">
+        <ActionableMessage
+          type="danger"
+          primaryAction={{
+            label: this.context.t('tryAnywayOption'),
+            onClick: () => this.handleConfirmAnyways(),
+          }}
+          message={this.context.t('simulationErrorMessage')}
+          roundedButtons
+        />
+      </div>
+    );
+
+    return (
+      <div className="confirm-page-container-content__details">
+        {EIP_1559_V2 && <LowPriorityMessage />}
+        <TransactionDetail
+          disabled={isDisabled()}
+          onEdit={
+            renderSimulationFailureWarning ? null : () => this.handleEditGas()
+          }
+          rows={[
+            renderSimulationFailureWarning && simulationFailureWarning(),
+            !renderSimulationFailureWarning && renderGasDetailsItem(),
+            !renderSimulationFailureWarning && isMultiLayerFeeNetwork && (
              <MultiLayerFeeMessage
                transaction={txData}
                layer2fee={hexMinimumTransactionFee}
@ -918,10 +943,14 @@ export default class ConfirmTransactionBase extends Component {
      submitWarning,
      ethGasPriceWarning,
      editingGas,
+      confirmAnyways,
    } = this.state;

    const { name } = methodData;
    const { valid, errorKey } = this.getErrorKey();
+    const hasSimulationError = Boolean(txData.simulationFails);
+    const renderSimulationFailureWarning =
+      hasSimulationError && !confirmAnyways;
    const {
      totalTx,
      positionOfCurrentTx,
@ -934,6 +963,10 @@ export default class ConfirmTransactionBase extends Component {
      requestsWaitingText,
    } = this.getNavigateTxData();

+    const isDisabled = () => {
+      return confirmAnyways ? false : !valid;
+    };
+
    let functionType = getMethodName(name);
    if (!functionType) {
      if (type) {
@ -965,6 +998,7 @@ export default class ConfirmTransactionBase extends Component {
        identiconAddress={identiconAddress}
        errorMessage={submitError}
        errorKey={errorKey}
+        hasSimulationError={hasSimulationError}
        warning={submitWarning}
        totalTx={totalTx}
        positionOfCurrentTx={positionOfCurrentTx}
@ -976,7 +1010,9 @@ export default class ConfirmTransactionBase extends Component {
        lastTx={lastTx}
        ofText={ofText}
        requestsWaitingText={requestsWaitingText}
+        hideConfirmAnyways={!isDisabled()}
        disabled={
+          renderSimulationFailureWarning ||
          !valid ||
          submitting ||
          hardwareWalletRequiresConnection ||
@ -986,6 +1022,7 @@ export default class ConfirmTransactionBase extends Component {
        onCancelAll={() => this.handleCancelAll()}
        onCancel={() => this.handleCancel()}
        onSubmit={() => this.handleSubmit()}
+        onConfirmAnyways={() => this.handleConfirmAnyways()}
        hideSenderToRecipient={hideSenderToRecipient}
        origin={txData.origin}
        ethGasPriceWarning={ethGasPriceWarning}