fix: temporary workaround for thorchain and cosmos ledger app check missing in hdwallet #6850
Conversation
…missing in hdwallet
| }), | ||
| ), | ||
| ) | ||
|
|
||
| const fulfilledAccountIdsAndMetadata = settledAccountIdsAndMetadata.reduce<AccountMetadataById[]>( | ||
| (acc, result) => { | ||
| if (isRejected(result)) { | ||
| console.error(result.reason) | ||
| if (throwOnReject) { |
| }, | ||
| "errors": { | ||
| "accountMetadataFailedToFetch": { | ||
| "title": "Account Metadata Fetch Failed", |
There was a problem hiding this comment.
This seems like quite a low-level messaging, but given the root cause is eventually going to get fixed, fine with that!
There was a problem hiding this comment.
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:
|
|
Root cause fixed in hdwallet, can remove the workaround but need to keep paging reset. |
|
Superseeded by #6866 with pagination hunks applied there |
Description
When attempting to manage accounts for thorchain or cosmos, the app crashes due to missing support for
validateCurrentAppon these chains. When callingvalidateCurrentApp, the promise is rejected withUnable 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
throwOnRejectflag 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
Issue (if applicable)
progresses #6806 - will not be closed until hdwallet is updated to support these chains and the patch removed from web.
Risk
Low risk
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)
Screen.Recording.2024-05-08.at.1.55.23.PM.mov