Skip to content

Commit 95fad52

Browse files
PastaPastaPastaPastaPastaPasta
authored
Merge pull request dashpay#5782 from knst/fix-circular-net-processing
refactor: remove circular dependencies through net_processing (1/N)
2 parents d2a8946 + a9401cc commit 95fad52

28 files changed

+2685
-245
lines changed

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ BITCOIN_CORE_H = \
324324
util/enumerate.h \
325325
util/epochguard.h \
326326
util/error.h \
327+
util/expected.h \
327328
util/fastrange.h \
328329
util/fees.h \
329330
util/golombrice.h \

‎src/coinjoin/client.cpp‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#include <evo/deterministicmns.h>
1212
#include <masternode/meta.h>
1313
#include <masternode/sync.h>
14-
#include <net_processing.h>
14+
#include <net.h>
1515
#include <netmessagemaker.h>
1616
#include <shutdown.h>
1717
#include <util/check.h>
@@ -29,33 +29,32 @@
2929
#include <memory>
3030
#include <univalue.h>
3131

32-
void CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
32+
PeerMsgRet CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv)
3333
{
34-
if (fMasternodeMode) return;
35-
if (!m_mn_sync.IsBlockchainSynced()) return;
34+
if (fMasternodeMode) return {};
35+
if (!m_mn_sync.IsBlockchainSynced()) return {};
3636

3737
if (msg_type == NetMsgType::DSQUEUE) {
38-
CCoinJoinClientQueueManager::ProcessDSQueue(peer, peerman, vRecv);
38+
return CCoinJoinClientQueueManager::ProcessDSQueue(peer, vRecv);
3939
}
40+
return {};
4041
}
4142

42-
void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, PeerManager& peerman, CDataStream& vRecv)
43+
PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream& vRecv)
4344
{
4445
CCoinJoinQueue dsq;
4546
vRecv >> dsq;
4647

4748
if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
48-
peerman.Misbehaving(peer.GetId(), 100);
49-
return;
49+
return tl::unexpected{100};
5050
}
5151

5252
if (dsq.masternodeOutpoint.IsNull()) {
5353
auto mnList = deterministicMNManager->GetListAtChainTip();
5454
if (auto dmn = mnList.GetValidMN(dsq.m_protxHash)) {
5555
dsq.masternodeOutpoint = dmn->collateralOutpoint;
5656
} else {
57-
peerman.Misbehaving(peer.GetId(), 10);
58-
return;
57+
return tl::unexpected{10};
5958
}
6059
}
6160

@@ -67,33 +66,32 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, PeerManager&
6766
// process every dsq only once
6867
for (const auto &q: vecCoinJoinQueue) {
6968
if (q == dsq) {
70-
return;
69+
return {};
7170
}
7271
if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) {
7372
// no way the same mn can send another dsq with the same readiness this soon
7473
LogPrint(BCLog::COINJOIN, /* Continued */
7574
"DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n",
7675
peer.GetLogString(), dsq.masternodeOutpoint.ToStringShort());
77-
return;
76+
return {};
7877
}
7978
}
8079
} // cs_vecqueue
8180

8281
LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString());
8382

84-
if (dsq.IsTimeOutOfBounds()) return;
83+
if (dsq.IsTimeOutOfBounds()) return {};
8584

8685
auto mnList = deterministicMNManager->GetListAtChainTip();
8786
auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint);
88-
if (!dmn) return;
87+
if (!dmn) return {};
8988

9089
if (dsq.m_protxHash.IsNull()) {
9190
dsq.m_protxHash = dmn->proTxHash;
9291
}
9392

9493
if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) {
95-
peerman.Misbehaving(peer.GetId(), 10);
96-
return;
94+
return tl::unexpected{10};
9795
}
9896

9997
// if the queue is ready, submit if we can
@@ -104,7 +102,7 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, PeerManager&
104102
})) {
105103
LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue (%s) is ready on masternode %s\n", dsq.ToString(),
106104
dmn->pdmnState->addr.ToString());
107-
return;
105+
return {};
108106
} else {
109107
int64_t nLastDsq = mmetaman->GetMetaInfo(dmn->proTxHash)->GetLastDsq();
110108
int64_t nDsqThreshold = mmetaman->GetDsqThreshold(dmn->proTxHash, mnList.GetValidMNsCount());
@@ -114,7 +112,7 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, PeerManager&
114112
if (nLastDsq != 0 && nDsqThreshold > mmetaman->GetDsqCount()) {
115113
LogPrint(BCLog::COINJOIN, "DSQUEUE -- Masternode %s is sending too many dsq messages\n",
116114
dmn->proTxHash.ToString());
117-
return;
115+
return {};
118116
}
119117

120118
mmetaman->AllowMixing(dmn->proTxHash);
@@ -129,9 +127,10 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, PeerManager&
129127
}
130128
} // cs_ProcessDSQueue
131129
dsq.Relay(connman);
130+
return {};
132131
}
133132

134-
void CCoinJoinClientManager::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
133+
void CCoinJoinClientManager::ProcessMessage(CNode& peer, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
135134
{
136135
if (fMasternodeMode) return;
137136
if (!CCoinJoinClientOptions::IsEnabled()) return;
@@ -150,7 +149,7 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, PeerManager& peerman, C
150149
AssertLockNotHeld(cs_deqsessions);
151150
LOCK(cs_deqsessions);
152151
for (auto& session : deqSessions) {
153-
session.ProcessMessage(peer, peerman, connman, mempool, msg_type, vRecv);
152+
session.ProcessMessage(peer, connman, mempool, msg_type, vRecv);
154153
}
155154
}
156155
}
@@ -164,7 +163,7 @@ CCoinJoinClientSession::CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletMa
164163
m_queueman(queueman)
165164
{}
166165

167-
void CCoinJoinClientSession::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
166+
void CCoinJoinClientSession::ProcessMessage(CNode& peer, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
168167
{
169168
if (fMasternodeMode) return;
170169
if (!CCoinJoinClientOptions::IsEnabled()) return;

‎src/coinjoin/client.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
#include <coinjoin/util.h>
99
#include <coinjoin/coinjoin.h>
10+
11+
#include <net_types.h>
1012
#include <util/translation.h>
1113

1214
#include <atomic>
@@ -23,7 +25,6 @@ class CoinJoinWalletManager;
2325
class CNode;
2426
class CMasternodeSync;
2527
class CTxMemPool;
26-
class PeerManager;
2728

2829
class UniValue;
2930

@@ -159,7 +160,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
159160
explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, const CMasternodeSync& mn_sync,
160161
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman);
161162

162-
void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);
163+
void ProcessMessage(CNode& peer, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);
163164

164165
void UnlockCoins();
165166

@@ -196,8 +197,8 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
196197
explicit CCoinJoinClientQueueManager(CConnman& _connman, CoinJoinWalletManager& walletman, const CMasternodeSync& mn_sync) :
197198
connman(_connman), m_walletman(walletman), m_mn_sync(mn_sync) {};
198199

199-
void ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
200-
void ProcessDSQueue(const CNode& peer, PeerManager& peerman, CDataStream& vRecv);
200+
PeerMsgRet ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
201+
PeerMsgRet ProcessDSQueue(const CNode& peer, CDataStream& vRecv);
201202
void DoMaintenance();
202203
};
203204

@@ -245,7 +246,7 @@ class CCoinJoinClientManager
245246
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
246247
m_wallet(wallet), m_walletman(walletman), m_mn_sync(mn_sync), m_queueman(queueman) {}
247248

248-
void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_deqsessions);
249+
void ProcessMessage(CNode& peer, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_deqsessions);
249250

250251
bool StartMixing();
251252
void StopMixing();

‎src/coinjoin/server.cpp‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@
1010
#include <masternode/meta.h>
1111
#include <masternode/node.h>
1212
#include <masternode/sync.h>
13-
#include <net_processing.h>
13+
#include <net.h>
1414
#include <netmessagemaker.h>
1515
#include <script/interpreter.h>
1616
#include <shutdown.h>
17+
#include <streams.h>
1718
#include <txmempool.h>
1819
#include <util/moneystr.h>
1920
#include <util/ranges.h>
@@ -25,20 +26,21 @@
2526

2627
constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10};
2728

28-
void CCoinJoinServer::ProcessMessage(CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
29+
PeerMsgRet CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv)
2930
{
30-
if (!fMasternodeMode) return;
31-
if (!m_mn_sync.IsBlockchainSynced()) return;
31+
if (!fMasternodeMode) return {};
32+
if (!m_mn_sync.IsBlockchainSynced()) return {};
3233

3334
if (msg_type == NetMsgType::DSACCEPT) {
3435
ProcessDSACCEPT(peer, vRecv);
3536
} else if (msg_type == NetMsgType::DSQUEUE) {
36-
ProcessDSQUEUE(peer, peerman, vRecv);
37+
return ProcessDSQUEUE(peer, vRecv);
3738
} else if (msg_type == NetMsgType::DSVIN) {
3839
ProcessDSVIN(peer, vRecv);
3940
} else if (msg_type == NetMsgType::DSSIGNFINALTX) {
4041
ProcessDSSIGNFINALTX(vRecv);
4142
}
43+
return {};
4244
}
4345

4446
void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
@@ -106,58 +108,55 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
106108
}
107109
}
108110

109-
void CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, PeerManager& peerman, CDataStream& vRecv)
111+
PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv)
110112
{
111113
CCoinJoinQueue dsq;
112114
vRecv >> dsq;
113115

114116
if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
115-
peerman.Misbehaving(peer.GetId(), 100);
116-
return;
117+
return tl::unexpected{100};
117118
}
118119

119120
if (dsq.masternodeOutpoint.IsNull()) {
120121
auto mnList = deterministicMNManager->GetListAtChainTip();
121122
if (auto dmn = mnList.GetValidMN(dsq.m_protxHash)) {
122123
dsq.masternodeOutpoint = dmn->collateralOutpoint;
123124
} else {
124-
peerman.Misbehaving(peer.GetId(), 10);
125-
return;
125+
return tl::unexpected{10};
126126
}
127127
}
128128

129129
{
130130
TRY_LOCK(cs_vecqueue, lockRecv);
131-
if (!lockRecv) return;
131+
if (!lockRecv) return {};
132132

133133
// process every dsq only once
134134
for (const auto& q : vecCoinJoinQueue) {
135135
if (q == dsq) {
136-
return;
136+
return {};
137137
}
138138
if (q.fReady == dsq.fReady && q.masternodeOutpoint == dsq.masternodeOutpoint) {
139139
// no way the same mn can send another dsq with the same readiness this soon
140140
LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %s is sending WAY too many dsq messages for a masternode with collateral %s\n", peer.GetLogString(), dsq.masternodeOutpoint.ToStringShort());
141-
return;
141+
return {};
142142
}
143143
}
144144
} // cs_vecqueue
145145

146146
LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString());
147147

148-
if (dsq.IsTimeOutOfBounds()) return;
148+
if (dsq.IsTimeOutOfBounds()) return {};
149149

150150
auto mnList = deterministicMNManager->GetListAtChainTip();
151151
auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint);
152-
if (!dmn) return;
152+
if (!dmn) return {};
153153

154154
if (dsq.m_protxHash.IsNull()) {
155155
dsq.m_protxHash = dmn->proTxHash;
156156
}
157157

158158
if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) {
159-
peerman.Misbehaving(peer.GetId(), 10);
160-
return;
159+
return tl::unexpected{10};
161160
}
162161

163162
if (!dsq.fReady) {
@@ -167,17 +166,18 @@ void CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, PeerManager& peerman, CD
167166
//don't allow a few nodes to dominate the queuing process
168167
if (nLastDsq != 0 && nDsqThreshold > mmetaman->GetDsqCount()) {
169168
LogPrint(BCLog::COINJOIN, "DSQUEUE -- Masternode %s is sending too many dsq messages\n", dmn->pdmnState->addr.ToString());
170-
return;
169+
return {};
171170
}
172171
mmetaman->AllowMixing(dmn->proTxHash);
173172

174173
LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString());
175174

176175
TRY_LOCK(cs_vecqueue, lockRecv);
177-
if (!lockRecv) return;
176+
if (!lockRecv) return {};
178177
vecCoinJoinQueue.push_back(dsq);
179178
dsq.Relay(connman);
180179
}
180+
return {};
181181
}
182182

183183
void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv)

‎src/coinjoin/server.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
#define BITCOIN_COINJOIN_SERVER_H
77

88
#include <coinjoin/coinjoin.h>
9-
#include <net.h>
9+
10+
#include <net_types.h>
1011

1112
class CChainState;
1213
class CCoinJoinServer;
14+
class CDataStream;
15+
class CNode;
1316
class CTxMemPool;
14-
class PeerManager;
1517

1618
class UniValue;
1719

@@ -71,7 +73,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
7173
void RelayCompletedTransaction(PoolMessage nMessageID) LOCKS_EXCLUDED(cs_coinjoin);
7274

7375
void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
74-
void ProcessDSQUEUE(const CNode& peer, PeerManager& peerman, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
76+
PeerMsgRet ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
7577
void ProcessDSVIN(CNode& peer, CDataStream& vRecv) LOCKS_EXCLUDED(cs_coinjoin);
7678
void ProcessDSSIGNFINALTX(CDataStream& vRecv) LOCKS_EXCLUDED(cs_coinjoin);
7779

@@ -87,7 +89,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
8789
fUnitTest(false)
8890
{}
8991

90-
void ProcessMessage(CNode& pfrom, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv);
92+
PeerMsgRet ProcessMessage(CNode& pfrom, std::string_view msg_type, CDataStream& vRecv);
9193

9294
bool HasTimedOut() const;
9395
void CheckTimeout();

0 commit comments

Comments
 (0)