Skip to content

Add gem_keystore crate with BIP39 recovery phrase suggestions#1154

Draft
0xh3rman wants to merge 1 commit into
mainfrom
recovery-phrase-suggest
Draft

Add gem_keystore crate with BIP39 recovery phrase suggestions#1154
0xh3rman wants to merge 1 commit into
mainfrom
recovery-phrase-suggest

Conversation

@0xh3rman
Copy link
Copy Markdown
Collaborator

Introduce gem_keystore as the foundation crate for keystore and key management work. Initial surface is a Mnemonic helper that exposes BIP39 English word suggestions, wired through gemstone via UniFFI as suggest_recovery_phrase_words.

Introduce gem_keystore as the foundation crate for keystore and key
management work. Initial surface is a Mnemonic helper that exposes
BIP39 English word suggestions, wired through gemstone via UniFFI as
suggest_recovery_phrase_words.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new gem_keystore crate and a keystore module within the gemstone package to provide mnemonic word suggestions based on the BIP39 standard. The implementation includes a suggest_recovery_phrase_words function exported via UniFFI for cross-platform support. Feedback indicates that the suggestion logic should normalize input to lowercase to ensure case-insensitive matching against the wordlist. Additionally, there is a concern regarding performance overhead when returning the full 2048-word list for empty prefixes during FFI serialization, suggesting a limit should be applied.


impl Mnemonic {
pub fn suggest(prefix: &str) -> Vec<String> {
Language::English.words_by_prefix(prefix).iter().map(|word| word.to_string()).collect()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The suggestion logic is currently case-sensitive because the BIP39 wordlist is entirely lowercase. Normalizing the input to lowercase would improve the user experience (e.g., allowing "Ab" to match "abandon").

Additionally, returning all 2048 words for an empty prefix might lead to performance overhead when serializing/deserializing across the FFI boundary to mobile platforms. Consider if a limit (e.g., .take(20)) should be applied for the UI's benefit.

Suggested change
Language::English.words_by_prefix(prefix).iter().map(|word| word.to_string()).collect()
Language::English.words_by_prefix(&prefix.to_lowercase()).iter().map(|word| word.to_string()).collect()

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.

1 participant