fromMnemonic returns a private key which shouldn't be used

[litt3]

This isn't a bug, but rather a way that an SDK user can easily misuse the PrivateKey::fromMnemonic function.

By default, calling fromMnemonic returns the key at derivation path m/44'/3030'/0'/0' (A in the figure below). If a programmer isn't familiar with derivation paths, they will probably just use the resulting private key without a further thought. This behavior seems to be enforced by the online documentation, which shows simple usage of fromMnemonic

//Use the mnemonic to recover the private key
PrivateKey privateKey = PrivateKey.fromMnemonic(mnemonic);
PublicKey publicKey = privateKey.publicKey();

//v2.0.0

However, the private key created from fromMnemonic should not be used directly - the programmer ought to go one layer deeper in the derivation path, to m/44'/3030'/0'/0'/0', m/44'/3030'/0'/0'/1', etc.. (B in the figure below). This requirement is documented, but only briefly, in the function documentation

     * @return the recovered key; use {@link #derive(int)} to get a key for an account index (0
     * for default account)

My understanding is that receiving to A is out of line with expected BIP44 behavior, and doing so might result in different wallets not finding existing funds. It seems worth it to me to make it harder for a wallet developer to mistakenly receive here.

[ochikov]

Just to add here, that in some of the SDKs, fromMnemonic is deprecated and the suggestion is to use Mnemonic.from[Words|String]().to[Ed25519|Ecdsa]PrivateKey()

[litt3]

One possible suggestion:

Have 3 different versions of fromMnemonic (or equivalent)

1. fromMnemonic(mnemonic, passphrase, derivationPath) allows the user to derive any arbitrary child in the tree
2. fromMnemonic(mnemonic, passphrase) returns m/44'/3030'/0'/0'/0'
   • This is B in the picture above, what I would consider the "default" address. Programmers who don't want to know anything about derivation paths should recover this private key from an end user's mnemonic
3. fromMnemonic(mnemonic, passphrase, int index) returns the key at m/44'/3030'/0'/0'/index, for when a user wants a new address in the same "account"
   • index must be checked, and attempting to use a non-hardened index for ed25519 should fail

[dikel]

Hey @alittley, we had an internal discussion and we think the best option is to leave the current method (fromMnemonic(mnemonic) as it is because it might break current applications, add fromMnemonic(mnemonic, passphrase, int index) and change all examples to use fromMnemonic(mnemonic, 0).

We think fromMnemonic(mnemonic, derivationPath) won't be used and there is no point in adding it.

What is your opinion?

[litt3]

@dikel I agree, fromMnemonic(mnemonic, derivationPath) probably wouldn't be used 👍

I'm also on board with keeping the existing function, and adding fromMnemonic(mnemonic, passphrase, int index)

Would it make sense to deprecate the existing fromMnemonic function at some point? That would allow existing applications to keep working, but hopefully steer future implementors away from using it incorrectly.

[litt3]

One more consideration-

since Ed25519 supports only hardened derivation, we need to decide whether Ed25519PrivateKey::fromMnemonic(mnemonic, 0) interprets "0" to mean the 0th hardened child index (2147483648), or whether this should throw an error due to invalid child index

I think my preference would be that Ed25519PrivateKey::fromMnemonic(mnemonic, 0) kicks back an error, and the user would have to do something like Ed25519PrivateKey::fromMnemonic(mnemonic, toHardened(0)). But I can see this going either way, as long as there is consistency across SDKs

[dikel]

@alittley I did some investigation and it turns out the Java SDK only support creating Ed25519 keys from mnemonic. And it uses the path m/44/3030/0/0 (All are not-hardened). The JS SDK support creating both Ed25519 and ECDSA keys, but they both use the same path as the Java one (m/44/3030/0/0)

The correct path should be m/44'/3030'/0'/0'/0' for Ed25519 and m/44'/3030'/0'/0/0 ECDSA, right?

[litt3]

@dikel You're correct, java doesn't support ECDSA from mnemonic.

Concerning the path - PrivateKeyEd25519::derive in the Java SDK infers that every index is hardened, and does so with the following code:

        // write the index in big-endian order, setting the 31st bit to mark it "hardened"
        var indexBytes = new byte[4];
        ByteBuffer.wrap(indexBytes).order(ByteOrder.BIG_ENDIAN).putInt(index);
        indexBytes[0] |= (byte) 0b10000000;

The JS SDK makes the same assumption for Ed25519 keys in slip10::derive

    // set the index to hardened
    input[33] |= 128;

I haven't dug into the JS SDK to verify, but if you're correct that m/44/3030/0/0 is being used for ECDSA keys with no logic to harden the first 3 children... that's not in compliance with the BIP, and it's deriving the wrong keys.

The paths you've stated are correct 👍

[litt3]

I guess this means that if we were to change Ed25519 derivation to not infer all indices are hardened, that would be a breaking change 🤔 So we probably should continue inferring all Ed25519 indices are hardened. But to get a hardened ECDSA child, it must be explicitly chosen