Skip to content
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

Seed phrase recovery #304

Merged
merged 3 commits into from
May 7, 2019
Merged

Seed phrase recovery #304

merged 3 commits into from
May 7, 2019

Conversation

VolkerSchiewe
Copy link
Contributor

@VolkerSchiewe VolkerSchiewe commented Apr 17, 2019

I think it is a good idea to move every thing that is mnemonic related to the lib, but I'm not sure if adding the getMnemonic method to the key provider is a good idea as it is kind of similar to the getPrivateKey-antipattern, isn't it?

cloeses #300

@VolkerSchiewe VolkerSchiewe changed the title 300/seed phrase recovery Seed phrase recovery Apr 17, 2019
@VolkerSchiewe VolkerSchiewe self-assigned this Apr 17, 2019
@VolkerSchiewe VolkerSchiewe force-pushed the 300/seed-phrase-recovery branch from 2c8d450 to 14c7cef Compare April 17, 2019 13:04
@VolkerSchiewe VolkerSchiewe changed the base branch from master to develop April 17, 2019 13:05
Copy link
Contributor

@chunningham chunningham left a comment

Choose a reason for hiding this comment

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

should the recoverIdentity function also return an IdentityWallet? the returned vkp could be used to construct one and then the function wouldnt depend on a registry and wouldnt need to be async. In that case it would basically be a constructor for a vkp. I think we discussed this before but I dont quite remember the conclusion we came to

@VolkerSchiewe
Copy link
Contributor Author

@chunningham I moved the recovery method to be part of the vkp and works basically like a constructor for it.

@chunningham chunningham merged commit 2981532 into develop May 7, 2019
@chunningham chunningham deleted the 300/seed-phrase-recovery branch May 7, 2019 09:01
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.

2 participants