dashpay / dash

Dash - Reinventing Cryptocurrency
https://www.dash.org
MIT License
1.49k stars 1.2k forks source link

bug: crash on wallet encrypt #5715

Open PastaPastaPasta opened 10 months ago

PastaPastaPasta commented 10 months ago

Steps to reproduce

  1. Create a new wallet, call it 1000000 leave all checkboxes as default.
  2. In the console, swap to that wallet and issue keypoolrefill 1000000
  3. Here a bug emerges, I had to close numerous popups that claimed the refill progress, but I could see in the debug.log that the refill was already done.
  4. Exit the app.
  5. Start the app and switch to the wallet named 1000000
  6. Select encrypt and choose password of 1234
  7. This appears to hang, after a few seconds click anywhere on the window and the assert appears.

Stack trace

$ ./src/qt/dash-qt --testnet [±v20.x ●] Assertion failure: assertion: false file: wallet.cpp, line: 661 function: EncryptWallet 0#: (0x105C0E004) stacktraces.cpp:629 - __assert_rtn 1#: (0x10505444C) wallet.cpp - ??? 2#: (0x10495DA6C) askpassphrasedialog.cpp:149 - ??? 3#: (0x1056B0D7C) slice.h - ??? 4#: (0x105894618) slice.h - ??? 5#: (0x1056B0D7C) slice.h - ??? 6#: (0x1058188C0) slice.h - ??? 7#: (0x1058184F4) slice.h - ??? 8#: (0x1058DE910) slice.h - ??? 9#: (0x10579BE30) slice.h - ??? 10#: (0x10495E1DC) askpassphrasedialog.cpp - ??? 11#: (0x105769E44) slice.h - ??? 12#: (0x10576B4C4) slice.h - ??? 13#: (0x105693F24) slice.h - ??? Assertion failed: (false), function EncryptWallet, file wallet.cpp, line 661. [1] 948 abort ./src/qt/dash-qt --testnet

PastaPastaPasta commented 10 months ago

I added a bunch of asserts into this to figure out what exactly is failing and it appears to me to be this that is actually failing https://github.com/dashpay/dash/blob/65ff12c303f1ca8303e608209fa101b066f6f508/src/wallet/walletdb.cpp#L134

panleone commented 7 months ago

Pretty sure this happens because all keys are encrypted in a single Transaction (and we do this because if something fails we don't end up with half keys encrypted on db and half keys still unencrypted or still not saved on db).

In this case there are too many keys in a single transaction, which gets too big and at some point we fail to write in the WalletBatch.

panleone commented 7 months ago

A solution would be doing multiple smaller transactions, but then a smart way to revert everything in case of failure should be implemented too