wallet: Allow user to navigate options while encrypting at creation

[hernanmarino]

This fixes #394.
It adds a "Go back" button to the "Confirm wallet encryption" window, allowing the users to change the password if they want to. It also adds a Cancel button to the "Wallet to be encrypted" window.
Prior to this change users had no option to alter the password, and were forced to either go ahead with wallet creation or cancel the whole process. Also, at the final window, they were shown a warning but with no option to cancel.
The new workflow for wallet encryption and creation is similar to the following:

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	alfonsoromanz, BrandonOdiwuor, hebasto
Concept ACK	luke-jr
Stale ACK	jarolrod, pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[pablomartin4btc]

Tested ACK.
It works as expected according to the description of the PR and it fulfils the requirements of the related issue.
Just as a reminder for other reviewers, if you want to verify if the wallet was encrypted or not, when a user creates an encrypted wallet, the password won't be requested on loading or switching to the encrypted wallet, only on "send"; the private keys are encrypted not the wallet (.dat) itself.

[hebasto]

Concept ACK.

[hernanmarino]

Updated the code as per @hebasto suggestions.

[pablomartin4btc]

re-ACK 78660e7. cc @jarolrod.

I seewallet_create_tx.py --descriptorstest is failing and you are re-running it, I don't think it's related with your change but let's see.

[jarolrod]

ACK 78660e7

Tested for functionality and sanity checked the user flows between master and this PR.

[hernanmarino]

After @hebasto 's comment I decided to update the code to explicitly call reject() when cancelling the first dialog. This introduces one less change from the original code, and also makes the intention more explicit to developers / reviewers.

[luke-jr]

Concept ACK

But shouldn't the final confirmation dialog also have a "Go back" instead of "Cancel"?

(Better yet, why don't we just show these warnings upfront in the password entry dialog?)

[pablomartin4btc]

But shouldn't the final confirmation dialog also have a "Go back" instead of "Cancel"?

I think at that point, going back to the previous window, where you have to re-enter the password, won't make much sense.

This is how it's now:

Now that I've re-tested it, once you have entered the passphrase, when you click on "Back", doesn't go back to anything, just close/ cancel the entire process, so if we agree on the use case I rather put back "Cancel" on the button label or I wouldn't go back to the password either but to the "Create Wallet" window. Having said that I see in @jarolrod's issue #394 that he wanted to go back to the "Encrypt wallet" window where you have to re-enter the password, so I'm not sure if we could add a "second" back, one to the creation wallet window and one for the encrypt wallet to re-enter the password, the reason behind going back to the "Create wallet window" is that perhaps you just realised you don't want to bother, just create a regular (non encrypted) one.

So, same for the final confirmation @luke-jr, I would go "Back" to the "Create Wallet" window or just leave it as cancel.

And, I think I'd change button label "Cancel" on the "Encrypt wallet" and put "Back" there to go back to creation window, also I'd add "Create wallet -" (or "New wallet - #name) on the window's title so you see the context if you alt-tab to another app or anything, also because I see this window is also used when you have a wallet selected and want to encrypt it (I'd add the name of the wallet in the title, perhaps to be consistent with when you create a new one) :

(Better yet, why don't we just show these warnings upfront in the password entry dialog?)

I agree with this one, I'd add it when you enter the pwd (current "Encrypt wallet" window) and I'd leave it also on the confirmation pop-up.

@GBKS, @jarolrod, what do you think? And others ofc.

[pablomartin4btc]

re-Tested nACK.

As I mentioned in the above response to @luke-jr, last change made the fix not to work anymore (see animated gif). Please consider my comments above if others also see them useful.

[hernanmarino]

Rolled back to undo changes detected by @pablomartin4btc . Now changes are at commit 78660e7 , the last one ACK ed by @jarolrod and others.

[pablomartin4btc]

re-ACK 78660e7 as previously here.

[BrandonOdiwuor]

Tested ACK 78660e7

[alfonsoromanz]

Tested ACK 78660e7

[hernanmarino]

Rebased for CI, no actual code changes in my code.

[alfonsoromanz]

Re-Tested ACK cccddc0

[BrandonOdiwuor]

re-Tested ACK cccddc0

[hebasto]

ACK cccddc0, tested on Ubuntu 24.04.

[maflcko]

I don't understand this change. It looks like a "Back" button is added to the warning pop-up, but it will only take you back to enter a different password, not to disable the password selection in the create wallet dialog. Wouldn't it be better to add the "Back" button to the password dialog?

Also, wouldn't it be better if the text from the two warning pop-ups was added to the top of the password dialog? Otherwise, it seems odd to first ask for a password from the user, then print two warnings about it, when the warnings could have been provided in one go?

[maflcko]

Also, this doesn't need release notes, does it?

[hebasto]

It looks like a "Back" button is added to the warning pop-up, but it will only take you back to enter a different password, not to disable the password selection in the create wallet dialog. Wouldn't it be better to add the "Back" button to the password dialog?

Also, wouldn't it be better if the text from the two warning pop-ups was added to the top of the password dialog? Otherwise, it seems odd to first ask for a password from the user, then print two warnings about it, when the warnings could have been provided in one go?

@GBKS

What kind of a workflow will suit this part of the UI in your opinion?

[GBKS]

Good points. I think it could work well to combine the two warnings to a single dialog. It could be placed between the initial Create wallet dialog, and the password input.

1. Create wallet dialog: The user checks Encrypt wallet and clicks Continue.
2. Password warning dialog: The application informs the user about the risks of using a password and offers Back and Continue options. The user considers, decides to move forward with the password, and clicks Continue.
3. Password input dialog: The user enters their password and clicks Continue (the other option would be Back). It could be an option to have a I have stored this password in a safe place check box here. Another option is a password difficulty indicator.

This avoids the stack of dialog-on-dialog by putting them in a linear sequence with consistent Continue and Back options.

When dialogs are stacked like this and you mix buttons like Cancel and Back, it can be hard to know what they reference. Do you cancel the whole wallet creation or just the password option, or just the dialog? What does Back refer to in a dialog that sits on top of another dialog? Keeping the sequence linear makes this more obvious.

Just some thoughts, not sure if you even want to revisit this or not right after working through this PR.

[hebasto]

@hernanmarino

Are you still working on this PR? If so, what do you think about GBKS's suggestion?

[hernanmarino]

@hernanmarino

Are you still working on this PR? If so, what do you think about GBKS's suggestion?

I like them. I can work on them and open a follow up PR soon.