Skip to content
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
merged 18 commits into from
Jan 28, 2025

Conversation

httpJunkie
Copy link
Contributor

@httpJunkie httpJunkie commented Jan 27, 2025

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

  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]
#29869

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
stx-off-to-on.mp4

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
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

  • I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards
  • I've completed the PR template to the best of my ability
  • I've included tests:
    • Updated migration tests to verify new behavior
    • Updated component tests to work with new migration logic
    • Fixed existing integration tests
  • I've documented my code using JSDoc format where applicable
  • I've applied the right labels on the PR

Pre-merge reviewer checklist

  • I've manually tested the PR
  • I confirm that this PR addresses all acceptance criteria and includes necessary testing evidence

…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.
@httpJunkie httpJunkie requested a review from a team as a code owner January 27, 2025 07:11
@httpJunkie httpJunkie marked this pull request as draft January 27, 2025 07:11
Copy link
Contributor

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
@httpJunkie httpJunkie self-assigned this Jan 27, 2025
@httpJunkie httpJunkie added the team-transactions Transactions team label Jan 27, 2025
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 httpJunkie marked this pull request as ready for review January 28, 2025 08:39
@httpJunkie 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 httpJunkie changed the title fix: Update STX Banner Alert, include chainSupportsSmartTransactions & STX liveness check fix: Update STX Banner Alert, include chainSupportsSmartTransactions Jan 28, 2025
dan437
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.
@metamaskbot
Copy link
Collaborator

Builds ready [c78fc09]
Page Load Metrics (1914 ± 196 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43125181767396190
domContentLoaded144933031873408196
load145533171914407196
domInteractive25282535929
backgroundConnect992413014
firstReactRender1597402512
getState592202210
initialActions01000
loadScripts102127331398374179
setupStore861232010
uiStartup163137512228510245
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 279 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@httpJunkie httpJunkie requested a review from dan437 January 28, 2025 13:42
@metamaskbot
Copy link
Collaborator

Builds ready [cf90c03]
Page Load Metrics (1624 ± 72 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14301992162514972
domContentLoaded13991978160314771
load14311992162415072
domInteractive247536168
backgroundConnect86323189
firstReactRender15108442914
getState431863
initialActions01000
loadScripts9541456114711153
setupStore767212210
uiStartup160925951903223107
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 279 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@httpJunkie httpJunkie added this pull request to the merge queue Jan 28, 2025
Merged via the queue into main with commit 564c825 Jan 28, 2025
70 checks passed
@httpJunkie httpJunkie deleted the fix/stx-by-default--add-stx-chain-condition branch January 28, 2025 15:14
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2025
@metamaskbot 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants