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

Ledger "unexpected application error!" after clicking the account to connect with "No LedgerRequestKeysContext found" #4786

Closed
314159265359879 opened this issue Jan 9, 2024 · 20 comments · Fixed by #4888 or #5357
Assignees
Labels
area:ledger bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds effort:medium Expected to take 2-5 days of integration work

Comments

@314159265359879
Copy link
Contributor

Reproduce with these steps

  1. Sign out wallet
  2. Connect wallet to Stacks with Ledger, so account balance shows in the extension
  3. Then disconnect the ledger device from computer
  4. Sign in to a dapp for example lockstacks.com
  5. Select account to connect with
  6. See error:

image

To fix issue: connect Ledger Device to your computer again, start the Stacks app on the Ledger device then try again.

I think instead of this error the wallet should show better guidance for the user.

@314159265359879 314159265359879 added bug Functionality broken bug-p3 Non-critical functionality broken for many users, or there are clear workarounds area:ledger labels Jan 9, 2024
@wizard1786
Copy link

How about for edge browser?

@wizard1786
Copy link

Did not work with edge browser got the same error message
Any help on transferring stx out of leather wallet ?

@edgarkhanzadian
Copy link
Contributor

@314159265359879 I've tried reproducing the error but i didn't get any result. What version did you get this error on? Can you, please, try reproducing it on the dev branch now?

Note: you won't be able to see it in the video, but i disconnected the ledger device right after signing in into leather with stacks app.

reproductionAttemptLedgerError.mov

@wizard1786
Copy link

wizard1786 commented Jan 24, 2024 via email

@wizard1786
Copy link

wizard1786 commented Jan 24, 2024 via email

@edgarkhanzadian
Copy link
Contributor

@wizard1786 using a different ledger device shouldn't make a difference in theory, but let me know if you are able to reproduce it on another type of device. I was unable to repro it on ledger nano X

@314159265359879
Copy link
Contributor Author

@edgarkhanzadian I first tested this with version 6.22.0 of the wallet and a Nano S plus, it is still the same for me today with version 6.24.0.
Testing on Chrome version 120.0.6099.227
firmware version 1.1.1

You're video isn't playing for me (looks like an issue with github).

2024-01-25_15-40-03.mp4

@wizard1786
Copy link

wizard1786 commented Jan 25, 2024 via email

@wizard1786
Copy link

wizard1786 commented Jan 25, 2024 via email

@314159265359879
Copy link
Contributor Author

@wizard1786 We have enough video material, thanks for the effort though.

If you keep hitting this error even when the Ledger device is connected to your computer and it is the same device used to setup your Leather wallet. I assume there is an issue with connection or cable. Try it on another device, another browser and/or with another cable to exclude any issues with those.

Before signing in on a dapp, make sure your Leather wallet is on the same account you want to sign in with. Then sign in on the page. Please connect via support@leather.io for further assistance if you see this persist.

@kyranjamie
Copy link
Collaborator

Reopening this issue, saw it happen.

When trying to sign a transaction with Ledger, and the device isn't able to connect, we route to an error page (that isn't within the context so we see this error).

@kyranjamie kyranjamie added bug-p2 Critical functionality broken for few users, with no clear workarounds and removed bug-p3 Non-critical functionality broken for many users, or there are clear workarounds labels Mar 24, 2024
@wizard1786

This comment was marked as off-topic.

@kyranjamie kyranjamie assigned kyranjamie and unassigned kyranjamie Mar 25, 2024
@pete-watters pete-watters self-assigned this Apr 11, 2024
@pete-watters pete-watters added the effort:medium Expected to take 2-5 days of integration work label Apr 19, 2024
@pete-watters
Copy link
Contributor

pete-watters commented May 9, 2024

I've been investigating this but as yet unable to reproduce. Once signed in to a DAPP with ledger, if I then disconnect, when I try and perform an action it prompts me to connect the Ledger again.

  • I signed in to Gamma
  • initiated a purchase of a BTC collectible
  • disconnected the ledger
  • observed that the popup prompts me to connect my Ledger and doesn't error

I'll keep investigating and try with a Stacks interaction instead in case there is some bug there.

@kyranjamie do you remember exactly what you were doing to cause the error? Maybe its different than the device not being connected and it's an issue with communicating with a connected device?

@kyranjamie
Copy link
Collaborator

kyranjamie commented May 9, 2024

This was a routing issue I think, where you get routed somewhere outside the context. Try with lockstacks.com

@pete-watters
Copy link
Contributor

I could be missing something here but if my Ledger is not connected, I consistently get a prompt to ask me to connect it rather than hitting any kind of uncaught error.

No LedgerRequestKeysContext found is an error we throw in useLedgerRequestKeysContext and I cannot find any instances of it in Sentry, albeit it for the past 90 days.

If I try with lockstacks.com to:

In both cases, the Leather popup handles the lack of Ledger connection gracefully and shows the We're unable to connect to your Ledger device warning.

Looking at the code and two places where we use useLedgerRequestKeysContext and could potentially hit this error are in:

  • ConnectLedgerRequestKeys
  • AddMoreKeysLayout

If I am testing ConnectLedgerRequestKeys AKA the initial Route - ConnectLedger - connect-your-ledger it seems to be handled well.

If it is still a bug it seems likely related to AddMoreKeysLayout but when I look deeper into that, it seems like dead code no longer used. In our code we only have:

  • a reference to only the route RouteUrls.LedgerAddMoreKeys in route-urls
  • Handling of that route in ledgerRequestKeysRoutes with component AddMoreKeysLayout

@kyranjamie : do you know what the LedgerAddMoreKeys route is for? Does it relate to #2502? It was added in this PR - #4042

@314159265359879 : I can't play your repro video linked in #4786 (comment). When you have time can you please help me try and reproduce this?

@kyranjamie
Copy link
Collaborator

Thanks for the investigation @pete-watters. Could be that it's been fixed 👍🏼

RE: RouteUrls.LedgerAddMoreKey. At one point, we had a route that would allow you to add more keys, after you'd added the first, but I think this has been removed as we show the connect button on homepage. As such, think we can remove it.

@pete-watters
Copy link
Contributor

OK thanks. I will remove that route and when Werner is back I will double check with him and if he cannot reproduce I will close.

@pete-watters
Copy link
Contributor

@314159265359879 when you have a chance can you please help test if this is still a problem? I was unable to reproduce so I think it is fixed but if you can still reproduce it please let me know

@314159265359879
Copy link
Contributor Author

@pete-watters I retested this too, and it looks resolved indeed. It is handled gracefully now.

@pete-watters
Copy link
Contributor

OK that's great, thanks for confirming 👍

kyranjamie pushed a commit that referenced this issue May 14, 2024
## [6.39.0](v6.38.0...v6.39.0) (2024-05-14)

### Features

* hide asset list unsupported tokens in accordion, closes [#16](#16) ([f37bb1b](f37bb1b))

### Bug Fixes

* add schema validation for stamps ([7a89337](7a89337))
* change default to pulling 10 keys ([b5c9c79](b5c9c79))
* compliance checks reenabled ([d0c17ec](d0c17ec))
* constant retrying of metadata ([540c349](540c349))
* double stacks asset in ledger mode ([5d9895f](5d9895f))
* remove ledger add more keys route, ref [#4786](#4786) ([45c9968](45c9968))
* use nakamoto testnet link when network is set to nakamoto testnet ([e522113](e522113))

### Internal

* input calc helper methods ([16a9e6e](16a9e6e))
* post-release merge back ([9caef96](9caef96))
* remove multiple recipients components ([8f83bcc](8f83bcc))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ledger bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds effort:medium Expected to take 2-5 days of integration work
Projects
None yet
6 participants