fix: finding and updating transaction errors in confirmation pages st…

…ate (#7319)
MetaMask · Oct 17, 2023 · 9a37d5a · 9a37d5a
1 parent b3f0e31
commit 9a37d5a
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 23 deletions.
diff --git a/app/components/UI/TransactionEditor/index.js b/app/components/UI/TransactionEditor/index.js
@@ -136,6 +136,8 @@ class TransactionEditor extends PureComponent {
     toFocused: false,
     ensRecipient: undefined,
     ready: false,
+    // here error is defaulted to true until its confirmed that there is no error
+    error: true,
     data: undefined,
     amountError: '',
     toAddressError: '',
@@ -151,7 +153,7 @@ class TransactionEditor extends PureComponent {
     suggestedMaxFeePerGas: undefined,
   };
 
-  computeGasEstimates = (gasEstimateTypeChanged) => {
+  computeGasEstimates = async (gasEstimateTypeChanged) => {
     const {
       transaction,
       gasEstimateType,
@@ -213,6 +215,7 @@ class TransactionEditor extends PureComponent {
         });
       }
 
+      await this.validate(EIP1559GasData);
       // eslint-disable-next-line react/no-did-update-set-state
       this.setState(
         {
@@ -264,6 +267,7 @@ class TransactionEditor extends PureComponent {
         });
       }
 
+      await this.validate(undefined, LegacyGasData);
       // eslint-disable-next-line react/no-did-update-set-state
       this.setState(
         {
@@ -397,6 +401,14 @@ class TransactionEditor extends PureComponent {
         this.computeGasEstimates(gasEstimateTypeChanged);
       }
     }
+
+    if (
+      prevProps.transaction !== this.props.transaction ||
+      prevProps.selectedAddress !== this.props.selectedAddress ||
+      prevProps.contractBalances !== this.props.contractBalances
+    ) {
+      this.validate();
+    }
   };
 
   componentWillUnmount = () => {
@@ -609,7 +621,7 @@ class TransactionEditor extends PureComponent {
     this.props?.onModeChange(REVIEW);
   };
 
-  validate = async () => {
+  validate = async (EIP1559GasData, LegacyGasData) => {
     const {
       transaction: {
         assetType,
@@ -621,7 +633,9 @@ class TransactionEditor extends PureComponent {
     } = this.props;
 
     const totalError = this.validateTotal(
-      this.state.EIP1559GasData.totalMaxHex ||
+      EIP1559GasData?.totalMaxHex ||
+        this.state.EIP1559GasData.totalMaxHex ||
+        LegacyGasData?.totalHex ||
         this.state.LegacyGasData.totalHex,
     );
     const amountError = await validateAmount(
@@ -634,7 +648,11 @@ class TransactionEditor extends PureComponent {
       false,
     );
     const toAddressError = this.validateToAddress();
-    this.setState({ amountError: totalError || amountError, toAddressError });
+    this.setState({
+      amountError: totalError || amountError,
+      toAddressError,
+      error: totalError || amountError || toAddressError,
+    });
     return totalError || amountError || toAddressError;
   };
 
@@ -800,6 +818,7 @@ class TransactionEditor extends PureComponent {
     } = this.props;
     const {
       ready,
+      error,
       over,
       EIP1559GasData,
       EIP1559GasDataTemp,
@@ -837,8 +856,8 @@ class TransactionEditor extends PureComponent {
               <TransactionReview
                 onCancel={this.onCancel}
                 onConfirm={this.onConfirm}
-                validate={this.validate}
                 ready={ready}
+                error={error}
                 gasSelected={gasSelected}
                 transactionConfirmed={transactionConfirmed}
                 over={over}

diff --git a/app/components/UI/TransactionReview/index.js b/app/components/UI/TransactionReview/index.js
@@ -156,10 +156,6 @@ class TransactionReview extends PureComponent {
      * Transaction object associated with this transaction
      */
     transaction: PropTypes.object,
-    /**
-     * Callback to validate transaction in parent state
-     */
-    validate: PropTypes.func,
     /**
      * Browser/tab information
      */
@@ -192,6 +188,10 @@ class TransactionReview extends PureComponent {
      * ETH or fiat, depending on user setting
      */
     primaryCurrency: PropTypes.string,
+    /**
+     * Error blockaid transaction execution, undefined value signifies no error.
+     */
+    error: PropTypes.oneOf[(PropTypes.bool, PropTypes.string)],
     /**
      * Whether or not basic gas estimates have been fetched
      */
@@ -265,7 +265,6 @@ class TransactionReview extends PureComponent {
     actionKey: strings('transactions.tx_review_confirm'),
     showHexData: false,
     dataVisible: false,
-    error: undefined,
     assetAmount: undefined,
     conversionRate: undefined,
     fiatValue: undefined,
@@ -300,13 +299,11 @@ class TransactionReview extends PureComponent {
   componentDidMount = async () => {
     const {
       accounts,
-      validate,
       transaction,
       transaction: { data, to, value, from },
       tokens,
       chainId,
       tokenList,
-      ready,
     } = this.props;
     let { showHexData } = this.props;
     let assetAmount, conversionRate, fiatValue;
@@ -315,7 +312,6 @@ class TransactionReview extends PureComponent {
       data &&
       data.substr(0, 10) === APPROVE_FUNCTION_SIGNATURE &&
       (!value || isZeroValue(value));
-    const error = ready && validate && (await validate());
     const actionKey = await getTransactionReviewActionKey(transaction, chainId);
     if (approveTransaction) {
       let contract = tokenList[safeToChecksumAddress(to)];
@@ -337,7 +333,6 @@ class TransactionReview extends PureComponent {
     );
 
     this.setState({
-      error,
       actionKey,
       showHexData,
       assetAmount,
@@ -377,14 +372,6 @@ class TransactionReview extends PureComponent {
     clearInterval(intervalIdForEstimatedL1Fee);
   };
 
-  async componentDidUpdate(prevProps) {
-    if (this.props.ready !== prevProps.ready) {
-      const error = this.props.validate && (await this.props.validate());
-      // eslint-disable-next-line react/no-did-update-set-state
-      this.setState({ error });
-    }
-  }
-
   getRenderValues = () => {
     const {
       transaction: { value, selectedAsset, assetType },
@@ -500,10 +487,10 @@ class TransactionReview extends PureComponent {
       chainId,
       transaction,
       transaction: { to, origin, from, ensRecipient, securityAlertResponse },
+      error,
     } = this.props;
     const {
       actionKey,
-      error,
       assetAmount,
       conversionRate,
       fiatValue,

diff --git a/app/components/UI/TransactionReview/index.test.tsx b/app/components/UI/TransactionReview/index.test.tsx
@@ -231,4 +231,21 @@ describe('TransactionReview', () => {
     const confirmButton = getByRole('button', { name: 'Confirm' });
     expect(confirmButton.props.disabled).toBe(true);
   });
+
+  it('should have confirm button disabled if error is defined', async () => {
+    jest.mock('react-redux', () => ({
+      ...jest.requireActual('react-redux'),
+      useSelector: (fn: any) => fn(mockState),
+    }));
+    const { getByRole } = renderWithProvider(
+      <TransactionReview
+        EIP1559GasData={{}}
+        generateTransform={generateTransform}
+        error="You need 1 more ETH to complete the transaction"
+      />,
+      { state: mockState },
+    );
+    const confirmButton = getByRole('button', { name: 'Confirm' });
+    expect(confirmButton.props.disabled).toBe(true);
+  });
 });