Skip to content

Cherry-picking commits from cherry-pick-13-43-0-3cad329 to release/13.43.0 for PR #45158 - #45244

Merged
sleepytanya merged 2 commits into
release/13.43.0from
cherry-pick-13-43-0-3cad329
Aug 5, 2026
Merged

Cherry-picking commits from cherry-pick-13-43-0-3cad329 to release/13.43.0 for PR #45158#45244
sleepytanya merged 2 commits into
release/13.43.0from
cherry-pick-13-43-0-3cad329

Conversation

@runway-github

@runway-github runway-github Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

The Ledger integration is presenting an odd UX where the operation
"Account Import" never settles, the possible root cause for this issue
is a failure during a device roundtrip operation using WebHID, since
this operation runs under a keyring mutex with no timeout we never see
the promise resolve/reject and therefore the operation never finishes.

Three changes are introduced to mitigate this issue:

  • Revert of fix(hardware-wallets): bound stuck account-creation spinner with device-read timeout cp-13.42.0 #45048. A
    previous temporal fix.
  • Add timeouts to LedgerOffscreenBridge as a safety net
    • getPublicKey now has a 30s timeout — no user confirmation needed
  • deviceSignTransaction, deviceSignMessage, deviceSignTypedData
    now has a 5 min timeout - needs on device confirmation
  • Timeout rejection message updated to: Ledger device did not respond to "<action>" within <n>ms
    • The timeout are arbitrary
  • Reuse transport across actions in LedgerLegacyHandler
  • handleAction no longer calls await this.closeTransport() in its
    finally. Instead it cancels any pending idle close at entry and
    schedules a new one after the action — so consecutive actions reuse the
    open WebHID transport instead of open/close churning (the churn that
    probably desyncs WebHID).
    • Added scheduleIdleClose()/clearIdleClose() helpers.
  • closeTransport() now calls clearIdleClose() (a manual
    disconnect/close supersedes a pending idle close).
  • destroy() clears the idle timer before tearing down
    listeners/transport.
  • The HID disconnect listener still calls closeTransport() immediately
    on unplug.

Changelog

CHANGELOG entry: reuse ledger transport for multiple operations

Related issues

Fixes: #45027
Fixes: https://consensyssoftware.atlassian.net/browse/MUL-2108

Manual testing steps

Connect and import multiple accounts

  1. Connect a Ledger device and import multiple accounts (at least 7).
  2. Forget device and disconnect
  3. Connect the device again and try to import multiple accounts (more
    accounts than imported in step 1).

Signature

  1. Using the imported Ledger accounts try to execute different
    operations like message sign, swap, bridge, and send

Screenshots/Recordings

Screen.Recording.2026-08-03.at.2.28.17.PM.mov

Pre-merge author checklist

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.

Note

Medium Risk
Changes core Ledger offscreen transport lifecycle,
mutex/serialization, and hardware account unlock paths; mistakes could
cause device lock errors, premature timeouts on signing, or stuck
transports.

Overview
Addresses stuck Ledger flows (e.g. account import) by keeping the
WebHID transport open
across bursts of offscreen actions (5s idle
close) instead of closing after every call, with forceReset to
drop a hung transport when recovery is needed.

LedgerOffscreenBridge adds action-specific timeouts (30s for
getPublicKey, 5min for signing) and clearer timeout errors;
unlockHardwareWalletAccount drops its separate account-creation
timeout in favor of that stack.

ledger-router serializes concurrent offscreen messages on one
promise chain, races each action against 60s / 330s backstops (sign vs
read), calls forceReset on timeout, and swallows late rejections
so the chain can continue. DMK stub forwards forceReset to legacy.
Tests cover concurrency, timeout recovery, bootstrap error swallowing,
and init/switch races.

Reviewed by Cursor Bugbot for commit
70e07e5. Bugbot is set up for automated
code reviews on this repo. Configure
here.


Co-authored-by: Cursor cursoragent@cursor.com 3cad329

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

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

The Ledger integration is presenting an odd UX where the operation
"Account Import" never settles, the possible root cause for this issue
is a failure during a device roundtrip operation using WebHID, since
this operation runs under a keyring mutex with no timeout we never see
the promise resolve/reject and therefore the operation never finishes.

Three changes are introduced to mitigate this issue:

- Revert of #45048. A
previous temporal fix.
- Add timeouts to `LedgerOffscreenBridge` as a safety net
  - `getPublicKey` now has a 30s timeout — no user confirmation needed
- `deviceSignTransaction`, `deviceSignMessage`, `deviceSignTypedData`
now has a 5 min timeout - needs on device confirmation
- Timeout rejection message updated to: `Ledger device did not respond
to "<action>" within <n>ms`
  - The timeout are arbitrary
- Reuse transport across actions in `LedgerLegacyHandler`
- `handleAction` no longer calls await `this.closeTransport()` in its
finally. Instead it cancels any pending idle close at entry and
schedules a new one after the action — so consecutive actions reuse the
open WebHID transport instead of open/close churning (the churn that
probably desyncs WebHID).
  - Added `scheduleIdleClose()`/`clearIdleClose()` helpers.
- `closeTransport()` now calls `clearIdleClose()` (a manual
disconnect/close supersedes a pending idle close).
- `destroy()` clears the idle timer before tearing down
listeners/transport.
- The HID disconnect listener still calls `closeTransport()` immediately
on unplug.

<!--
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`
3. 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: reuse ledger transport for multiple operations

Fixes: #45027
Fixes: https://consensyssoftware.atlassian.net/browse/MUL-2108

1. Connect a Ledger device and import multiple accounts (at least 7).
2. Forget device and disconnect
3. Connect the device again and try to import multiple accounts (more
accounts than imported in step 1).

1. Using the imported Ledger accounts try to execute different
operations like message sign, swap, bridge, and send

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

https://github.com/user-attachments/assets/7a52c1cb-ea98-4118-97ad-468908261b92

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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 Ledger offscreen transport lifecycle,
mutex/serialization, and hardware account unlock paths; mistakes could
cause device lock errors, premature timeouts on signing, or stuck
transports.
>
> **Overview**
> Addresses stuck Ledger flows (e.g. account import) by **keeping the
WebHID transport open** across bursts of offscreen actions (5s idle
close) instead of closing after every call, with **`forceReset`** to
drop a hung transport when recovery is needed.
>
> **`LedgerOffscreenBridge`** adds action-specific timeouts (30s for
`getPublicKey`, 5min for signing) and clearer timeout errors;
**`unlockHardwareWalletAccount`** drops its separate account-creation
timeout in favor of that stack.
>
> **`ledger-router`** serializes concurrent offscreen messages on one
promise chain, races each action against 60s / 330s backstops (sign vs
read), calls **`forceReset`** on timeout, and swallows late rejections
so the chain can continue. DMK stub forwards **`forceReset`** to legacy.
Tests cover concurrency, timeout recovery, bootstrap error swallowing,
and init/switch races.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
70e07e5. 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: Cursor <cursoragent@cursor.com>
@runway-github
runway-github Bot requested review from a team as code owners August 5, 2026 15:53
@metamask-ci metamask-ci Bot added the team-bots Bot team (for MetaMask Bot, Runway Bot, etc.) label Aug 5, 2026
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@metamask-ci

metamask-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
Builds ready [8cc8db8] [reused from f216617]
⚡ Performance Benchmarks (Total: 🟢 0 pass · 🟡 0 warn · 🔴 0 fail)

Baseline (latest main): 171ed20 | Date: 7/28/2026 | Pipeline: 31023109091 | Baseline logs

Interaction Benchmarks · Samples: 5

⚠️ Missing data: chrome/webpack/interactionUserActions, firefox/webpack/interactionUserActions

✅ No regressions detected

Startup Benchmarks · Samples: 100

⚠️ Missing data: chrome/webpack/startupStandardHome, chrome/webpack/startupPowerUserHome, firefox/webpack/startupStandardHome, firefox/webpack/startupPowerUserHome

✅ No regressions detected

User Journey Benchmarks · Samples: 5 · mock API

⚠️ Missing data: chrome/webpack/userJourneyOnboardingImport, chrome/webpack/userJourneyOnboardingNew, chrome/webpack/userJourneyAssets, chrome/webpack/userJourneyAccountManagement, chrome/webpack/userJourneyTransactions, firefox/webpack/userJourneyOnboardingImport, firefox/webpack/userJourneyOnboardingNew, firefox/webpack/userJourneyAssets, firefox/webpack/userJourneyAccountManagement, firefox/webpack/userJourneyTransactions

✅ No regressions detected

Dapp Page Load Benchmarks · Samples: 100

⚠️ Missing data: chrome/webpack/pageLoadBenchmark

✅ No regressions detected

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 4.15 KiB (0.03%)
  • ui: 33.98 KiB (0.19%)
  • common: 0 Bytes (0%)
  • other: 0 Bytes (0%)
  • contentScripts: 476 Bytes (0.02%)
  • zip: 11.59 KiB (0.05%)

@owencraston owencraston left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in depth by connecting a Ledger Flex.

  • connected 6 accounts including clicking next on the account selector page
  • tested removing and forgetting an account
  • tested erc20 sends
  • tested monad sends
  • tested swaps
  • tested gasless swap on BNB
  • tested gasless send on Linea and BNB
  • tested personal sign and sign type data 4 on the e2e test dapp
  • tested adding a new account

@owencraston
owencraston enabled auto-merge (squash) August 5, 2026 18:10
@sleepytanya
sleepytanya disabled auto-merge August 5, 2026 18:36
@sleepytanya
sleepytanya merged commit a694e1e into release/13.43.0 Aug 5, 2026
128 checks passed
@sleepytanya
sleepytanya deleted the cherry-pick-13-43-0-3cad329 branch August 5, 2026 18:37
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 5, 2026
@metamaskbot metamaskbot added the release-13.43.0 Issue or pull request that will be included in release 13.43.0 label Aug 6, 2026
@gauthierpetetin

Copy link
Copy Markdown
Contributor

No release label on PR. Adding release label release-13.43.0 on PR, as PR was cherry-picked in branch 13.43.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-13.43.0 Issue or pull request that will be included in release 13.43.0 risk:medium team-bots Bot team (for MetaMask Bot, Runway Bot, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants