hashgraph / hashgraph/hedera-sdk-reference
Refactor key derivation
- Dominant language
- HTML
- Stars
- 7
- Forks
- 3
- PR merge metrics
- No merged PRs in 30d
Description
There are problems with how key derivations are currently implemented. This issue is for discussing the best solution to the existing problems, and settling on a course of action.
## Background
- https://github.com/hashgraph/hedera-sdk-java/issues/1263
- https://github.com/hashgraph/hedera-sdk-js/issues/1350
## Proposal (will be updated as discussion occurs)
### Converting Mnemonics to Private Keys
1. `Mnemonic.toStandardEd25519PrivateKey(passphrase="", index=0)`
- returns the child key at derivation path `m/44'/3030'/0'/0'/index'`
- this function automatically hardens `index`. Passing in a pre-hardened index **should fail**
2. `Mnemonic.toStandardECDSAsecp256k1PrivateKey(passphrase="", index=0)`
- returns the child key at derivation path `m/44'/3030'/0'/0/index`
- if the user wants a hardened child, they must **manually** harden `index`
3. `Mnemonic.toLegacyPrivateKey()` returns an `Ed25519` private key
- this function should choose the *type* of legacy derivation based on mnemonic word count / composition
- 22 words from the non-standard list means legacy derivation v1
- 24 words from the standard list means legacy derivation v2
### Deriving Child Keys
1. `PrivateKey.derive(index)`
- returns the child key at `index`
- if `PrivateKey` is an Ed25519 key, `index` is automatically hardened. passing in a pre-hardened index **should fail**
- if `PrivateKey` is an ECDSA key, `index` must be manually hardened, if desired
2. `PrivateKey.legacyDerive(index)`
- returns the child key at `index`, using the legacy algorithm
- only valid for Ed25519 keys. Calling this on an ECDSA key **should fail**
### Deprecated Functions (including but not necessarily limited to)
1. `Mnemonic.toEd25519PrivateKey(passphrase = "", path = ...)`
- since using the default path returns a key that shouldn't be used
2. `Mnemonic.toECDSAPrivateKey(passphrase = "", path = ...)`
- since the default path is entirely wrong, and returns a non-standard key (JS only)
### Tests
1. All test vectors described in [SLIP10](https://github.com/satoshilabs/slips/blob/master/slip-0010.md#test-vectors), ~[BIP32](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki)~, and [BIP39](https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#generating-the-mnemonic) should be checked for all key types
- Some refactoring will be necessary to support test vectors from seed
- BIP-32 test requirement removed for now, since base58 extended key encoding isn't supported
2. The following test vectors should be created and standardized across SDKs
- `legacyMnemonicV1`
- mnemonic to key
- legacy derivation (if applicable, see question # 1 below)
- `legacyMnemonicV2`
- mnemonic to key
- legacy derivation (if applicable, see question # 1 below)
- `toStandardEd25519PrivateKey`
- mnemonic to key with no passphrase and index `0`
- mnemonic to key with passphrase and index `0`
- mnemonic to key with passphrase and index `max`
- `toStandardECDSAsecp256k1PrivateKey`
- mnemonic to key with no passphrase and index `0`
- mnemonic to key with no passphrase and index `0'`
- mnemonic to key with passphrase and index `0`
- mnemonic to key with passphrase and index `0'`
- mnemonic to key with passphrase and index `max`
- mnemonic to key with passphrase and index `max'`
### Possible future functionality (not included in the current proposal)
1. Functions that allow the user to explicitly define a full derivation path
- The functions that do this are being deprecated
- Since this is presumably not a commonly used feature, it doesn't need to be added urgently
2. *Public key* derivation
- inherently not possible with Ed25519
- should eventually be implemented for ECDSA
### Questions
1. When is `PrivateKey.legacyDerive(index)` actually needed? Is this just for *one* of the legacy versions, or *both*?
- this needs to be figured out and documented
2. Can we remove the `isLegacy` flag from `Mnemonic`?
- There currently exists some functionality to construct a mnemonic with an `isLegacy` flag, or to infer `isLegacy` based on word count
- I personally don't like this at all. Not all SDKs do this the same way, and `isLegacy` actually seems to mean "is legacy v1", since legacy v2 mnemonics are notably *not* `isLegacy`
- IMO we should rip this confusing bandaid off now, and just require the wallet implementor to explicitly call `toLegacyPrivateKey`, rather than inconsistently and imperfectly inferring `isLegacy`
4. Should we specify the curve in the ECDSA function names? (see [below](https://github.com/hashgraph/hedera-sdk-reference/issues/73#issuecomment-1353196841) for details)
Contributor guide
Assessment
This issue has not been assessed yet.