-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: Migration fails with extension version 12.7.x #29970
Labels
regression-RC-12.11.0
Regression bug that was found in release candidate (RC) for release 12.11.0
release-12.12.0
Issue or pull request that will be included in release 12.12.0
team-transactions
Transactions team
type-bug
Something isn't working
Comments
7 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 30, 2025
## **Description** Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x Default Migration](#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** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ### **After** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ## **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.
httpJunkie
added a commit
that referenced
this issue
Jan 30, 2025
## **Description** Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x Default Migration](#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** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ### **After** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ## **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.
This was referenced Jan 30, 2025
7 tasks
dbrans
pushed a commit
that referenced
this issue
Jan 30, 2025
… state (#30010) cherry-picks #29968 ## **Description** Only remaining QA issue for feature: [[EXT] STX Banner Alert & STX x Default Migration](#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** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ### **After** <img width="150" alt="after-fix" src="https://github.com/user-attachments/assets/fe33da33-7828-45e2-b7a1-8b7d60b12de5" /> ## **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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
regression-RC-12.11.0
Regression bug that was found in release candidate (RC) for release 12.11.0
release-12.12.0
Issue or pull request that will be included in release 12.12.0
team-transactions
Transactions team
type-bug
Something isn't working
Describe the bug
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
Screenshots/Recordings
No response
Steps to reproduce
Error messages or log output
Detection stage
During release testing
Version
12.11.0
Build type
None
Browser
Chrome
Operating system
MacOS
Hardware wallet
No response
Additional context
Same behaviour in Firefox
Severity
sev2
The text was updated successfully, but these errors were encountered: