Skip to content

Commit 152c10b

Browse files
UdjinM6UdjinM6
authored
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
1 parent e0c5624 commit 152c10b

File tree

4 files changed

+42
-36
lines changed

4 files changed

+42
-36
lines changed

‎src/privatesend/privatesend-client.cpp‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,17 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string&
5454
if (q == dsq) {
5555
return;
5656
}
57+
if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) {
58+
// no way the same mn can send another dsq with the same readiness this soon
59+
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n", pfrom->GetLogString(), dsq.masternodeOutpoint.ToStringShort());
60+
return;
61+
}
5762
}
5863
} // cs_vecqueue
5964

6065
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- %s new\n", dsq.ToString());
6166

62-
if (dsq.IsExpired()) return;
67+
if (dsq.IsTimeOutOfBounds()) return;
6368

6469
auto mnList = deterministicMNManager->GetListAtChainTip();
6570
auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint);
@@ -83,18 +88,6 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string&
8388
}
8489
}
8590
} else {
86-
LOCK(cs_deqsessions); // have to lock this first to avoid deadlocks with cs_vecqueue
87-
TRY_LOCK(cs_vecqueue, lockRecv);
88-
if (!lockRecv) return;
89-
90-
for (const auto& q : vecPrivateSendQueue) {
91-
if (q.masternodeOutpoint == dsq.masternodeOutpoint) {
92-
// no way same mn can send another "not yet ready" dsq this soon
93-
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- Masternode %s is sending WAY too many dsq messages\n", dmn->pdmnState->ToString());
94-
return;
95-
}
96-
}
97-
9891
int64_t nLastDsq = mmetaman.GetMetaInfo(dmn->proTxHash)->GetLastDsq();
9992
int nThreshold = nLastDsq + mnList.GetValidMNsCount() / 5;
10093
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- nLastDsq: %d threshold: %d nDsqCount: %d\n", nLastDsq, nThreshold, mmetaman.GetDsqCount());
@@ -107,12 +100,17 @@ void CPrivateSendClientManager::ProcessMessage(CNode* pfrom, const std::string&
107100
mmetaman.AllowMixing(dmn->proTxHash);
108101

109102
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- new PrivateSend queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString());
103+
104+
LOCK(cs_deqsessions);
110105
for (auto& session : deqSessions) {
111106
CDeterministicMNCPtr mnMixing;
112107
if (session.GetMixingMasternodeInfo(mnMixing) && mnMixing->collateralOutpoint == dsq.masternodeOutpoint) {
113108
dsq.fTried = true;
114109
}
115110
}
111+
112+
TRY_LOCK(cs_vecqueue, lockRecv);
113+
if (!lockRecv) return;
116114
vecPrivateSendQueue.push_back(dsq);
117115
dsq.Relay(connman);
118116
}

‎src/privatesend/privatesend-server.cpp‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,6 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm
9797
}
9898

9999
} else if (strCommand == NetMsgType::DSQUEUE) {
100-
TRY_LOCK(cs_vecqueue, lockRecv);
101-
if (!lockRecv) return;
102-
103100
if (pfrom->nVersion < MIN_PRIVATESEND_PEER_PROTO_VERSION) {
104101
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- peer=%d using obsolete version %i\n", pfrom->GetId(), pfrom->nVersion);
105102
connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", MIN_PRIVATESEND_PEER_PROTO_VERSION)));
@@ -109,16 +106,26 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm
109106
CPrivateSendQueue dsq;
110107
vRecv >> dsq;
111108

112-
// process every dsq only once
113-
for (const auto& q : vecPrivateSendQueue) {
114-
if (q == dsq) {
115-
return;
109+
{
110+
TRY_LOCK(cs_vecqueue, lockRecv);
111+
if (!lockRecv) return;
112+
113+
// process every dsq only once
114+
for (const auto& q : vecPrivateSendQueue) {
115+
if (q == dsq) {
116+
return;
117+
}
118+
if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) {
119+
// no way the same mn can send another dsq with the same readiness this soon
120+
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n", pfrom->GetLogString(), dsq.masternodeOutpoint.ToStringShort());
121+
return;
122+
}
116123
}
117-
}
124+
} // cs_vecqueue
118125

119126
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- %s new\n", dsq.ToString());
120127

121-
if (dsq.IsExpired()) return;
128+
if (dsq.IsTimeOutOfBounds()) return;
122129

123130
auto mnList = deterministicMNManager->GetListAtChainTip();
124131
auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint);
@@ -131,14 +138,6 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm
131138
}
132139

133140
if (!dsq.fReady) {
134-
for (const auto& q : vecPrivateSendQueue) {
135-
if (q.masternodeOutpoint == dsq.masternodeOutpoint) {
136-
// no way same mn can send another "not yet ready" dsq this soon
137-
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- Masternode %s is sending WAY too many dsq messages\n", dmn->pdmnState->addr.ToString());
138-
return;
139-
}
140-
}
141-
142141
int64_t nLastDsq = mmetaman.GetMetaInfo(dmn->proTxHash)->GetLastDsq();
143142
int nThreshold = nLastDsq + mnList.GetValidMNsCount() / 5;
144143
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- nLastDsq: %d threshold: %d nDsqCount: %d\n", nLastDsq, nThreshold, mmetaman.GetDsqCount());
@@ -150,6 +149,9 @@ void CPrivateSendServer::ProcessMessage(CNode* pfrom, const std::string& strComm
150149
mmetaman.AllowMixing(dmn->proTxHash);
151150

152151
LogPrint(BCLog::PRIVATESEND, "DSQUEUE -- new PrivateSend queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString());
152+
153+
TRY_LOCK(cs_vecqueue, lockRecv);
154+
if (!lockRecv) return;
153155
vecPrivateSendQueue.push_back(dsq);
154156
dsq.Relay(connman);
155157
}
@@ -524,10 +526,11 @@ void CPrivateSendServer::ConsumeCollateral(CConnman& connman, const CTransaction
524526
void CPrivateSendServer::CheckTimeout(CConnman& connman)
525527
{
526528
if (!fMasternodeMode) return;
527-
if (nState == POOL_STATE_IDLE) return;
528529

529530
CheckQueue();
530531

532+
if (nState == POOL_STATE_IDLE) return;
533+
531534
int nTimeout = (nState == POOL_STATE_SIGNING) ? PRIVATESEND_SIGNING_TIMEOUT : PRIVATESEND_QUEUE_TIMEOUT;
532535
bool fTimeout = GetTime() - nTimeLastSuccessfulStep >= nTimeout;
533536

‎src/privatesend/privatesend.cpp‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ bool CPrivateSendQueue::Relay(CConnman& connman)
8282
return true;
8383
}
8484

85+
bool CPrivateSendQueue::IsTimeOutOfBounds() const
86+
{
87+
return GetAdjustedTime() - nTime > PRIVATESEND_QUEUE_TIMEOUT || nTime - GetAdjustedTime() > PRIVATESEND_QUEUE_TIMEOUT;
88+
}
89+
8590
uint256 CPrivateSendBroadcastTx::GetSignatureHash() const
8691
{
8792
return SerializeHash(*this);
@@ -174,8 +179,8 @@ void CPrivateSendBaseManager::CheckQueue()
174179
// check mixing queue objects for timeouts
175180
auto it = vecPrivateSendQueue.begin();
176181
while (it != vecPrivateSendQueue.end()) {
177-
if ((*it).IsExpired()) {
178-
LogPrint(BCLog::PRIVATESEND, "CPrivateSendBaseManager::%s -- Removing expired queue (%s)\n", __func__, (*it).ToString());
182+
if ((*it).IsTimeOutOfBounds()) {
183+
LogPrint(BCLog::PRIVATESEND, "CPrivateSendBaseManager::%s -- Removing a queue (%s)\n", __func__, (*it).ToString());
179184
it = vecPrivateSendQueue.erase(it);
180185
} else {
181186
++it;
@@ -190,7 +195,7 @@ bool CPrivateSendBaseManager::GetQueueItemAndTry(CPrivateSendQueue& dsqRet)
190195

191196
for (auto& dsq : vecPrivateSendQueue) {
192197
// only try each queue once
193-
if (dsq.fTried || dsq.IsExpired()) continue;
198+
if (dsq.fTried || dsq.IsTimeOutOfBounds()) continue;
194199
dsq.fTried = true;
195200
dsqRet = dsq;
196201
return true;

‎src/privatesend/privatesend.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ class CPrivateSendQueue
271271

272272
bool Relay(CConnman& connman);
273273

274-
/// Is this queue expired?
275-
bool IsExpired() { return GetAdjustedTime() - nTime > PRIVATESEND_QUEUE_TIMEOUT; }
274+
/// Check if a queue is too old or too far into the future
275+
bool IsTimeOutOfBounds() const;
276276

277277
std::string ToString() const
278278
{

0 commit comments

Comments
 (0)