New wallet wizard edge case with 1.5.3 dev branch on Windows #79

This could be related to my dev environment and build of 1.5.3, but documenting anyway. Steps to reproduce:

Ensure there is no existing silentdragonlite-wallet.dat
Open SDL and be presented with "New wallet wizard"
Accept terms, enter passphrase, choose to restore wallet from seed, and click Next
Enter seed phrase
Note that wallet Birthday defaults to 180000 and Quantity defaults to 10
I deleted wallet Birthday entirely and manually typed 840000
I deleted Quantity entirely and manually typed 10
Click Finish
silentdragonlite-wallet.dat is created and wallet starts syncing at block 840000
Upon sync finishing, silentdragonlite-wallet.dat is automatically deleted and there is no longer any wallet file
Rescan from Edit > Rescan
Restore your Wallet dialog appears with seed phrase, birthday of 840000, and quantity of 10 auto-populated
Clicking restore creates a silentdragonlite-wallet.dat file and starts syncing at block 840000 with "Failed to parse wallet birthday" warning alert behind it.
Upon sync finishing, silentdragonlite-wallet.dat is deleted automatically and no wallet file exists
Clicking OK on "Failed to parse wallet birthday" warning alert closes SDL. Note that there is still no wallet file.
Re-open SDL and you are presented with "New wallet wizard" again due to no wallet file existing

Edge case with seeing new wallet wizard every open only happens if you can not normally close SDL at least once for silentdragonlite-wallet-enc.dat to be created. silentdragonlite-wallet.dat deletion also occurs in 1.5.2 and this edge case may only occur in that version if SDL crashes on first use. If you close SDL normally after step 9, it will create silentdragonlite-wallet-enc.dat. Re-opening SDL will allow you to restore wallet from seed or enter passphrase.

These are additional steps when re-opening SDL while silentdragonlite-wallet-enc.dat exists:

Upon re-opening SDL with a silentdraonlite-wallet-enc.dat file existing, enter passphrase and wallet syncs any new blocks since last sync.
When selecting Edit > Rescan, seed phrase auto populates with birthday autopopulated to 840000 and quantity 10.
Clicking "Restore" starts syncing wallet at 840000 with "Failed to parse wallet birthday" warning alert behind it.
Wallet fully syncs.
Clicking OK on warning alert closes SDL
Re-open SDL and wallet is still fully synced

I am testing compiling 1.5.2 in same environment to see if "Failed to parse wallet birthday" alert shows when performing Edit > Rescan. It does not show in released version of 1.5.2. This alert is the one in mainwindow.cpp and not the one in firsttimewizard.cpp. Adding simplified() to conversion makes no difference other than showing a different single character value for birthday.

This could be related to my dev environment and build of 1.5.3, but documenting anyway. Steps to reproduce: 1. Ensure there is no existing silentdragonlite-wallet.dat 2. Open SDL and be presented with "New wallet wizard" 3. Accept terms, enter passphrase, choose to restore wallet from seed, and click Next 3. Enter seed phrase 4. Note that wallet Birthday defaults to 180000 and Quantity defaults to 10 5. I deleted wallet Birthday entirely and manually typed 840000 6. I deleted Quantity entirely and manually typed 10 7. Click Finish 8. silentdragonlite-wallet.dat is created and wallet starts syncing at block 840000 9. Upon sync finishing, silentdragonlite-wallet.dat is automatically deleted and there is no longer any wallet file 10. Rescan from Edit > Rescan 11. Restore your Wallet dialog appears with seed phrase, birthday of 840000, and quantity of 10 auto-populated 12. Clicking restore creates a silentdragonlite-wallet.dat file and starts syncing at block 840000 with "Failed to parse wallet birthday" warning alert behind it. 13. Upon sync finishing, silentdragonlite-wallet.dat is deleted automatically and no wallet file exists 14. Clicking OK on "Failed to parse wallet birthday" warning alert closes SDL. Note that there is still no wallet file. 15. Re-open SDL and you are presented with "New wallet wizard" again due to no wallet file existing Edge case with seeing new wallet wizard every open only happens if you can not normally close SDL at least once for silentdragonlite-wallet-enc.dat to be created. silentdragonlite-wallet.dat deletion also occurs in 1.5.2 and this edge case may only occur in that version if SDL crashes on first use. If you close SDL normally after step 9, it will create silentdragonlite-wallet-enc.dat. Re-opening SDL will allow you to restore wallet from seed or enter passphrase. These are additional steps when re-opening SDL while silentdragonlite-wallet-enc.dat exists: 1. Upon re-opening SDL with a silentdraonlite-wallet-enc.dat file existing, enter passphrase and wallet syncs any new blocks since last sync. 2. When selecting Edit > Rescan, seed phrase auto populates with birthday autopopulated to 840000 and quantity 10. 3. Clicking "Restore" starts syncing wallet at 840000 with "Failed to parse wallet birthday" warning alert behind it. 4. Wallet fully syncs. 5. Clicking OK on warning alert closes SDL 6. Re-open SDL and wallet is still fully synced I am testing compiling 1.5.2 in same environment to see if "Failed to parse wallet birthday" alert shows when performing Edit > Rescan. It does not show in released version of 1.5.2. This alert is the one in [mainwindow.cpp](https://git.hush.is/hush/SilentDragonLite/src/branch/dev/src/mainwindow.cpp#L208) and not the one in firsttimewizard.cpp. Adding `simplified()` to conversion makes no difference other than showing a different single character value for `birthday`.

@fekt this is a good bug report, thank you.

I added .simplified() just in case a user does accidentally put whitespace before or after the birthday, then things should work. I wasn't expecting that to fix this bug based on what you told me about it looking like corrupt binary data.

I really don't know what is causing the birthday to fail to parse, but from the behavior, it seems that the variable that the birthday is stored it gets corrupted or overwritten somehow, or maybe freed too early.

One idea I have is if the wallet birthday fails to be parsed, we could ask for it again, but that might go into a loop of failing to parse it. If you have suggestions for how things should change, they are welcome.

@duke I'm not sure on a fix for the alert because it seems to still accept the birthday and fully sync even though the conversion says it fails. I tried adding .simplified() because I was seeing differences in the value after conversion if I fully deleted the auto-populated value and manually typed it back in as well as differences if using .simplified() or not and thought maybe the auto-population was adding whitespace or new lines.

Is there a reason the encrypted version of the wallet.dat file isn't created after the unencrypted version is deleted after syncing and only created when closing SDL? I doubt many would hit this issue on first use and just curious if it needs to be that way or not.

@duke Safe to say the alert is something with my dev environment. I am getting the same alert in my compiled version of 1.5.2, but official release is fine. Do you know who compiled that originally so I can dig around to see if they left any notes on environment? What version of Rust should I be using?

@fekt To answer "Is there a reason the encrypted version of the wallet.dat file isn't created after the unencrypted version is deleted after syncing and only created when closing SDL? I doubt many would hit this issue on first use and just curious if it needs to be that way or not." that is intended as a feature, not a bug. Let me explain.

Originally, SDL wallet had very poor encryption that we inherited from upstream. Only privkeys were encrypted but all other metadata in the wallet was plaintext. Then I worked with the dev Denio to improve the situation, and we made all contents of the wallet encrypted. We also wanted to reduce the time the unencrypted version of the wallet existed on disk.

So the basic idea is we have an encrypted wallet on disk when SDL is not running, and when a user correctly logs in via passphrase, we decrypt the wallet. When SDL exits/closes, we re-encrypt the wallet. The code attempts to not have both an encrypted and decrypted wallet file at the same time on disk, because if an error/crash happens, they could be out of sync.

It sounds like you have discovered an edge case where we have decrypted the wallet, the encrypted wallet is stored in memory, but we have not yet written out any file to disk. It should not happen in normal cases, but could happen in Out Of Memory situations and other kinds of errors like that.

@fekt as for Rust version, the readme says to use 1.49 and gives instructions for that. As for who made the winbin, I think it was @odinzu

@duke Thanks for the explanation regarding wallet encryption. Makes sense. Looked like Zcash ported to a newer lite wallet quite some time ago so upstream issues are expected.

I have done different builds based on versions in readme, versions in DEVELOPING.md, and stable versions. All present the alert when performing Edit > Rescan.

I was thinking it was an older dev and second guessing environment because the source for 1.5.2 uses Zcash Rust crates from old GitHub. Just wanted to have everything match exactly from whoever built it last, but that may not be documented or known. The win-static-build.sh script mentioned in DEVELOPING.md was also removed at some point, but I pulled it from N1CK145's repo on GitHub. I was primarily setting up my dev environment based on DEVELOPING.md, but using newer versions of QT, openSSL, and Rust. Maybe you or someone else can see something that stands out with the dependencies or QT config? I will keep debugging for now.

@fekt that script was removed in this commit if you want to look at it: 44c80b4732

As for using rust crates from github, we changed them to use the versions from git.hush.is very recently, but the sha256 checksums never changed, we just download them from git.hush.is vs github now.

Do we know if this is a Windows-only bug or has this ever happened on Linux?

@duke I have only been testing dev branch on Windows. It's likely Windows-only bug with my build but I will test on Linux to be sure. I wasn't seeing any commits since 1.5.2 that would have introduced the alert issue and my environment produces the same alert when compiling 1.5.2, which doesn't happen in official release. Edge case with wallet.dat probably exists everywhere but would be difficult to hit without the alert issue or SDL crashing on first use. I have been cross-compiling on Ubuntu 18.0.4 and in process of setting up environment on Debian 11 since that's what I use for SD.

@duke I think others need to test on Linux to get solid feedback. I compiled and ran on Debian 11. After initial new wallet creation, I was seeing two animation screens at the same time and had a seg fault. My seed phrase also had two of the same words, which is a first for me. It re-opened fine and still had a wallet.dat. I can not reproduce the double animation screens and may have been an initial run fluke right after build.

Edit > Rescan appears to work fine and I am not getting a birthday alert like my Windows build.

I am ocassionally able to reproduce the new wallet wizard and wallet.dat issue by doing an Edit > Rescan, closing the rescan before finished, and then closing SDL, but seems sporadic. Closing SDL shows the animation screen status text duplicated with "Please wait for SilentDragonLite to exit" and it's actually continuing to sync. It'll say the saveWallet finished, but then that the file doesn't exist, so then new wallet wizard is presented on re-open.

I was seeing stuff like this:

doRPC :  "sietch"
No Hush contacts found on disk!
doRPCSetConnection : wallet path = QFileInfo(/home/fekt/.silentdragonlite/silentdragonlite-wallet.dat)
operator() : saveWallet finished
litelib_process_response :  [
  "zs1lq5nqpq95tfe5wa4her6h37ujqj6fdmn56xkx8j5yjyz2t2lppd8wnjsm6ykzq0jc8fm75ukemd"
]
File not exits "/home/fekt/.silentdragonlite/silentdragonlite-wallet.dat"
litelib_process_response :  [
  "zs108l95kal0rr2zpl1h3ztfy3dmucv8ucaz3puh07u73r2vvfhmw4kqgwpgx0n7k3fc3vvwgktpmq"
]

Reopen and see this

Wallet exists: false
doAutoConnect : no existing wallet

It is sporadic and not always reproducible. Cancelling the rescan, closing SDL, and closing the animation screen, would also still continue to sync and wallet.dat would still exist on re-open most of the time just not be fully synced, while showing this unclean shutdown. I think almost every time I got this message, wallet.dat still existed and re-opened fine without new wallet wizard.

shutdownhushd : No zrpc object, unclean shutdown and unable to call saveWallet!

@duke I think others need to test on Linux to get solid feedback. I compiled and ran on Debian 11. After initial new wallet creation, I was seeing two animation screens at the same time and had a seg fault. My seed phrase also had two of the same words, which is a first for me. It re-opened fine and still had a wallet.dat. I can not reproduce the double animation screens and may have been an initial run fluke right after build. Edit > Rescan appears to work fine and I am not getting a birthday alert like my Windows build. I am ocassionally able to reproduce the new wallet wizard and wallet.dat issue by doing an Edit > Rescan, closing the rescan before finished, and then closing SDL, but seems sporadic. Closing SDL shows the animation screen status text duplicated with "Please wait for SilentDragonLite to exit" and it's actually continuing to sync. It'll say the saveWallet finished, but then that the file doesn't exist, so then new wallet wizard is presented on re-open. I was seeing stuff like this: ``` doRPC : "sietch" No Hush contacts found on disk! doRPCSetConnection : wallet path = QFileInfo(/home/fekt/.silentdragonlite/silentdragonlite-wallet.dat) operator() : saveWallet finished litelib_process_response : [ "zs1lq5nqpq95tfe5wa4her6h37ujqj6fdmn56xkx8j5yjyz2t2lppd8wnjsm6ykzq0jc8fm75ukemd" ] File not exits "/home/fekt/.silentdragonlite/silentdragonlite-wallet.dat" litelib_process_response : [ "zs108l95kal0rr2zpl1h3ztfy3dmucv8ucaz3puh07u73r2vvfhmw4kqgwpgx0n7k3fc3vvwgktpmq" ] ``` Reopen and see this ``` Wallet exists: false doAutoConnect : no existing wallet ``` It is sporadic and not always reproducible. Cancelling the rescan, closing SDL, and closing the animation screen, would also still continue to sync and wallet.dat would still exist on re-open most of the time just not be fully synced, while showing this unclean shutdown. I think almost every time I got this message, wallet.dat still existed and re-opened fine without new wallet wizard. ``` shutdownhushd : No zrpc object, unclean shutdown and unable to call saveWallet! ```

@fekt FYI, it is valid and possible to get the same seed word multiple times in a seed phrase, that is not a bug. It doesn't happen often, but I have seen it various times.

As for the rest of your comments, I think you are finding valid bugs. They are edge cases from uncommon GUI actions. The "two animation screens at once" is new to me, I have never seen that before.

This is a known bug and I think there is an issue for it: Closing SDL shows the animation screen status text duplicated with "Please wait for SilentDragonLite to exit" and it's actually continuing to sync.

I think it's related to the fact that there is no way to tell SDL to stop syncing. Once it starts, it only knows how to finish syncing.

The print "shutdownhushd : No zrpc object, unclean shutdown and unable to call saveWallet!" means that we had no connection to the lite wallet server upon shutdown and could not call the saveWallet method. I think the latest code attempts to create a new connection, just once, and if that fails, it gives up.