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

Disable back arrow at File Backup after word import #246

Open
danimoh opened this issue Mar 15, 2019 · 6 comments
Open

Disable back arrow at File Backup after word import #246

danimoh opened this issue Mar 15, 2019 · 6 comments

Comments

@danimoh
Copy link
Member

danimoh commented Mar 15, 2019

Otherwise, it's possible to import multiple wallets by words in one import request which is probably not what we want.

@sisou
Copy link
Member

sisou commented Mar 15, 2019

I think a cleaner approach is fixing the problem, not the symptom. Meaning I'd rather reset the keys array to empty when going from words to password input.

@sisou
Copy link
Member

sisou commented Mar 15, 2019

I believe this is actually a non-issue since the array is already reset when completing the words input: https://github.com/nimiq/keyguard-next/blob/master/src/request/import/ImportWords.js#L237.

Are you sure this is a working bug, @danimoh?

@Bettelstab
Copy link
Contributor

Are you sure this is a working bug, @danimoh?

xD

@danimoh
Copy link
Member Author

danimoh commented Mar 16, 2019

Yes, but the wallet is immediately stored to the database with _storeKeys after a password was defined.
I believe this is an actual bug.

@sisou
Copy link
Member

sisou commented Mar 18, 2019

Ok, I just tried it out:
Yes, the first account is immediately stored after setting a password. When you then go back (two times) to the word import form, all the fields are still filled out. So you actively have to remove every single word or replace every single word to import new words. I think this makes it pretty clear that this is not meant for that purpose.

Then, when finishing the second set of words, the _results array is reset, so that it now only contains the second imported key; every reference to the first key is lost.

Only the second key's info is thus returned to the AccountsManager, which then derives the accounts for it.

Thus I think the only bug here is that it is possible to create a dead key in the Keyguard, for which no reference or knowledge in the AccountsManager exists. - Which requires actual reset work of the user.

I'd say this is a low-priority bug.

@nibhar
Copy link
Member

nibhar commented Mar 18, 2019

An option would be to remove the newly created account(s) from the indexedDb again in case the user imports different words before these words are stored (therefore not only overwriting the returned keyInfo but also the key itself).
Another option would be to not store the key until the request actually resolves, which definitely would be a bigger change in the code.

I agree with @sisou that it is probably not very high priority.

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

No branches or pull requests

4 participants