KintabaHQ / EtherKit

Apache License 2.0
42 stars 6 forks source link

Major refactor to KeyManager + Addition of BIP32 support. #27

Closed SirensOfTitan closed 6 years ago

SirensOfTitan commented 6 years ago

This is a pretty huge diff, really sorry in advance.

The previous KeyManager was a bit of a disaster. It was written to work and because of the already large issues with readability with Apple's Security APIs and our very light wrapper on the secp256k1 C wrapper, it was very unreadable, which is super bad for security.

This rewrites key manager. We introduce a KeyType which is a protocol describing what a key should look like.

... each PrivateKeyType has a StorageStrategy, which describes how to both store inside a particular secure container (like Keychain, SecureEnclave) and to map a private key value from it. This allows us to apply composition better (See MnemonicStorageStrategy and EnclaveStorageStrategy) in a more declarative style.

Then, I've split keys into two types: Key (basic secp256k1 key pair) HDKey (BIP32 key pair)

I think it's important to support both HD keys and non HD keys, as sometimes you don't need the complexity of HD keys. As above, we use a heavily compositional style, and HDKeys are composed of normal Keys. Normal Keys do not support recovery, their private bytes are generated and then used as if they were inside the enclave.

HDKey supports two types of keys: mnemonic based and seed based. In BIP39, we use a pseudo-random key generation function to build a seed for a BIP32 (HD) wallet. The seed cannot be translated back into the mnemonic, so user friendly wallets are sort of left to dry. Because of this, you can create a HDKey where the mnemonic is stored inside the enclave OR the seed is.

This is pretty raw right now, as I just sort of finished and haven't gotten a chance to reabsorb some decisions. It should be in almost pretty good shape, and I've written some code to test BIP32 basically using the test vectors inside BIP32 spec.

SirensOfTitan commented 6 years ago

Also realized my use of swiftformat caused me to touch a couple files that aren't relevant in this diff. Sorry about that!