Expose Mnemonic as interface to bindings users - #1059
Conversation
|
👋 Thanks for assigning @benthecarman as a reviewer! |
benthecarman
left a comment
benthecarman
left a comment
There was a problem hiding this comment.
nits otherwise lgtm
| #[uniffi::constructor] | ||
| pub fn generate(word_count: Option<WordCount>) -> Self { | ||
| let word_count = word_count.unwrap_or(WordCount::Words24).word_count(); | ||
| let inner = Bip39Mnemonic::generate(word_count) |
There was a problem hiding this comment.
Hmm, no, I'd prefer to just call through to bip39 here, though that should do it soon, see rust-bitcoin/rust-bip39#105 (we just need to get a release out)
|
|
||
| /// Returns the number of words in the mnemonic. | ||
| pub fn word_count(&self) -> u8 { | ||
| self.inner.word_count() as u8 |
There was a problem hiding this comment.
changing the type here is kinda weird
There was a problem hiding this comment.
Yeah, unfortunately no usize in uniffi. Would you prefer we just no expose the method?
There was a problem hiding this comment.
Could also wait until after rust-bitcoin/rust-bip39#98 lands and ships, when we'll re-introduce WordCount (just seemed weird to keep it for bindings only for now)
There was a problem hiding this comment.
This is fine for now, we can move to that in the future. No reason we'd ever break the u8 here for any practical reason
Invalid mnemonic strings currently fail during implicit custom-type lifting, which leaves generated bindings without a catchable validation error. Parse mnemonic phrases through a fallible object constructor while keeping bip39::Mnemonic in the native Rust API. Co-Authored-By: HAL 9000
Mnemonic generation no longer needs a separate global entry point now that bindings expose a real mnemonic object. Use numeric word counts for both native and binding constructors so the APIs stay aligned without a binding-specific enum. Co-Authored-By: HAL 9000
9e8f8ed to
ec9567f
Compare
|
Amended with some fixes to make CI pass. |
3 participants
And, since we now expose
Mnemonic::generateto bindings users directly, we can get rid of our globalgenerate_entropy_mnemonichelper.