fix: issue in blockaid spinner for batched confirmations (#10658)

MetaMask · Aug 21, 2024 · ce9227d · ce9227d
1 parent 37e3897
commit ce9227d
Show file tree

Hide file tree

Showing 13 changed files with 102 additions and 139 deletions.
diff --git a/app/components/Views/confirmations/Approval/index.js b/app/components/Views/confirmations/Approval/index.js
@@ -51,6 +51,7 @@ import { withMetricsAwareness } from '../../../../components/hooks/useMetrics';
 import { STX_NO_HASH_ERROR } from '../../../../util/smart-transactions/smart-publish-hook';
 import { getSmartTransactionMetricsProperties } from '../../../../util/smart-transactions';
 import { selectTransactionMetrics } from '../../../../core/redux/slices/transactionMetrics';
+import { selectCurrentTransactionSecurityAlertResponse } from '../../../../selectors/confirmTransaction';
 import { selectTransactions } from '../../../../selectors/transactionController';
 import { selectShowCustomNonce } from '../../../../selectors/settings';
 import { buildTransactionParams } from '../../../../util/confirmation/transactions';
@@ -130,6 +131,11 @@ class Approval extends PureComponent {
      * Object containing transaction metrics by id
      */
     transactionMetricsById: PropTypes.object,
+
+    /**
+     * Object containing blockaid validation response for confirmation
+     */
+    securityAlertResponse: PropTypes.object,
   };
 
   state = {
@@ -306,19 +312,10 @@ class Approval extends PureComponent {
   };
 
   getBlockaidMetricsParams = () => {
-    const { transaction } = this.props;
-
-    let blockaidParams = {};
-
-    if (
-      transaction.id === transaction.currentTransactionSecurityAlertResponse?.id
-    ) {
-      blockaidParams = getBlockaidMetricsParams(
-        transaction.currentTransactionSecurityAlertResponse?.response,
-      );
-    }
-
-    return blockaidParams;
+    const { securityAlertResponse } = this.props;
+    return securityAlertResponse
+      ? getBlockaidMetricsParams(securityAlertResponse)
+      : {};
   };
 
   getAnalyticsParams = ({ gasEstimateType, gasSelected } = {}) => {
@@ -663,6 +660,7 @@ const mapStateToProps = (state) => ({
   activeTabUrl: getActiveTabUrl(state),
   shouldUseSmartTransaction: selectShouldUseSmartTransaction(state),
   transactionMetricsById: selectTransactionMetrics(state),
+  securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
 });
 
 const mapDispatchToProps = (dispatch) => ({

diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js
@@ -114,6 +114,7 @@ import { withMetricsAwareness } from '../../../../../components/hooks/useMetrics
 import {
   selectTransactionGasFeeEstimates,
   selectCurrentTransactionMetadata,
+  selectCurrentTransactionSecurityAlertResponse,
 } from '../../../../../selectors/confirmTransaction';
 import { selectGasFeeControllerEstimateType } from '../../../../../selectors/gasFeeController';
 import { createBuyNavigationDetails } from '../../../../UI/Ramp/routes/utils';
@@ -265,6 +266,10 @@ class Confirm extends PureComponent {
      * Indicates whether the transaction simulations feature is enabled
      */
     useTransactionSimulations: PropTypes.bool,
+    /**
+     * Object containing blockaid validation response for confirmation
+     */
+    securityAlertResponse: PropTypes.object,
   };
 
   state = {
@@ -1172,27 +1177,15 @@ class Confirm extends PureComponent {
   };
 
   getConfirmButtonStyles() {
-    const { transactionMeta } = this.state;
-    const { transaction } = this.props;
-    const { currentTransactionSecurityAlertResponse } = transaction;
+    const { securityAlertResponse } = this.props;
     const colors = this.context.colors || mockTheme.colors;
     const styles = createStyles(colors);
 
     let confirmButtonStyle = {};
-    if (
-      transactionMeta.id &&
-      currentTransactionSecurityAlertResponse?.id &&
-      currentTransactionSecurityAlertResponse.id === transactionMeta.id
-    ) {
-      if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Malicious
-      ) {
+    if (securityAlertResponse) {
+      if (securityAlertResponse?.result_type === ResultType.Malicious) {
         confirmButtonStyle = styles.confirmButtonError;
-      } else if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Warning
-      ) {
+      } else if (securityAlertResponse?.result_type === ResultType.Warning) {
         confirmButtonStyle = styles.confirmButtonWarning;
       }
     }
@@ -1478,8 +1471,6 @@ const mapStateToProps = (state) => ({
   gasFeeEstimates: selectTransactionGasFeeEstimates(state),
   gasEstimateType: selectGasFeeControllerEstimateType(state),
   isPaymentRequest: state.transaction.paymentRequest,
-  securityAlertResponse:
-    state.transaction.currentTransactionSecurityAlertResponse,
   isNativeTokenBuySupported: isNetworkRampNativeTokenSupported(
     selectChainId(state),
     getRampNetworks(state),
@@ -1489,6 +1480,7 @@ const mapStateToProps = (state) => ({
   transactionSimulationData:
     selectCurrentTransactionMetadata(state)?.simulationData,
   useTransactionSimulations: selectUseTransactionSimulations(state),
+  securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
 });
 
 const mapDispatchToProps = (dispatch) => ({

diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.test.tsx b/app/components/Views/confirmations/SendFlow/Confirm/index.test.tsx
@@ -56,9 +56,8 @@ const mockInitialState: DeepPartial<RootState> = {
     showHexData: true,
   },
   transaction: {
-    currentTransactionSecurityAlertResponse: {
-      id: 1,
-      response: {
+    securityAlertResponses: {
+      1: {
         result_type: 'Malicious',
         reason: 'blur_farming',
         providerRequestsCount: {},

diff --git a/app/components/Views/confirmations/components/ApproveTransactionReview/index.js b/app/components/Views/confirmations/components/ApproveTransactionReview/index.js
@@ -85,6 +85,7 @@ import {
 import { selectTokenList } from '../../../../../selectors/tokenListController';
 import { selectTokensLength } from '../../../../../selectors/tokensController';
 import { selectAccountsLength } from '../../../../../selectors/accountTrackerController';
+import { selectCurrentTransactionSecurityAlertResponse } from '../../../../../selectors/confirmTransaction';
 import Text, {
   TextVariant,
 } from '../../../../../component-library/components/Texts/Text';
@@ -283,6 +284,10 @@ class ApproveTransactionReview extends PureComponent {
      * Boolean that indicates if smart transaction should be used
      */
     shouldUseSmartTransaction: PropTypes.bool,
+    /**
+     * Object containing blockaid validation response for confirmation
+     */
+    securityAlertResponse: PropTypes.object,
   };
 
   state = {
@@ -731,23 +736,13 @@ class ApproveTransactionReview extends PureComponent {
   };
 
   getConfirmButtonState() {
-    const { transaction } = this.props;
-    const { id, currentTransactionSecurityAlertResponse } = transaction;
+    const { securityAlertResponse } = this.props;
     let confirmButtonState = ConfirmButtonState.Normal;
-    if (
-      id &&
-      currentTransactionSecurityAlertResponse?.id &&
-      currentTransactionSecurityAlertResponse.id === id
-    ) {
-      if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Malicious
-      ) {
+
+    if (securityAlertResponse) {
+      if (securityAlertResponse.result_type === ResultType.Malicious) {
         confirmButtonState = ConfirmButtonState.Error;
-      } else if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Warning
-      ) {
+      } else if (securityAlertResponse.result_type === ResultType.Warning) {
         confirmButtonState = ConfirmButtonState.Warning;
       }
     }
@@ -1306,6 +1301,7 @@ const mapStateToProps = (state) => ({
     getRampNetworks(state),
   ),
   shouldUseSmartTransaction: selectShouldUseSmartTransaction(state),
+  securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
 });
 
 const mapDispatchToProps = (dispatch) => ({

diff --git a/...ews/confirmations/components/TransactionBlockaidBanner/TransactionBlockaidBanner.test.tsx b/...ews/confirmations/components/TransactionBlockaidBanner/TransactionBlockaidBanner.test.tsx
@@ -34,9 +34,9 @@ const mockState: DeepPartial<RootState> = {
     },
   },
   transaction: {
-    currentTransactionSecurityAlertResponse: {
-      id: '123',
-      response: {
+    id: 123,
+    securityAlertResponses: {
+      123: {
         result_type: ResultType.Warning,
         reason: Reason.approvalFarming,
         block: 123,
@@ -69,23 +69,18 @@ describe('TransactionBlockaidBanner', () => {
     expect(await wrapper.queryByTestId(TESTID_ACCORDION_CONTENT)).toBeNull();
   });
 
-  it('should not render if currentTransactionSecurityAlertResponse.id is undefined', async () => {
-    const wrapper = renderWithProvider(<TransactionBlockaidBanner />, {
-      state: {
-        ...mockState,
-        transaction: {
-          currentTransactionSecurityAlertResponse: {
-            response: {
-              result_type: ResultType.Warning,
-              reason: Reason.approvalFarming,
-              block: 123,
-              req: {},
-              chainId: '0x1',
-            },
+  it('should not render if securityAlertResponses.id is undefined', async () => {
+    const wrapper = renderWithProvider(
+      <TransactionBlockaidBanner transactionId="123" />,
+      {
+        state: {
+          ...mockState,
+          transaction: {
+            securityAlertResponses: {},
           },
         },
       },
-    });
+    );
 
     expect(wrapper).toMatchSnapshot();
     expect(await wrapper.queryByTestId(TESTID_ACCORDIONHEADER)).toBeNull();

diff --git a/...ts/Views/confirmations/components/TransactionBlockaidBanner/TransactionBlockaidBanner.tsx b/...ts/Views/confirmations/components/TransactionBlockaidBanner/TransactionBlockaidBanner.tsx
@@ -1,32 +1,25 @@
 import React from 'react';
-import { TransactionBlockaidBannerProps } from './TransactionBlockaidBanner.types';
-import BlockaidBanner from '../BlockaidBanner/BlockaidBanner';
 import { useSelector } from 'react-redux';
 
+import { selectCurrentTransactionSecurityAlertResponse } from '../../../../../selectors/confirmTransaction';
+import BlockaidBanner from '../BlockaidBanner/BlockaidBanner';
+import { TransactionBlockaidBannerProps } from './TransactionBlockaidBanner.types';
+
 const TransactionBlockaidBanner = (
   bannerProps: TransactionBlockaidBannerProps,
 ) => {
   const { transactionId, ...rest } = bannerProps;
 
   const securityAlertResponse = useSelector(
-    // TODO: Replace "any" with type
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    (state: any) => state.transaction.currentTransactionSecurityAlertResponse,
+    selectCurrentTransactionSecurityAlertResponse,
   );
 
-  if (
-    !transactionId ||
-    !securityAlertResponse?.id ||
-    securityAlertResponse.id !== transactionId
-  ) {
+  if (!transactionId || !securityAlertResponse) {
     return null;
   }
 
   return (
-    <BlockaidBanner
-      securityAlertResponse={securityAlertResponse.response}
-      {...rest}
-    />
+    <BlockaidBanner securityAlertResponse={securityAlertResponse} {...rest} />
   );
 };
 

diff --git a/...omponents/TransactionBlockaidBanner/__snapshots__/TransactionBlockaidBanner.test.tsx.snap b/...omponents/TransactionBlockaidBanner/__snapshots__/TransactionBlockaidBanner.test.tsx.snap
@@ -1,9 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`TransactionBlockaidBanner should not render if currentTransactionSecurityAlertResponse.id is undefined 1`] = `null`;
-
 exports[`TransactionBlockaidBanner should not render if transactionId passed is undefined 1`] = `null`;
 
+exports[`TransactionBlockaidBanner should not render if securityAlertResponses.id is undefined 1`] = `null`;
+
 exports[`TransactionBlockaidBanner should render correctly 1`] = `
 <View
   securityAlertResponse={

diff --git a/app/components/Views/confirmations/components/TransactionReview/index.js b/app/components/Views/confirmations/components/TransactionReview/index.js
@@ -10,7 +10,10 @@ import { MetaMetricsEvents } from '../../../../../core/Analytics';
 import AppConstants from '../../../../../core/AppConstants';
 import Engine from '../../../../../core/Engine';
 import { SDKConnect } from '../../../../../core/SDKConnect/SDKConnect';
-import { selectCurrentTransactionMetadata } from '../../../../../selectors/confirmTransaction';
+import {
+  selectCurrentTransactionMetadata,
+  selectCurrentTransactionSecurityAlertResponse,
+} from '../../../../../selectors/confirmTransaction';
 import {
   selectConversionRate,
   selectCurrentCurrency,
@@ -269,6 +272,10 @@ class TransactionReview extends PureComponent {
      * Boolean that indicates if transaction simulations should be enabled
      */
     useTransactionSimulations: PropTypes.bool,
+    /**
+     * Object containing blockaid validation response for confirmation
+     */
+    securityAlertResponse: PropTypes.object,
   };
 
   state = {
@@ -358,13 +365,12 @@ class TransactionReview extends PureComponent {
   };
 
   onContactUsClicked = () => {
-    const { transaction, metrics } = this.props;
+    const { securityAlertResponse, metrics } = this.props;
     const additionalParams = {
-      ...getBlockaidMetricsParams(
-        transaction?.currentTransactionSecurityAlertResponse,
-      ),
+      ...getBlockaidMetricsParams(securityAlertResponse),
       external_link_clicked: 'security_alert_support_link',
     };
+
     metrics.trackEvent(
       MetaMetricsEvents.TRANSACTIONS_CONFIRM_STARTED,
       additionalParams,
@@ -470,23 +476,13 @@ class TransactionReview extends PureComponent {
   }
 
   getConfirmButtonState() {
-    const { transaction } = this.props;
-    const { id, currentTransactionSecurityAlertResponse } = transaction;
+    const { securityAlertResponse } = this.props;
     let confirmButtonState = ConfirmButtonState.Normal;
-    if (
-      id &&
-      currentTransactionSecurityAlertResponse?.id &&
-      currentTransactionSecurityAlertResponse.id === id
-    ) {
-      if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Malicious
-      ) {
+
+    if (securityAlertResponse) {
+      if (securityAlertResponse?.result_type === ResultType.Malicious) {
         confirmButtonState = ConfirmButtonState.Error;
-      } else if (
-        currentTransactionSecurityAlertResponse?.response?.result_type ===
-        ResultType.Warning
-      ) {
+      } else if (securityAlertResponse?.result_type === ResultType.Warning) {
         confirmButtonState = ConfirmButtonState.Warning;
       }
     }
@@ -571,7 +567,7 @@ class TransactionReview extends PureComponent {
               confirmDisabled={
                 transactionConfirmed || Boolean(error) || isAnimating
               }
-              confirmButtonState={this.getConfirmButtonState()}
+              confirmButtonState={this.getConfirmButtonState.bind(this)()}
             >
               <View style={styles.actionViewChildren}>
                 <ScrollView nestedScrollEnabled>
@@ -708,6 +704,7 @@ const mapStateToProps = (state) => ({
   transactionSimulationData:
     selectCurrentTransactionMetadata(state)?.simulationData,
   useTransactionSimulations: selectUseTransactionSimulations(state),
+  securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
 });
 
 TransactionReview.contextType = ThemeContext;