[pull] main from MetaMask:main#541
Merged
Merged
Conversation
## **Description** Phase 2 analytics migration (Batch 2-13): migrate Card's hooks, onboarding components, views, and UI components from `useMetrics` to the new analytics system. **Reason**: Deprecate MetaMetrics in favour of the shared analytics utility and AnalyticsController. **Changes**: All 27 Card source files now use `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics` and `MetaMetricsEvents` from `app/core/Analytics`. Test mocks updated to mock `useAnalytics` instead of `useMetrics`. 15 additional files were discovered via grep that were not in the original batch definition and have been included. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MCWP-297 (Batch 2-13) ## **Manual testing steps** ```gherkin Feature: Card analytics Scenario: user triggers a Card flow event Given app is open and user is in a Card flow When user performs an action that triggers analytics (e.g. card welcome page view, onboarding step, asset selection, spending limit change) Then the event is tracked on Mixpanel ``` ## **Screenshots/Recordings** N/A – analytics migration, no UI change. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Largely mechanical import/mocking changes that should not affect runtime behavior beyond analytics emission; main risk is mismapped event constants or broken test mocks causing missing telemetry. > > **Overview** > Migrates MetaMask Card UI (views, onboarding screens, bottom sheets, and related hooks) from the legacy `useMetrics` hook to the new `useAnalytics` hook, and switches event constants imports to `MetaMetricsEvents` from `app/core/Analytics`. > > Updates unit tests accordingly by replacing `useMetrics` mocks with `useAnalytics` mocks and adjusting assertions to use the centralized `MetaMetricsEvents` constants (instead of string literals in mocks). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ca45463. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## **Description** Defensive hardening for edge cases in `PerpsController` surfaced by static analysis on Core PR [#7941](MetaMask/core#7941). None of these bugs are currently triggerable in production — the app works correctly today — but all represent latent issues that could bite under future changes or unusual network conditions. ### What changed **1. `toggleTestnet()` — false success on silent init failure** `performInitialization()` catches errors internally and sets `InitializationState.Failed` instead of throwing. `toggleTestnet()` was not checking for this, so it could return `{ success: true }` even when the network switch failed. This patch: - Adds the same `InitializationState.Failed` check after `await this.init()` - Rolls back `isTestnet` to its previous value on failure **2. `depositWithConfirmation()` — never-resolving promise when `placeOrder=true`** The `placeOrder` path created `new Promise(() => {})` — a promise that can never be GC'd and would hang any consumer who awaits it. Replaced with `Promise.resolve(transactionMeta.id)` for proper fire-and-forget semantics. **3. `depositWithConfirmation()` — failed deposits remain permanently pending** `depositWithConfirmation` adds a pending entry to `state.depositRequests` before validating `networkClientId`. If the validation throws, the deposit request stays `status: 'pending'` forever. This patch marks the deposit request as `failed` in the outer catch when a pre-submission error occurs. **4. Feature flag listener lost after disconnect** `disconnect()` unsubscribed `RemoteFeatureFlagController:stateChange` but no reconnect path re-subscribed. After disconnect → reconnect, geo-blocking and HIP-3 flag changes stopped propagating. The feature-flag subscription is a controller-lifetime concern (not session-lifetime), so we removed the unsubscribe from `disconnect()` entirely — the subscription now lives for the full controller lifecycle. **5. `depositWithConfirmation()` — deposit+order request stays pending forever** When `placeOrder=true`, the `if (!placeOrder)` guard skips the `.then()/.catch()` lifecycle block that transitions the deposit request to `completed`/`failed`. No other handler exists for this path. Added an `else if` branch that attaches lifecycle tracking to `addResult.result` (the real transaction promise) for the deposit+order flow. **6. Non-EVM account switch leaves stale cache** The account-change handler only cleared cached user data when `currentAddress` is truthy. Switching to an account group with no EVM account (e.g., Bitcoin-only) skipped cache clearing, leaving stale positions/orders visible. Restructured the condition so cache is always cleared when the account group changes, with preload guarded separately behind `if (currentAddress)`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: bugbot findings from Core PR [#7941](MetaMask/core#7941) ## **Manual testing steps** These are defensive fixes for edge cases not reachable through normal UI flows. Verification is via unit tests. ```gherkin Feature: Perps network toggle resilience Scenario: toggleTestnet reports accurate status when init fails silently Given user is on mainnet with perps initialized When user toggles to testnet and initialization fails internally Then toggleTestnet returns { success: false } And isTestnet is rolled back to its previous value Feature: Perps deposit promise contract Scenario: depositWithConfirmation result promise resolves when placeOrder is true Given user initiates a deposit with placeOrder=true When the transaction is submitted Then the result promise resolves with the transaction ID Feature: Perps deposit request lifecycle Scenario: deposit request is marked failed on pre-submission error Given user initiates a deposit and the deposit request is added as pending When the networkClientId lookup fails Then the deposit request status is updated to 'failed' Scenario: deposit+order request transitions from pending Given user initiates a deposit with placeOrder=true When the transaction completes or fails Then the deposit request status is updated to 'completed' or 'failed' Feature: Feature flag subscription resilience Scenario: geo-blocking flags propagate after disconnect/reconnect Given perps controller is initialized with feature flag subscription When disconnect() is called and controller reconnects Then feature flag changes still propagate correctly Feature: Account switch cache clearing Scenario: switching to non-EVM account clears stale perps cache Given user has cached perps data from an EVM account When user switches to a Bitcoin-only account group Then cached positions, orders, and account state are cleared ``` ## **Screenshots/Recordings** N/A — no UI changes, logic-only hardening. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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 - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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.
## **Description** Phase 2 analytics migration (Batch 2-12): migrate metamask-assets's asset components, token lists, collectible modals, DeFi positions, and token details from `useMetrics`/`withMetricsAwareness` to the new analytics system (`useAnalytics`/`withAnalyticsAwareness`). **Reason**: Deprecate MetaMetrics in favour of the shared analytics utility and AnalyticsController. **Changes**: 20 source files + 16 test files migrated. React components now use `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics` instead of `useMetrics`; class components use `withAnalyticsAwareness` instead of `withMetricsAwareness` (or removed the HOC when unused). `MetaMetricsEvents` imports moved from `hooks/useMetrics` to `core/Analytics`. Test mocks updated to mock `useAnalytics` instead of `useMetrics`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MCWP-297 (Batch 2-12) ## **Manual testing steps** ```gherkin Feature: Assets analytics Scenario: user triggers an assets flow event Given app is open and user is in an assets flow When user performs an action that triggers analytics (e.g. add custom token, hide token, view NFT details, open token details) Then the event is tracked on Mixpanel ``` ## **Screenshots/Recordings** N/A – analytics migration, no UI change. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Mostly mechanical analytics-hook/HOC and type import migrations with no functional UI or state-flow changes beyond event plumbing; risk is limited to missing/incorrect analytics injection causing lost tracking or test failures. > > **Overview** > Migrates assets-related UI flows (custom token import, token list, asset/collectible detail modals, DeFi positions, and asset actions) from legacy `useMetrics`/`withMetricsAwareness` to the new analytics API (`useAnalytics`/`withAnalyticsAwareness`), updating call sites to use the new injected `analytics` prop/hook methods. > > Updates supporting utilities (`goToAddEvmToken`, `removeEvmToken`) to use `AnalyticsEventBuilder` types and moves `MetaMetricsEvents` imports to `core/Analytics`. Adjusts a large set of unit tests to mock `useAnalytics` instead of `useMetrics`, and removes the metrics HOC wrapper from `Views/Asset` where it’s no longer needed. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c1807bf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
… connections cp-7.67.0 (#26324) ## **Description** Fixes [TAT-2597](https://consensys-mesh.atlassian.net/browse/TAT-2597) and [TAT-2598](https://consensys-mesh.atlassian.net/browse/TAT-2598): After the preload PR merged, slow connections caused StreamChannels to exhaust 150 polling retries (30s) in `ensureReady()` and silently give up, leaving users with stale REST cache and no live WebSocket data — positions not appearing after trade, missing prices. **Root Cause:** `StreamChannel.ensureReady()` used blind polling (`isReady` every 200ms × 150 retries) with no awareness of WebSocket connection state. On slow connections, the connection had not even established yet, so polling burned through all retries before data could arrive. **Fix:** - `PerpsConnectionManager.waitForConnection()` — exposes init/reconnect promises so channels can `await` instead of blind-polling - `StreamChannel.ensureReady()` — detects `isConnecting` state and awaits the connection promise via `awaitConnectionThenConnect()` **Result:** PriceStreamChannel retries dropped from **33 → 0** on device after this fix. ## **Changelog** CHANGELOG entry: Fixed stale cache on slow connections where positions and prices were not updating after a trade ## **Related issues** Fixes: [TAT-2597](https://consensys-mesh.atlassian.net/browse/TAT-2597), [TAT-2598](https://consensys-mesh.atlassian.net/browse/TAT-2598) ## **Manual testing steps** ```gherkin Feature: Perps live data on slow connections Scenario: user opens a trade on a slow connection Given the app is connected to a slow 3G network And the user has navigated to the Perps trading screen When user opens a new position Then the position appears immediately in the positions list And price stream connects without excessive retries Scenario: user recovers from network drop Given the user is viewing live perps positions And the network connection drops momentarily When the network connection is restored Then live WebSocket data resumes without stale cache ``` ## **Screenshots/Recordings** ### **Before** <!-- PriceStreamChannel: 33 retries before data appears, positions missing on slow connections --> ### **After** <!-- PriceStreamChannel: 0 retries, positions appear immediately --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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 - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. [TAT-2597]: https://consensyssoftware.atlassian.net/browse/TAT-2597?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [TAT-2598]: https://consensyssoftware.atlassian.net/browse/TAT-2598?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [TAT-2597]: https://consensyssoftware.atlassian.net/browse/TAT-2597?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches Perps connection/stream reconnection timing and retry logic; mistakes could cause missed subscriptions or delayed real-time updates, though changes are localized and well-covered by tests. > > **Overview** > Prevents Perps stream channels from burning through retry polling on slow connections by making `StreamChannel.ensureReady()` detect `isConnecting` and wait on the connection manager before retrying `connect()`. > > Adds `PerpsConnectionManager.waitForConnection()` (awaits init/reconnect promises, swallowing rejections) and introduces `awaitConnectionThenConnect()` with a sentinel timer to avoid duplicate awaits, plus a small `deferConnect()` timer cleanup fix and a shared `PERPS_CONSTANTS.ConnectRetryDelayMs` constant. > > Expands unit tests to cover the new await/guard behavior (duplicate-await prevention, resolve/reject fallbacks) and updates market-data channel tests to use the new retry delay constant. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7e0d599. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…26230) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** - Consolidated all BrowserStack API interactions into the existing BrowserStackAPI.ts class, eliminating the redundant BrowserStackCredentials.js utility - Made BrowserStackAPI constructor resilient — no longer throws when credentials are missing; API methods return null gracefully instead - Added getAppProfilingData() and getNetworkLogs() endpoints to BrowserStackAPI.ts - Updated AppProfilingDataHandler.js, PerformanceTracker.js, and custom-reporter.js to use BrowserStackAPI instead of BrowserStackCredentials + raw axios/fetch calls - Removed manual credential checks in custom-reporter.js — the API handles missing credentials internally - Deleted BrowserStackCredentials.js and cleaned up all references <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMQA-1457 ## **Manual testing steps** N/A ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A <!-- [screenshots/recordings] --> ### **After** N/A <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches CI/test reporting flows and changes error/credential behavior (now returning `null` instead of throwing), which could alter how BrowserStack artifacts are fetched and retried; impact is limited to test infrastructure, not production code. > > **Overview** > Centralizes BrowserStack interactions into `BrowserStackAPI.ts`: the client no longer throws on missing env credentials, API methods return `null` with warnings, requests use a shared 8s timeout, and failures now throw a `BrowserStackAPIError` that preserves HTTP `status` and response `body`. > > Adds new API methods for app profiling (`/appprofiling/v2`) and session network logs (`/networklogs`), exports the new error type, and updates performance reporters (`AppProfilingDataHandler`, `PerformanceTracker`, `custom-reporter`) to stop using `axios`/manual auth checks, fetch `buildId` explicitly for network logs, and skip/fallback cleanly when `buildId` or credentials are unavailable. Removes the redundant `BrowserStackCredentials.js` helper and associated README mention. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit cd2dfeb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This feature was shipped a month ago, this is a cleanup since we will not be reverting this feature now. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: chore: remove explore feature flag ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2736 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core navigation/tab registration and back/close routing behavior, which can impact user flows if any routes assumed the old flag-gated Browser tab behavior. > > **Overview** > **Removes the `assetsTrendingTokens`/Explore feature flag** and makes Trending/Explore navigation unconditional. > > `TabBar` now always navigates to `Routes.TRENDING_VIEW`, and `MainNavigator` always registers the Explore tab plus a hidden `Routes.BROWSER.HOME` tab and always registers Explore-related stack screens (`EXPLORE_SEARCH`, `SITES_FULL_VIEW`, `BROWSER.HOME`). `BrowserTab` close behavior is simplified to always route back to Trending when not opened from Trending/Perps, and unit tests/snapshots are updated while the now-dead `assetsTrendingTokens` selector + tests are deleted. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5499952. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
This PR fix the ledger transaction speedup and cancel not able to run
issue.
The root clause of this issue was caused by when we introduce the new
unified transaction view and we forgot to check account is ledger
devices or not, and didn't handle ledger differently because ledger
require BLE approve to trigger the speed up and cancl transaction.
If you look at legacy behavior defined in `Transaction/index.js` file
which has this code to handle ledger device check and then do speedup
and cancel
```
const isLedgerAccount = isHardwareAccount(this.props.selectedAddress, [...]);
...
if (isLedgerAccount) {
await this.signLedgerTransaction({...replacementParams...});
} else {
await speedUpTransaction(...);
}
```
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry: Modify `useUnifiedTxActions.ts` to specially handle
ledger transaction speed up and cancel transaction.
## **Related issues**
Fixes: #24544
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes transaction replacement execution paths and fee parameter
wiring for Ledger accounts, which can affect speed up/cancel behavior;
coverage is improved with new unit tests but the flow is
user-funds-adjacent.
>
> **Overview**
> Fixes unified transaction *speed up* and *cancel* for Ledger accounts
by detecting Ledger keyring addresses and routing replacement
transactions through `LedgerTransactionModal` (BLE confirmation) instead
of calling `speedUpTransaction`/`TransactionController.stopTransaction`
directly.
>
> Extends Ledger replacement parameters to support *legacy* `gasPrice`
as `legacyGasFee` (in addition to EIP-1559 fees), passes the appropriate
fee shape through the modal, and ensures modal state is cleaned up even
when the user rejects on the Ledger confirmation screen. Adds
comprehensive unit tests covering Ledger vs non-Ledger paths and legacy
vs EIP-1559 fee handling.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
fe13682. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
bump SonarCloud action to the last stable one
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry:
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [ ] I've included tests if applicable
- [ ] 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-mobile/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.
## **Description** ### Problem The `redux-persist-filesystem-storage` library has two problematic behaviors when handling arbitrary keys in `StorageService`: 1. **Slashes (`/`) create subdirectories**: Keys containing slashes are stored in subdirectories, making them unreachable via `getAllKeys`. This breaks the `clear` method for affected keys. 2. **Hyphens (`-`) get corrupted**: The library's internal `fromFileName` function converts hyphens to colons (`:`), meaning keys like `simple-key` are returned as `simple:key` by `getAllKeys`. This causes a permanent mismatch between stored keys and returned keys. ### Solution This PR introduces URI-style encoding for problematic characters in **both namespace and key** portions of storage keys: - `/` → `%2F` (prevents subdirectory creation) - `-` → `%2D` (prevents hyphen-to-colon corruption) - `%` → `%25` (prevents double-encoding issues) The encoding is applied via shared utility functions (`encodeStorageKey`/`decodeStorageKey`) used by both: - `mobileStorageAdapter` in `storage-service-init.ts` - Migration 118 which stores snap source code ### Backward Compatibility **This change is backward compatible and will NOT break existing production keys.** - Existing keys like `storageService:TokenListController:tokensChainsCache:0x1` are **unaffected** because: - The namespace (`TokenListController`) has no special characters → encoding produces identical output - Colons (`:`) are **not encoded** - they pass through unchanged - The key portion (`tokensChainsCache:0x1`) contains no hyphens or slashes - **Strings without `-`, `/`, or `%` characters produce identical output when encoded** - This means all current production keys work exactly as before, while future keys with special characters will be handled correctly ### Examples ``` # No special characters → unchanged (backward compatible) storageService:TokenListController:tokensChainsCache:0x1 → storageService:TokenListController:tokensChainsCache:0x1 # Snap ID with slashes and hyphens → encoded storageService:SnapController:npm:@metamask/bip32-keyring-snap → storageService:SnapController:npm:@MetaMask%2Fbip32%2Dkeyring%2Dsnap # Namespace with hyphen → encoded storageService:Some-Controller:some-key → storageService:Some%2DController:some%2Dkey # Key with slashes → encoded (prevents subdirectory creation) storageService:TestController:nested/path/key → storageService:TestController:nested%2Fpath%2Fkey ``` ### Files Changed | File | Change | |------|--------| | `app/core/Engine/utils/storage-service-utils.ts` | New utility with `encodeStorageKey`/`decodeStorageKey` functions | | `app/core/Engine/utils/storage-service-utils.test.ts` | 35 unit tests for the encoding utilities | | `app/core/Engine/controllers/storage-service-init.ts` | Apply encoding in `mobileStorageAdapter` methods | | `app/core/Engine/controllers/storage-service-init.test.ts` | 22 new tests for key and namespace encoding behavior | | `app/store/migrations/119.ts` | Encode snap IDs when storing snap source code | ## **Changelog** CHANGELOG entry: null ## **Related issues** Refs: StorageService key handling issues with `redux-persist-filesystem-storage` ## **Manual testing steps** ```gherkin Feature: StorageService key encoding Scenario: Keys with hyphens are stored and retrieved correctly Given the app is running When StorageService stores a key containing hyphens (e.g., "npm:@metamask/bip32-keyring-snap") Then the key is encoded as "npm:@MetaMask%2Fbip32%2Dkeyring%2Dsnap" on disk And getAllKeys returns the original key "npm:@metamask/bip32-keyring-snap" And getItem with the original key returns the stored data Scenario: Keys with slashes are stored and retrieved correctly Given the app is running When StorageService stores a key containing slashes (e.g., "nested/path/key") Then the key is stored as a single file (not in subdirectories) And getAllKeys returns the original key "nested/path/key" And clear removes the key correctly Scenario: Existing keys with colons remain unchanged Given existing production keys like "storageService:TokenListController:tokensChainsCache:0x1" When the app starts with this fix Then the existing keys are still accessible And no migration is required for existing data ``` ## **Screenshots/Recordings** N/A - No UI changes ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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 - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the on-disk key format for any namespaces/keys containing `%`, `/`, or `-`, which could impact access to previously stored data for those cases; behavior is covered by new unit tests but touches persistence and migration logic. > > **Overview** > Fixes StorageService filesystem key handling by **URI-encoding namespaces and keys** (encoding `%`, `/`, `-` while leaving `:` intact) before calling `redux-persist-filesystem-storage`, and decoding keys returned from `getAllKeys`. > > Updates `mobileStorageAdapter` to apply this encoding in `getItem`/`setItem`/`removeItem`, use encoded namespace prefixes in `getAllKeys`/`clear`, and adds a shared `storage-service-utils` module with thorough unit coverage. Migration `119` is updated (and its test adjusted) to store snap source code under the **encoded snap ID** so snap IDs containing slashes/hyphens are retrievable/clearable. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 59b39f4. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Make TooltipModal CTA optional to prevent rendering the CTA of the tooltip on top of the swaps CTA causing misclicks. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: remove quote details tooltip cta and fix paddings ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/SWAPS-4129 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <img width="499" height="236" alt="εικόνα" src="https://github.com/user-attachments/assets/cd4e5d5a-a572-4416-b96e-2db674e62935" /> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small UI param removal limited to tooltip modal configuration and test updates; low chance of behavioral impact beyond tooltip spacing. > > **Overview** > Bridge `QuoteDetailsCard` no longer passes a hardcoded `bottomPadding` to the tooltip modal for quote rate, network fee, slippage, minimum received, price impact, and rewards tooltips, removing the now-unused `TOOLTIP_BOTTOM_PADDING` constant. > > Associated navigation assertions in `QuoteDetailsCard.test.tsx` were updated to expect tooltip modal params without `bottomPadding`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 00ad73f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )