BIP-39: Add shortcomings section, drop comments headers.#2184
Conversation
ajtowns
force-pushed
the
202605-bip39-unanimity
branch
from
050142c to
9444507
Compare
|
cc @scgbckbone @prusnak ; ref #1677 (comment) |
| * Generated seed depends on the wordlist used for the mnemonic. Because | ||
| the "mnemonic to seed" process uses the mnemonic sentence directly | ||
| rather than the original entropy, translating the mnemonic to a | ||
| different wordlist necessary creates a completely different seed. |
There was a problem hiding this comment.
Maybe I'd add that this shortcoming has been addressed by wallets by not-allowing generating seeds in other languages than English to maximize compatibility across wallets.
There was a problem hiding this comment.
Never attempting to translate to a different wordlist avoids the problem, sure, but it's still a shortcoming given the BIP includes 10 different wordlists. It's your doc, so happy to reword if you prefer, but I think "despite supporting different languages, you can't actually translate mnemonics" is a real shortcoming.
There was a problem hiding this comment.
I was advocating for removing other wordlists for years but the friction to change the BIP used to be too high for me to still care; especially when the editors marked the BIP as "Unanimously Discouraged for implementation".
If there is now editors' will to edit the BIP, I would remove the other wordlists and encourage all wallets to generate only English mnemonics.
There was a problem hiding this comment.
ACK on removing it all besides english
There was a problem hiding this comment.
Removing the wordlists seems like it would be annoying for anyone trying to be compatible with old mnemonics that were created with different languages. I've reworded to emphasise focussing on the english wordlist.
There was a problem hiding this comment.
I’m biased, but I think editors now are a bit more available. Given that there are a number of Bitcoin projects depending on the wordlists in other languages (especially in countries where English proficiency is perhaps less common), it seems to me that even with willing BIP Editors, there would probably be a few hurdles to clear, so that a removal of other lists from BIP39 wouldn’t break deployed software.
| * No versioning scheme. When originally introduced, there was no way to | ||
| distinguish the address format that should be used for a BIP-0039 key. | ||
| This is now largely mitigated by using descriptor wallets (BIP-0380) | ||
| in addition to a seed however. |
There was a problem hiding this comment.
I still do think this is a feature not bug, allowing the seed to be future proof which has proven to be very useful.
There was a problem hiding this comment.
Any suggestion for rewording it?
jonatack
added
Proposed BIP modification
scgbckbone
left a comment
scgbckbone
left a comment
There was a problem hiding this comment.
ACK
That said, I do not think the new Shortcomings section is very useful as written; it is also incomplete (check rust-bitcoin BIP39 discussion).
Enumerating shortcomings directly in the BIP feels unusual; I do not think many BIPs include this kind of retrospective critique of themselves. Framed this way, it also raises the opposite question: should the BIP also include a section on the practical interoperability benefits BIP39 has provided across many wallets over the years? I would call that section Great Success.
So overall, I support removing Comments-Summary; this was long overdue. I do not feel strongly about whether the Shortcomings section is added or not, since this BIP has long contained language that does not really match how it is used in practice.
ajtowns
force-pushed
the
202605-bip39-unanimity
branch
from
9444507 to
9429dbb
Compare
Added some things from there.
It definitely is unusual (cf this post) but maybe it should be less unusual? The "unanimously discourage" / "supported almost everywhere" dichotomy with this BIP always bugged me, so it was the first test case that came to mind.
There is a pre-existing "Post mortem" in the form of bip 50 which could potentially work; but "why my bip was awesome, by me, the author of an awesome bip" doesn't sound like something that'll be useful in general to me... |
ajtowns
force-pushed
the
202605-bip39-unanimity
branch
2 times, most recently
from
6290e6a to
f36c8d0
Compare
danielabrozzoni
left a comment
danielabrozzoni
left a comment
There was a problem hiding this comment.
I think it's slightly cleaner to have shortcomings in the BIP itself rather than having comments in the wiki, but I don't have a strong opinion either way.
While doing some research on why Core doesn't implement BIP39 I found this stackexchange answer from Ava:
Also, more generally, many Bitcoin Core contributors don't consider BIP 39 to be secure. It uses PBKDF2 which is generally regarded to be a fairly weak KDF so it isn't considered to be good for the secure storage of all of your Bitcoin. Some software (such as Electrum) use their own mnemonic algorithm because of this weakness in BIP 39.
Should this be in the shortcomings as well?
I think we could consider an informational BIP that gathers issues, learned lessons, and best practices regarding an idea proposed previously by another BIP. |
|
Approach ACK f36c8d0. Bringing this information into the BIP as done here seems a helpful improvement. No strong opinion on creating an informational retrospective BIP, apart from it being perhaps less convenient for readers. |
ajtowns
force-pushed
the
202605-bip39-unanimity
branch
from
f36c8d0 to
0269641
Compare
|
I don't really think the mnemonic to seed function not being very secure is much of a shortcoming -- you can't make that too slow, because it has to be performed on signing devices which might themselves be slow, and you should have high entropy when generating the mnemonic in the first place anyway. Updated for wording/layout nits. |
|
Concept ACK I think comments of this BIP should be included in the BIPS repository, but I think it's not tidy having them inside the BIP itself. I'm not finding 'shortcomings' as a standard in BIP 3, so I'm unsure what is really happening here. |
|
ACK 0269641 Pending feedback or sign-off by the BIP authors. |
7 participants
No description provided.