The PR branch HEAD was b2e2829 at the time of this review club meeting.
A wallet rescans the blockchain in order to find transactions related
to its keys. A wallet starts rescanning the blockchain upon the import
of keys, descriptors, etc. A rescan can also be invoked by calling
Wallet rescans were previously discussed in review clubs
#15845 and #25957.
When a wallet is rescanning, it needs to be able to top-up its keypool.
A wallet’s keypool is its reserve of unused keys of a denoted size (the
default size is 1000 keys). When a wallet is using BIP 32
to generate deterministic keys, an unused key is considered to be any key
whose BIP 32 index is greater than the index of the last key which was
provided to the user by the wallet or which received funds.
Wallets can optionally be encrypted using a passphrase. Only the
private key material of the wallet is encrypted using a user-provided
An encrypted wallet is locked by default;
it can be unlocked by calling walletpassphrase,
which then stores the passphrase in memory for a specified amount of
time before the wallet is locked again
Certain RPCs which make use of information relating to private keys, such as
sendtoaddress and signrawtransactionwithwallet
require the wallet to be unlocked in order to work correctly. If the wallet is
not unlocked, the RPC typically throws an error.
The cs_walletmutexguards access to the passphrase when stored in memory. For example, cs_wallet must be
held before the passphrase can be deleted, so
holding the cs_wallet mutex for the duration of the RPC ensures another thread
cannot delete the passphrase while it is in use.
The rescanblockchain and importdescriptors RPCs don’t hold
cs_wallet during the entire rescan, only for certain relatively fast
operations, because a rescan can take over an hour, and the user might
want to use other RPCs in the meantime which require the cs_wallet
lock to be held.
<rot13maxi> Will other signing operations with the wallet fail/error if the wallet is in the middle of rescanning? In other words, if my wallet normally re-locks after 30 seconds, but is "held open" because it's rescanning, could an RPC to send a transaction come in while its rescanning, or would that RPC fail because there's a scan-in-progress?
<rot13maxi> if other signing RPCs don't currently fail/error when a rescan is in progress, then we might want to have the ` if (pwallet->IsScanningWithPassphrase())` guard on those other operations, right?
<LarryRuane> while rescanning, we're discovered UTXOs that we own (have keys for), but it should be okay to, for example, `sendmany` which would just use the UTXOs that we do know about? (rather than waiting for the rescan to complete?)
<ishaanam[m]> LarryRuane: Yeah, when writing this I had played around with using mocktime but it didn't seem to make a difference. However, if you can think of any ways to make this more reliable please let me know! Perhaps you can increase the "range" and "next_index" values in the descriptor object that the scan is being done for?
<ishaanam[m]> pablomartin: Currently it is somewhat difficult for the functional test suite to have a rescan going on in the backgroung while other RPCs are being run, but I think I remember there being a PR open for fixing that.
<ishaanam[m]> Theoretically while ␑cs_wallet␏ could be used, ␑m_relock_mutex␏ is added because rescanning a wallet can take a long time, and we would still want other RPCs that lock ␑cs_wallet␏ to be able to run.
<LarryRuane> but there's a general push to move away from RecursiveMutex because it encourages sloppy code ... I'm not sure why a `Mutex` wouldn't work here (i saw that there's a comment on the PR that you replied to)
<ishaanam[m]> LarryRuane: it is because sometimes ␑cs_wallet␏ is locked, and then ␑CWallet::Lock␏ is called, which locks ␑m_rescan_mutex␏ and then ␑cs_wallet␏, which is inconsistent from a few other places
<ishaanam[m]> glozow: yes, but if there is a strong preference towards Mutex then I can revisit the code and perhaps do it differently such that this is no longer a concern. This would require a few more changes then what is there currently from what I can tell.