Skip to content
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

Remove key index restrictions from EIP-2334 #2670

Closed
wants to merge 1 commit into from

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented May 26, 2020

This PR removes the specifics for Ethereum 2 validator keys from EIP-2334.

EIP-2334 provides important information to allow for standardisation of key paths, however the addition of specifics about how to generate validator keys makes it impossible to fully implement in common staking situations.

This part of the spec removed in this PR defines a specific relationship between validator and withdrawal keys that often does not exist in the real world. Situations that this part of the spec precludes include:

  • multi-party computation, where a validator key is created via distributed key generation (hence the validator key cannot be a child of the withdrawal key)
  • staking services, where separate entities hold the validator and withdrawal keys (hence the keys are not generated from the same seed)
  • hardware wallets, where the withdrawal key is held securely whilst the validator key is used for attesting (hence the keys are not generated from the same seed)

Removing these restrictions allow this EIP to be adhered to by more participants.

@alonmuroch
Copy link

@CarlBeek @mcdee
I agree that strict coupling of withdrawal keys and signing keys is less useful.

how about: m/12381/3600/{wallet_index}/{0 for withdrawal, 1 for signing}/{account_index}

@mcdee
Copy link
Contributor Author

mcdee commented May 27, 2020

I don't think that any system linking the keys is necessary or useful; the suggestion will still require the same seed for both.

@alonmuroch
Copy link

Having the ability to create different wallets with their own set of keys is very useful, been around for years. I don't see reason to get backwards here..

@mcdee
Copy link
Contributor Author

mcdee commented May 27, 2020

Different wallets is fine, different wallets with the same seed less so.

@alonmuroch
Copy link

that's how every wallet outside of eth 2 works.. 1 see that supports many wallets

@mcdee
Copy link
Contributor Author

mcdee commented May 27, 2020

They allow multiple accounts, not wallets. The terminology can be a bit different from one client to another, but the idea is that a single seed can generate multiple accounts. What we don't want is a single seed generating multiple streams of accounts, as that could cause user confusion (and endangerment of funds) as above.

@alonmuroch
Copy link

Current crypto wallets enable you dynamicy on 2 levels

  • coin type
  • wallet (for bitcoin like wallets there is another level for change)

A user, from a single seed, can create multiple wallets for the same coin type:
ethereum/wallet1
ethereum/wallet2
For each of those, many "accounts" (addresses) can be created.

That's the functionality everyone would expect from any key derivation scheme.
A validator signing key only really needs the private key not the seed, how you deterministically generate that key is what's important.

@mcdee
Copy link
Contributor Author

mcdee commented May 28, 2020

I haven't seen that two-level functionality in any Ethereum wallets I've used. Regardless, this is about the removal of a specific requirement for certain indices in key derivation to enable the use cases I listed in the original post. Your suggestion does not enable the use cases.

@CarlBeek
Copy link
Contributor

I am not generally in favour of this change because I think it introduces areas of potential confusion for users (and loss of keys within the keyspace). To tackle the particular arguments put forward here:

  1. multi-party computation - by construction, no party involved in the MPC can ever know where a key exists in the key-tree and therefore talking about paths for MPC keys is irrelevant IMO. No MPC mechanism can say they are EIP2334 compliant, what would that even mean?

  2. staking services - In the case where a user generates the withdrawal keys and a 3rd party creates the signing keys, ideally both parties would just use the appropriate keys from their trees. The user would use m/12381/3600/i/0 and the service would use m/12381/3600/j/0/0 for whatever the next free i and j indices are in their respective trees. If this idea is not clearly communicated in the EIP, I am happy to amend it to elaborate.

  3. hardware wallets - hardware wallets could export keystores (EIP2335) with the relevant signing keys whilst retaining the withdrawal keys themselves.

  4. dual mnemonics - IMO this is a failure of design. Everything (practical) that can be achieved with dual mnemonics should also be achievable by using separate paths within the tree. If there is a need for this, I'd be curious to hear about it.

5 wallets/accounts - I think @alonmuroch is referring to the accounts and indices defined in BIP44. The difference between these two is whether they are public or privately derivable. It is functionality that is not really used in the Ethereum space, to my knowledge.

@paulhauner
Copy link

I think the current path derivation scheme (i.e., what is removed in this PR) is quite desirable as it guides the user to separate the voting and withdrawal key-spaces.

To achieve @mcdee's goals (as I understand them) I would prefer a change that retains the current "Validator keys" section, but:

  1. Specifically states that it is valid for a single validator to use voting/withdrawal keys from different seeds.
  2. Moves the signing key from m/12381/3600/i/0/0 to m/12381/3600/i/1.

I don't know enough about MPC make a firm statement as to whether or not (2) is indeed necessary.

Optionally, if we implement (1) we could also specify that that i needs to be the same in each path. It adds an additional degree of uniformity that seems appealing.

@mcdee
Copy link
Contributor Author

mcdee commented Jun 16, 2020

I think the current path derivation scheme (i.e., what is removed in this PR) is quite desirable as it guides the user to separate the voting and withdrawal key-spaces.

@paulhauner I agree with desirability of separating voting and withdrawal keyspaces, but this is an EIP not a best practices guide. If one person wants to use m/12381/3600/{wallet_index}/{0 for withdrawal, 1 for signing}/{account_index} and another m/12381/3600/{account_index}/{0 for withdrawal, 1 for signing} the resultant keys will work in any client, they will both provide a system for knowing one from the other, but one will end up EIP-compliant and the other not.

@CarlBeek
It is important for us to have a standard derivation root (m/12381/3600/...) and some idea of the primary index for account generation, and I'm all for that. But putting an arbitrary set of rules in to the EIP for key generation in a specific case (i.e. staking) overreaches its purpose.

If there are a number of validator best practices that you believe should be made available for stakers place they could go in a separate document, but definition of the path and example uses of the path should not be tied together.

@paulhauner
Copy link

@paulhauner I agree with desirability of separating voting and withdrawal keyspaces, but this is an EIP not a best practices guide. If one person wants to use m/12381/3600/{wallet_index}/{0 for withdrawal, 1 for signing}/{account_index} and another m/12381/3600/{account_index}/{0 for withdrawal, 1 for signing} the resultant keys will work in any client, they will both provide a system for knowing one from the other, but one will end up EIP-compliant and the other not.

If I were to generate keys into m/42/{account_index}/{0 for withdrawal, 1 for signing} the keys will also "work in any client" and have "a system for knowing one from the other" and be not EIP-compliant. However, I would assume that you are comfortable with this being a violation of the EIP.

I'm also well aware that this is an EIP. EIPs are very much in the business of creating cryptography specifications that attempt to prevent users from having enough rope to hang themselves.

I found this comment to be an off-the-cuff dismissal which did not address the content of my statement and, frankly, a little condescending. I'm not interested in hammering on this point, I just wanted to make my discontentment clear.

If there are a number of validator best practices that you believe should be made available for stakers place they could go in a separate document, but definition of the path and example uses of the path should not be tied together.

This point is fair and I'm not opposed to it all.

However, if we were to remove the "Validator keys" section I would advocate for creating an EIP which replaces it.

Consider this scenario: Alice creates a genesis validator and then exits a few weeks after genesis for whatever reason. Now, a year or two has passed and phase 1 has arrived and Alice wishes to withdraw her previously-staked ETH.

Now, we didn't standardize on a path for validator keys and there are several competing standards, some of which included extra nodes in the path. Alice has forgotten which tool she used (it's been over a year, after all) and all she has is a twelve-word mnemonic. Alice is now in the territory of brute-forcing her keys from:

  • m/12381/3600/i/0/0
  • m/12381/3600/i/0
  • m/12381/3600/x/i/0/0
  • m/12381/3600/x/y/i/0/0

If we can make assumptions about i, x and y being low integers, then she's probably going to have success. Regardless, she's going to have a bad time and it cost someone time/money to build that brute-force tool. This is a scenario that I think we should at least try to avoid by providing a standard. Of course, if anyone who wishes to do their own thing is free to do so.

@mcdee
Copy link
Contributor Author

mcdee commented Jun 16, 2020

I found this comment to be an off-the-cuff dismissal which did not address the content of my statement and, frankly, a little condescending.

I offer my apologies for this, it was not my intent.

However, if we were to remove the "Validator keys" section I would advocate for creating an EIP which replaces it.

I'd be very much behind this.

@CarlBeek
Copy link
Contributor

However, if we were to remove the "Validator keys" section I would advocate for creating an EIP which replaces it.

I am not against creating a separate EIP for tracking this, but I am not convinced that this EIP specifies much if that section is removed, arguably only the m / purpose / coin_type / account / use structure.

Already there is some confusion as to exactly what each level should be used for as per the discussions in this thread and in eth1 this has been a train-wreck with 4 different paths in use and the subsequent EIPs (600 & 601) which haven't seen widespread adoption attempting to address this problem.

@mcdee
Copy link
Contributor Author

mcdee commented Jun 18, 2020

@CarlBeek I'd fully support a generic definition of paths to use, e.g. m/12381/3600/i/0 for the ith account from the seed, and think it would have a lot of value as you point out in terms of a standard to move easily from seeds to keys across multiple wallet implementations. And I agree it makes sense to have that as part of EIP-2334.

@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Oct 27, 2020
@mcdee mcdee closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants