-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat(types)!: use Mnemonic
type from crypto.rs
#471
feat(types)!: use Mnemonic
type from crypto.rs
#471
Conversation
Mnemonic
type that is ZeroizeOnDrop
Mnemonic
type that is ZeroizeOnDrop
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.
CI fails
9cc642c
to
a8de867
Compare
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.
We should wait and use the Mnemonic from iotaledger/crypto.rs#200
* Allow SecretManage to be extensible * fix type * more feature gating * Fix stronghold backup/restore * Fix backup restore bugs * clippy * feature 🐊 * LINE PIECE | Co-authored-by: Thibault Martinez <[email protected]> * imports * fix merge * PR suggestions * changelog * cleanup * fix merge * fix borked merge * nit * Remove Debug requirement from StorageManage * comment * clippy * Remove generate addresses that is not needed * remove more generate addresses * cleanup * Fix features and clippy * flip the switch --------- Co-authored-by: Thibault Martinez <[email protected]>
iotaledger/crypto.rs#200 was merged, I don't know if there was something else in this PR that we would keep? |
Things we probably want to keep from this PR: |
I agree, those aspects should be maintained. |
Waiting for #715 to be merged which will make their |
Mnemonic
type that is ZeroizeOnDrop
Mnemonic
type from crypto.rs
@@ -154,6 +154,76 @@ impl MnemonicSecretManager { | |||
} | |||
} | |||
|
|||
// impl From<Mnemonic> for MnemonicSecretManager { |
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.
What's up with all 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.
don't think too much about it ... I merged develop into this branch to see in GH if there's something left of use and just out-commented this for the time being.
Changes of this PR were already done in #715 |
Description of change
Consistent with the new
Password
type in #468 this PR adds aMnemonic
type, that makes sure sensitive data gets zeroed when the owner is being dropped.Links to any relevant issues
None
Type of change
How the change has been tested
Unit tests