Merge #6433: fix: possible bug due to using unspecified behavior of std::move

6f75a7f601 refactor: use swap instead assign+clean in GetAndClearDirtyGovernanceObjectHashes (Konstantin Akimov)
83e0bb6e35 fix: potential bug due to unspecified state of std::vector after move (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  All standard library objects that have been moved from are placed in a "valid but unspecified state", meaning the object's class invariants hold (so functions without preconditions, such as the assignment operator, can be safely used on the object after it was moved from). See example:

  ```
  std::vector<int> v = {2, 3, 3};
  std::vector<int> u = std::move(v); // the value of v is unspecified
  v.clear(); // we are good now
  ```

  Instead, let's have swap better!
  ```
  WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns));
  ```

  ## What was done?
  Fixed `CSigSharesManager::SignPendingSigShares`, `CSigningManager::ProcessPendingReconstructedRecoveredSigs` and refactored `CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes` for unification.

  ## How Has This Been Tested?
  Run unit and functional tests.

  ## 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 6f75a7f601
  UdjinM6:
    utACK 6f75a7f601

Tree-SHA512: 5dd8664dbe9ce78329dfae24f6e8b67b7032ff7d2c066da0e01c4ed1b13bf76359a0307ee76e5b006820318693f067b505e59408614f47ee2fa8a979a1e0dc4d

src/llmq/signing.cpp

@@ -520,10 +520,8 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
 void CSigningManager::ProcessPendingReconstructedRecoveredSigs()
 {
     decltype(pendingReconstructedRecoveredSigs) m;
-    {
+    WITH_LOCK(cs_pending, swap(m, pendingReconstructedRecoveredSigs));
-        LOCK(cs_pending);
+
-        m = std::move(pendingReconstructedRecoveredSigs);
-    }
     for (const auto& p : m) {
         ProcessRecoveredSig(p.second);
     }

src/llmq/signing_shares.cpp

@@ -1490,10 +1490,7 @@ void CSigSharesManager::AsyncSign(const CQuorumCPtr& quorum, const uint256& id,
 void CSigSharesManager::SignPendingSigShares()
 {
     std::vector<PendingSignatureData> v;
-    {
+    WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns));
-        LOCK(cs_pendingSigns);
-        v = std::move(pendingSigns);
-    }
 
     for (const auto& [pQuorum, id, msgHash] : v) {
         auto opt_sigShare = CreateSigShare(pQuorum, id, msgHash);

src/masternode/meta.cpp

@@ -122,9 +122,8 @@ void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObject
 
 std::vector<uint256> CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes()
 {
-    LOCK(cs);
+    std::vector<uint256> vecTmp;
-    std::vector<uint256> vecTmp = std::move(vecDirtyGovernanceObjectHashes);
+    WITH_LOCK(cs, vecTmp.swap(vecDirtyGovernanceObjectHashes));
-    vecDirtyGovernanceObjectHashes.clear();
     return vecTmp;
 }