-
Notifications
You must be signed in to change notification settings - Fork 37
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 refactor #218
Ledger refactor #218
Conversation
✅ Deploy Preview for cheery-moxie-4f1121 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 880d09f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial tACK 99398ac
Code looks good, Ledger itself works very nicely for me - can't really test the Access Denied
catch since I've now fixed my udev rules, hah.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-tACK 99398ac
much better now, ledger is working, no access errors on my end.
Abstract
This PR aims to refactor ledger to prevent most of the common errors that commonly pop up.
All MasterKey functions are now non-async.
HardwareMasterKey is now simply a view-only HdMasterKey with one addtional function that verifies the address. All non-important addresses must be derived using
getAddress
as normal.When an address needs to be used to transfer funds (for instance to generate change addresses), verifyAddress must also be called, which will derive the address using the ledger and will ask the user to validate it.
Testing