feat: Improve content of banner for errors and warning (#25058)

## **Description** This PR fixes the logic for PPOM banners, hardcoding the `Warning` banners for any `reason`. `Error` and `Malicious` banners are still dependent on the reasons. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25058?quickstart=1) ## **Related issues** Fixes: [#1758](MetaMask/MetaMask-planning#1758) ## **Manual testing steps** 1. Open up metamask test-dapp. 2. Connect wallet 3. Open the console 4. Issue a transaction that approves USDT to unverified contract by invoking that transaction for example: ``` window.ethereum.sendAsync({ "method": "eth_sendTransaction", "params": [ { "from": "INSERT_YOUR_WALLET_ADDRESS", "to": "0xdac17f958d2ee523a2206206994597c13d831ec7", "data": "0x095ea7b3000000000000000000000000b208222089e9c48f3a0680b91be5a79d79ad429effffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" } ], "timestamp": 1693229271999 }) ``` ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Jun 6, 2024 · a37816d · a37816d
1 parent 470d4a7
commit a37816d
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 19 deletions.
diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
diff --git a/...vider-banner-alert/blockaid-banner-alert/__snapshots__/blockaid-banner-alert.test.js.snap b/...vider-banner-alert/blockaid-banner-alert/__snapshots__/blockaid-banner-alert.test.js.snap
@@ -209,12 +209,12 @@ exports[`Blockaid Banner Alert should render 'warning' UI when securityAlertResp
     <p
       class="mm-box mm-text mm-text--body-lg-medium mm-box--color-text-default"
     >
-      This is a deceptive request
+      Be careful
     </p>
     <p
       class="mm-box mm-text mm-text--body-md mm-box--margin-top-2 mm-box--color-text-default"
     >
-      If you approve this request, a third party known for scams might take all your assets.
+      This could be a deceptive request. Only continue if you trust every address involved.
     </p>
     <div
       class="mm-box mm-box--margin-top-3"
@@ -307,12 +307,12 @@ exports[`Blockaid Banner Alert should render details section even when features
       <p
         class="mm-box mm-text mm-text--body-lg-medium mm-box--color-text-default"
       >
-        This is a deceptive request
+        Be careful
       </p>
       <p
         class="mm-box mm-text mm-text--body-md mm-box--margin-top-2 mm-box--color-text-default"
       >
-        If you approve this request, a third party known for scams might take all your assets.
+        This could be a deceptive request. Only continue if you trust every address involved.
       </p>
       <div
         class="mm-box mm-box--margin-top-3"
@@ -406,12 +406,12 @@ exports[`Blockaid Banner Alert should render details when provided 1`] = `
       <p
         class="mm-box mm-text mm-text--body-lg-medium mm-box--color-text-default"
       >
-        This is a deceptive request
+        Be careful
       </p>
       <p
         class="mm-box mm-text mm-text--body-md mm-box--margin-top-2 mm-box--color-text-default"
       >
-        If you approve this request, a third party known for scams might take all your assets.
+        This could be a deceptive request. Only continue if you trust every address involved.
       </p>
       <div
         class="mm-box mm-box--margin-top-3"
@@ -517,12 +517,12 @@ exports[`Blockaid Banner Alert should render link to report url 1`] = `
       <p
         class="mm-box mm-text mm-text--body-lg-medium mm-box--color-text-default"
       >
-        This is a deceptive request
+        Be careful
       </p>
       <p
         class="mm-box mm-text mm-text--body-md mm-box--margin-top-2 mm-box--color-text-default"
       >
-        If you approve this request, a third party known for scams might take all your assets.
+        This could be a deceptive request. Only continue if you trust every address involved.
       </p>
       <div
         class="mm-box mm-box--margin-top-3"

diff --git a/.../components/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.js b/.../components/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.js
@@ -84,18 +84,30 @@ function BlockaidBannerAlert({ txData, ...props }) {
     reason,
     result_type: resultType,
   } = securityAlertResponse;
+
+  let title, description;
   if (resultType === BlockaidResultType.Benign) {
     return null;
+  } else if (resultType === BlockaidResultType.Warning) {
+    // When `result_type` is `Warning`, the `reason` is no longer relevant for
+    // determining the copy. This is because Blockaid has lower certainty when
+    // they flag something as warning so that requires a softer and broader
+    // message than the ones we use when `result_type` is `Malicious` or
+    // `Error`.
+
+    title = t(REASON_TO_TITLE_TKEY[BlockaidReason.errored]);
+    description = t('blockaidDescriptionWarning');
+  } else {
+    if (!REASON_TO_DESCRIPTION_TKEY[reason]) {
+      captureException(`BlockaidBannerAlert: Unidentified reason '${reason}'`);
+    }
+
+    title = t(REASON_TO_TITLE_TKEY[reason] || 'blockaidTitleDeceptive');
+    description = t(
+      REASON_TO_DESCRIPTION_TKEY[reason] || REASON_TO_DESCRIPTION_TKEY.other,
+    );
   }
 
-  if (!REASON_TO_DESCRIPTION_TKEY[reason]) {
-    captureException(`BlockaidBannerAlert: Unidentified reason '${reason}'`);
-  }
-
-  const description = t(
-    REASON_TO_DESCRIPTION_TKEY[reason] || REASON_TO_DESCRIPTION_TKEY.other,
-  );
-
   const details = features?.length ? (
     <Text as="ul" overflowWrap={OverflowWrap.BreakWord}>
       {features.map((feature, i) => (
@@ -106,13 +118,14 @@ function BlockaidBannerAlert({ txData, ...props }) {
 
   const isFailedResultType = resultType === BlockaidResultType.Errored;
 
+  // On the banner colors:
+  // Malicious -> red
+  // Error and Warning -> orange
   const severity =
     resultType === BlockaidResultType.Malicious
       ? BannerAlertSeverity.Danger
       : BannerAlertSeverity.Warning;
 
-  const title = t(REASON_TO_TITLE_TKEY[reason] || 'blockaidTitleDeceptive');
-
   /** Data we pass to Blockaid false reporting portal. As far as I know, there are no documents that exist that specifies these key values */
   const reportUrl = (() => {
     const reportData = {

diff --git a/...onents/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.test.js b/...onents/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert.test.js
@@ -132,7 +132,10 @@ describe('Blockaid Banner Alert', () => {
     const { getByText } = renderWithProvider(
       <BlockaidBannerAlert
         txData={{
-          securityAlertResponse: mockSecurityAlertResponse,
+          securityAlertResponse: {
+            ...mockSecurityAlertResponse,
+            result_type: BlockaidResultType.Errored,
+          },
         }}
       />,
       configureStore(),
@@ -163,6 +166,7 @@ describe('Blockaid Banner Alert', () => {
         txData={{
           securityAlertResponse: {
             ...mockSecurityAlertResponse,
+            result_type: BlockaidResultType.Errored,
             reason: BlockaidReason.rawSignatureFarming,
           },
         }}
@@ -306,6 +310,7 @@ describe('Blockaid Banner Alert', () => {
             txData={{
               securityAlertResponse: {
                 ...mockSecurityAlertResponse,
+                result_type: BlockaidResultType.Errored,
                 reason,
               },
             }}
@@ -327,6 +332,7 @@ describe('Blockaid Banner Alert', () => {
           txData={{
             securityAlertResponse: {
               ...mockSecurityAlertResponse,
+              result_type: BlockaidResultType.Errored,
               reason: 'unmappedReason',
             },
           }}