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

SLIP-0039 wallet recovery #6917

Merged
merged 5 commits into from
Jun 22, 2021
Merged

SLIP-0039 wallet recovery #6917

merged 5 commits into from
Jun 22, 2021

Conversation

andrewkozlik
Copy link
Contributor

Implements #5419.
Note that this PR does not include support for SLIP-0039 in the Kivy GUI.

@Davincible
Copy link

Why was this never picked up?

@prusnak
Copy link
Contributor

prusnak commented Mar 25, 2021

@andrewkozlik can you please rebase?

@jonathancross
Copy link
Contributor

@andrewkozlik please rebase again, thanks.

@prusnak
Copy link
Contributor

prusnak commented May 3, 2021

@ecdsa @SomberNight will you please have a look and provide feedback?

- fix kivy wizard restore-from-seed
- qt seed dialog: disable "next share" if current share is invalid
- fix tests: file paths should not depend on $PWD (working dir)
Copy link
Member

@SomberNight SomberNight left a comment

Choose a reason for hiding this comment

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

I've reviewed the changes except for slip39.py which I just glanced over.

Re slip39.py, given the json test vectors file passes, I trust it is correct. Also, thanks for adding test cases to test_wallet_vertical.py.

I wish the changes needed for qt/seed_dialog.py could somehow be encapsulated even more - but that does not seem easy.

There are some minor fixes I've pushed as a follow-up. In particular, not sure if it was deliberate but the UI allowed the user to enter an incorrect share and press "next" - I've changed this as I find it confusing.

Apologies for the delayed review. Thank you for the contribution.

@SomberNight SomberNight merged commit b828627 into spesmilo:master Jun 22, 2021
@prusnak
Copy link
Contributor

prusnak commented Jun 22, 2021

Thank you @SomberNight for the review and for the merge!

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.

5 participants