Skip to content

Conversation

@SNeedlewoods
Copy link

In Monero Tech Meeting 116 (log) I mentioned that changing a password in a password prompt in any way other than appending letters, QT will create copies in memory and we have no easy access to wipe those copies. So I added a property bool isSecret to the LineEdit.qml component that you can set to true for password fields. With that setting, it does not allow Backspace, Delete or Arrow keys or selecting Text with the mouse.
If anyone thinks it's worth it, I could investigate if we can make this an optional wallet setting, for those who prefer the old UX. In that case I'd like to know your opinion which option should be default.
Another note from that meeting:

FWIW just pasting the password also works fine, meaning the password will get wiped.


Some things changed for "Settings->Info-Wallet restore height"

  • Now you have to provide a password to change restore height, because that information is stored in the .keys file and to retrigger rewrite of that file we call setPassword(), which used the cached password before.
  • Before, when you changed the restore height and canceled the second prompt asking "Are you sure you want to rebuild the wallet cache?", the restore height in the settings was changed (even after closing and reopening the wallet), but the actual restore height the wallet used to scan did not change. Now both will change only after confirming the second prompt.
  • Before, when confirming to rebuild wallet cache, the wallet cache file got moved from <wallet_name> to <wallet_name>.old_cache. So if you do something that needs the wallet cache file afterwards it can lead to undefined behavior. E.g. if you try to change your password, you'll get: Error: boost::filesystem::canonical: No such file or directory [generic:2]: "/path/to/Monero/wallets/<wallet_name>/<wallet_name>". Now with a call to storeAsync() in main.qml callback connectWallet() we can prevent this.

If you create a view-only wallet with Settings->Wallet->Create a view-only wallet, you now have to type a new password. Before, it used the cached password so both the original wallet and the new view-only wallet had the same password. A minor problem: without creating a new "new password dialog" there is no confirm-password field, so if you mistyped the desired password you can't open the view-only wallet, but I think it's not a big problem, you can just create new view-only wallets until you get the password right.


To test this you need to pull this PR into monero/ subdir.
In order to make debug I changed origin/master to <YOUR_LOCAL_BRANCH_NAME> in CMakeLists.txt.

-      execute_process(COMMAND ${GIT_EXECUTABLE} checkout -f origin/master WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/monero RESULT_VARIABLE GIT_CHECKOUT_MASTER_RESULT)
+      execute_process(COMMAND ${GIT_EXECUTABLE} checkout -f <YOUR_LOCAL_BRANCH_NAME> WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/monero RESULT_VARIABLE GIT_CHECKOUT_MASTER_RESULT)

@plowsof
Copy link
Contributor

plowsof commented Apr 28, 2025

just done a quick test to confirm it builds / the bug you found is fixed: so now cancelling the wallet restore height change will not apply the unconfirmed value.

some "marked ‘override’" complaints with builds

@SNeedlewoods
Copy link
Author

SNeedlewoods commented Apr 29, 2025

Thanks,
I should make another PR for monero-project/monero#9464 based on release, that's where I added those WalletListener callback functions, I think once that is merged the "marked override" complains for CI builds should disappear.


Edit: monero-project/monero#9918

@SNeedlewoods SNeedlewoods force-pushed the x_replace_cached_password_with_verifyPassword branch from c112612 to 7b6f23d Compare May 8, 2025 16:40
@SNeedlewoods SNeedlewoods force-pushed the x_replace_cached_password_with_verifyPassword branch from 7b6f23d to f11d6d3 Compare July 8, 2025 18:21
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.

2 participants