Merge pull request #4909 from UdjinM6/bp_rc8

backport: v18 rc8

configure.ac

@@ -2,7 +2,7 @@ AC_PREREQ([2.69])
 define(_CLIENT_VERSION_MAJOR, 18)
 define(_CLIENT_VERSION_MINOR, 0)
 define(_CLIENT_VERSION_BUILD, 0)
-define(_CLIENT_VERSION_RC, 7)
+define(_CLIENT_VERSION_RC, 8)
 define(_CLIENT_VERSION_IS_RELEASE, false)
 define(_COPYRIGHT_YEAR, 2022)
 define(_COPYRIGHT_HOLDERS,[The %s developers])

src/llmq/blockprocessor.cpp

@@ -152,18 +152,18 @@ bool CQuorumBlockProcessor::ProcessBlock(const CBlock& block, const CBlockIndex*
             break;
         }
 
-        bool isCommitmentRequired = IsCommitmentRequired(params, pindex->nHeight);
+        const size_t numCommitmentsRequired = GetNumCommitmentsRequired(params, pindex->nHeight);
         const auto numCommitmentsInNewBlock = qcs.count(params.type);
 
-        if (!isCommitmentRequired && numCommitmentsInNewBlock > 0) {
+        if (numCommitmentsRequired < numCommitmentsInNewBlock) {
             return state.DoS(100, false, REJECT_INVALID, "bad-qc-not-allowed");
         }
 
-        if (isCommitmentRequired && numCommitmentsInNewBlock == 0) {
+        if (numCommitmentsRequired > numCommitmentsInNewBlock) {
             return state.DoS(100, false, REJECT_INVALID, "bad-qc-missing");
         }
         if (llmq::CLLMQUtils::IsQuorumRotationEnabled(params.type, pindex)) {
-            LogPrintf("[ProcessBlock] h[%d] isCommitmentRequired[%d] numCommitmentsInNewBlock[%d]\n", pindex->nHeight, isCommitmentRequired, numCommitmentsInNewBlock);
+            LogPrintf("[ProcessBlock] h[%d] numCommitmentsRequired[%d] numCommitmentsInNewBlock[%d]\n", pindex->nHeight, numCommitmentsRequired, numCommitmentsInNewBlock);
         }
     }
 
@@ -431,26 +431,26 @@ bool CQuorumBlockProcessor::IsMiningPhase(const Consensus::LLMQParams& llmqParam
     return false;
 }
 
-bool CQuorumBlockProcessor::IsCommitmentRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const
+size_t CQuorumBlockProcessor::GetNumCommitmentsRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const
 {
     AssertLockHeld(cs_main);
 
-    if (!IsMiningPhase(llmqParams, nHeight)) return false;
+    if (!IsMiningPhase(llmqParams, nHeight)) return 0;
 
     // Note: This function can be called for new blocks
     assert(nHeight <= ::ChainActive().Height() + 1);
     const auto pindex = ::ChainActive().Height() < nHeight ? ::ChainActive().Tip() : ::ChainActive().Tip()->GetAncestor(nHeight);
 
     bool rotation_enabled = CLLMQUtils::IsQuorumRotationEnabled(llmqParams.type, pindex);
     size_t quorums_num = rotation_enabled ? llmqParams.signingActiveQuorumCount : 1;
+    size_t ret{0};
 
     for (int quorumIndex = 0; quorumIndex < quorums_num; ++quorumIndex) {
         uint256 quorumHash = GetQuorumBlockHash(llmqParams, nHeight, quorumIndex);
-        if (quorumHash.IsNull()) return false;
-        if (HasMinedCommitment(llmqParams.type, quorumHash)) return false;
+        if (!quorumHash.IsNull() && !HasMinedCommitment(llmqParams.type, quorumHash)) ++ret;
     }
 
-    return true;
+    return ret;
 }
 
 // WARNING: This method returns uint256() on the first block of the DKG interval (because the block hash is not known yet)
@@ -711,7 +711,7 @@ std::optional<std::vector<CFinalCommitment>> CQuorumBlockProcessor::GetMineableC
 
     std::vector<CFinalCommitment> ret;
 
-    if (!IsCommitmentRequired(llmqParams, nHeight /*, 0*/)) {
+    if (GetNumCommitmentsRequired(llmqParams, nHeight) == 0) {
         // no commitment required
         return std::nullopt;
     }

src/llmq/blockprocessor.h

@@ -70,7 +70,7 @@ class CQuorumBlockProcessor
     static bool GetCommitmentsFromBlock(const CBlock& block, const CBlockIndex* pindex, std::multimap<Consensus::LLMQType, CFinalCommitment>& ret, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, CValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     bool IsMiningPhase(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-    bool IsCommitmentRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    size_t GetNumCommitmentsRequired(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     static uint256 GetQuorumBlockHash(const Consensus::LLMQParams& llmqParams, int nHeight, int quorumIndex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 };
 

src/llmq/utils.cpp

@@ -757,11 +757,11 @@ void CLLMQUtils::AddQuorumProbeConnections(const Consensus::LLMQParams& llmqPara
             continue;
         }
         auto lastOutbound = mmetaman.GetMetaInfo(dmn->proTxHash)->GetLastOutboundSuccess();
-        // re-probe after 50 minutes so that the "good connection" check in the DKG doesn't fail just because we're on
-        // the brink of timeout
-        if (curTime - lastOutbound > 50 * 60) {
-            probeConnections.emplace(dmn->proTxHash);
+        if (curTime - lastOutbound < 10 * 60) {
+            // avoid re-probing nodes too often
+            continue;
         }
+        probeConnections.emplace(dmn->proTxHash);
     }
 
     if (!probeConnections.empty()) {

src/masternode/meta.cpp

@@ -8,7 +8,7 @@
 
 CMasternodeMetaMan mmetaman;
 
-const std::string CMasternodeMetaMan::SERIALIZATION_VERSION_STRING = "CMasternodeMetaMan-Version-2";
+const std::string CMasternodeMetaMan::SERIALIZATION_VERSION_STRING = "CMasternodeMetaMan-Version-3";
 
 UniValue CMasternodeMetaInfo::ToJson() const
 {
@@ -18,6 +18,7 @@ UniValue CMasternodeMetaInfo::ToJson() const
 
     ret.pushKV("lastDSQ", nLastDsq);
     ret.pushKV("mixingTxCount", nMixingTxCount);
+    ret.pushKV("outboundAttemptCount", outboundAttemptCount);
     ret.pushKV("lastOutboundAttempt", lastOutboundAttempt);
     ret.pushKV("lastOutboundAttemptElapsed", now - lastOutboundAttempt);
     ret.pushKV("lastOutboundSuccess", lastOutboundSuccess);

src/masternode/meta.h

@@ -16,6 +16,7 @@
 class CConnman;
 
 static constexpr int MASTERNODE_MAX_MIXING_TXES{5};
+static constexpr int MASTERNODE_MAX_FAILED_OUTBOUND_ATTEMPTS{5};
 
 // Holds extra (non-deterministic) information about masternodes
 // This is mostly local information, e.g. about mixing and governance
@@ -35,6 +36,7 @@ class CMasternodeMetaInfo
     // KEEP TRACK OF GOVERNANCE ITEMS EACH MASTERNODE HAS VOTE UPON FOR RECALCULATION
     std::map<uint256, int> mapGovernanceObjectsVotedOn GUARDED_BY(cs);
 
+    std::atomic<int> outboundAttemptCount{0};
     std::atomic<int64_t> lastOutboundAttempt{0};
     std::atomic<int64_t> lastOutboundSuccess{0};
 
@@ -59,6 +61,7 @@ class CMasternodeMetaInfo
                 obj.nLastDsq,
                 obj.nMixingTxCount,
                 obj.mapGovernanceObjectsVotedOn,
+                obj.outboundAttemptCount,
                 obj.lastOutboundAttempt,
                 obj.lastOutboundSuccess
                 );
@@ -78,9 +81,10 @@ class CMasternodeMetaInfo
 
     void RemoveGovernanceObject(const uint256& nGovernanceObjectHash);
 
-    void SetLastOutboundAttempt(int64_t t) { lastOutboundAttempt = t; }
+    bool OutboundFailedTooManyTimes() const { return outboundAttemptCount > MASTERNODE_MAX_FAILED_OUTBOUND_ATTEMPTS; }
+    void SetLastOutboundAttempt(int64_t t) { lastOutboundAttempt = t; ++outboundAttemptCount; }
     int64_t GetLastOutboundAttempt() const { return lastOutboundAttempt; }
-    void SetLastOutboundSuccess(int64_t t) { lastOutboundSuccess = t; }
+    void SetLastOutboundSuccess(int64_t t) { lastOutboundSuccess = t; outboundAttemptCount = 0; }
     int64_t GetLastOutboundSuccess() const { return lastOutboundSuccess; }
 };
 using CMasternodeMetaInfoPtr = std::shared_ptr<CMasternodeMetaInfo>;

src/net.cpp

@@ -1107,9 +1107,9 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
         nInbound--;
     }
 
-    // don't accept incoming connections until fully synced
-    if(fMasternodeMode && !masternodeSync.IsSynced()) {
-        LogPrint(BCLog::NET, "AcceptConnection -- masternode is not synced yet, skipping inbound connection attempt\n");
+    // don't accept incoming connections until blockchain is synced
+    if(fMasternodeMode && !masternodeSync.IsBlockchainSynced()) {
+        LogPrint(BCLog::NET, "AcceptConnection -- blockchain is not synced yet, skipping inbound connection attempt\n");
         CloseSocket(hSocket);
         return;
     }
@@ -2476,8 +2476,11 @@ void CConnman::ThreadOpenMasternodeConnections()
         });
         if (!connected) {
             LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- connection failed for masternode  %s, service=%s\n", __func__, connectToDmn->proTxHash.ToString(), connectToDmn->pdmnState->addr.ToString(false));
-            // reset last outbound success
-            mmetaman.GetMetaInfo(connectToDmn->proTxHash)->SetLastOutboundSuccess(0);
+            // Will take a few consequent failed attempts to PoSe-punish a MN.
+            if (mmetaman.GetMetaInfo(connectToDmn->proTxHash)->OutboundFailedTooManyTimes()) {
+                LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- failed to connect to masternode %s too many times, resetting outbound success time\n", __func__, connectToDmn->proTxHash.ToString());
+                mmetaman.GetMetaInfo(connectToDmn->proTxHash)->SetLastOutboundSuccess(0);
+            }
         }
     }
 }
@@ -2494,6 +2497,15 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
     if (!fNetworkActive) {
         return;
     }
+
+    auto getIpStr = [&]() {
+        if (fLogIPs) {
+            return addrConnect.ToString(false);
+        } else {
+            return std::string("new peer");
+        }
+    };
+
     if (!pszDest) {
         // banned or exact match?
         if ((m_banman && m_banman->IsBanned(addrConnect)) || FindNode(addrConnect.ToStringIPPort()))
@@ -2502,21 +2514,21 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
         bool fAllowLocal = Params().AllowMultiplePorts() && addrConnect.GetPort() != GetListenPort();
         if (!fAllowLocal && IsLocal(addrConnect))
             return;
-        // if multiple ports for same IP are allowed, search for IP:PORT match, otherwise search for IP-only match
-        if ((!Params().AllowMultiplePorts() && FindNode(static_cast<CNetAddr>(addrConnect))) ||
-            (Params().AllowMultiplePorts() && FindNode(static_cast<CService>(addrConnect))))
+        // Search for IP:PORT match:
+        //  - if multiple ports for the same IP are allowed,
+        //  - for probe connections
+        // Search for IP-only match otherwise
+        bool searchIPPort = Params().AllowMultiplePorts() || masternode_probe_connection;
+        bool skip = searchIPPort ?
+                FindNode(static_cast<CService>(addrConnect)) :
+                FindNode(static_cast<CNetAddr>(addrConnect));
+        if (skip) {
+            LogPrintf("CConnman::%s -- Failed to open new connection to %s, already connected\n", __func__, getIpStr());
             return;
+        }
     } else if (FindNode(std::string(pszDest)))
         return;
 
-    auto getIpStr = [&]() {
-        if (fLogIPs) {
-            return addrConnect.ToString(false);
-        } else {
-            return std::string("new peer");
-        }
-    };
-
     LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- connecting to %s\n", __func__, getIpStr());
     CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection);