Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow optional 24th word input #43

Closed
wants to merge 3 commits into from

Conversation

jimbojw
Copy link
Contributor

@jimbojw jimbojw commented Jan 4, 2021

Currently, SeedPicker requires exactly 23 words as input, from which the 24th word is calculated. This affords 253 bits of entropy (23 x 11 = 253).

This pull request introduces a checkbox to allow the user to supply a 24th word as input. When supplied, the 24th word contributes 3 bits to the overall entropy.

As a part of this change, the logic function firstChecksumWordAlphabetically() no longer calls through to allChecksumWords(). I believe the latter to be unnecessary, as computing the checksum is a deterministic operation.

This pull request builds on and includes the changes in PR #42.

@jimbojw
Copy link
Contributor Author

jimbojw commented Jan 4, 2021

This build is failing for the same "Chrome version" issue facing PR #42. Tests pass.

@merland
Copy link
Owner

merland commented Jan 4, 2021

Thanks for this! I haven't had the chance to test it yet, but I like the idea.
Did you see #16? It's also about adding the 24th word, but for a different reason (if I understand your motivation right).

Regarding the "lost" entropy: This has been on the table before and it was decided that the clarity/simplicity of having the 24th word deterministic far outweighs the very small increase in entropy. See #13.

In summary, I think this may be worth doing but not for the entropy reason :)
Will test and comment more.

@jimbojw
Copy link
Contributor Author

jimbojw commented Jan 4, 2021

I had not seen these other issues.

Regarding #13, this seems to be making SeedPicker deterministically produce a 24th word given an initial 23 (I assume it used to be random?). This seems like a good change to me. Under the current implementation, SeedPicker will always find a checksum word whose eleven bit binary representation starts with 000. That is, exactly one word in the range from "abandon" to "cable" inclusive.

Regarding #16, this seems like reasonable use case. It would also act as confirmation that the checksum word that you believed was correct is indeed so (you haven't accidentally mistranscribed any words, including the checksum).

Regarding my current PR in progress: Adding the 24th word as input means that the first three bits of entropy it encodes contribute to the checksum. While not all of the 24th word's bits are used, the transformation is still deterministic. Given any 24 word input, the output will be the same every time.

If this seems unreasonable, no worries! I'm happy to withdraw my PR. Just let me know.

@merland
Copy link
Owner

merland commented Jan 5, 2021

Yes, the 24th word was selected randomly before #13 was implemented.

Tell me more about your use case in practice. Is it that you have a full 24 word seed phrase (from another source of entropy) and want to validate it?

@merland
Copy link
Owner

merland commented Jan 5, 2021

@jimbojw I did some project maintenance on master, would be great if you could try a build on master and maybe rebase your PR.

@jimbojw
Copy link
Contributor Author

jimbojw commented Jan 7, 2021

Tell me more about your use case in practice. Is it that you have a full 24 word seed phrase (from another source of entropy) and want to validate it?

My use case is the same as the SeedPicker project use case. I'm generating my own offline entropy (seed words) manually. Just like in the original use case, I need to generate the 24th word because its last 8 bits encode a checksum which cannot be computed by hand. My intent is to run SeedPicker offline on an eternally quarantined machine (no Internet, no HDD) to act as one member of a Specter Desktop quorum.

The normal flow for SeedPicker is to allow 23 words of input, each of which encodes 11 bits of a Bitcoin wallet seed (23 x 11 = 253 bits). But a seed contains 256 total bits. SeedPicker currently assigns those last three bits to 000.

My proposed change allows a user to opt into providing a 24th word whose first 3 bits of entropy contribute to the seed value. This operation is still deterministic, but rather than always picking the 1st alphabetical checksum candidate, or picking a random one, the one is picked whose first 3 bits match the bits represented by the user-supplied 24th word.

My change makes SeedPicker compatible with Specter DIY, which is implementing the same feature. See cryptoadvance/specter-diy#85

@merland
Copy link
Owner

merland commented Jan 8, 2021

@jimbojw Thanks, this provides good context. I didn't see the connection to SpecterDIY first.

To me, it seems that the notion of "fixing" the 24th word could be confusing to newbies. That was my thought when I first heard about it, and it still is. And - as I mentioned - since the gain in entropy is minimal, the tradeoff seems fair.

I would like SeedPicker to be as simple as possible for people learning about these things, so for now my vote is on not adding this feature.

I would love to have @mflaxman's view on this too. :)

@jimbojw
Copy link
Contributor Author

jimbojw commented Jan 8, 2021

Sounds good, I will withdraw my PR. I agree that this topic is probably confusing for newbies, and for them the loss of three bits of entropy is probably a reasonable tradeoff for some.

@jimbojw jimbojw closed this Jan 8, 2021
@mflaxman
Copy link

I would love to have @mflaxman's view on this too. :)

Allowing different 24th words seems like an experts-only feature. I'd imagine almost every user who want to use that feature doesn't actually doesn't understand what they're doing and shouldn't.

I like allowing different checksums for advanced/cautious users who want to confirm a seed->xpub but didn't generate it with this algo (perhaps they used dice, which I think are much inferior). IDK if @merland wants to add that, it'd def be advanced users only feature and that could complicate the UX. For now, that's accessible to advanced users in my python and golang CLI libraries.

@merland
Copy link
Owner

merland commented Jan 13, 2021

Thanks for weighing in @mflaxman. Well said.
If anything, I aim to make SeedPicker less advanced, and even more newbie friendly. I do like the "validate a full seed phrase" (#16) feature but I think that should be done separately. Maybe on a whole new page or tab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants