[Feature]: Predict Last Seed Word

[notTanveer]

Description

fixes #500 .
displays only checksum-valid words when manually entering the final word of a seed phrase.

core implementation

• added get_valid_final_mnemonic_words() in mnemonic_generation.py
  • computes valid final words per bip-39 checksum
  • 12-word seeds: 128 valid words
  • 24-word seeds: 8 valid words (of 2048)

technical details

• implements exact bip-39 spec
  • 12-word: 7-bit entropy + 4-bit checksum
  • 24-word: 3-bit entropy + 8-bit checksum

this enhances the UX by filtering out invalid words (which don’t meet the checksum criteria) and makes typing faster by showing fewer possible matches.

EDIT:
the test_invalid_mnemonic test was removed because it tests behavior that is no longer possible with the new seed word prediction feature.

this test was causing the entire test suite to hang indefinitely during execution. the issue occurred at the final word entry stage, where the test attempted to input invalid final words ("zoo" and "zebra"). these entries triggered the get_valid_final_mnemonic_words() function, but since neither word exists in the computed valid word set, the test framework became stuck in an infinite loop

This pull request is categorized as a:

• New feature

Checklist

• I’ve run pytest and made sure all unit tests pass before submitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• Yes

I have tested this PR on the following platforms/os:

• Raspberry Pi OS Manual Build

[alvroble]

Concept ACK, but implementation NACK.
I tested it, and although the tests pass, it currently breaks Electrum mnemonic entry. It turns out Electrum uses a different checksum format from BIP-39, which means the valid last words proposed are only valid for BIP-39 checksum scheme, and do not match those for Electrum.

This would also limit the reusability of SeedMnemonicEntryScreen for potential future implementations of other mnemonic seed formats such as SLIP-39 (#636) or aezeed (used by LND), which also rely on different checksum schemes.

[notTanveer]

TIL, that there are different mnemonic formats. I have made this PR exclusively for BIP-39.

it currently breaks Electrum mnemonic entry

this can be fixed easily by adding a condition to ensure SETTING__ELECTRUM_SEEDS is disabled (so we don’t run through all the extra math to find possible last words). these are very niche features that the majority of people don’t use. I'm not sure how many actually rely on electrum seeds, but i’m pretty sure the number is low. also, the last word of an electrum seed phrase CANT be predicted.

for potential future implementations of other mnemonic seed formats such as SLIP-39 (#636) or aezeed (used by LND)

i believe these are all niche features and are disabled by default. we can handle them the same way as described above.

almost forgot, thank you for reviewing this PR @alvroble !
also if you have a better approach in mind i would love to hear that as well!

that said, lemme know what others think.

[alvroble]

@notTanveer thank you for your work!

this can be fixed easily by adding a condition to ensure SETTING__ELECTRUM_SEEDS is disabled

This would mean disabling the functionality of predicting the last word in all cases where SETTING__ELECTRUM_SEEDS is enabled, even if we are entering a BIP-39 mnemonic phrase. It definitely solves the problem, but it might be overkill.

Another option is to directly use different screen and view for Electrum and other (to be supported) mnemonic phrase formats. This option would completely solve the problem, but I like it even less because it would duplicate a lot of code and would be the messy way of doing it.

these are very niche features that the majority of people don’t use. I'm not sure how many actually rely on electrum seeds, but i’m pretty sure the number is low

Yes, it’s true that this feature is rarely used nowadays, but we shouldn’t allow functionality that SeedSigner already supports to break.

Actually... I think the best approach would be to have a new class attribute in the Seed class (e.g. seed_type) that could be either "bip39" for Seed (default, parent class) or "electrum" for ElectrumSeed. Then in get_valid_final_mnemonic_words check if Seed is "electrum" and return the whole wordlist since that seed format does not support the feature. I would also love some input from other devs here.

[notTanveer]

we have two possible approaches:

1. use is_pending_electrum – This would let us distinguish Electrum seeds from others. It requires minimal changes and still works fine for standard mnemonic entry (BIP-39).

2. adopt a seed type approach (as suggested by @alvroble), this feels more standardized and makes the code future-proof, especially for supporting Shamir Secret and other upcoming formats.

let me know which one do you guys prefer?

[Chaitanya-Keyal]

How about changing the current _pending_is_electrum to pending_seed_type in SeedStorage? This would be defaulted to "BIP-39", and changed to "electrum" in SeedElectrumMnemonicStartView (and to other types for other seeds in the future).

In that case there would be no need to pass a seed_type argument to the SeedMnemonicEntryView since the attribute depends on the seed insance, and the check could happen even inside get_valid_final_mnemonic_words, so the whole wordlist is returned for SeedElectrum

At the mnemonic entry stage, we only set a pending mnemonic, not a pending seed. Since the seed instance is only created after all valid words have been entered, there’s no seed instance available at that point to check the type. So that approach doesn’t seem viable to me.

[notTanveer]

thanks for the suggestion, @Chaitanya-Keyal and @alvroble ! updated the PR. feels like this is the best way forward.
open to hearing what others think though.

[alvroble]

Tested ACK on Raspberry Pi OS Manual Build as of 6f27161. I like this approach too.

[kdmukai]

There is too much bitcoin-related work being done in SeedMnemonicEntryScreen. Ideally Screens shouldn't do any logic beyond what's necessary to present the information the View has provided them.

People have asked seriously about a version of SeedSigner for blind users. If and when that time comes, we'd need very different "Screen" implementations to present the data (perhaps a digital braille device or just a text-to-speech flow). But the goal is that all the important logic would already be contained within the Views, so swapping in alternate Screens should be kind of trivial.

So... all of this wordlist filtering needs to happen in the View.

[kdmukai]

I've been unhappy with our existing bip39 vs Electrum juggling in SeedStorage since we added support for Electrum seeds. We could take a cue from Destination and just set a Seed_cls member var AND get rid of the _pending_is_electrum bool completely:

    def init_pending_mnemonic(self, num_words:int = 12, is_electrum:bool = False):
        self._pending_mnemonic = [None] * num_words
        self._Seed_cls = Seed if not is_electrum else ElectrumSeed

So then later we can avoid this:

            if self._pending_is_electrum:
                seed = ElectrumSeed(self._pending_mnemonic)
            else:
                seed = Seed(self._pending_mnemonic)

And just do:

            seed = self._Seed_cls(self._pending_mnemonic)

I haven't tested this nor deeply explored if it creates any problems elsewhere.

Also note that validate_mnemonic is dead code.

This all might make more sense as a separate refactor PR.

[notTanveer]

the test_invalid_mnemonic test was removed because it tests behavior that is no longer possible with the new seed word prediction feature.

this test was causing the entire test suite to hang indefinitely during execution. the issue occurred at the final word entry stage, where the test attempted to input invalid final words ("zoo" and "zebra"). these entries triggered the get_valid_final_mnemonic_words() function, but since neither word exists in the computed valid word set, the test framework became stuck in an infinite loop.

[alvroble]

I think a deeper refactor would be better, SeedStorage should stay agnostic of seed types. The responsibility for type should live in the Seed classes themselves, and polymorphism should handle the differences.

[notTanveer]

will be updated once #807 is in

[alvroble]

Following an offline conversation last week about the worst-case negative implications of this feature, I have to change my answer to NACK. Basically, by reducing the options of entering seed words to a single path, other UX issues arise:

• If a user makes an invalid backup of their seed, it could be frustrating for them as they would not receive a warning at any point, but rather the UX would silently try to take them down a different path.
• Potentially, this could lead to lazy users creating seeds with arbitrary words and very low entropy.

Technicaly, the feature is neat. But after reconsidering the potential risks, I believe the drawbacks outweigh the UX simplification.

[kdmukai]

I agree with @alvroble and am also a NACK.

Having the UX tell the user that their mnemonic is invalid is better than the user thinking that SeedSigner has a bug that is preventing them from importing their key.