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

Handle Ledger THORChain & Cosmos app open request #6806

Closed
0xApotheosis opened this issue May 3, 2024 · 3 comments · Fixed by #6866
Closed

Handle Ledger THORChain & Cosmos app open request #6806

0xApotheosis opened this issue May 3, 2024 · 3 comments · Fixed by #6866
Assignees

Comments

@0xApotheosis
Copy link
Member

0xApotheosis commented May 3, 2024

Detecting if the THORChain app is open on Ledger is currently broken, which I suspect is caused by await this.transport.call(null, "getAppAndVersion") returning THORChain, whereas the slip key is written as Thorchain in HDWallet's slip44Table (I suspect incorrectly, as SLIP-0044 lists it as THORChain too).

This might require a slight tweak in HDWallet.

@0xApotheosis 0xApotheosis converted this from a draft issue May 3, 2024
@0xApotheosis 0xApotheosis self-assigned this May 3, 2024
@0xApotheosis 0xApotheosis moved this from Backlog to Up next / groomed in ShapeShift Dashboard May 3, 2024
@0xApotheosis 0xApotheosis added this to the account management milestone May 3, 2024
@0xApotheosis 0xApotheosis changed the title Handle THORChain app open request Handle Ledger THORChain app open request May 3, 2024
@woodenfurniture woodenfurniture moved this from Up next / groomed to In progress in ShapeShift Dashboard May 8, 2024
@0xApotheosis
Copy link
Member Author

Cormos is in the same boat, debug this at the same time 🙏

@0xApotheosis 0xApotheosis changed the title Handle Ledger THORChain app open request Handle Ledger THORChain & Cosmos app open request May 8, 2024
@woodenfurniture
Copy link
Contributor

When attempting to manage accounts for thorchain or cosmos, the app crashes due to missing support for validateCurrentApp on these chains. When calling validateCurrentApp, the promise is rejected with Unable to find associated app name for coin: ${coin} because support for these chains is actually missing from hdwallet.

This issue never presented in web previously because we never attempt to validate the correct app being open on ledger, and instead swallow errors (hence the requirement for the new throwOnReject flag added in this PR.

The workaround here is to bypass this check and allow the user to attempt to proceed, then display an error toast if account fetching fails.

We'll have to follow up with a fix in hdwallet and remove this temporary patch.

@woodenfurniture
Copy link
Contributor

Once shapeshift/hdwallet#670 is merged, will have to follow up in web and remove the patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants