-
Notifications
You must be signed in to change notification settings - Fork 3
feat(crypto): Add ability to create a private key from a mnemonic #347
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
Conversation
thibault-martinez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this deserves a new example or an addition to an existing one?
| println!("Public Key With Flag: {flagged_public_key}"); | ||
| println!("Address: {address}"); | ||
|
|
||
| let private_key = Secp256r1PrivateKey::from_mnemonic(MNEMONIC, None, None)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe at least one example with a password and/or path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
crates/iota-sdk-crypto/src/lib.rs
Outdated
| } | ||
|
|
||
| /// Defines the scheme of a private key | ||
| pub trait PrivateKeyScheme { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting this in 2 feels over the top to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well only some types can have a const scheme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok but you can't really implement scheme() if you don't have one? Or what am I missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SimpleKeypair can impl PrivateKeyScheme but not the const version
crates/iota-sdk-crypto/src/lib.rs
Outdated
| } | ||
|
|
||
| /// Defines a type which can be constructed from bytes | ||
| pub trait FromBytes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these 2 traits when ToFromBech32 is one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because some types cannot impl FromBytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SimpleKeypair
|
|
||
| let path = path.into().unwrap_or_else(|| { | ||
| format!( | ||
| "m/{}'/{}'/0'/0'/0'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am asking this because I am unsure about this myself, but are you sure about the hardening in this path? AFAIK this scheme allows safe non-hardened derivation as compared to ED25519. So just checking whether you've thought about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not thought about it and I do not know what the best way is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
monorepo has this
/// Secp256k1 follows BIP-32/44 using path where the first 3 levels are
/// hardened: m/54'/4218'/0'/0/{index} Secp256r1 follows BIP-32/44 using path
/// where the first 3 levels are hardened: m/74'/4218'/0'/0/{index}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still wonder whether we should take the whole path. Maybe two functions?
* feat(crypto): Add ability to create a private key from a mnemonic * clippy * allow passing password and path * fix mnemonic parsing and add example * bindings examples * clippy * add password and path to example * remove impls from `SimpleKeypair` * add tests and fix secp256 generation * split mnemonic methods * fix python example * use constant arrays
Description
Allow creating private keys from mnemonics. While working on this, it made sense to separate the
PrivateKeyExttrait into several smaller traits.Closes #266