From 564c825467c969166ba661b797c4c73bbd529f73 Mon Sep 17 00:00:00 2001
From: Eric Bishard <eric@httpjunkie.com>
Date: Tue, 28 Jan 2025 09:41:29 -0500
Subject: [PATCH] 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 <ramon.acitores@consensys.net>
---
app/scripts/migrations/135.test.ts | 16 +--
app/scripts/migrations/135.ts | 3 +-
privacy-snapshot.json | 1 +
test/e2e/tests/swaps/shared.ts | 6 -
test/e2e/tests/swaps/swap-eth.spec.ts | 5 -
.../tests/swaps/swaps-notifications.spec.ts | 3 +-
.../smart-transactions-banner-alert.test.tsx | 116 +++++++++++++++---
.../smart-transactions-banner-alert.tsx | 16 ++-
.../transaction-alerts.test.js | 22 +++-
9 files changed, 149 insertions(+), 39 deletions(-)
diff --git a/app/scripts/migrations/135.test.ts b/app/scripts/migrations/135.test.ts
index b2cca43b7733..19b3134698e1 100644
--- 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
index 277aafa66227..8d3413934bd4 100644
--- 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
index c39909e6cf42..499423cdbaf5 100644
--- 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
index fa55b3a7f0a8..75a965e1dbb6 100644
--- 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
index 2841f80b0aa3..926a1ac73936 100644
--- 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
index 246e6e4b5ec7..e800ca041e94 100644
--- 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/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.test.tsx b/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.test.tsx
index 79be7acec262..1585eba751e1 100644
--- a/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.test.tsx
+++ b/ui/pages/confirmations/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/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx b/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx
index 6a2739c0eeb1..e6b0e82bc0ec 100644
--- a/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx
+++ b/ui/pages/confirmations/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
index 64a9ce92113a..5b796e353e49 100644
--- 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,8 +585,17 @@ 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,
},
@@ -594,6 +603,17 @@ describe('Smart Transactions Migration Alert', () => {
smartTransactionsOptInStatus: true,
smartTransactionsMigrationApplied: true,
},
+ featureFlags: {
+ smartTransactionsEnabled: true,
+ },
+ swapsState: {
+ swapsFeatureFlags: {
+ smartTransactions: {
+ extensionActive: true,
+ mobileActive: false,
+ },
+ },
+ },
},
},
});