fix: Update STX Banner Alert, include chainSupportsSmartTransactions (

#29911) - Added selectors for chain support and preference enabled - Updated alertConditions to include chain support and preference enabled checks The banner will now only show if the chain supports smart transactions and they are enabled in preferences. ## **Description** 1. Reason for change: The Smart Transactions banner was showing incorrectly for users who had STX enabled before migration. The banner should only show for users when STX is newly enabled by the migration. 2. Improvement/solution: Modified migration script to only set `smartTransactionsMigrationApplied` when STX is newly enabled during migration, rather than setting it for all migrated users. This ensures the banner's display logic correctly identifies users who need to see the STX announcement. ## **Related issues** Fixes: #[issue number] ## **Manual testing steps** 1. Install older version of MetaMask with STX enabled 2. Upgrade to version with these changes 3. Verify banner does not show for pre-enabled STX users 4. Install older version with STX disabled 5. Upgrade to version with these changes 6. Verify banner shows when migration enables STX ## **Screenshots/Recordings** Load old version 12.5.0 and turn STX OFF, go through migration and ensure that: - STX is now ON - Banner Alert shows only on STX supported chains https://github.com/user-attachments/assets/f85da8e9-5e4f-46eb-9132-95a8129a4f4e Load old version 12.5.0 and turn STX ON, go through migration and ensure that: - STX is still ON - Banner Alert does not show https://github.com/user-attachments/assets/0bff702c-a24d-4d52-b6a3-2d9615589fd5 ### **Before** Banner shows for all users after migration, even those who had STX previously enabled. ### **After** Banner only shows for users where migration newly enabled STX. ## **Pre-merge author checklist** - [x] I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards - [x] I've completed the PR template to the best of my ability - [x] I've included tests: - Updated migration tests to verify new behavior - Updated component tests to work with new migration logic - Fixed existing integration tests - [x] I've documented my code using JSDoc format where applicable - [x] I've applied the right labels on the PR ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR - [x] I confirm that this PR addresses all acceptance criteria and includes necessary testing evidence --------- Co-authored-by: Ramon Acitores <[email protected]>
MetaMask · Jan 28, 2025 · 564c825 · 564c825
1 parent fcfd96f
commit 564c825
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 39 deletions.
diff --git a/app/scripts/migrations/135.test.ts b/app/scripts/migrations/135.test.ts
@@ -18,7 +18,7 @@ describe('migration #135', () => {
     expect(newStorage.meta).toStrictEqual({ version: 135 });
   });
 
-  it('should set stx opt-in to true and migration flag when stx opt-in status is null', async () => {
+  it('should set stx opt-in to true and mark as migration-enabled when opt-in status is null', async () => {
     const oldStorage: VersionedData = {
       meta: { version: prevVersion },
       data: {
@@ -41,7 +41,7 @@ describe('migration #135', () => {
     ).toBe(true);
   });
 
-  it('should set stx opt-in to true and migration flag when stx opt-in status is undefined', async () => {
+  it('should set stx opt-in to true and mark as migration-enabled when opt-in status is undefined', async () => {
     const oldStorage: VersionedData = {
       meta: { version: prevVersion },
       data: {
@@ -60,7 +60,7 @@ describe('migration #135', () => {
     ).toBe(true);
   });
 
-  it('should set stx opt-in to true and migration flag when stx opt-in is false and no existing mainnet smart transactions', async () => {
+  it('should set stx opt-in to true and mark as migration-enabled when opt-in is false and no existing mainnet transactions', async () => {
     const oldStorage: VersionedData = {
       meta: { version: prevVersion },
       data: {
@@ -91,7 +91,7 @@ describe('migration #135', () => {
     ).toBe(true);
   });
 
-  it('should not change stx opt-in when stx opt-in is false but has existing smart transactions, but should set migration flag', async () => {
+  it('should preserve disabled stx state when user has transaction history', async () => {
     const oldStorage: VersionedData = {
       meta: { version: prevVersion },
       data: {
@@ -118,10 +118,10 @@ describe('migration #135', () => {
     expect(
       newStorage.data.PreferencesController?.preferences
         ?.smartTransactionsMigrationApplied,
-    ).toBe(true);
+    ).toBe(false);
   });
 
-  it('should not change stx opt-in when stx opt-in is already true, but should set migration flag', async () => {
+  it('should preserve existing stx enabled state', async () => {
     const oldStorage: VersionedData = {
       meta: { version: prevVersion },
       data: {
@@ -141,7 +141,7 @@ describe('migration #135', () => {
     expect(
       newStorage.data.PreferencesController?.preferences
         ?.smartTransactionsMigrationApplied,
-    ).toBe(true);
+    ).toBe(false);
   });
 
   it('should initialize preferences object if it does not exist', async () => {
@@ -161,7 +161,7 @@ describe('migration #135', () => {
     expect(
       newStorage.data.PreferencesController?.preferences
         ?.smartTransactionsMigrationApplied,
-    ).toBe(true);
+    ).toBe(false);
   });
 
   it('should capture exception if PreferencesController state is invalid', async () => {

diff --git a/app/scripts/migrations/135.ts b/app/scripts/migrations/135.ts
@@ -38,7 +38,6 @@ function transformState(state: VersionedData['data']) {
   }
 
   const { PreferencesController } = state;
-
   const currentOptInStatus =
     PreferencesController.preferences?.smartTransactionsOptInStatus;
 
@@ -55,7 +54,7 @@ function transformState(state: VersionedData['data']) {
   } else {
     state.PreferencesController.preferences = {
       ...state.PreferencesController.preferences,
-      smartTransactionsMigrationApplied: true,
+      smartTransactionsMigrationApplied: false, // Always set it, but false for existing users
     };
   }
 

diff --git a/privacy-snapshot.json b/privacy-snapshot.json
@@ -15,6 +15,7 @@
   "bafkreifvhjdf6ve4jfv6qytqtux5nd4nwnelioeiqx5x2ez5yrgrzk7ypi.ipfs.dweb.link",
   "bafybeidxfmwycgzcp4v2togflpqh2gnibuexjy4m4qqwxp7nh3jx5zlh4y.ipfs.dweb.link",
   "bridge.api.cx.metamask.io",
+  "bridge.dev-api.cx.metamask.io",
   "cdn.segment.com",
   "cdn.segment.io",
   "cdnjs.cloudflare.com",

diff --git a/test/e2e/tests/swaps/shared.ts b/test/e2e/tests/swaps/shared.ts
@@ -190,12 +190,6 @@ export const checkActivityTransaction = async (
   await driver.clickElement('[data-testid="popover-close"]');
 };
 
-export const closeSmartTransactionsMigrationNotification = async (
-  driver: Driver,
-) => {
-  await driver.clickElement('[aria-label="Close"]');
-};
-
 export const checkNotification = async (
   driver: Driver,
   options: { title: string; text: string },

diff --git a/test/e2e/tests/swaps/swap-eth.spec.ts b/test/e2e/tests/swaps/swap-eth.spec.ts
@@ -7,7 +7,6 @@ import {
   checkActivityTransaction,
   changeExchangeRate,
   mockEthDaiTrade,
-  closeSmartTransactionsMigrationNotification,
 } from './shared';
 
 // TODO: (MM-PENDING) These tests are planned for deprecation as part of swaps testing revamp
@@ -27,10 +26,6 @@ describe('Swap Eth for another Token', function () {
           swapTo: 'DAI',
         });
 
-        // Close the STX notification immediately after buildQuote
-        // This ensures the UI is clear before we proceed with quote review
-        await closeSmartTransactionsMigrationNotification(driver);
-
         await reviewQuote(driver, {
           amount: 2,
           swapFrom: 'TESTETH',

diff --git a/test/e2e/tests/swaps/swaps-notifications.spec.ts b/test/e2e/tests/swaps/swaps-notifications.spec.ts
@@ -6,7 +6,6 @@ import {
   buildQuote,
   reviewQuote,
   checkNotification,
-  closeSmartTransactionsMigrationNotification,
 } from './shared';
 
 async function mockSwapsTransactionQuote(mockServer: Mockttp) {
@@ -81,7 +80,7 @@ describe('Swaps - notifications', function () {
           amount: 2,
           swapTo: 'INUINU',
         });
-        await closeSmartTransactionsMigrationNotification(driver);
+
         await checkNotification(driver, {
           title: 'Potentially inauthentic token',
           text: 'INUINU is only verified on 1 source. Consider verifying it on Etherscan before proceeding.',

diff --git a/...tions/components/smart-transactions-banner-alert/smart-transactions-banner-alert.test.tsx b/...tions/components/smart-transactions-banner-alert/smart-transactions-banner-alert.test.tsx
@@ -8,6 +8,8 @@ import { renderWithProvider } from '../../../../../test/jest/rendering';
 import configureStore from '../../../../store/store';
 import { AlertTypes } from '../../../../../shared/constants/alerts';
 import { setAlertEnabledness } from '../../../../store/actions';
+import { mockNetworkState } from '../../../../../test/stub/networks';
+import { CHAIN_IDS } from '../../../../../shared/constants/network';
 import { SmartTransactionsBannerAlert } from './smart-transactions-banner-alert';
 
 type TestConfirmContextValue = {
@@ -56,10 +58,41 @@ describe('SmartTransactionsBannerAlert', () => {
         smartTransactionsOptInStatus: true,
         smartTransactionsMigrationApplied: true,
       },
+      featureFlags: {
+        smartTransactionsEnabled: true,
+      },
+      smartTransactionsFeatureFlags: {
+        enabled: true,
+      },
+      swapsState: {
+        swapsFeatureFlags: {
+          ethereum: {
+            extensionActive: true,
+            mobileActive: false,
+            smartTransactions: {
+              expectedDeadline: 45,
+              maxDeadline: 150,
+              extensionReturnTxHashAsap: false,
+            },
+          },
+          smartTransactions: {
+            extensionActive: true,
+            mobileActive: false,
+          },
+        },
+      },
+      smartTransactionsState: {
+        liveness: true,
+      },
+      ...mockNetworkState({
+        id: 'network-configuration-id-1',
+        chainId: CHAIN_IDS.MAINNET,
+        rpcUrl: 'https://mainnet.infura.io/v3/',
+      }),
     },
   };
 
-  it('renders banner when alert is enabled, STX is opted in, and migration is applied', () => {
+  it('renders banner when all conditions are met', () => {
     const store = configureStore(mockState);
     renderWithProvider(<SmartTransactionsBannerAlert />, store);
 
@@ -79,14 +112,12 @@ describe('SmartTransactionsBannerAlert', () => {
 
   it('does not render when alert is disabled', () => {
     const disabledState = {
+      ...mockState,
       metamask: {
+        ...mockState.metamask,
         alertEnabledness: {
           [AlertTypes.smartTransactionsMigration]: false,
         },
-        preferences: {
-          smartTransactionsOptInStatus: true,
-          smartTransactionsMigrationApplied: true,
-        },
       },
     };
     const store = configureStore(disabledState);
@@ -99,12 +130,11 @@ describe('SmartTransactionsBannerAlert', () => {
 
   it('does not render when migration has not been applied', () => {
     const noMigrationState = {
+      ...mockState,
       metamask: {
-        alertEnabledness: {
-          [AlertTypes.smartTransactionsMigration]: true,
-        },
+        ...mockState.metamask,
         preferences: {
-          smartTransactionsOptInStatus: true,
+          ...mockState.metamask.preferences,
           smartTransactionsMigrationApplied: false,
         },
       },
@@ -117,6 +147,67 @@ describe('SmartTransactionsBannerAlert', () => {
     ).not.toBeInTheDocument();
   });
 
+  it('does not render when chain does not support smart transactions', () => {
+    const unsupportedChainState = {
+      ...mockState,
+      metamask: {
+        ...mockState.metamask,
+        ...mockNetworkState({
+          id: 'network-configuration-id-2',
+          chainId: CHAIN_IDS.POLYGON,
+          rpcUrl: 'https://polygon-rpc.com',
+        }),
+      },
+    };
+    const store = configureStore(unsupportedChainState);
+    renderWithProvider(<SmartTransactionsBannerAlert />, store);
+
+    expect(
+      screen.queryByTestId('smart-transactions-banner-alert'),
+    ).not.toBeInTheDocument();
+  });
+
+  it('does not render when smart transactions preference is disabled', () => {
+    const disabledPreferenceState = {
+      ...mockState,
+      metamask: {
+        ...mockState.metamask,
+        preferences: {
+          ...mockState.metamask.preferences,
+          smartTransactionsOptInStatus: false, // Add this
+        },
+        featureFlags: {
+          smartTransactionsEnabled: false,
+        },
+        smartTransactionsFeatureFlags: {
+          enabled: false,
+        },
+        swapsState: {
+          swapsFeatureFlags: {
+            ethereum: {
+              extensionActive: false,
+              smartTransactions: {
+                expectedDeadline: 45,
+                maxDeadline: 150,
+                extensionReturnTxHashAsap: false,
+              },
+            },
+            smartTransactions: {
+              extensionActive: false,
+              mobileActive: false,
+            },
+          },
+        },
+      },
+    };
+    const store = configureStore(disabledPreferenceState);
+    renderWithProvider(<SmartTransactionsBannerAlert />, store);
+
+    expect(
+      screen.queryByTestId('smart-transactions-banner-alert'),
+    ).not.toBeInTheDocument();
+  });
+
   it('dismisses banner when close button or link is clicked', () => {
     const store = configureStore(mockState);
 
@@ -224,7 +315,6 @@ describe('SmartTransactionsBannerAlert', () => {
 
   it('handles being outside of ConfirmContext correctly', () => {
     const store = configureStore(mockState);
-
     renderWithProvider(<SmartTransactionsBannerAlert />, store);
 
     expect(
@@ -235,12 +325,10 @@ describe('SmartTransactionsBannerAlert', () => {
   it('automatically dismisses banner when Smart Transactions is manually disabled', () => {
     const store = configureStore({
       metamask: {
-        alertEnabledness: {
-          [AlertTypes.smartTransactionsMigration]: true,
-        },
+        ...mockState.metamask,
         preferences: {
+          ...mockState.metamask.preferences,
           smartTransactionsOptInStatus: false,
-          smartTransactionsMigrationApplied: true,
         },
       },
     });

diff --git a/...firmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx b/...firmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx
@@ -19,6 +19,10 @@ import {
   getSmartTransactionsOptInStatusInternal,
   getSmartTransactionsMigrationAppliedInternal,
 } from '../../../../../shared/modules/selectors/smart-transactions';
+import {
+  getCurrentChainSupportsSmartTransactions,
+  getSmartTransactionsPreferenceEnabled,
+} from '../../../../../shared/modules/selectors';
 
 type MarginType = 'default' | 'none' | 'noTop' | 'onlyTop';
 
@@ -55,6 +59,14 @@ export const SmartTransactionsBannerAlert: React.FC<SmartTransactionsBannerAlert
       getSmartTransactionsMigrationAppliedInternal,
     );
 
+    const chainSupportsSmartTransactions = useSelector(
+      getCurrentChainSupportsSmartTransactions,
+    );
+
+    const smartTransactionsPreferenceEnabled = useSelector(
+      getSmartTransactionsPreferenceEnabled,
+    );
+
     const dismissAlert = useCallback(() => {
       setAlertEnabledness(AlertTypes.smartTransactionsMigration, false);
     }, []);
@@ -68,7 +80,9 @@ export const SmartTransactionsBannerAlert: React.FC<SmartTransactionsBannerAlert
     const alertConditions =
       alertEnabled &&
       smartTransactionsOptIn &&
-      smartTransactionsMigrationApplied;
+      smartTransactionsMigrationApplied &&
+      chainSupportsSmartTransactions &&
+      smartTransactionsPreferenceEnabled;
 
     const shouldRender =
       currentConfirmation === null

diff --git a/ui/pages/confirmations/components/transaction-alerts/transaction-alerts.test.js b/ui/pages/confirmations/components/transaction-alerts/transaction-alerts.test.js
@@ -570,7 +570,7 @@ describe('TransactionAlerts', () => {
 });
 
 describe('Smart Transactions Migration Alert', () => {
-  it('shows when alert is enabled, opted in, and migration applied', () => {
+  it('shows when stx was enabled by migration', () => {
     const { getByTestId } = render({
       componentProps: {
         txData: {
@@ -585,15 +585,35 @@ describe('Smart Transactions Migration Alert', () => {
           networkConfigurationsByChainId: {
             [CHAIN_ID_MOCK]: {
               chainId: CHAIN_ID_MOCK,
+              rpcEndpoints: [
+                {
+                  rpcUrl: 'https://mainnet.infura.io/v3/',
+                  networkClientId: '1',
+                  blockExplorerUrl: 'https://etherscan.io',
+                },
+              ],
+              defaultBlockExplorerUrlIndex: 0,
             },
           },
+          selectedNetworkClientId: '1',
           alertEnabledness: {
             [AlertTypes.smartTransactionsMigration]: true,
           },
           preferences: {
             smartTransactionsOptInStatus: true,
             smartTransactionsMigrationApplied: true,
           },
+          featureFlags: {
+            smartTransactionsEnabled: true,
+          },
+          swapsState: {
+            swapsFeatureFlags: {
+              smartTransactions: {
+                extensionActive: true,
+                mobileActive: false,
+              },
+            },
+          },
         },
       },
     });