-
Notifications
You must be signed in to change notification settings - Fork 6
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
Make 'QuaiHDWallet' class to extend 'HDWallet' #151
Make 'QuaiHDWallet' class to extend 'HDWallet' #151
Conversation
7fadcb6
to
f37ad6a
Compare
@alejoacosta74 pretty much everything in this PR looks good and implements what we have previously agreed on. However, at some point I think we may have introduced an error in the mental model we use regarding the purpose of HD Wallets as they are implemented in ethers/quais. I am going to outline my thoughts below on how I believe they were intended to work and how our updates may go against the use case. Let's start by looking at how the
With this example is it obvious to see that calling So, where did we go wrong? Our misstep occurred (I think) when we started to imagine how this implementation could be adapted for use with Qi. There is a starkly different requirement of Qi wallets in that they must support aggregated signatures by many private keys. This requirement alone violates the initial HD wallet implementation which only contains a single public/private key pair per HD node instance. I see two pretty clear options to move forward here: scrap the node base implementation and allow people to only specify either full paths or path missing only |
f37ad6a
to
0060cc6
Compare
This PR changes the following:
QuaiHDWallet
class to parentHDWallet
classgetAddress
method forQuaiHDWallet