From d7f31d9a4a368d60a2c0f3c553123cee1d7e2a77 Mon Sep 17 00:00:00 2001 From: Eric Bishard Date: Thu, 30 Jan 2025 13:47:26 -0500 Subject: [PATCH] cherry-pick 29968: fix: handle null STX status containing pre-enabled state (#30010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry-picks https://github.com/MetaMask/metamask-extension/pull/29968 ## **Description** Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x Default Migration](https://github.com/MetaMask/metamask-extension/pull/28854) Update migration to treat null preference value as an existing enabled state, preventing unneeded banner display for v12.6.0-12.7.2 migrations. The update removes the `null` check in the migration's `transformState` function (near line 46 of `135.ts`) ```ts if ( currentOptInStatus === undefined || currentOptInStatus === null || // remove this line (currentOptInStatus === false && !hasExistingSmartTransactions(state)) ) { ``` And let the condition fall through to the else block. This will ensure that `smartTransactionsMigrationApplied: false` and ensure that we don't see a banner on Transaction Confirmation. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29968?quickstart=1) ## **Related issues** Fixes: #29970 **Description** Migration fails when the origin extension is version 12.7.x (tested 1 and 2) In this cases, the alert banner is displayed in the confirmation screen (e.g: send and swaps) when the parameter STX in settings was enabled in the older extension. **Expected behavior** The behaviour should be: extension 12.11.0 < with STX activated -> update to 12.11.0 and no banner is displayed in the confirmation screen ## **Manual testing steps for fix** 1. Migrate from any version 12.6.* to 12.7.* to 12.10.1 (or 12.11.0 depending) 2. Ensure that STX is ON in the old version (before migrating to the newer version) 3. After migration try to Swap or Send on Ethereum Mainnet & Sepolia and ensure there is no STX Banner Alert shown ## **Screenshots/Recordings** For users with STX ON before and after Migration: ### **Before** after-fix ### **After** after-fix ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --- app/scripts/migrations/135.test.ts | 29 ++++++++++++++++++++++++++--- app/scripts/migrations/135.ts | 1 - 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/scripts/migrations/135.test.ts b/app/scripts/migrations/135.test.ts index 19b3134698e1..06e78a7d6238 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 mark as migration-enabled when opt-in status is null', async () => { + it('should preserve existing state when opt-in status is null (default-enabled from previous versions)', async () => { const oldStorage: VersionedData = { meta: { version: prevVersion }, data: { @@ -34,11 +34,34 @@ describe('migration #135', () => { expect( newStorage.data.PreferencesController?.preferences ?.smartTransactionsOptInStatus, - ).toBe(true); + ).toBe(null); expect( newStorage.data.PreferencesController?.preferences ?.smartTransactionsMigrationApplied, - ).toBe(true); + ).toBe(false); + }); + + it('should preserve existing state when opt-in status is null (default-enabled from previous versions)', async () => { + const oldStorage: VersionedData = { + meta: { version: prevVersion }, + data: { + PreferencesController: { + preferences: { + smartTransactionsOptInStatus: null, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + expect( + newStorage.data.PreferencesController?.preferences + ?.smartTransactionsOptInStatus, + ).toBe(null); + expect( + newStorage.data.PreferencesController?.preferences + ?.smartTransactionsMigrationApplied, + ).toBe(false); }); it('should set stx opt-in to true and mark as migration-enabled when opt-in status is undefined', async () => { diff --git a/app/scripts/migrations/135.ts b/app/scripts/migrations/135.ts index 8d3413934bd4..1f6d00b5e869 100644 --- a/app/scripts/migrations/135.ts +++ b/app/scripts/migrations/135.ts @@ -43,7 +43,6 @@ function transformState(state: VersionedData['data']) { if ( currentOptInStatus === undefined || - currentOptInStatus === null || (currentOptInStatus === false && !hasExistingSmartTransactions(state)) ) { state.PreferencesController.preferences = {