Cosmos-sdk: keys: Should support ed25519

Created on 28 Jul 2018  路  8Comments  路  Source: cosmos/cosmos-sdk

Currently we don't support ed25519

gaiacli keys add --type ed25519 potato
override the existing name potato [y/n]:y
Enter a passphrase for your key:
Repeat the passphrase:
ERROR: unsupported signing algo: only secp256k1 is supported

I'm _pretty_ sure we want to support ed25519 at launch. (This error was a pretty big surprise to me, thanks @amrali for asking questions that led to debugging this!) ~If desired, I can argue for supporting it at launch here, if for some reason ed25519 isn't supposed to be here at launch.~ (this can easily be added back post launch)

My guess is that ed25519 support got removed from gaiacli when we added proper HD derivation support. There is currently an ed25519-bip32 spec by dmitry written here (https://cardanolaunch.com/assets/Ed25519_BIP.pdf - not sure if this is the latest version, I don't have a quick way to check), for the case without ristretto. I don't think we should support this spec at launch due to it not being standardized. (Plus it would be awesome if we could just jump to using ristretto soon after launch)

However all we _really_ need is to support a mnemonic to remember. This is quite easy to do (conceptually, implementation may not be). We can just use an error correction code of suitable strength on the private key, and then convert those bytes into words in the same way we do for secp256k1. We should probably prefix the bytes after the error correction code but before the wordlist with a 0x00 byte, just so its easy to identify in the future.

(Note that we should only be using the first 32 bytes of the private key, since the latter 32 bytes are the public key, see tendermint/ed25519 godoc for more details)

keys proposal

Most helpful comment

cc @Liamsi

I think we could add in ed25519 support (without HD derivation) for user keys pretty easily, although I guess it would require adding extra functions to the keybase API.

All 8 comments

If we add back ed25519 support for users, we should also add an option to gaiad init to make the account pubkey ed25519. (The validator privkey is still ed25519, we just don't display that to users, perhaps we should?)

If we add back ed25519 support for users, we should also add an option to gaiad init to make the account pubkey ed25519. (The validator privkey is still ed25519, we just don't display that to users, perhaps we should?)

I looked into the code base to figure out what this mnemonic seed is for (the one from gaiad init), it turned out it is for the Secp256k1 account key. So there's at least a UX issue here where it is unclear what the output information is for.

BIP-39 implements its own checksum, it's not clear why the error correction code (for the sake of consistency with how it is done for Secp256k1 private key?)

I think it be better to have different mnemonic seeds for each of the validator and the owner keys.

BIP-39 implements its own checksum, it's not clear why the error correction code (for the sake of consistency with how it is done for Secp256k1 private key?)

Oh cool. I didn't realize bip-39 was the thing that handled mnemonics + error correction codes. I had only read through bip-32 before. We should totally just use bip-39.

So there's at least a UX issue here where it is unclear what the output information is for.

Agreed. We should definitely have a better message here. I also agree that we should provide different mnemonics for both the validator and owner key.

cc @Liamsi

I think we could add in ed25519 support (without HD derivation) for user keys pretty easily, although I guess it would require adding extra functions to the keybase API.

Disagree on Prelaunch.

All fundraiser keys are secp keys. No one can even really use a new keypair until transfers are enabled.

You're right. Removed the prelaunch tags.

I've somehow missed this discussion. I agree with @zmanian on this!

I think we could add in ed25519 support (without HD derivation) for user keys pretty easily, although I guess it would require adding extra functions to the keybase API.

That doesn't sound right to me. We should either add ed25519 with some form of HD derivation, or, we should not have the interfaces changed. Maybe, we could add support for ed25519 without changing the keybase APi though.

That sounds good to me! This is definitely something that will be easier to support later. (Doesn't even involve changing the version) I think I'll just close this, and we can look into ed25519 again post launch / after hd derivation is suitably standardized.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rigelrozanski picture rigelrozanski  路  3Comments

fedekunze picture fedekunze  路  3Comments

kevlubkcm picture kevlubkcm  路  3Comments

hendrikhofstadt picture hendrikhofstadt  路  3Comments

fedekunze picture fedekunze  路  3Comments