fix: A set of qdata/qwatch related fixes (dashpay#5745)

## Issue being fixed or feature implemented
Fix/tidy up a few `qdata`/`qwatch` related parts, improve performance
for regular non-watching nodes

~based on dashpay#5744 atm~

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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 _(for repository
code-owners and collaborators only)_

Author: UdjinM6

src/llmq/dkgsessionmgr.cpp

@@ -189,10 +189,21 @@ void CDKGSessionManager::ProcessMessage(CNode& pfrom, const CQuorumManager& quor
     }
 
     if (msg_type == NetMsgType::QWATCH) {
+        if (!fMasternodeMode) {
+            // non-masternodes should never receive this
+            m_peerman->Misbehaving(pfrom.GetId(), 10);
+            return;
+        }
         pfrom.qwatch = true;
         return;
     }
 
+    if ((!fMasternodeMode && !utils::IsWatchQuorumsEnabled())) {
+        // regular non-watching nodes should never receive any of these
+        m_peerman->Misbehaving(pfrom.GetId(), 10);
+        return;
+    }
+
     if (vRecv.empty()) {
         m_peerman->Misbehaving(pfrom.GetId(), 100);
         return;
@@ -429,13 +440,15 @@ bool CDKGSessionManager::GetEncryptedContributions(Consensus::LLMQType llmqType,
         }
     }
     if (nRequestedMemberIdx == std::numeric_limits<size_t>::max()) {
+        LogPrint(BCLog::LLMQ, "CDKGSessionManager::%s -- not a member, nProTxHash=%s\n", __func__, nProTxHash.ToString());
         return false;
     }
 
     for (const auto i : irange::range(members.size())) {
         if (validMembers[i]) {
             CBLSIESMultiRecipientObjects<CBLSSecretKey> encryptedContributions;
             if (!db->Read(std::make_tuple(DB_ENC_CONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), members[i]->proTxHash), encryptedContributions)) {
+                LogPrint(BCLog::LLMQ, "CDKGSessionManager::%s -- can't read from db, nProTxHash=%s\n", __func__, nProTxHash.ToString());
                 return false;
             }
             vecRet.emplace_back(encryptedContributions.Get(nRequestedMemberIdx));

src/llmq/quorums.cpp

@@ -221,7 +221,7 @@ void CQuorumManager::Stop()
 
 void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex) const
 {
-    if (!fMasternodeMode || !utils::QuorumDataRecoveryEnabled() || pIndex == nullptr) {
+    if ((!fMasternodeMode && !utils::IsWatchQuorumsEnabled()) || !utils::QuorumDataRecoveryEnabled() || pIndex == nullptr) {
         return;
     }
 
@@ -272,15 +272,13 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)
 
 void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload) const
 {
-    if (!m_mn_sync->IsBlockchainSynced()) {
-        return;
-    }
+    if (!m_mn_sync->IsBlockchainSynced()) return;
 
     for (const auto& params : Params().GetConsensus().llmqs) {
         CheckQuorumConnections(params, pindexNew);
     }
 
-    {
+    if (fMasternodeMode || utils::IsWatchQuorumsEnabled()) {
         // Cleanup expired data requests
         LOCK(cs_data_requests);
         auto it = mapQuorumDataRequests.begin();
@@ -299,6 +297,8 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitial
 
 void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pindexNew) const
 {
+    if (!fMasternodeMode && !utils::IsWatchQuorumsEnabled()) return;
+
     auto lastQuorums = ScanQuorums(llmqParams.type, pindexNew, (size_t)llmqParams.keepOldConnections);
 
     auto connmanQuorumsToDelete = connman.GetMasternodeQuorums(llmqParams.type);
@@ -450,8 +450,8 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp
         LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Version must be %d or greater.\n", __func__, LLMQ_DATA_MESSAGES_VERSION);
         return false;
     }
-    if (pfrom->GetVerifiedProRegTxHash().IsNull() && !pfrom->qwatch) {
-        LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pfrom is neither a verified masternode nor a qwatch connection\n", __func__);
+    if (pfrom->GetVerifiedProRegTxHash().IsNull()) {
+        LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pfrom is not a verified masternode\n", __func__);
         return false;
     }
     if (!GetLLMQParams(llmqType).has_value()) {
@@ -470,8 +470,8 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp
     LOCK(cs_data_requests);
     const CQuorumDataRequestKey key(pfrom->GetVerifiedProRegTxHash(), true, pQuorumBaseBlockIndex->GetBlockHash(), llmqType);
     const CQuorumDataRequest request(llmqType, pQuorumBaseBlockIndex->GetBlockHash(), nDataMask, proTxHash);
-    auto [old_pair, exists] = mapQuorumDataRequests.emplace(key, request);
-    if (!exists) {
+    auto [old_pair, inserted] = mapQuorumDataRequests.emplace(key, request);
+    if (!inserted) {
         if (old_pair->second.IsExpired(/*add_bias=*/true)) {
             old_pair->second = request;
         } else {
@@ -752,8 +752,8 @@ void CQuorumManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, C
     }
 
     if (msg_type == NetMsgType::QDATA) {
-        if ((!fMasternodeMode && !utils::IsWatchQuorumsEnabled()) || (pfrom.GetVerifiedProRegTxHash().IsNull() && !pfrom.qwatch)) {
-            errorHandler("Not a verified masternode or a qwatch connection");
+        if ((!fMasternodeMode && !utils::IsWatchQuorumsEnabled()) || pfrom.GetVerifiedProRegTxHash().IsNull()) {
+            errorHandler("Not a verified masternode and -watchquorums is not enabled");
             return;
         }
 
@@ -1051,7 +1051,7 @@ void CQuorumManager::StartCleanupOldQuorumDataThread(const CBlockIndex* pIndex)
     // window and it's better to have more room so we pick next cycle.
     // dkgMiningWindowStart for small quorums is 10 i.e. a safe block to start
     // these calculations is at height 576 + 24 * 2 + 10 = 576 + 58.
-    if (!fMasternodeMode || pIndex == nullptr || (pIndex->nHeight % 576 != 58)) {
+    if ((!fMasternodeMode && !utils::IsWatchQuorumsEnabled()) || pIndex == nullptr || (pIndex->nHeight % 576 != 58)) {
         return;
     }
 

test/functional/p2p_quorum_data.py

@@ -154,6 +154,9 @@ def test_basics():
                           "and does bump our score")
             p2p_node0.test_qgetdata(qgetdata_all, response_expected=False)
             wait_for_banscore(node0, id_p2p_node0, 10)
+            self.log.info("Check that normal node bumps our score for qwatch")
+            p2p_node0.send_message(msg_qwatch())
+            wait_for_banscore(node0, id_p2p_node0, 20)
             # The masternode should not respond to qgetdata for non-masternode connections
             self.log.info("Check that masternode doesn't respond to "
                           "non-masternode connection and does bump our score")
@@ -383,6 +386,10 @@ def test_watchquorums():
                 p2p_node0.wait_for_qmessage("qgetdata")
                 p2p_node0.send_message(p2p_mn2.get_qdata())
                 wait_for_banscore(node0, id_p2p_node0, (1 - len(extra_args)) * 10)
+                # Non-masternodes should bump peer's score for qwatch no matter
+                # whether they (non-masternodes) are watching or not.
+                p2p_node0.send_message(msg_qwatch())
+                wait_for_banscore(node0, id_p2p_node0, (1 - len(extra_args)) * 10 + 10)
                 node0.disconnect_p2ps()
                 mn2.node.disconnect_p2ps()