release: 8.4.2 (OTA) - #34500
Open
metamask-ci[bot] wants to merge 14 commits into
Open
Conversation
…oller cp-8.5.0 (#33822) ## **Description** Perps orders funded with a custom pay token fail at publish with `MetaMask Pay: Cannot submit without quote`. The publish guard is not the cause; it is the last thing to notice a problem that starts on the order screen. The order amount reaches `TransactionPayController` through a chain of effects: `depositAmount` sets the pending amount, then `amountHuman` triggers `updateTokenAmount`. Until that lands, the required token still has `amountRaw: "0"`. A zero required amount produces no source amounts, so no quote is ever requested. Nothing gates the Place Order button in that window. `shouldBlockBecauseOfFeesLoading` only reads `isPayTotalsLoading`, which is false because the fetch has not started, and the no-quotes alert cannot fire because it needs a non-empty `sourceAmounts`. Tapping there sends an unfunded deposit to publish, which correctly rejects it. Logs from a local build show the window lasting about three seconds after the payment token changes, with loading false, no quotes, and the required amount still zero. The app even emits `Perp Trade Quote Received` with `status: success` and `0ms` latency during it. This change treats a stale pay amount as "not ready", so the CTA stays disabled until the controller has the real amount. Predict already solved the same problem with its `isPaySystemSettling` machine in `usePredictBuyConditions`; Perps never had an equivalent. Porting that machine over is the fuller fix and is worth a follow-up, but this keeps the hotfix small and cannot leave the button stuck, because the amount always arrives. The publish guard is left exactly as it is. It is the correct backstop, and loosening it would only turn a visible error back into a silent unfunded deposit. ## **Changelog** CHANGELOG entry: Fixed a bug that stopped some Perps orders from being placed when paying with another token ## **Related issues** Fixes: CONF-1731 ## **Manual testing steps** ~~~gherkin Feature: Perps order paying with another token Scenario: user taps Place Order before the pay amount is ready Given the user opens a Perps market and chooses Long And the user changes "Pay with" to a token that is not Arbitrum USDC When the user taps Place Order as soon as the button looks tappable Then the button stays disabled until the fees have settled And no "Cannot submit without quote" error is raised Scenario: user places the order after the fees settle Given the user selected a custom pay token And the fees and quote have finished loading When the user taps Place Order Then the deposit and order are submitted as before Scenario: user pays from the existing Perps balance Given the Perps balance is selected as the payment method When the user taps Place Order Then the order is placed directly with no deposit transaction ~~~ ## **Screenshots/Recordings** N/A — no visual change. The only difference is that an existing button stays disabled for a few seconds longer while the pay amount propagates. ### **Before** Tapping Place Order in the window right after changing the pay token fails with a "Deposit failed" toast. Reproduced on `main` with debug logging, which captured the full chain: the pay state with `amountRaw: "0"` and no executable quote, the publish-guard throw `MetaMask Pay: Cannot submit without quote`, and the failed transaction with no hash. The same user-facing toast also reproduces on 8.2.0 from a real device, before that publish guard existed, so the ungated CTA window predates 8.3.0; the guard only made the failure visible. ### **After** The button remains disabled through that window and the order submits normally once the quote is ready. Verified on this branch with the same logging: the window still appears, with loading false, no quotes and a zero amount for about three seconds, but no tap reaches publish, no guard error is raised, and no toast is shown. ## **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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **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 Perps order submission gating and MetaMask Pay readiness for custom tokens; logic is narrow but user-facing on a money path. > > **Overview** > Fixes a race where **Place Order** could stay enabled right after switching to a custom pay token, because quote loading was still false while `TransactionPayController` still had `amountRaw: "0"` and never requested a quote—leading to **Cannot submit without quote** on publish. > > `PerpsOrderView` now reads **`useTransactionPayRequiredTokens`** and treats a stale pay amount (`amountRaw === '0'` for tokens that are not `skipIfBalance`) as **not ready**, alongside existing pay-quote loading. That **`isPayStateNotReady`** flag feeds **`shouldBlockBecauseOfFeesLoading`** and fee-loading UI for custom-token flows only; paying from the Perps balance is unchanged. > > Tests mock required tokens and assert the button stays disabled with a zero amount, enables once a real amount arrives, ignores zero amounts when balance covers the token, and does not block Perps-balance orders. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1e58769. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…33962) - fix: route WalletConnect relay through built-in WebSocket (#33871) ## **Description** **What:** Routes `relay.walletconnect.org` / `relay.walletconnect.com` through React Native's built-in WebSocket instead of the Nitro adapter (hostname match, both prod and dev installs). Every other endpoint keeps the Nitro transport, so the performance work from #32472 is untouched. **Why:** #32472 (shipped in 8.3.0) replaced `global.WebSocket` with `react-native-nitro-websockets`. Its Android native layer (`WebSocketConnection.cpp` `connect()`) sends `Sec-WebSocket-Protocol: nitro-ws` (libwebsockets transmits `lws_client_connect_info.protocol` as that request header) and an `Origin` header set to the server's own host on every connection whose caller requested no subprotocol — which is exactly what WalletConnect's transport does (`new WebSocket(url, [], undefined)`). The WalletConnect relay rejects that handshake, the relay socket never opens, `pairing.pair()` never settles, and every WalletConnect connection on Android 8.3.0 hangs on the "Connecting to MetaMask…" sheet. Lenient endpoints (e.g. HyperLiquid feeds) ignore the extra headers, which is why all other websocket features work and this slipped through. [Slack thread where the issue is first surfaced](https://consensys.slack.com/archives/C08U6DYNJ1G/p1785165035226969) Android-only because the package's iOS implementation (`NWWebSocketConnection.mm`, NSURLSession) only sets `Sec-WebSocket-Protocol` when protocols are actually requested and sets no Origin. Root-cause proof (A/B test on emulator): with only the relay host routed through the built-in WebSocket and everything else on Nitro, the relay connects immediately and the session proposal → approval → `wc_sessionSettle` flow completes end-to-end (verified from the dapp side via CDP). A follow-up to fix the Android native layer itself (bind the lws callback via `local_protocol_name`, leave `protocol` null when no subprotocol is requested, drop the fabricated `Origin`) is tracked in WAPI-1574; once that lands this bypass can be removed. ## **Changelog** CHANGELOG entry: Fixed WalletConnect connections hanging forever on Android. ## **Related issues** Fixes: [WAPI-1574](https://consensyssoftware.atlassian.net/browse/WAPI-1574) Refs: #32472 ## **Manual testing steps** Feature: WalletConnect pairing on Android Scenario: user connects a dapp via WalletConnect Given MetaMask Android (8.3.0 or main) with an unlocked wallet When user taps WalletConnect → MetaMask on https://metamask.github.io/test-dapp/ (or Hyperliquid) and approves the permissions sheet Then the loading sheet resolves, the session settles, and the dapp shows the connected account Scenario: other websocket consumers stay on Nitro Given the fix applied When user opens Perps and Predictions (Polymarket) Then prices and positions still update live over the Nitro WebSocket transport ## **Screenshots/Recordings** ### **Before** WalletConnect relay socket fails on every attempt; `pairing.pair()` never settles; "Connecting to MetaMask…" sheet hangs indefinitely. ### **After** Relay connects immediately; session proposal → approval → settle completes; dapp receives `wc_sessionSettle` within a second of approval. ## **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. [WAPI-1574]: https://consensyssoftware.atlassian.net/browse/WAPI-1574?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches global `WebSocket` routing used by WalletConnect and all `wss` traffic; behavior is well-tested but any hostname-routing bug could affect connectivity or regress Nitro performance paths. > > **Overview** > Fixes Android WalletConnect hangs by **not** sending `relay.walletconnect.org` / `relay.walletconnect.com` through the Nitro WebSocket adapter, which adds handshake headers the relay rejects. > > **Production** no longer replaces `global.WebSocket` with Nitro only. It installs a **routing constructor** that keeps Nitro for compatible `wss` URLs and delegates WalletConnect relay URLs (and non-string URLs like `URL` objects) to React Native’s built-in `WebSocket`. **Dev** keeps `ws://` on the built-in socket and `wss://` on Nitro, but applies the same relay bypass for `wss` relay hosts. > > Hostname matching is done with a lightweight `getWsHostname` parser (ports, userinfo, query-string false positives). Re-install is **idempotent** via a symbol so Fast Refresh/tests don’t nest wrappers. Missing built-in `WebSocket` still falls back to bare Nitro but now **logs a warning** that relay routing is lost. > > Tests cover relay routing in dev and prod, other hosts staying on Nitro, WalletConnect’s `(url, [], undefined)` shape, and the no-built-in warning. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4558569. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [80fcbb9](80fcbb9) [WAPI-1574]: https://consensyssoftware.atlassian.net/browse/WAPI-1574?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Alex Donesky <adonesky@gmail.com>
Runway bumped your version to 8.4.1 Co-authored-by: runway-github[bot] <73448015+runway-github[bot]@users.noreply.github.com>
…33979) - feat(perps): block place order until the pay amount reaches the controller cp-8.5.0 (#33822) ## **Description** Perps orders funded with a custom pay token fail at publish with `MetaMask Pay: Cannot submit without quote`. The publish guard is not the cause; it is the last thing to notice a problem that starts on the order screen. The order amount reaches `TransactionPayController` through a chain of effects: `depositAmount` sets the pending amount, then `amountHuman` triggers `updateTokenAmount`. Until that lands, the required token still has `amountRaw: "0"`. A zero required amount produces no source amounts, so no quote is ever requested. Nothing gates the Place Order button in that window. `shouldBlockBecauseOfFeesLoading` only reads `isPayTotalsLoading`, which is false because the fetch has not started, and the no-quotes alert cannot fire because it needs a non-empty `sourceAmounts`. Tapping there sends an unfunded deposit to publish, which correctly rejects it. Logs from a local build show the window lasting about three seconds after the payment token changes, with loading false, no quotes, and the required amount still zero. The app even emits `Perp Trade Quote Received` with `status: success` and `0ms` latency during it. This change treats a stale pay amount as "not ready", so the CTA stays disabled until the controller has the real amount. Predict already solved the same problem with its `isPaySystemSettling` machine in `usePredictBuyConditions`; Perps never had an equivalent. Porting that machine over is the fuller fix and is worth a follow-up, but this keeps the hotfix small and cannot leave the button stuck, because the amount always arrives. The publish guard is left exactly as it is. It is the correct backstop, and loosening it would only turn a visible error back into a silent unfunded deposit. ## **Changelog** CHANGELOG entry: Fixed a bug that stopped some Perps orders from being placed when paying with another token ## **Related issues** Fixes: CONF-1731 ## **Manual testing steps** ~~~gherkin Feature: Perps order paying with another token Scenario: user taps Place Order before the pay amount is ready Given the user opens a Perps market and chooses Long And the user changes "Pay with" to a token that is not Arbitrum USDC When the user taps Place Order as soon as the button looks tappable Then the button stays disabled until the fees have settled And no "Cannot submit without quote" error is raised Scenario: user places the order after the fees settle Given the user selected a custom pay token And the fees and quote have finished loading When the user taps Place Order Then the deposit and order are submitted as before Scenario: user pays from the existing Perps balance Given the Perps balance is selected as the payment method When the user taps Place Order Then the order is placed directly with no deposit transaction ~~~ ## **Screenshots/Recordings** N/A — no visual change. The only difference is that an existing button stays disabled for a few seconds longer while the pay amount propagates. ### **Before** Tapping Place Order in the window right after changing the pay token fails with a "Deposit failed" toast. Reproduced on `main` with debug logging, which captured the full chain: the pay state with `amountRaw: "0"` and no executable quote, the publish-guard throw `MetaMask Pay: Cannot submit without quote`, and the failed transaction with no hash. The same user-facing toast also reproduces on 8.2.0 from a real device, before that publish guard existed, so the ungated CTA window predates 8.3.0; the guard only made the failure visible. ### **After** The button remains disabled through that window and the order submits normally once the quote is ready. Verified on this branch with the same logging: the window still appears, with loading false, no quotes and a zero amount for about three seconds, but no tap reaches publish, no guard error is raised, and no toast is shown. ## **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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **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 Perps order submission gating and MetaMask Pay readiness for custom tokens; logic is narrow but user-facing on a money path. > > **Overview** > Fixes a race where **Place Order** could stay enabled right after switching to a custom pay token, because quote loading was still false while `TransactionPayController` still had `amountRaw: "0"` and never requested a quote—leading to **Cannot submit without quote** on publish. > > `PerpsOrderView` now reads **`useTransactionPayRequiredTokens`** and treats a stale pay amount (`amountRaw === '0'` for tokens that are not `skipIfBalance`) as **not ready**, alongside existing pay-quote loading. That **`isPayStateNotReady`** flag feeds **`shouldBlockBecauseOfFeesLoading`** and fee-loading UI for custom-token flows only; paying from the Perps balance is unchanged. > > Tests mock required tokens and assert the button stays disabled with a zero amount, enables once a real amount arrives, ignores zero amounts when balance covers the token, and does not block Perps-balance orders. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1e58769. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [e817797](e817797)
The zip artifact path used by the EAS/OTA pipelines strips Unix executable bits, so consumers rely on restore-node-modules-permissions to put them back. Its globs required an exact "bin" path segment, which never matched the platform-suffixed directories hermes-compiler uses (linux64-bin, osx-bin, win64-bin). Metro spawns that binary directly during bundling, so OTA pushes failed with EACCES once React Native 0.83.6 moved hermesc out of the react-native package, where it had lived under sdks/ and was covered incidentally. Also covers *.sh scripts, which are spawned directly by Xcode build phases and packager helpers and carry the same exposure on this path. Co-authored-by: Cursor <cursoragent@cursor.com>
…s (8.4.1) (#34005) ## **Description** Backport of #34004 to unblock the `8.4.1` OTA release. **This backport is required separately** because CI composite actions resolve from the checked-out branch, not from `main`. `Runway OTA RC` is dispatched against `release/8.4.1-ota`, so it uses that branch's copy of `.github/actions/restore-node-modules-permissions` — meaning the `main` fix alone does not affect this release. Failing run: https://github.com/MetaMask/metamask-mobile/actions/runs/30454324689/job/90593099632 ``` Error: spawn .../node_modules/hermes-compiler/hermesc/linux64-bin/hermesc EACCES ``` Both `Push EAS Update (iOS)` and `Push EAS Update (Android)` fail at Metro bundling because `hermesc` lost its executable bit in the zip artifact round-trip and the permission-restore globs required an exact `bin` path segment, never matching `hermes-compiler`'s `linux64-bin` / `osx-bin` / `win64-bin` layout. Full root-cause analysis, blast-radius review, and testing notes are in #34004. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: OTA push for 8.4.1 Scenario: Runway OTA RC after backport Given this PR is merged into release/8.4.1-ota When Runway OTA RC is re-run for 8.4.1 Then Push EAS Update (iOS) and (Android) bundle without EACCES ``` After merge, re-run **Runway OTA RC** for `8.4.1`. ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **Note for other release branches** `release/8.4.0` and `release/8.5.0` carry the same combination (RN `0.83.6` plus the un-patched action) and will hit this on their next OTA push, so they likely want the same backport. ## **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. Made with [Cursor](https://cursor.com)
#34493) since 148 not in prod yet we "swapped" and set `148.ts` as THIS FIX and old 148.ts becomes 149.ts. This way, cherry-picks will be easier and numbering will be preserved. **Context:** Back in June, migration 145 added the Arc network to every user's NetworkController state ahead of schedule (it was meant for a mid-June release; Arc now ships mid-September). Migrations run sequentially and can't be retroactively removed, so 145 still runs for anyone on it. As a result, some users a now seeing Arc as enabled network on their account, when it should have been hidden - unless added manually. Note that this is unrelated to Feature Flags: Although Arc is turned off via Feature Flags added networks aren't affected by Feature Flags whitelists or blacklist, only non-added Networks are. This means that when Arc was added (by the user or by migration 145) it will show to the user regardless of Feature Flags. This is a known behavior, not a bug. The issue is that the network is enabled when it shouldn't have been. **Solution:** New Migration 149: - Undoes 145's effect, but only where the resulting Arc network configuration has the RPC URL that has been set by 145. When other RPC URLs, it will be considered as a "custom network" addition - user added network manually and network was not auto-added. - Also falls back selectedNetworkClientId to `mainnet` if the user happened to be on the removed Arc client, mirroring the existing precedent in migration 111. - And manually enables `mainnet` in the unlikely event where Arc is the only enabled network. <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> <!-- mms-check: type=text required=true --> <!-- 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? --> <!-- mms-check: type=changelog required=true blocking=true --> <!-- 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: prevent Arc from showing as added for some users <!-- mms-check: type=issue-link required=true --> Fixes: https://consensyssoftware.atlassian.net/browse/WPN-1803 Case 1: Gradual upgrades - Install release 7.81.0 on a fresh state (new account with seed phrase) and setup account. - Install release 8.1.0 ontop of existing install (triggers migration 145) -> Arc is enabled when it shouldn't. - Install build from this PR ontop of existing install -> Arc should not be enabled (and should not be in the default list either). Case 2: Skip-the-line upgrades - Install release 7.81.0 on a fresh state (new account with seed phrase) and setup account. - Install build from this PR ontop of existing install -> Arc should not be enabled (and should not be in the default list either). Essentially both migration 145 and this fix migration should run in sequence and Arc should never appear to the user. Case 3: Custom network preserved - Install release 7.81.0 on a fresh state (new account with seed phrase) and setup account. - Add Arc manually as a custom network (Ex: with dummy RPC). - Install build from this PR ontop of existing install -> Arc should still be enabled with user-specific configuration. <!-- mms-check: type=screenshot required=true --> <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> <!-- mms-check: type=checklist required=true --> <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] 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). - [ ] 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 - [ ] 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. - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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 persisted network and enablement state on upgrade; logic is guarded and well-tested but a mistaken match could remove Arc or leave users without an enabled network if edge cases are wrong. > > **Overview** > Adds **migration 148** to undo migration 145’s premature Arc network auto-add for users who still have the untouched default Infura RPC (`arc-mainnet.infura.io/v3/...`). It removes Arc from `NetworkController` and `NetworkEnablementController`, resets `selectedNetworkClientId` to mainnet when the user was on Arc, and force-enables mainnet if Arc was the only enabled EVM network. > > **User-customized Arc is preserved:** different RPC URLs, failover URLs, extra endpoints, or manual adds are left unchanged; cosmetic edits (name, currency, explorer) still allow reversion when the private RPC is unchanged. Revert is skipped when `MM_INFURA_PROJECT_ID` is missing or the literal `"null"`. Invalid controller state is validated with Sentry reporting; enablement cleanup is skipped when that controller is invalid or missing. > > Registers migration **148** in the migrations index and adds broad unit tests (including 145→148 sequence). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a3ca586. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> ## **Description** <!-- mms-check: type=text required=true --> <!-- 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** <!-- mms-check: type=changelog required=true blocking=true --> <!-- 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** <!-- mms-check: type=issue-link required=true --> Fixes: ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```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** <!-- mms-check: type=screenshot required=true --> <!-- 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** <!-- mms-check: type=checklist required=true --> <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] 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). - [ ] 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 - [ ] 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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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. Co-authored-by: maxime-oe <maxime.ouairy-ext@consensys.net>
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. |
tommasini
previously approved these changes
Aug 7, 2026
Contributor
🚀 RC Builds Ready for Testing
More Info
🍒 What's in this RCCherry-picks (22 commits)
Changelog (372 commits from main at RC cut)
AI Test Plan
Executive SummaryRelease Focus: OTA hotfix 8.4.2 that reverts the prematurely-added Arc network from all user wallets and fixes a WalletConnect regression introduced in 8.3.0 by routing WalletConnect relay hosts through the native WebSocket instead of Nitro. Key Changes:
Critical Areas: WalletConnect pairing and session establishment on Android after the Nitro WebSocket routing fix, Arc network removal migration correctness across all user state variants (default config, customized config, Arc as selected network), Network switching and RPC endpoint integrity after migration 148 runs, NetworkEnablementController state consistency post-migration, WebSocket routing correctness ensuring non-WalletConnect wss:// connections still use Nitro Overall Risk: HIGH Recommendation: Conditional go — the WalletConnect fix and Arc network removal are both critical correctness changes that directly impact user-facing functionality. All high-risk scenarios around WalletConnect pairing on Android, migration state integrity, and network switching must pass before release. The migration's guard logic (isDefaultArcConfiguration) must be verified against real device state to ensure no unintended data loss for users who customized Arc. Release Scenarios (11)High Risk Scenarios (7)1. WalletConnectRisk Level: HIGH Why This Matters: NitroWebSocketSetup.ts introduced NITRO_INCOMPATIBLE_HOSTS containing relay.walletconnect.org and relay.walletconnect.com. The Android Nitro native layer was injecting a 'nitro-ws' subprotocol header and a fabricated Origin header that caused WalletConnect's strict relay server to reject the handshake, leaving pairing permanently hung (WAPI-1574). The fix routes these hosts through RN's built-in WebSocket via isNitroIncompatibleUrl(). If the hostname extraction (getWsHostname) or the Set lookup is wrong, the regression persists on Android. Preconditions:
Test Steps:
Expected Outcomes:
2. WalletConnectRisk Level: HIGH Why This Matters: The createRoutingWebSocket factory now uses a NITRO_ROUTING_INSTALLED symbol to prevent double-wrapping on Fast Refresh. Multiple concurrent WebSocket connections to WalletConnect relays must all be routed through the built-in WebSocket. If the routing wrapper is applied multiple times or the idempotency guard misfires, concurrent sessions could use different WebSocket implementations causing inconsistent behavior. Preconditions:
Test Steps:
Expected Outcomes:
3. WalletConnectRisk Level: HIGH Why This Matters: The NITRO_INCOMPATIBLE_HOSTS blocklist and getWsHostname extraction logic runs on both platforms. While the Nitro header bug was Android-specific, the routing code change affects iOS too. Verifying iOS WalletConnect and that other wss:// connections still use Nitro ensures the routing predicate (shouldUseNitro) correctly inverts the incompatibility check. Preconditions:
Test Steps:
Expected Outcomes:
4. Network ManagementRisk Level: HIGH Why This Matters: Migration 148 deletes networkConfigurationsByChainId[ARC_CHAIN_ID] when isDefaultArcConfiguration() returns true. This is a destructive state mutation. If the migration runs incorrectly or the isDefaultArcConfiguration guard has a bug, users could lose their Arc network entry even if they customized it, or conversely, Arc could remain when it should be removed. The migration also cleans NetworkEnablementController state. Preconditions:
Test Steps:
Expected Outcomes:
5. Network ManagementRisk Level: HIGH Why This Matters: Migration 148 explicitly handles the case where selectedNetworkClientId === arcNetworkClientId by resetting it to 'mainnet'. If this reset fails or the arcNetworkClientId comparison is incorrect, the app would reference a non-existent network client, potentially causing crashes or blank wallet state on every launch. Preconditions:
Test Steps:
Expected Outcomes:
6. Network ManagementRisk Level: HIGH Why This Matters: The isDefaultArcConfiguration() guard in migration 148 is the critical safety valve that prevents data loss for users who intentionally configured Arc. The migration comment explicitly states: 'if the user replaced/removed that RPC endpoint, added a failover to it, added an additional endpoint, or added Arc manually with their own RPC, that's a real customization and the network is left alone entirely.' Any bug in this guard could silently delete user-configured network data. Preconditions:
Test Steps:
Expected Outcomes:
7. Network ManagementRisk Level: HIGH Why This Matters: Migration 148 has a separate code path to clean up NetworkEnablementController state. The migration warns and skips this step if NetworkEnablementController state is missing or invalid. If Arc remains in the enabledNetworkMap after migration, it could cause UI inconsistencies or errors in dApp permission flows that enumerate enabled networks. Preconditions:
Test Steps:
Expected Outcomes:
Medium Risk Scenarios (4)1. Network ManagementRisk Level: MEDIUM Why This Matters: Migration 148 has an early exit: if ARC_CHAIN_ID is not in networkConfigurationsByChainId, it returns immediately. Fresh installs should never have had Arc added (migration 145 would have added it, but fresh 8.4.2 installs skip 145 since they start at the latest migration). Verifying this path ensures the migration's hasProperty guard works correctly. Preconditions:
Test Steps:
Expected Outcomes:
2. Network ManagementRisk Level: MEDIUM Why This Matters: Migration 148 uses cloneDeep on the entire state and then performs targeted deletions on networkConfigurationsByChainId. While the migration only targets ARC_CHAIN_ID, any bug in the cloneDeep or state reconstruction could corrupt adjacent network configurations. This scenario validates that non-Arc network data is completely unaffected. Preconditions:
Test Steps:
Expected Outcomes:
3. TransactionsRisk Level: MEDIUM Why This Matters: The NitroWebSocketSetup refactor changes how ALL WebSocket connections are routed, not just WalletConnect. The shouldUseNitro predicate now uses isNitroIncompatibleUrl() which returns true for WalletConnect hosts and false for everything else. If the logic is inverted or the hostname extraction fails for edge-case URLs, non-WalletConnect wss:// connections could be incorrectly routed to the built-in WebSocket, degrading performance for price feeds and other real-time data. Preconditions:
Test Steps:
Expected Outcomes:
4. App StabilityRisk Level: MEDIUM Why This Matters: Migration 148 is registered in migrations/index.ts and will run once for all users upgrading from any version where migration 148 has not yet run. The migration uses captureException from Sentry for error reporting. A failed migration that throws an unhandled exception could leave the app in an inconsistent state. The cloneDeep at the start of migrate() is the safety net — verifying the upgrade path with complex state ensures no edge cases in the migration logic cause data corruption. Preconditions:
Test Steps:
Expected Outcomes:
Excluded Features - Feature Flags Disabled (99)The following features are disabled via feature flags and should NOT be tested:
Generated by AI Test Plan Analyzer (claude-sonnet-4-6) at 2026-08-07T18:21:05.647Z AI generated test plan (JSON): Available as artifact |
Co-authored-by: Cursor <cursoragent@cursor.com>
sleepytanya
force-pushed
the
release/8.4.2-ota
branch
from
August 7, 2026 20:19
664b7a0 to
175aa80
Compare
sleepytanya
previously approved these changes
Aug 7, 2026
sleepytanya
marked this pull request as ready for review
August 7, 2026 21:00
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 175aa80. Configure here.
| * Kept here (not only in ota.config.js) so changes there do not alter the Expo fingerprint and break CI. | ||
| */ | ||
| export const OTA_VERSION: string = 'vX.XX.X'; | ||
| export const OTA_VERSION: string = 'v8.4.2'; |
Contributor
There was a problem hiding this comment.
OTA targets wrong runtime line
High Severity
OTA_VERSION is set to v8.4.2 for this release/8.4.2-ota hotfix, but RUNTIME_VERSION still comes from package.json as 8.5.0 and is what Expo publishes as runtimeVersion. OTA clients only fetch updates for their native runtime, so this bundle would target 8.5.0 installs rather than the 8.4.x line this release is meant to patch.
Reviewed by Cursor Bugbot for commit 175aa80. Configure here.
| targetSdkVersion rootProject.ext.targetSdkVersion | ||
| versionName "8.5.0" | ||
| versionCode 6291 | ||
| versionCode 6278 |
Contributor
There was a problem hiding this comment.
Native build numbers inconsistently reverted
Medium Severity
Android versionCode and iOS CURRENT_PROJECT_VERSION drop from 6291 to 6278, but versionName / MARKETING_VERSION stay at 8.5.0. OTA branches are supposed to match the already-shipped native build identity; this partial revert leaves store/build metadata inconsistent and can break fingerprint or store upload assumptions if a native artifact is produced from the branch.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 175aa80. Configure here.
This PR updates the change log for 8.4.2. (Hotfix - no test plan generated.) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog and link updates; no application code changes. > > **Overview** > Documents **v8.4.2** and **v8.4.1** in `CHANGELOG.md` and refreshes the release compare links at the bottom of the file. > > **8.4.2** records a single fix: Arc no longer shows up as an added/enabled network for users who got it from a pre-release default migration (#34445). **8.4.1** entries cover Perps custom pay-token order placement and WalletConnect relay reliability (#33822, #33871). The `[Unreleased]` → `v8.5.0` link now compares from `v8.4.2`, with new links for `8.4.2` and `8.4.1`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c0bc7bb. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: metamaskbot <metamaskbot@users.noreply.github.com> Co-authored-by: Tatiana Bobritsky <sleepytanya@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
🔍 Smart E2E Test Selection⏭️ Smart E2E selection skipped - PR targets a release or stable branch (release/* or stable) All E2E tests pre-selected. |
sleepytanya
approved these changes
Aug 7, 2026
|
sleepytanya
temporarily deployed
to
build-production
August 7, 2026 21:37 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
sleepytanya
temporarily deployed
to
build-production
August 7, 2026 21:37 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
OTA hotfix: branch
release/8.4.2-ota.OTA_VERSIONinapp/constants/ota.tsisv8.4.2.8.4.2/v8.4.2; the-otasuffix is branch-only.Note
Medium Risk
Persisted wallet state migration and global WebSocket routing affect all users on upgrade; logic is guarded and well-tested but mistakes could drop networks or break dapp connectivity.
Overview
OTA hotfix
v8.4.2setsOTA_VERSIONtov8.4.2and updates CHANGELOG with 8.4.2 (Arc revert) and 8.4.1 (Perps pay token, WalletConnect) entries plus compare links.Migration 148 undoes migration 145’s premature Arc network add when the entry still matches the single default Infura RPC 145 wrote (cosmetic edits still revert; custom RPC/failovers/extra endpoints or manual Arc are left alone). It removes Arc from NetworkController and NetworkEnablementController, falls back selectedNetworkClientId and mainnet enablement when Arc was the only active EVM network, and ships extensive tests.
NitroWebSocketSetupno longer replacesglobal.WebSocketwholesale in production: WalletConnect relay hosts (relay.walletconnect.org/.com) use React Native’s built-in WebSocket so strict handshake requirements aren’t broken by Nitro’s extra subprotocol/Origin headers (regression from 8.3.0). Dev builds keepwss://→ Nitro routing except for those hosts; installs are idempotent. Tests cover relay routing, hostname parsing edge cases, and WalletConnect’s(url, [], undefined)constructor shape.CI
restore-node-modules-permissionsalso chmods*-bin/*paths (e.g. Hermeshermesc) and*.shscripts undernode_modules.Native build numbers in Android
versionCodeand iOSCURRENT_PROJECT_VERSIONchange 6291 → 6278 while app versionName stays 8.5.0 in the snippet shown.Reviewed by Cursor Bugbot for commit 16f3f01. Bugbot is set up for automated code reviews on this repo. Configure here.