fix: temporary workaround for thorchain and cosmos ledger app check missing in hdwallet

woodenfurniture commented 3 weeks ago

Description

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.

Pull Request Type

[x] :bug: Bug fix (Non-breaking Change: Fixes an issue)
[ ] :hammer_and_wrench: Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
[ ] :nail_care: New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

progresses #6806 - will not be closed until hdwallet is updated to support these chains and the patch removed from web.

Risk

High Risk PRs Require 2 approvals

Low risk

What protocols, transaction types or contract interactions might be affected by this PR?

Testing

Attempt to manage thorchain or cosmos accounts without the corresponding ledger app being open on the device. A toast should appear with an appropriate error message.

Managing an account with the corresponding ledger app open should not show error toast, but should behave normally.

Engineering

Operations

Screenshots (if applicable)

https://github.com/shapeshift/web/assets/125113430/e7f40b23-607b-445f-8a55-c85bd6f41f21

woodenfurniture commented 3 weeks ago

Tested locally, confirmed this is happy, though with an issue related to one of the two hard things in software engineering.

Not sure if we want to get this in in this state though however - if this is eventually going to get fixed upstream, it may not worth the effort to fix the caching bug and rather fix the root cause in hdwallet instead?

Confirmed it's impossible to currently continue with the wrong app on Ledger ✅

There's a double toast, although this is eventually going to be removed

The toast(s) only appear once 🚫

Screen.Recording.2024-05-08.at.14.02.48.mov

Connecting the correct app is still happy ❓

Though on a second try, similarly to the toasts that never appear again, caching means auto-fetching is borked. If hard refreshing the app and trying with the correct app on the first try, this is happy.

Screen.Recording.2024-05-08.at.14.04.03.mov

Ok, thanks for the testing ser! New changes pushed:

Clear account paging when opening account import for ledger wallets. This fixes the case where it's possible to view accounts from a previous load without the correct ledger app open on the device.
The above also fixes the error toast only appearing once.
Fixed double toast on error.
Moved toast to top-right for consistency with other toasts in account management.

woodenfurniture commented 3 weeks ago

Root cause fixed in hdwallet, can remove the workaround but need to keep paging reset.

gomesalexandre commented 3 weeks ago

Superseeded by https://github.com/shapeshift/web/pull/6866 with pagination hunks applied there

shapeshift / web