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

UX upgrades to new key screen and easier display of recovery phrase #157

Merged
merged 4 commits into from
Jun 10, 2021

Conversation

moneymanolis
Copy link
Contributor

  1. Generate new key
  • New word numbering, words from the BIP39 wordlist are now displayed from 1-2048 instead of 0-2047
  • "Next button" replaced by a "done button" to make clear that there is no other step.

grafik

grafik

  1. Display of recovery phrase
  • Thus far, the display of the recovery phrase was a bit hidden, moved it one level higher to the settings screen.

grafik

@openoms
Copy link
Contributor

openoms commented Jun 9, 2021

just to check using a smartcard with firmware v1.5.4 the show recovery phrase does not work.
The device asks for the PIN and returns for the menu without showing the seed.

Might not be touched by this PR, but a good place to test / reproduce.

"""Manage storage and display of the recovery phrase"""
# This class can only show mnemonic, can't save
await self.show_mnemonic()
# async def storage_menu(self):
Copy link
Collaborator

@stepansnigirev stepansnigirev Jun 9, 2021

Choose a reason for hiding this comment

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

So here you made storage_menu optional for the KeyStore class.
In this case it makes sense to check if keystore has storage_menu attribute and only then display corresponding button in Specter settings.

I think we should do the same with show_mnemonic function - potentially keystore object may not be able to display a mnemonic (i.e. when all crypto happens on the smartcard and the keys never leave it). Currently it's not the case but better to think about that in advance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should all be addressed with bea5a4c

@stepansnigirev
Copy link
Collaborator

just to check using a smartcard with firmware v1.5.4 the show recovery phrase does not work.
The device asks for the PIN and returns for the menu without showing the seed.

Might not be touched by this PR, but a good place to test / reproduce.

That's strange, I just checked both in simulator and with the real device and smartcard - I can get the recovery phrase displayed.

Can you double-check?

@openoms
Copy link
Contributor

openoms commented Jun 9, 2021

Managed to reproduce the problem, but needs more detail.
The recovery words are not shown if I switch the device on and proceed to load the keys from the smart card (it is stored encrypted there).

If I switch the device on and generate a keys the words can be shown.
Then without switching off I can load the keys from the smartcard and in this case the (correct) words are shown again from the menu.

The problem is only when loading the keys from the smartcard without generating keys beforehand.

…nd show_mnemonic are available to control display of respective buttons
# wait for menu selection
menuitem = await self.gui.menu(buttons, last=(255, None), note="Firmware version %s" % get_version())

# process the menu button:
# back button
if menuitem == 255:
return self.mainmenu
elif menuitem == 0:
elif menuitem == 1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed this to 1 to have a proper ordering (now: 1, 2, 3, 4, 5).

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.

3 participants