Add error message for user when ledger fails when attempting to sign #692
Conversation
👷 Deploy request for dapper-rolypoly-9814ad pending review.Visit the deploys page to approve it
|
|
| const isLedgerError = (error: any) => { | ||
| if (error.code === -32603) { | ||
| // Error message when rejecting from ledger or blind signing and debug not enabled | ||
| if ( |
There was a problem hiding this comment.
Obviously could do if (... && ...) but followed commenting pattern of isUserRejectionError
Happy to change if this is unnecessary.
There was a problem hiding this comment.
Hey @valefar-on-discord do you happen to know the error code that results from not having the proper Ledger settings enabled? Adding error handling here seems like a minimally invasive and safe improvement.
|
Friendly ping on this. I am still assisting users with this issue and though this isn't the optimal solution, it will still help. |
| const isLedgerError = (error: any) => { | ||
| if (error.code === -32603) { | ||
| // Error message when rejecting from ledger or blind signing and debug not enabled | ||
| if ( | ||
| error.message.includes('Ledger: Unknown error while signing transaction') | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
@valefar-on-discord Oops sorry, was on mobile and missed the context...
So -32603 is the error code... is there a reason we need to also check for the "Ledger:" string below? If so, I may consider just checking for .toLowerCase().includes("ledger") so any error message with "Ledger" in it will be flagged.
And also, we could simplify this to just return error.code === -32603 && error.message.toLowerCase().includes("ledger") as a boolean:
| const isLedgerError = (error: any) => { | |
| if (error.code === -32603) { | |
| // Error message when rejecting from ledger or blind signing and debug not enabled | |
| if ( | |
| error.message.includes('Ledger: Unknown error while signing transaction') | |
| ) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| }; | |
| // Error message when rejecting from ledger or blind signing and debug not enabled | |
| const isLedgerError = (error: any) => | |
| error.code === -32603 && error.message.toLowerCase().includes("ledger") |
There was a problem hiding this comment.
Yup yup, sounds good.
Thanks for taking a look!
src/pages/Transactions/index.tsx
Outdated
| blindSigning: <b>Blind Signing</b>, | ||
| debugData: <b>Debug Data</b>, |
There was a problem hiding this comment.
@valefar-on-discord Double checking here, do we want these to be translated? I'm not deeply familiar with any non-English Ledger interface. If we want these terms to be translated, we'll need to wrap them in their own FormattedMessage component.
There was a problem hiding this comment.
That's a great question but I don't believe they are translated.
If you take a look at the ledger documentation in a different language, it will show "Blind Signing"
There was a problem hiding this comment.
I'm going to update the instructions based on this little discovery.
There was a problem hiding this comment.
Updated to extract hard-coded words and used <strong>
Updated version |
Description
When a user attempts to perform a deposit with a Ledger device, if Blind Signing and Debug Data is not enabled, the transaction will fail with seemingly no reason. This has been a problem for years and still people ask about it on the EthStaker discord. Though this issue may be resolved by updating the web3-react library from v6 to v8, a brief attempt at doing so resulted in dependency hell and would be quite a drastic change for this one small issue. I had a quick chat with @wackerow about this and we agreed an error message is the best approach.
The error message will appear if either Blind Signing or Debug Data are disabled or, sadly, if the user simply rejects from the Ledger device. We have no way to differentiate but I feel the benefit of the guidance outweighs the false positive.
I am a little uncertain on exact wording or if there is a stylistic standard around presenting error messages but happy to update as necessary.
Related Issues