From 1acd4742c437908300229460d13191f55c0457eb Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 9 Oct 2019 19:48:32 +0300 Subject: [PATCH] Various fixes for mixing queues (#3138) * Always check for expired queues on masternodes * Check if a queue is too old or too far into the future Instead of only checking that it's to old * Check that no masternode can spam us with dsqs regardless of dsq readiness --- src/privatesend-client.cpp | 24 ++++++++++------------ src/privatesend-server.cpp | 41 ++++++++++++++++++++------------------ src/privatesend.cpp | 11 +++++++--- src/privatesend.h | 4 ++-- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index b4550a688..94f6151be 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -55,12 +55,17 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string& // LogPrint("privatesend", "DSQUEUE -- %s seen\n", dsq.ToString()); return; } + if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) { + // no way the same mn can send another dsq with the same readiness this soon + LogPrint("privatesend", "DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n", pfrom->GetLogString(), dsq.masternodeOutpoint.ToStringShort()); + return; + } } } // cs_vecqueue LogPrint("privatesend", "DSQUEUE -- %s new\n", dsq.ToString()); - if (dsq.IsExpired()) return; + if (dsq.IsTimeOutOfBounds()) return; auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint); @@ -84,18 +89,6 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string& } } } else { - LOCK(cs_deqsessions); // have to lock this first to avoid deadlocks with cs_vecqueue - TRY_LOCK(cs_vecqueue, lockRecv); - if (!lockRecv) return; - - for (const auto& q : vecPrivateSendQueue) { - if (q.masternodeOutpoint == dsq.masternodeOutpoint) { - // no way same mn can send another "not yet ready" dsq this soon - LogPrint("privatesend", "DSQUEUE -- Masternode %s is sending WAY too many dsq messages\n", dmn->pdmnState->ToString()); - return; - } - } - int64_t nLastDsq = mmetaman.GetMetaInfo(dmn->proTxHash)->GetLastDsq(); int nThreshold = nLastDsq + mnList.GetValidMNsCount() / 5; LogPrint("privatesend", "DSQUEUE -- nLastDsq: %d threshold: %d nDsqCount: %d\n", nLastDsq, nThreshold, mmetaman.GetDsqCount()); @@ -108,12 +101,17 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string& mmetaman.AllowMixing(dmn->proTxHash); LogPrint("privatesend", "DSQUEUE -- new PrivateSend queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); + + LOCK(cs_deqsessions); for (auto& session : deqSessions) { CDeterministicMNCPtr mnMixing; if (session.GetMixingMasternodeInfo(mnMixing) && mnMixing->collateralOutpoint == dsq.masternodeOutpoint) { dsq.fTried = true; } } + + TRY_LOCK(cs_vecqueue, lockRecv); + if (!lockRecv) return; vecPrivateSendQueue.push_back(dsq); dsq.Relay(connman); } diff --git a/src/privatesend-server.cpp b/src/privatesend-server.cpp index 7e79c4d9b..a86d2891c 100644 --- a/src/privatesend-server.cpp +++ b/src/privatesend-server.cpp @@ -93,9 +93,6 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm } } else if (strCommand == NetMsgType::DSQUEUE) { - TRY_LOCK(cs_vecqueue, lockRecv); - if (!lockRecv) return; - if (pfrom->nVersion < MIN_PRIVATESEND_PEER_PROTO_VERSION) { LogPrint("privatesend", "DSQUEUE -- peer=%d using obsolete version %i\n", pfrom->id, pfrom->nVersion); connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", MIN_PRIVATESEND_PEER_PROTO_VERSION))); @@ -105,17 +102,27 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm CPrivateSendQueue dsq; vRecv >> dsq; - // process every dsq only once - for (const auto& q : vecPrivateSendQueue) { - if (q == dsq) { - // LogPrint("privatesend", "DSQUEUE -- %s seen\n", dsq.ToString()); - return; + { + TRY_LOCK(cs_vecqueue, lockRecv); + if (!lockRecv) return; + + // process every dsq only once + for (const auto& q : vecPrivateSendQueue) { + if (q == dsq) { + // LogPrint("privatesend", "DSQUEUE -- %s seen\n", dsq.ToString()); + return; + } + if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) { + // no way the same mn can send another dsq with the same readiness this soon + LogPrint("privatesend", "DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n", pfrom->GetLogString(), dsq.masternodeOutpoint.ToStringShort()); + return; + } } - } + } // cs_vecqueue LogPrint("privatesend", "DSQUEUE -- %s new\n", dsq.ToString()); - if (dsq.IsExpired()) return; + if (dsq.IsTimeOutOfBounds()) return; auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint); @@ -128,14 +135,6 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm } if (!dsq.fReady) { - for (const auto& q : vecPrivateSendQueue) { - if (q.masternodeOutpoint == dsq.masternodeOutpoint) { - // no way same mn can send another "not yet ready" dsq this soon - LogPrint("privatesend", "DSQUEUE -- Masternode %s is sending WAY too many dsq messages\n", dmn->pdmnState->addr.ToString()); - return; - } - } - int64_t nLastDsq = mmetaman.GetMetaInfo(dmn->proTxHash)->GetLastDsq(); int nThreshold = nLastDsq + mnList.GetValidMNsCount() / 5; LogPrint("privatesend", "DSQUEUE -- nLastDsq: %d threshold: %d nDsqCount: %d\n", nLastDsq, nThreshold, mmetaman.GetDsqCount()); @@ -147,6 +146,9 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm mmetaman.AllowMixing(dmn->proTxHash); LogPrint("privatesend", "DSQUEUE -- new PrivateSend queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); + + TRY_LOCK(cs_vecqueue, lockRecv); + if (!lockRecv) return; vecPrivateSendQueue.push_back(dsq); dsq.Relay(connman); } @@ -507,10 +509,11 @@ void CPrivateSendServer::ChargeRandomFees(CConnman& connman) void CPrivateSendServer::CheckTimeout(CConnman& connman) { if (!fMasternodeMode) return; - if (nState == POOL_STATE_IDLE) return; CheckQueue(); + if (nState == POOL_STATE_IDLE) return; + int nTimeout = (nState == POOL_STATE_SIGNING) ? PRIVATESEND_SIGNING_TIMEOUT : PRIVATESEND_QUEUE_TIMEOUT; bool fTimeout = GetTime() - nTimeLastSuccessfulStep >= nTimeout; diff --git a/src/privatesend.cpp b/src/privatesend.cpp index f96f0dfba..78840d199 100644 --- a/src/privatesend.cpp +++ b/src/privatesend.cpp @@ -80,6 +80,11 @@ bool CPrivateSendQueue::Relay(CConnman& connman) return true; } +bool CPrivateSendQueue::IsTimeOutOfBounds() const +{ + return GetAdjustedTime() - nTime > PRIVATESEND_QUEUE_TIMEOUT || nTime - GetAdjustedTime() > PRIVATESEND_QUEUE_TIMEOUT; +} + uint256 CPrivateSendBroadcastTx::GetSignatureHash() const { return SerializeHash(*this); @@ -149,8 +154,8 @@ void CPrivateSendBaseManager::CheckQueue() // check mixing queue objects for timeouts std::vector::iterator it = vecPrivateSendQueue.begin(); while (it != vecPrivateSendQueue.end()) { - if ((*it).IsExpired()) { - LogPrint("privatesend", "CPrivateSendBaseManager::%s -- Removing expired queue (%s)\n", __func__, (*it).ToString()); + if ((*it).IsTimeOutOfBounds()) { + LogPrint("privatesend", "CPrivateSendBaseManager::%s -- Removing a queue (%s)\n", __func__, (*it).ToString()); it = vecPrivateSendQueue.erase(it); } else ++it; @@ -164,7 +169,7 @@ bool CPrivateSendBaseManager::GetQueueItemAndTry(CPrivateSendQueue& dsqRet) for (auto& dsq : vecPrivateSendQueue) { // only try each queue once - if (dsq.fTried || dsq.IsExpired()) continue; + if (dsq.fTried || dsq.IsTimeOutOfBounds()) continue; dsq.fTried = true; dsqRet = dsq; return true; diff --git a/src/privatesend.h b/src/privatesend.h index 7b315e2ba..3c0b4475c 100644 --- a/src/privatesend.h +++ b/src/privatesend.h @@ -229,8 +229,8 @@ public: bool Relay(CConnman& connman); - /// Is this queue expired? - bool IsExpired() { return GetAdjustedTime() - nTime > PRIVATESEND_QUEUE_TIMEOUT; } + /// Check if a queue is too old or too far into the future + bool IsTimeOutOfBounds() const; std::string ToString() const {