-
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
fix: Update STX Banner Alert, include chainSupportsSmartTransactions
#29911
Merged
httpJunkie
merged 18 commits into
main
from
fix/stx-by-default--add-stx-chain-condition
Jan 28, 2025
Merged
fix: Update STX Banner Alert, include chainSupportsSmartTransactions
#29911
httpJunkie
merged 18 commits into
main
from
fix/stx-by-default--add-stx-chain-condition
Jan 28, 2025
+149
−39
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…STX liveness checks. - Added imports for feature flag and STX liveness checks - Added Redux dispatch hook - Added selectors for chain support and preference enabled - Added useEffect to fetch feature flags and STX liveness when alert is enabled - Updated alertConditions to include chain support and preference enabled checks The banner will now wait for feature flags and STX liveness to be fetched before potentially rendering, and will only show if the chain supports smart transactions and they are enabled in preferences.
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Updates test state structure to match production selectors: - Integrated `mockNetworkState` helper for network configuration WAS KEY - Added required `swapsState` structure (`extensionActive`, `smartTransactions config`) - Fixes `preference-disabled` tests w/ complete feat flag state The `swapsState` configuration contains essential settings that control how smart transactions behave Adding `swapsState` was needed to match network-specific MetaMask smart transaction settings, including extension state and transaction params These settings determine feature availability and transaction behavior across different platforms and networks
Changes: - `135.ts` - Updated migration script to only `set smartTransactionsMigrationApplied` when STX is newly enabled - `135.test.ts` - Updated tests to verify migration behavior with clearer test descriptions `transaction-alerts.test.js` - Updated test to properly mock network configuration for the banner component
httpJunkie
added
team-confirmations
Push issues to confirmations team
regression-RC-12.11.0
Regression bug that was found in release candidate (RC) for release 12.11.0
release-12.11.0
Issue or pull request that will be included in release 12.11.0
type-bug
release-blocker
This bug is blocking the next release
Sev1-high
High severity; partial loss of service with severe impact upon users, with no workaround.
labels
Jan 28, 2025
httpJunkie
changed the title
fix: Update STX Banner Alert, include
fix: Update STX Banner Alert, include Jan 28, 2025
chainSupportsSmartTransactions
& STX liveness checkchainSupportsSmartTransactions
dan437
reviewed
Jan 28, 2025
dan437
previously approved these changes
Jan 28, 2025
…com:MetaMask/metamask-extension into fix/stx-by-default--add-stx-chain-condition
…entry reporting Sets flag to false instead of undefined for existing STX users to maintain consistent state shape while preserving banner behavior.
Builds ready [c78fc09]
Page Load Metrics (1914 ± 196 ms)
Bundle size diffs
|
dan437
approved these changes
Jan 28, 2025
Builds ready [cf90c03]
Page Load Metrics (1624 ± 72 ms)
Bundle size diffs
|
pedronfigueiredo
approved these changes
Jan 28, 2025
metamaskbot
added
the
release-12.12.0
Issue or pull request that will be included in release 12.12.0
label
Jan 28, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
regression-RC-12.11.0
Regression bug that was found in release candidate (RC) for release 12.11.0
release-12.11.0
Issue or pull request that will be included in release 12.11.0
release-12.12.0
Issue or pull request that will be included in release 12.12.0
release-blocker
This bug is blocking the next release
Sev1-high
High severity; partial loss of service with severe impact upon users, with no workaround.
team-confirmations
Push issues to confirmations team
team-transactions
Transactions team
type-bug
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to: #29869
This fix ensures that the SmartTransactionsBannerAlert shows for STX supported chains and that it does not show for users that were not affected by the STX by Default migrations (part of the same feature).
Changes to migration #135 to ensure smartTransactionsMigrationApplied always has a boolean value in state:
true
when STX is newly enabled (banner shows)false
for pre-existing STX users (banner doesn't show)This maintains consistent state shape for Sentry reporting while preserving the banner's intended behavior. Previously undefined values are now explicitly false.
Description
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.
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]
#29869
Manual testing steps
Screenshots/Recordings
Load old version 12.5.0 and turn STX OFF, go through migration and ensure that:
stx-off-to-on.mp4
Load old version 12.5.0 and turn STX ON, go through migration and ensure that:
stx-on-to-on.mp4
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
Pre-merge reviewer checklist