For discussion: Should invalid mnemonic generate error/exception?

[dan-da]

For background, see: https://github.com/dan-da/hd-wallet-derive/issues/24

If I export a seed phrase from electrum (which does not match bip39), bitcoin-php will happily accept it, and then it can be derived from.

$ ./hd-wallet-derive.php -g --mnemonic="merit december trigger right round farm receive level buffalo bounce faculty cruise" --cols=path,address --preset=electrum_legacy --numderive=3

+-------+------------------------------------+
| path  | address                            |
+-------+------------------------------------+
| m/0/0 | 1BARuweKofYP8KUMYm6Am1x1sCoYyDdLrh |
| m/0/1 | 156PEqz6YajFe1awoxrYkZJuGqbRmQWdHC |
| m/0/2 | 1JkQtneJqKMptUpjpjg1wYbndrev7cUDYZ |
+-------+------------------------------------+

However, Ian Coleman's bip39 tool rejects it with an "invalid mnemonic" error message.

So, perhaps/probably bitcoin-php should detect it is invalid and throw an exception. Or maybe there is a good reason not to?

btw, I just use a passphrase with total gibberish words, and a seed is still generated without error.

$ ./hd-wallet-derive.php -g --mnemonic="ew2rqewr wejrwqejrqe adskfjqweijr as qewrqe asfds asdfdsaf" --cols=path,address --preset=electrum_legacy --numderive=3

+-------+------------------------------------+
| path  | address                            |
+-------+------------------------------------+
| m/0/0 | 1HW61TnEA64V5o9RfZN4XxL3a3kmTqakqB |
| m/0/1 | 1K1zJq2BUeroy4E4e1rJkHXrjtPs3iqbQa |
| m/0/2 | 18SRNRYJ2bzPdv4G2aUwxr4ZSGnT6au2uP |
+-------+------------------------------------+

Maybe there should be an API flag to indicate strict or anything goes.

ps, I'm not using the very latest bitcoin-php, so apologies if this is already addressed.

[afk11]

Hey! Thanks for opening the issue. I agree that code is a foot gun right now.

Short term, this is decodes and checks the checksum:

try {
    $bip30->mnemonicToEntropy($mnemonic);
} catch (\Exception $e) {
    // reject as invalid
}
// ok

We could check the mnemonic for issue #1 (invalid checksum, probably wrong word or typo) by also requiring the appropriate WordList object be provided, except as-is, the mnemonicToEntropy step works for all languages right now.

Maybe we can move the current behavior to an *Unsafe method, and add a new method of the old name that starts to introduce the wordlist requirement over time, or an explicit ArbitraryWordlist to disable checks.. or something so it's difficult to make the code behave as it does now.

I'll have a think about this soon!

[dan-da]

Sounds like you have a good handle on it. Flexibility is nice, and also defaulting to "the expected thing" for the most common case.