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

twelve words backup screen UI and logic #78

Merged
merged 5 commits into from Apr 7, 2019
Merged

Conversation

@yhaspel
Copy link
Contributor

@yhaspel yhaspel commented Apr 3, 2019

In this commit:
Create 12 words backup page. Displaying the words only for now. (no print or copy yet).

Screen Shot 2019-04-03 at 2 33 29 PM

@yhaspel yhaspel requested a review from IlyaVi Apr 3, 2019
@@ -14,7 +14,7 @@ class KeyGeneratorService {
const seedAsUint8Array = Buffer.from(seed, 'hex');
const left32BitsOfSeed = seedAsUint8Array.slice(0, nacl.sign.seedLength);
const { publicKey, secretKey } = nacl.sign.keyPair.fromSeed(left32BitsOfSeed);
return { publicKey: Buffer.from(publicKey).toString('hex'), secretKey: Buffer.from(secretKey).toString('hex'), seed };
return { publicKey: Buffer.from(publicKey).toString('hex'), secretKey: Buffer.from(secretKey).toString('hex'), seed, resolvedMnemonic };

This comment has been minimized.

@IlyaVi

IlyaVi Apr 3, 2019
Collaborator

rename to mnemonic

This comment has been minimized.

@yhaspel

yhaspel Apr 3, 2019
Author Contributor

This will clash with "mnemonic" param in wallet actions.

This comment has been minimized.

@IlyaVi

IlyaVi Apr 3, 2019
Collaborator

you can rename it like this { mnemonic: returnedMnemonic }

This comment has been minimized.

@yhaspel

yhaspel Apr 3, 2019
Author Contributor

Renaming on destructuring still does not solve the issue. Unless I change the "mnemonic" parmam name in the saveNewWallet fuction to something else, like so:
in keyGenService:
return { publicKey: Buffer.from(publicKey).toString('hex'), secretKey: Buffer.from(secretKey).toString('hex'), seed, mnemonic: resolvedMnemonic };

in wallet actions:

Screen Shot 2019-04-03 at 5 20 14 PM

This comment has been minimized.

@IlyaVi

IlyaVi Apr 4, 2019
Collaborator

this works just fine:
Screen Shot 2019-04-04 at 10 10 06
Screen Shot 2019-04-04 at 10 09 27

What you've done - changed param name in function params destruct without changing param name in all usages of this function => broken app functionality

This comment has been minimized.

@yhaspel

yhaspel Apr 4, 2019
Author Contributor

OK np, this brings us back to how it was coded to begin with.

This comment has been minimized.

@IlyaVi

IlyaVi Apr 4, 2019
Collaborator

it's not the same, the important thing is not breaking the app with untested changes

@IlyaVi IlyaVi changed the title twelve words backup screen - initial dev twelve words backup screen UI and logic Apr 4, 2019
@IlyaVi
IlyaVi approved these changes Apr 7, 2019
@IlyaVi IlyaVi merged commit 237803d into develop Apr 7, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@IlyaVi IlyaVi deleted the wallet-backup-12words branch Apr 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants