From 143a5c4a53127a7783c97417a4c746e6d9afd8cd Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Mon, 22 Nov 2021 16:13:30 +0400 Subject: [PATCH] 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 * Always return a value for hasSimulationError Signed-off-by: Akintayo A. Olusegun * Use primary button of action message Signed-off-by: Akintayo A. Olusegun * Remove hasSimulationError from getErrorKey Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun * Move confirm anyways logic to base component. Change message Signed-off-by: Akintayo A. Olusegun * Disable edit if there's simulation error Signed-off-by: Akintayo A. Olusegun * hide confirm anyways button once clicked. Signed-off-by: Akintayo A. Olusegun * Move Actionable Primary Action to flex-end Signed-off-by: Akintayo A. Olusegun * Fix unit tests Signed-off-by: Akintayo A. Olusegun * Fix nested ternary lint issues Signed-off-by: Akintayo A. Olusegun * add !confirmAnyways to conditions to show GasDetails. Signed-off-by: Akintayo A. Olusegun * ConfirmAnyways should be read from state Signed-off-by: Akintayo A. Olusegun * Rename tryAnywayOption Signed-off-by: Akintayo A. Olusegun * Lint fixes Signed-off-by: Akintayo A. Olusegun * Remove await tick Signed-off-by: Akintayo A. Olusegun * Lint issue fix Signed-off-by: Akintayo A. Olusegun * Lint fixes after rebase Signed-off-by: Akintayo A. Olusegun * description should show that it's content being tested. Signed-off-by: Akintayo A. Olusegun * 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 * Always return a value for hasSimulationError Signed-off-by: Akintayo A. Olusegun * Use primary button of action message Signed-off-by: Akintayo A. Olusegun * Remove hasSimulationError from getErrorKey Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun * Move confirm anyways logic to base component. Change message Signed-off-by: Akintayo A. Olusegun * Disable edit if there's simulation error Signed-off-by: Akintayo A. Olusegun * hide confirm anyways button once clicked. Signed-off-by: Akintayo A. Olusegun * Move Actionable Primary Action to flex-end Signed-off-by: Akintayo A. Olusegun * Fix unit tests Signed-off-by: Akintayo A. Olusegun * Fix nested ternary lint issues Signed-off-by: Akintayo A. Olusegun * add !confirmAnyways to conditions to show GasDetails. Signed-off-by: Akintayo A. Olusegun * ConfirmAnyways should be read from state Signed-off-by: Akintayo A. Olusegun * Rename tryAnywayOption Signed-off-by: Akintayo A. Olusegun * Lint fixes Signed-off-by: Akintayo A. Olusegun * Remove await tick Signed-off-by: Akintayo A. Olusegun * Lint issue fix Signed-off-by: Akintayo A. Olusegun * Lint fixes after rebase Signed-off-by: Akintayo A. Olusegun * description should show that it's content being tested. Signed-off-by: Akintayo A. Olusegun * 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 * lint fix * use an XOR operation. Signed-off-by: Akintayo A. Olusegun * The changes in this file are no longer needed because we hide the edit button now Signed-off-by: Akintayo A. Olusegun Co-authored-by: Dan Miller --- app/_locales/en/messages.json | 6 + ...onfirm-page-container-content.component.js | 25 +- ...m-page-container-content.component.test.js | 124 +++++++ .../actionable-message/actionable-message.js | 17 +- .../ui/actionable-message/index.scss | 10 +- .../confirm-transaction-base.component.js | 331 ++++++++++-------- 6 files changed, 363 insertions(+), 150 deletions(-) create mode 100644 ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 36654de69..adcc8298f 100644 --- 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" }, diff --git 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 index 574967420..42fe0c744 100644 --- 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 (
{warning ? : null} {ethGasPriceWarning && ( )} + {hasSimulationError && ( +
+ +
+ )} {this.renderContent()} - {(errorKey || errorMessage) && ( + {(errorKey || errorMessage) && !hasSimulationError && (
diff --git 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 new file mode 100644 index 000000000..edd7f7bd5 --- /dev/null +++ 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( + , + 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( + , + 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( + , + 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); + }); +}); diff --git a/ui/components/ui/actionable-message/actionable-message.js b/ui/components/ui/actionable-message/actionable-message.js index 8af6646c7..9e4f4728c 100644 --- 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 (
{useIcon ? : null} @@ -47,12 +51,19 @@ export default function ActionableMessage({ )}
{message}
{(primaryAction || secondaryAction) && ( -
+
{primaryAction && (
) : null; + const renderGasDetailsItem = () => { + return EIP_1559_V2 ? ( + + ) : ( + + {isMultiLayerFeeNetwork + ? t('transactionDetailLayer2GasHeading') + : t('transactionDetailGasHeading')} + + + + + ) : ( + <> + {isMultiLayerFeeNetwork + ? t('transactionDetailLayer2GasHeading') + : t('transactionDetailGasHeading')} + +

+ {t('transactionDetailGasTooltipIntro', [ + isMainnet ? t('networkNameEthereum') : '', + ])} +

+

{t('transactionDetailGasTooltipExplanation')}

+

+ + {t('transactionDetailGasTooltipConversion')} + +

+ + } + position="top" + > + + + + ) + } + detailTitleColor={COLORS.BLACK} + detailText={ + !isMultiLayerFeeNetwork && ( +
+ {renderHeartBeatIfNotInTest()} + +
+ ) + } + detailTotal={ +
+ {renderHeartBeatIfNotInTest()} + +
+ } + subText={ + !isMultiLayerFeeNetwork && + t('editGasSubTextFee', [ + {t('editGasSubTextFeeLabel')}, +
+ {renderHeartBeatIfNotInTest()} + +
, + ]) + } + subTitle={ + <> + {txData.dappSuggestedGasFees ? ( + + {t('transactionDetailDappGasMoreInfo')} + + ) : ( + '' + )} + {supportsEIP1559 && ( + + )} + + } + /> + ); + }; + + const simulationFailureWarning = () => ( +
+ this.handleConfirmAnyways(), + }} + message={this.context.t('simulationErrorMessage')} + roundedButtons + /> +
+ ); + return (
{EIP_1559_V2 && } this.handleEditGas()} + disabled={isDisabled()} + onEdit={ + renderSimulationFailureWarning ? null : () => this.handleEditGas() + } rows={[ - EIP_1559_V2 ? ( - - ) : ( - - {isMultiLayerFeeNetwork - ? t('transactionDetailLayer2GasHeading') - : t('transactionDetailGasHeading')} - - - - - ) : ( - <> - {isMultiLayerFeeNetwork - ? t('transactionDetailLayer2GasHeading') - : t('transactionDetailGasHeading')} - -

- {t('transactionDetailGasTooltipIntro', [ - isMainnet ? t('networkNameEthereum') : '', - ])} -

-

{t('transactionDetailGasTooltipExplanation')}

-

- - {t('transactionDetailGasTooltipConversion')} - -

- - } - position="top" - > - - - - ) - } - detailTitleColor={COLORS.BLACK} - detailText={ - !isMultiLayerFeeNetwork && ( -
- {renderHeartBeatIfNotInTest()} - -
- ) - } - detailTotal={ -
- {renderHeartBeatIfNotInTest()} - -
- } - subText={ - !isMultiLayerFeeNetwork && - t('editGasSubTextFee', [ - - {t('editGasSubTextFeeLabel')} - , -
- {renderHeartBeatIfNotInTest()} - -
, - ]) - } - subTitle={ - <> - {txData.dappSuggestedGasFees ? ( - - {t('transactionDetailDappGasMoreInfo')} - - ) : ( - '' - )} - {supportsEIP1559 && ( - - )} - - } - /> - ), - isMultiLayerFeeNetwork && ( + renderSimulationFailureWarning && simulationFailureWarning(), + !renderSimulationFailureWarning && renderGasDetailsItem(), + !renderSimulationFailureWarning && isMultiLayerFeeNetwork && ( { + 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}