Merge #6006: fix: resolve potential deadlocks in CJ

b2910fb fix: resolve potential deadlocks in CJ (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented

  ```
  POTENTIAL DEADLOCK DETECTED
  Previous lock order was:
   (2) 'cs_wallet' in wallet/wallet.cpp:3826 (in thread 'qt-init')
   (2) 'pwallet->cs_wallet' in wallet/walletdb.cpp:705 (in thread 'qt-init')
   (1) 'cs_KeyStore' in wallet/scriptpubkeyman.cpp:971 (in thread 'qt-init')
  Current lock order is:
   'cs_deqsessions' in coinjoin/client.cpp:261 (in thread 'main')
   (1) 'cs_KeyStore' in wallet/scriptpubkeyman.cpp:1522 (in thread 'main')
   (2) 'cs_wallet' in wallet/wallet.cpp:1629 (in thread 'main')
  ```

  This one is for `ResetPool()`.

  ## What was done?
  Lock `cs_wallet` when calling `keyHolderStorage.ReturnAll()` in some places* to ensure the right order of locks.

  (*In other places `cs_wallet` is held already, no need to double lock).

  ## How Has This Been Tested?
  Mixing

  ## Breaking Changes
  n/a

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK b2910fb

Tree-SHA512: 8be98df021f7683cd496ebe095dd7b32ebea76c7f9c7c085af3bc0a6901d9cfd4d4624e20a2eee1f3b0d69fd711f8fceb9a91c386b9bf02475632a23b3a0f09a

src/coinjoin/client.cpp

@@ -249,7 +249,7 @@ void CCoinJoinClientSession::ResetPool()
 {
     txMyCollateral = CMutableTransaction();
     UnlockCoins();
-    keyHolderStorage.ReturnAll();
+    WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
     WITH_LOCK(cs_coinjoin, SetNull());
 }
 
@@ -410,7 +410,7 @@ bool CCoinJoinClientSession::CheckTimeout()
 
     SetState(POOL_STATE_ERROR);
     UnlockCoins();
-    keyHolderStorage.ReturnAll();
+    WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
     nTimeLastSuccessfulStep = GetTime();
     strLastMessage = CoinJoin::GetMessageByID(ERR_SESSION);
 
@@ -521,7 +521,7 @@ void CCoinJoinClientSession::ProcessPoolStateUpdate(CCoinJoinStatusUpdate psssup
             WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- rejected by Masternode: %s\n", __func__, strMessageTmp.translated);
             SetState(POOL_STATE_ERROR);
             UnlockCoins();
-            keyHolderStorage.ReturnAll();
+            WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
             nTimeLastSuccessfulStep = GetTime();
             strLastMessage = strMessageTmp;
             break;
@@ -688,7 +688,7 @@ void CCoinJoinClientSession::CompletedTransaction(PoolMessage nMessageID)
         keyHolderStorage.KeepAll();
         WalletCJLogPrint(m_wallet, "CompletedTransaction -- success\n");
     } else {
-        keyHolderStorage.ReturnAll();
+        WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
         WalletCJLogPrint(m_wallet, "CompletedTransaction -- error\n");
     }
     UnlockCoins();