infincia / bip39-rs

A Rust implementation of BIP-0039
Apache License 2.0
54 stars 61 forks source link

Rethink API? #4

Closed dpc closed 4 years ago

dpc commented 7 years ago

Hi, first of all, thanks for providing this crate. I'm here to ask a question and complain about the API a bit. Sorry. :D

My question is: how to generate a mnemonic from existing key? Bip39 describes the general conversion of encoding a key into a human-friendly string. It could be used outside of Bitcoin, etc. and that was my plan. I already have a key generated.

struct Bip39 seems confusing. There is no such object as Bip39. There could be bip39:: Mnemnonic , but clamping all the functions into a struct doesn't seem to serve any purpose.

The whole crate could be just couple two functions:

fn mnemonic_to_key(&str) -> Result<Vec<u8>> and fn key_to_mnemonic(&[u8]) -> Result<String> + bit39::Result error types. Lenght of the string/vec already provides KeyType? The lang stuff could be skipped until someone actually makes an effort to implement it? :)

Please let me know what you think, etc. I am planing to use this crate for some stuff, so I was just wondering: maybe I could make a PR or something.

dpc commented 7 years ago

So to clarify about your comment in #5.

Wouldn't it be OK to have:

struct Seed;
struct Mnemnonic

And functions/methods that do entropy -> seed , seed (+passphrase) -> mnemonic and mnemonic (+passphrase)-> seed and then maybe, for people that really need it, under long and scary name seed -> entropy? (I actually don't need it myself)

I just don't understand why Bip39 has to be a one struct containing everything.

steveatinfincia commented 6 years ago

Hi again @dpc, I see what you mean by "no such object as Bip39", splitting it certainly makes sense.

My only goal is to make sure the type system enforces correct usage, for example Seed couldn't be turned back into a Mnemonic because the process of generating the seed is irreversible, and it couldn't be generated directly from entropy without going through a Mnemonic first anyway, so Mnemonic is probably the "main" type to have available.

This is partly why it's all one (poorly named) Bip39 struct at the moment, that's the "main" type.

Renaming that to Mnemonic would probably help a bit, I'll do that and then we can figure out the rest, like having it give out a separate Seed type that provides access to the bytes as well as hex.

steveatinfincia commented 6 years ago

I've refactored things a bit to accommodate cases where access to the original entropy is useful, as well as cleaned up the types so that they make more sense but still prevent incorrect use.

Mnemonic can be used to create new ones, or created from an existing phrase with from_string(), which will make sure it is OK before returning it.

Mnemonic provides access to the Seed using get_seed(), and you can get the byte or hex representation of the Seed with get_bytes() or get_hex().

Mnemonic also provides direct access to the original entropy using get_entropy(), which clones the internal entropy bytes and returns an owned Vec<u8>.

There are non-allocating as_*() versions of most of those as well that provide immutable access to the value as a slice.

steveatinfincia commented 6 years ago

I've also published an alpha version of the crate so the new crate docs would be generated, it's a bit easier to see how things are arranged that way: https://docs.rs/bip39/0.3.0-alpha.1/bip39/