Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Unable to pair to Ledger a second time after removing accounts #22837

Closed
plasmacorral opened this issue Feb 6, 2024 · 5 comments · Fixed by #25462
Closed

[Bug]: Unable to pair to Ledger a second time after removing accounts #22837

plasmacorral opened this issue Feb 6, 2024 · 5 comments · Fixed by #25462
Assignees
Labels
hardware-ledger regression-prod-11.9.0 Regression bug that was found in production in release 11.9.0 regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead regression-RC-11.10.0 release-11.10.0 Issue or pull request that will be included in release 11.10.0 release-12.2.0 Issue or pull request that will be included in release 12.2.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-hardware-wallets type-bug

Comments

@plasmacorral
Copy link
Contributor

Describe the bug

chrome 117.0.5938.92
mac Sonoma 14.2.1
Ledger Nano X running firmware 2.1.0
Ethereum app 1.10.1

If I pair the Ledger device, remove the accounts, and try to pair again I am stuck with an Unknown error and unable to add any further Ledger accounts.

The only way I have found so far to get out of this situation is to forget all HID devices and reload the extension.

This was observed in 11.10 RC testing, but confirmed to exist in 11.9.0.

Expected behavior

Should be able to add and remove Ledger accounts repeatedly without having to forget devices and reload extension.

Screenshots/Recordings

https://recordit.co/P9YMp1mOcN

Steps to reproduce

  1. Setup wallet
  2. Connect, unlock and open the Ethereum app on Ledger
  3. Use the add hardware flow to add some Ledger accounts
  4. Remove the Ledger accounts from full screen mode
  5. Use add hardware flow to try and add any Ledger accounts
  6. Note Unknown Error in extension

Error messages or log output

LEDGER:::CREATE APP ERROR 
TransportError {name: 'TransportError', message: "U2F browser support is needed for Ledger. Please u…ion. Also make sure you're on an HTTPS connection", id: 'U2FNotSupported', stack: 'Error\n    at new TransportError (https://metamask.…b.io/eth-ledger-bridge-keyring/bundle.js:22460:22'}
id
: 
"U2FNotSupported"
message
: 
"U2F browser support is needed for Ledger. Please use Chrome, Opera or Firefox with a U2F extension. Also make sure you're on an HTTPS connection"
name
: 
"TransportError"
stack
: 
"Error\n    at new TransportError (https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:18069:18)\n    at https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:22460:22"
[[Prototype]]
: 
Error at new TransportError (https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:18069:18) at https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:22460:22
stack
: 
"Error\n    at new TransportError (https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:18069:18)\n    at https://metamask.github.io/eth-ledger-bridge-keyring/bundle.js:22460:22"
[[Prototype]]
: 
Object
constructor
: 
ƒ Error()
message
: 
""
name
: 
"Error"
toString
: 
ƒ toString()
[[Prototype]]
: 
Object
constructor
: 
ƒ Object()
hasOwnProperty
: 
ƒ hasOwnProperty()
isPrototypeOf
: 
ƒ isPrototypeOf()
propertyIsEnumerable
: 
ƒ propertyIsEnumerable()
toLocaleString
: 
ƒ toLocaleString()
toString
: 
ƒ toString()
valueOf
: 
ƒ valueOf()
__defineGetter__
: 
ƒ __defineGetter__()
__defineSetter__
: 
ƒ __defineSetter__()
__lookupGetter__
: 
ƒ __lookupGetter__()
__lookupSetter__
: 
ƒ __lookupSetter__()
__proto__
: 
(...)
get __proto__
: 
ƒ __proto__()
set __proto__
: 
ƒ __proto__()

Version

11.9.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

Ledger

Additional context

No response

Severity

No response

@plasmacorral plasmacorral added type-bug Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. hardware-ledger regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead team-hardware-wallets release-11.10.0 Issue or pull request that will be included in release 11.10.0 labels Feb 6, 2024
@plasmacorral plasmacorral changed the title [Bug]: Unable to pair to ledger a second time, when webHID shows the device as paired [Bug]: Unable to pair to Ledger a second time after removing accounts Feb 6, 2024
@plasmacorral plasmacorral added Sev2-normal Normal severity; minor loss of service or inconvenience. and removed Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. labels Feb 6, 2024
@sleepytanya
Copy link
Contributor

Observed the same issue with Ledger on 11.10.0

  1. Create a tx
  2. Disconnect device while signing
  3. Create another tx
  4. Try to connect device
  5. See 'unknown error' message
  6. Can be resolved by deleting hardwallet account and reloading extension
    https://recordit.co/kdCDL4laWo

@plasmacorral plasmacorral added regression-prod-11.9.0 Regression bug that was found in production in release 11.9.0 and removed regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead labels Feb 9, 2024
@metamaskbot metamaskbot added the regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead label Feb 9, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Feb 19, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Apr 9, 2024
Copy link
Contributor

github-actions bot commented May 9, 2024

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label May 9, 2024
@github-actions github-actions bot removed the stale issues and PRs marked as stale label May 15, 2024
@dawnseeker8
Copy link
Contributor

dawnseeker8 commented Jun 19, 2024

i am unable to reproduce the issue in metamask extension 11.16.7 which is latest in develop branch.
i try both the cases described in comments and it still work to add ledger. the second case of disconnected ledger during signing transaction, will lead to transaction failure status for 10 mins. and then you can do transaction again.

@vivek-consensys
Copy link

Hi @plasmacorral, I have tried to reproduce using the latest versions but did not face this issue with the steps provided.

Chrome: 125.0.6422.142
Metamask Extension: 11.16.12
Ledger Nano X running firmware 2.2.4
Ethereum app 1.10.4

However, I am able to reproduce with the 'Unknown Error' when the ledger device is locked and the user is trying to add/remove the device into MM Extension.

@dawnseeker8 are you able to debug?

@dawnseeker8
Copy link
Contributor

hi, i have fixed the issue with patch to @metamask/eth-ledger-bridge-keyring. the pr is #25462

dawnseeker8 added a commit that referenced this issue Jul 2, 2024
<!--
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**

Issue #22837 descrip that ledger sometimes will display `Unknown Error`
during pairing. we have tried to replicate the issue and discover that
it happen during ledger is lock and not open Eth app. this PR will
replace the `Unknown Error` with more meaningful error message to guide
user solve the issue.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25462?quickstart=1)

## **Related issues**

Fixes: #22837 

## **Manual testing steps**

1. Setup wallet
2. Connect, unlock and open the Ethereum app on Ledger
3. Use the add hardware flow to add some Ledger accounts
4. Remove the Ledger accounts from full screen mode
5. Use add hardware flow to try and add any Ledger accounts
6. Select the `paired` with that ledger when it is available
7. ledger should be locked status or unlock ledger but not opening eth
app.
8. you will see new error message `Unlock your Ledger device and open
the ETH app`
9. After you unlock your ledger and open ETH app. and click paired again
10. you should be able to select accounts from next screen and import
those accounts from ledger.


## **Screenshots/Recordings**

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

### **Before**
https://recordit.co/kdCDL4laWo

<!-- [screenshots/recordings] -->

### **After**

https://github.com/MetaMask/metamask-extension/assets/7315988/f11a1867-ad3f-4796-8661-6d3469bed682


<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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-extension/blob/develop/.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.

---------

Co-authored-by: chloeYue <[email protected]>
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Jul 2, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jul 2, 2024
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Jul 2, 2024
@angelcheung22 angelcheung22 added this to the Q2 2024 CET hw milestone Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hardware-ledger regression-prod-11.9.0 Regression bug that was found in production in release 11.9.0 regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead regression-RC-11.10.0 release-11.10.0 Issue or pull request that will be included in release 11.10.0 release-12.2.0 Issue or pull request that will be included in release 12.2.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-hardware-wallets type-bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants