dominant-strategies / quais.js

MIT License
0 stars 3 forks source link

[Quais] - Rearchitect HD Wallets #161

Open mibuono opened 1 month ago

Juuddi commented 1 month ago

Context: https://github.com/dominant-strategies/quais-6.js/pull/151#issuecomment-2125564987

alejoacosta74 commented 1 month ago

from: https://github.com/dominant-strategies/quais-6.js/pull/151

@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 ethers package intended the HD wallet to be used. First, you will notice it is called an HDNodeWallet implying that each instance is supposed to represent a node within the derivation tree. When viewed through this lens, it becomes more clear the purpose of deriveChild() and there is no deriveAddress() present in the ethers implementation of HDNodeWallet. This is because an instance of an HD node does not require a full path, but rather supplies the method deriveChild() that allows for reaching an HD node that ultimately has a complete BIP44 path. Let's look at an example - a master HD node for deriving Quai addresses should be able to accept the path m/44/969, where the account, change, and address_index values are not yet specified. This master node instance could then create a child node instance for account 0 by calling master_quai_node.deriveChild(0) which would create a node with a resulting path of m/44/969/0. Still, at this depth we are not able to generate a meaningful address that would be used within the Quai ledger of any shard because we still must specify change and address_index values in the path. The following series of operations would get us to a point where we would have a node whose address created from the public key would (sharded address space aside) be correct according to BIP44 and thus usable within a wallet implementation:

account_0_node = master_quai_node.deriveChild(0)
account_0_non_change_node = account_0_node.deriveChild(0)
account_0_address_0_node = account_0_non_change_node.deriveChild(0)

With this example is it obvious to see that calling getAddress on any HD node instance other than account_0_address_0_node would result in a violation of the BIP44 implementation.

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 address_index or we create another layer of abstraction above the QiHDWallet that manages deriving nodes properly and then uses the private keys of each leaf node for performing signature aggregation.

alejoacosta74 commented 1 month ago

Image