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

wizard: move mnemonic seed into a separate screen #3677

Closed
wants to merge 2 commits into from

Conversation

rating89us
Copy link
Contributor

@rating89us rating89us commented Aug 23, 2021

Closes #2043

This PR introduces a dedicated page to the mnemonic seed + a pdf template to be printed (with 25 blank fields). When the user clicks on Next button, 5 random words are hidden and the user is requested to type them to confirm that the mnemonic seed was written down.

Main changes:

  • Create dedicated seed page on "Create a new wallet" flow
  • Add description explaining what is the mnemonic seed, its importance and detailed instructions on how to proceed
  • Display seed in 25 numbered fields. The seed is generated when the user clicks on "Create a new wallet" menu item. If the user is in seed page and click on previous button, the wallet file name and location can be changed and the seed will remain the same.
  • Wallet creation date + Wallet restore height
  • Add tooltips
  • "Create new seed" button: creates a new seed on the same page (keeps wallet name and location defined on the previous page)
  • "Copy to clipboard" button: copies the mnemonic seed to the clipboard (visible on advanced mode only because copying the seed to the clipboard is something potentially dangerous)
  • "Print a template" button: displays a pdf file with blank numbered fields, one for each word (always visible)
  • Fix width of wizard navigation buttons (180)
  • Use secondary color for Previous (back) button
  • Accessibility

Writing down seed and verifying it:
aaa123

Choosing 5 new random words to hide (on in each column) every time verification page is opened
aaa1234

Resizing from 5 columns (desktop) to 2 columns (mobile)
aaa12345

@rating89us rating89us force-pushed the patch-107 branch 2 times, most recently from 569579e to a737b14 Compare August 23, 2021 17:29
@rating89us
Copy link
Contributor Author

@GBKS, could you please create a new version of the pdf template with the following text:

  • change "Block height" to "Wallet restore height"
  • general instructions: "The 25 words above are your recovery phrase (mnemonic seed). They are the only thing needed to access your funds and restore your Monero wallet, so no one except you should be able to access it. It's strongly NOT recommended to store your recovery phrase digitally (in an email, online service, screenshot, photo or any other type of computer file)"
  • warning with large and bold font: "Print this paper, fill it out, and keep it in a safe location! NEVER SHARE YOUR MNEMONIC SEED WITH ANYBODY, ESPECIALLY WITH STRANGERS OFFERING TECHNICAL SUPPORT!"
  • instructions on how to restore wallet: "For instructions on how to restore this wallet, visit www.getmonero.org and go to Resources > User Guides > "How to restore a wallet from mnemonic seed". Use only Monero wallets that are trusted and recommended by the Monero community (see a list of them in www.getmonero.org/downloads)."

@GBKS
Copy link
Contributor

GBKS commented Sep 6, 2021

Hi there, here's an update to the template. I am not a fan of screaming at users with huge capitalized text and exclamation marks, so I put the instructions (and warning notice) at the top so it's clearly noticeable. Let me know what you think.

monero-onboarding-recovery-phrase-template-gbks-210906.pdf
monero-onboarding-recovery-phrase-template-gbks-210906

@rating89us
Copy link
Contributor Author

rating89us commented Sep 20, 2021

@GBKS Thank you, it's really nice. I've just finished coding this PR and had to modify some things, so I'd like to kindly ask you to do some changes:

Required changes:

  • Generate 4 different pdf files (with seed words in 2, 3, 4 and 5 columns), so that the user can copy exactly what is being displayed on Monero GUI. See examples below.
  • Remove wallet name field (the UI was too crowded and it's not needed for wallet restoring)
  • Change recovery phrase label to: "Recovery phrase (mnemonic seed)"
  • Change recovery phrase text to: "These words are a backup of your wallet. They are the only thing needed to access your funds and restore your Monero wallet, so keep this paper in a safe place and do not disclose it to anybody! It's strongly not recommended to store your recovery phrase digitally (in an email, online service, screenshot, photo, or any other type of computer file)."

Suggested changes:

  • "Never share your recovery phrase with anybody" in bold text
  • Add allowed (green check mark) and prohibited (red circle) icons on the remaining blank space, together with a paper and pen, a camera, desktop and wifi (web/internet) symbols.

Below are some examples of the required positioning of mnemonic seed words in each template (so that it matches what is displayed on Monero GUI):
image

image

image

image

@rating89us
Copy link
Contributor Author

rating89us commented Sep 23, 2021

@GBKS It would also be nice to have a dedicated icon for the "verify your recovery phrase" header, something like this:
image

@selsta
Copy link
Collaborator

selsta commented Oct 5, 2021

Did you get opening a PDF from qrc to work yet? I doesn't seem to work on Mac. As far as I know you have to write it to disk first before you can open it.

@selsta
Copy link
Collaborator

selsta commented Oct 5, 2021

diff --git a/wizard/SeedListItem.qml b/wizard/SeedListItem.qml
index 81b417cc..7fc19fe3 100644
--- a/wizard/SeedListItem.qml
+++ b/wizard/SeedListItem.qml
@@ -37,14 +37,8 @@ ColumnLayout {
                 } else {
                     return header.forceActiveFocus();
                 }
-            } else if (wordNumber >= 5 && wordNumber < 10) {
-                return parent.children[wizardCreateWallet2.firstHiddenWord].lineEdit.forceActiveFocus()
-            } else if (wordNumber >= 10 && wordNumber < 15) {
-                return parent.children[wizardCreateWallet2.secondHiddenWord].lineEdit.forceActiveFocus()
-            } else if (wordNumber >= 15 && wordNumber < 20) {
-                return parent.children[wizardCreateWallet2.thirdHiddenWord].lineEdit.forceActiveFocus()
-            } else {
-                return parent.children[wizardCreateWallet2.fourthHiddenWord].lineEdit.forceActiveFocus()
+            } else if (wordNumber >= 5 && wordNumber < 25) {
+                return parent.children[wizardCreateWallet2.hiddenWords[parseInt(wordNumber / 5) - 1]].lineEdit.forceActiveFocus()
             }
         } else {
             if (wordNumber == 0) {
@@ -61,14 +55,8 @@ ColumnLayout {
 
     function focusOnNextField() {
         if (wizardCreateWallet2.state == "verify") {
-            if (wordNumber < 5) {
-                return parent.children[wizardCreateWallet2.secondHiddenWord].lineEdit.forceActiveFocus()
-            } else if (wordNumber >= 5 && wordNumber < 10) {
-                return parent.children[wizardCreateWallet2.thirdHiddenWord].lineEdit.forceActiveFocus()
-            } else if (wordNumber >= 10 && wordNumber < 15) {
-                return parent.children[wizardCreateWallet2.fourthHiddenWord].lineEdit.forceActiveFocus()
-            } else if (wordNumber >= 15 && wordNumber < 20){
-                return parent.children[wizardCreateWallet2.fifthHiddenWord].lineEdit.forceActiveFocus()
+            if (wordNumber < 20) {
+                return parent.children[wizardCreateWallet2.hiddenWords[parseInt(wordNumber / 5) + 1]].lineEdit.forceActiveFocus()
             } else {
                 return navigation.btnPrev.forceActiveFocus()
             }
diff --git a/wizard/WizardCreateWallet2.qml b/wizard/WizardCreateWallet2.qml
index 98ca4227..a4160702 100644
--- a/wizard/WizardCreateWallet2.qml
+++ b/wizard/WizardCreateWallet2.qml
@@ -45,11 +45,7 @@ Rectangle {
     property string viewName: "wizardCreateWallet2"
     property var seedArray: wizardController.walletOptionsSeed.split(" ")
     property var seedListGrid: ""
-    property var firstHiddenWord: ""
-    property var secondHiddenWord: ""
-    property var thirdHiddenWord: ""
-    property var fourthHiddenWord: ""
-    property var fifthHiddenWord: ""
+    property var hiddenWords: [0, 5, 10, 15, 20]
 
     Clipboard { id: clipboard }
 
@@ -214,10 +210,10 @@ Rectangle {
 
                 function focusOnListGrid() {
                     if (wizardCreateWallet2.state == "verify") {
-                        if (seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].lineEdit.visible) {
-                            return seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].lineEdit.forceActiveFocus();
+                        if (seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[0]].lineEdit.visible) {
+                            return seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[0]].lineEdit.forceActiveFocus();
                         } else {
-                            return seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].forceActiveFocus();
+                            return seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[0]].forceActiveFocus();
                         }
                     } else {
                         return seedListGridColumn.children[0].children[0].forceActiveFocus();
@@ -249,21 +245,11 @@ Rectangle {
                 id: seedListGridColumn
 
                 function clearFields() {
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].wordText.visible = true;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].lineEdit.text = "";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].lineEdit.readOnly = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.secondHiddenWord].wordText.visible = true;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.secondHiddenWord].lineEdit.text = "";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.secondHiddenWord].lineEdit.readOnly = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.thirdHiddenWord].wordText.visible = true;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.thirdHiddenWord].lineEdit.text = "";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.thirdHiddenWord].lineEdit.readOnly = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fourthHiddenWord].wordText.visible = true;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fourthHiddenWord].lineEdit.text = "";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fourthHiddenWord].lineEdit.readOnly = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].wordText.visible = true;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].lineEdit.text = "";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].lineEdit.readOnly = false;
+                    for (var i = 0; i < wizardCreateWallet2.hiddenWords.length; i++) {
+                        seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[i]].wordText.visible = true;
+                        seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[i]].lineEdit.text = "";
+                        seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[i]].lineEdit.readOnly = false;
+                    }
                 }
             }
 
@@ -295,7 +281,7 @@ Rectangle {
                     text: qsTr("Print a template") + translationManager.emptyString
                     tooltip: qsTr("Print a template to write down your seed") + translationManager.emptyString
                     onClicked: {
-                        oshelper.openFile("template.pdf")
+                        Qt.openUrlExternally(":/template.pdf")
                     }
                     Accessible.role: Accessible.Button
                     Accessible.name: qsTr("Print a blank template to write down your seed") + translationManager.emptyString
@@ -391,30 +377,26 @@ Rectangle {
                     mobileDialog.visible = Qt.binding(function() { return wizardController.layoutScale == 4 })
                 }
                 btnPrevKeyNavigationBackTab: wizardCreateWallet2.state == "default" ? walletRestoreHeight
-                                                                                    : seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].lineEdit.visible ? seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].lineEdit
+                                                                                    : seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[4]].lineEdit.visible ? seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[4]].lineEdit
                                                                                                                                                                               : seedListGridColumn.children[0].children[24]
                 btnNextKeyNavigationTab: mobileDialog.visible ? mobileHeader : header
                 btnNext.enabled: walletCreationDate.opacity == 1 ? true
-                                                                 : seedListGridColumn.children[0].children[firstHiddenWord].icon.wordsMatch &&
-                                                                   seedListGridColumn.children[0].children[secondHiddenWord].icon.wordsMatch &&
-                                                                   seedListGridColumn.children[0].children[thirdHiddenWord].icon.wordsMatch &&
-                                                                   seedListGridColumn.children[0].children[fourthHiddenWord].icon.wordsMatch &&
-                                                                   seedListGridColumn.children[0].children[fifthHiddenWord].icon.wordsMatch
+                                                                 : seedListGridColumn.children[0].children[hiddenWords[0]].icon.wordsMatch &&
+                                                                   seedListGridColumn.children[0].children[hiddenWords[1]].icon.wordsMatch &&
+                                                                   seedListGridColumn.children[0].children[hiddenWords[2]].icon.wordsMatch &&
+                                                                   seedListGridColumn.children[0].children[hiddenWords[3]].icon.wordsMatch &&
+                                                                   seedListGridColumn.children[0].children[hiddenWords[4]].icon.wordsMatch
                 onNextClicked: {
                     //choose five random words to hide
-                    wizardCreateWallet2.firstHiddenWord = Math.floor(Math.random() * 5)
-                    wizardCreateWallet2.secondHiddenWord = Math.floor(Math.random() * 5) + 5
-                    wizardCreateWallet2.thirdHiddenWord = Math.floor(Math.random() * 5) + 10
-                    wizardCreateWallet2.fourthHiddenWord = Math.floor(Math.random() * 5) + 15
-                    wizardCreateWallet2.fifthHiddenWord = Math.floor(Math.random() * 5) + 20
+                    for (var i = 0; i < hiddenWords.length; i++) {
+                        wizardCreateWallet2.hiddenWords[i] = Math.floor(Math.random() * 5) + 5 * i
+                    }
 
                     wizardCreateWallet2.state = "verify";
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].wordText.visible = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.secondHiddenWord].wordText.visible = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.thirdHiddenWord].wordText.visible = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fourthHiddenWord].wordText.visible = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.fifthHiddenWord].wordText.visible = false;
-                    seedListGridColumn.children[0].children[wizardCreateWallet2.firstHiddenWord].lineEdit.forceActiveFocus();
+                    for (var i = 0; i < hiddenWords.length; i++) {
+                        seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[i]].wordText.visible = false;
+                    }
+                    seedListGridColumn.children[0].children[wizardCreateWallet2.hiddenWords[0]].lineEdit.forceActiveFocus();
                 }
             }
         }

Using an array makes this code simpler.

@rating89us
Copy link
Contributor Author

Did you get opening a PDF from qrc to work yet? I doesn't seem to work on Mac. As far as I know you have to write it to disk first before you can open it.

It's working on Windows and Ubuntu. I haven't tested on Mac yet.

@selsta
Copy link
Collaborator

selsta commented Oct 15, 2021

So the only thing remaining here is getting the PDF to work on Mac?

@rating89us
Copy link
Contributor Author

rating89us commented Nov 24, 2021

So the only thing remaining here is getting the PDF to work on Mac?

Yes

font.bold: true
textFormat: Text.RichText
color: MoneroComponents.Style.defaultFontColor
text: currentDate.toLocaleDateString(locale, Locale.ShortFormat)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about specifying it as YYYY-MM-DD? That's the format that is required for the restore height textbox.

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 prefer the default LocaleDateString. Later I want to display a calendar during wallet restore instead of asking for YYYY-MM-DD, which is not familiar to users from multiple countries.

Copy link
Contributor

Choose a reason for hiding this comment

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

ALOT of beginners forget the "-" or just type the date incorrectly. This would save alot of people pain if a calendar was implemented 👍

@rating89us
Copy link
Contributor Author

Added "create a new seed" button

Copy link
Collaborator

@selsta selsta left a comment

Choose a reason for hiding this comment

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

Couple minor comments. Did you test the binaries that were created with CI to check if the PDF works in them?

wizard/SeedListItem.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet3.qml Outdated Show resolved Hide resolved
… template; seed verification; responsive UI; accessibility
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
wizard/WizardCreateWallet2.qml Outdated Show resolved Hide resolved
Copy link
Collaborator

@selsta selsta left a comment

Choose a reason for hiding this comment

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

There is a merge conflict, a rebase is required.

diff --git a/src/main/oshelper.cpp b/src/main/oshelper.cpp
index a910dae8..ddf3d9ed 100644
--- a/src/main/oshelper.cpp
+++ b/src/main/oshelper.cpp
@@ -292,3 +292,9 @@ bool OSHelper::installed() const
     return false;
 #endif
 }
+
+void OSHelper::openSeedTemplate() const
+{
+    QFile::copy(":/wizard/template.pdf", QDir::tempPath() + "/seed_template.pdf");
+    openFile(QDir::tempPath() + "/seed_template.pdf");
+}
diff --git a/src/main/oshelper.h b/src/main/oshelper.h
index 2d8d41f0..d171fc57 100644
--- a/src/main/oshelper.h
+++ b/src/main/oshelper.h
@@ -53,6 +53,7 @@ public:
     Q_INVOKABLE QString temporaryPath() const;
     Q_INVOKABLE bool removeTemporaryWallet(const QString &walletName) const;
     Q_INVOKABLE bool isCapsLock() const;
+    Q_INVOKABLE void openSeedTemplate() const;
 
 private:
     bool installed() const;
diff --git a/wizard/WizardCreateWallet2.qml b/wizard/WizardCreateWallet2.qml
index 475e722f..04c2bf99 100644
--- a/wizard/WizardCreateWallet2.qml
+++ b/wizard/WizardCreateWallet2.qml
@@ -316,11 +316,10 @@ Rectangle {
                     id: printPDFTemplate
                     small: true
                     primary: false
-                    visible: !isMac
                     text: qsTr("Print a template") + translationManager.emptyString
                     tooltip: qsTr("Print a template to write down your seed") + translationManager.emptyString
                     onClicked: {
-                        oshelper.openFile("wizard/template.pdf")
+                        oshelper.openSeedTemplate();
                     }
                     Accessible.role: Accessible.Button
                     Accessible.name: qsTr("Print a template to write down your seed") + translationManager.emptyString

This solves saving the PDF on macOS. I did not test it on other OS.

@selsta
Copy link
Collaborator

selsta commented Feb 27, 2022

It saves the PDF to the temp directory and then it gets opened. The PDF inside the temp directory gets deleted after the next computer reboot.

@selsta
Copy link
Collaborator

selsta commented Mar 16, 2022

@rating89us did you time to update this so that we can merge it? :)

@selsta
Copy link
Collaborator

selsta commented Apr 6, 2022

Let's continue here: #3878

@selsta selsta closed this Apr 6, 2022
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.

[UX] Move seed into a separate screen of the onboarding flow
4 participants