From 270c8ddf401389d2795baaae2ff3f540798d4cf2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 14 Dec 2016 16:41:37 -0800 Subject: [PATCH 1/7] Dont deserialize nVersion into CNode, should fix #9212 --- src/net_processing.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f8ddb4042c09..ba9f59cc7792 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1122,7 +1122,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, CAddress addrFrom; uint64_t nNonce = 1; uint64_t nServiceInt; - vRecv >> pfrom->nVersion >> nServiceInt >> nTime >> addrMe; + int nVersion; + vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; pfrom->nServices = ServiceFlags(nServiceInt); if (!pfrom->fInbound) { @@ -1137,18 +1138,18 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return false; } - if (pfrom->nVersion < MIN_PEER_PROTO_VERSION) + if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version - LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, pfrom->nVersion); + LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, nVersion); connman.PushMessageWithVersion(pfrom, INIT_PROTO_VERSION, NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)); pfrom->fDisconnect = true; return false; } - if (pfrom->nVersion == 10300) - pfrom->nVersion = 300; + if (nVersion == 10300) + nVersion = 300; if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { @@ -1190,7 +1191,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Change version connman.PushMessageWithVersion(pfrom, INIT_PROTO_VERSION, NetMsgType::VERACK); - pfrom->SetSendVersion(min(pfrom->nVersion, PROTOCOL_VERSION)); + int nSendVersion = std::min(nVersion, PROTOCOL_VERSION); + pfrom->nVersion = nVersion; + pfrom->SetSendVersion(nSendVersion); if (!pfrom->fInbound) { From 130b786a5c84b54575fc067903c0b0c4c0070549 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 18 Jan 2017 18:15:00 -0500 Subject: [PATCH 2/7] net: deserialize the entire version message locally This avoids having some vars set if the version negotiation fails. Also copy it all into CNode at the same site. nVersion and fSuccessfullyConnected are set last, as they are the gates for the other vars. Make them atomic for that reason. --- src/net.h | 4 +-- src/net_processing.cpp | 57 +++++++++++++++++++++++++----------------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/net.h b/src/net.h index 52840c264500..b6466fc348ae 100644 --- a/src/net.h +++ b/src/net.h @@ -634,7 +634,7 @@ class CNode std::string addrName; CService addrLocal; int nNumWarningsSkipped; - int nVersion; + std::atomic nVersion; // strSubVer is whatever byte array we read from the wire. However, this field is intended // to be printed out, displayed to humans in various forms and so on. So we sanitize it and // store the sanitized version in cleanSubVer. The original should be used when dealing with @@ -646,7 +646,7 @@ class CNode bool fClient; bool fInbound; bool fNetworkNode; - bool fSuccessfullyConnected; + std::atomic_bool fSuccessfullyConnected; bool fDisconnect; // We use fRelayTxes for two purposes - // a) it allows us to not relay tx invs before receiving the peer's version message diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ba9f59cc7792..c6a0458dce58 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1122,16 +1122,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, CAddress addrFrom; uint64_t nNonce = 1; uint64_t nServiceInt; + ServiceFlags nServices; int nVersion; + int nSendVersion; + std::string strSubVer; + int nStartingHeight = -1; + bool fRelay = true; + vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; - pfrom->nServices = ServiceFlags(nServiceInt); + nSendVersion = std::min(nVersion, PROTOCOL_VERSION); + nServices = ServiceFlags(nServiceInt); if (!pfrom->fInbound) { - connman.SetServices(pfrom->addr, pfrom->nServices); + connman.SetServices(pfrom->addr, nServices); } - if (pfrom->nServicesExpected & ~pfrom->nServices) + if (pfrom->nServicesExpected & ~nServices) { - LogPrint("net", "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, pfrom->nServices, pfrom->nServicesExpected); + LogPrint("net", "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, nServices, pfrom->nServicesExpected); connman.PushMessageWithVersion(pfrom, INIT_PROTO_VERSION, NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, strprintf("Expected to offer services %08x", pfrom->nServicesExpected)); pfrom->fDisconnect = true; @@ -1153,16 +1160,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { - vRecv >> LIMITED_STRING(pfrom->strSubVer, MAX_SUBVERSION_LENGTH); - pfrom->cleanSubVer = SanitizeString(pfrom->strSubVer); + vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH); + } + if (!vRecv.empty()) { + vRecv >> nStartingHeight; } if (!vRecv.empty()) - vRecv >> pfrom->nStartingHeight; - if (!vRecv.empty()) - vRecv >> pfrom->fRelayTxes; // set to true after we get the first filter* message - else - pfrom->fRelayTxes = true; - + vRecv >> fRelay; // Disconnect if we connected to ourself if (pfrom->fInbound && !connman.CheckIncomingNonce(nNonce)) { @@ -1171,7 +1175,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return true; } - pfrom->addrLocal = addrMe; if (pfrom->fInbound && addrMe.IsRoutable()) { SeenLocal(addrMe); @@ -1181,19 +1184,29 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (pfrom->fInbound) PushNodeVersion(pfrom, connman, GetAdjustedTime()); - pfrom->fClient = !(pfrom->nServices & NODE_NETWORK); + connman.PushMessageWithVersion(pfrom, INIT_PROTO_VERSION, NetMsgType::VERACK); - // Potentially mark this peer as a preferred download peer. + pfrom->nServices = nServices; + pfrom->addrLocal = addrMe; + pfrom->strSubVer = strSubVer; + pfrom->cleanSubVer = SanitizeString(strSubVer); + pfrom->nStartingHeight = nStartingHeight; + pfrom->fClient = !(nServices & NODE_NETWORK); { - LOCK(cs_main); - UpdatePreferredDownload(pfrom, State(pfrom->GetId())); + LOCK(pfrom->cs_filter); + pfrom->fRelayTxes = fRelay; // set to true after we get the first filter* message } // Change version - connman.PushMessageWithVersion(pfrom, INIT_PROTO_VERSION, NetMsgType::VERACK); - int nSendVersion = std::min(nVersion, PROTOCOL_VERSION); - pfrom->nVersion = nVersion; pfrom->SetSendVersion(nSendVersion); + pfrom->nVersion = nVersion; + pfrom->fSuccessfullyConnected = true; + + // Potentially mark this peer as a preferred download peer. + { + LOCK(cs_main); + UpdatePreferredDownload(pfrom, State(pfrom->GetId())); + } if (!pfrom->fInbound) { @@ -1234,8 +1247,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, item.second.RelayTo(pfrom, connman); } - pfrom->fSuccessfullyConnected = true; - string remoteAddr; if (fLogIPs) remoteAddr = ", peeraddr=" + pfrom->addr.ToString(); @@ -1262,7 +1273,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, else if (strCommand == NetMsgType::VERACK) { - pfrom->SetRecvVersion(min(pfrom->nVersion, PROTOCOL_VERSION)); + pfrom->SetRecvVersion(std::min(pfrom->nVersion.load(), PROTOCOL_VERSION)); // Mark this node as currently connected, so we update its timestamp later. if (pfrom->fNetworkNode) { From da94553d7e81d7d403168ada4546fb866d5596e4 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Fri, 20 Jan 2017 20:34:57 -0500 Subject: [PATCH 3/7] net: don't run callbacks on nodes that haven't completed the version handshake Since ForEach* are can be used to send messages to all nodes, the caller may end up sending a message before the version handshake is complete. To limit this, filter out these nodes. While we're at it, may as well filter out disconnected nodes as well. Delete unused methods rather than updating them. --- src/activemasternode.cpp | 2 +- src/net.cpp | 9 ++++-- src/net.h | 65 ++++++++++++++++------------------------ 3 files changed, 33 insertions(+), 43 deletions(-) diff --git a/src/activemasternode.cpp b/src/activemasternode.cpp index 1d953f26fb0c..5fd3e49b7d9d 100644 --- a/src/activemasternode.cpp +++ b/src/activemasternode.cpp @@ -158,7 +158,7 @@ void CActiveMasternode::ManageStateInitial() // If we have some peers, let's try to find our local address from one of them g_connman->ForEachNodeContinueIf([&fFoundLocal, &empty, this](CNode* pnode) { empty = false; - if (pnode->fSuccessfullyConnected && pnode->addr.IsIPv4()) + if (pnode->addr.IsIPv4()) fFoundLocal = GetLocal(service, &pnode->addr) && CMasternode::IsValidNetAddr(service); return !fFoundLocal; }); diff --git a/src/net.cpp b/src/net.cpp index 14b80c8f01bc..5626b9dbcead 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2725,6 +2725,11 @@ void CNode::AskFor(const CInv& inv) mapAskFor.insert(std::make_pair(nRequestTime, inv)); } +bool CConnman::NodeFullyConnected(const CNode* pnode) +{ + return pnode && pnode->fSuccessfullyConnected && !pnode->fDisconnect; +} + std::vector CNode::CalculateKeyedNetGroup(CAddress& address) { if(vchSecretKey.size() == 0) { @@ -2802,7 +2807,7 @@ bool CConnman::ForNode(const CService& addr, std::function f break; } } - return found != nullptr && func(found); + return found != nullptr && NodeFullyConnected(found) && func(found); } bool CConnman::ForNode(NodeId id, std::function func) @@ -2815,7 +2820,7 @@ bool CConnman::ForNode(NodeId id, std::function func) break; } } - return found != nullptr && func(found); + return found != nullptr && NodeFullyConnected(found) && func(found); } int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) { diff --git a/src/net.h b/src/net.h index b6466fc348ae..17771ea58b99 100644 --- a/src/net.h +++ b/src/net.h @@ -178,8 +178,9 @@ class CConnman { LOCK(cs_vNodes); for (auto&& node : vNodes) - if(!func(node)) - return false; + if (NodeFullyConnected(node)) + if(!func(node)) + return false; return true; }; @@ -188,61 +189,40 @@ class CConnman { LOCK(cs_vNodes); for (const auto& node : vNodes) - if(!func(node)) - return false; + if (NodeFullyConnected(node)) + if(!func(node)) + return false; return true; }; - template - bool ForEachNodeContinueIfThen(Callable&& pre, CallableAfter&& post) - { - bool ret = true; - LOCK(cs_vNodes); - for (auto&& node : vNodes) - if(!pre(node)) { - ret = false; - break; - } - post(); - return ret; - }; - - template - bool ForEachNodeContinueIfThen(Callable&& pre, CallableAfter&& post) const - { - bool ret = true; - LOCK(cs_vNodes); - for (const auto& node : vNodes) - if(!pre(node)) { - ret = false; - break; - } - post(); - return ret; - }; - template void ForEachNode(Callable&& func) { LOCK(cs_vNodes); - for (auto&& node : vNodes) - func(node); + for (auto&& node : vNodes) { + if (NodeFullyConnected(node)) + func(node); + } }; template void ForEachNode(Callable&& func) const { LOCK(cs_vNodes); - for (const auto& node : vNodes) - func(node); + for (auto&& node : vNodes) { + if (NodeFullyConnected(node)) + func(node); + } }; template void ForEachNodeThen(Callable&& pre, CallableAfter&& post) { LOCK(cs_vNodes); - for (auto&& node : vNodes) - pre(node); + for (auto&& node : vNodes) { + if (NodeFullyConnected(node)) + pre(node); + } post(); }; @@ -250,8 +230,10 @@ class CConnman void ForEachNodeThen(Callable&& pre, CallableAfter&& post) const { LOCK(cs_vNodes); - for (const auto& node : vNodes) - pre(node); + for (auto&& node : vNodes) { + if (NodeFullyConnected(node)) + pre(node); + } post(); }; @@ -391,6 +373,9 @@ class CConnman void RecordBytesRecv(uint64_t bytes); void RecordBytesSent(uint64_t bytes); + // Whether the node should be passed out in ForEach* callbacks + static bool NodeFullyConnected(const CNode* pnode); + // Network usage totals CCriticalSection cs_totalBytesRecv; CCriticalSection cs_totalBytesSent; From 7ae3cc5b822ce557f61764e2b096a65bc6b8001c Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 26 Jan 2017 12:35:49 -0500 Subject: [PATCH 4/7] net: Disallow sending messages until the version handshake is complete This is a change in behavior, though it's much more sane now than before. --- src/net_processing.cpp | 6 +++--- src/test/DoS_tests.cpp | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c6a0458dce58..edae321033ea 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1200,7 +1200,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Change version pfrom->SetSendVersion(nSendVersion); pfrom->nVersion = nVersion; - pfrom->fSuccessfullyConnected = true; // Potentially mark this peer as a preferred download peer. { @@ -1288,6 +1287,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // nodes) connman.PushMessage(pfrom, NetMsgType::SENDHEADERS); } + pfrom->fSuccessfullyConnected = true; } @@ -2297,8 +2297,8 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg { const Consensus::Params& consensusParams = Params().GetConsensus(); { - // Don't send anything until we get its version message - if (pto->nVersion == 0) + // Don't send anything until the version handshake is complete + if (!pto->fSuccessfullyConnected || pto->fDisconnect) return true; // diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index deb7733bdcb1..180ad667a7ef 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -54,6 +54,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) dummyNode1.SetSendVersion(PROTOCOL_VERSION); GetNodeSignals().InitializeNode(&dummyNode1, *connman); dummyNode1.nVersion = 1; + dummyNode1.fSuccessfullyConnected = true; Misbehaving(dummyNode1.GetId(), 100); // Should get banned SendMessages(&dummyNode1, *connman, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); @@ -64,6 +65,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) dummyNode2.SetSendVersion(PROTOCOL_VERSION); GetNodeSignals().InitializeNode(&dummyNode2, *connman); dummyNode2.nVersion = 1; + dummyNode2.fSuccessfullyConnected = true; Misbehaving(dummyNode2.GetId(), 50); SendMessages(&dummyNode2, *connman, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... @@ -84,6 +86,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) dummyNode1.SetSendVersion(PROTOCOL_VERSION); GetNodeSignals().InitializeNode(&dummyNode1, *connman); dummyNode1.nVersion = 1; + dummyNode1.fSuccessfullyConnected = true; Misbehaving(dummyNode1.GetId(), 100); SendMessages(&dummyNode1, *connman, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); @@ -109,6 +112,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.SetSendVersion(PROTOCOL_VERSION); GetNodeSignals().InitializeNode(&dummyNode, *connman); dummyNode.nVersion = 1; + dummyNode.fSuccessfullyConnected = true; Misbehaving(dummyNode.GetId(), 100); SendMessages(&dummyNode, *connman, interruptDummy); From 8ca9a529f01a99e1727409f5ebf59d69f927c8ef Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 2 Feb 2017 14:33:41 -0500 Subject: [PATCH 5/7] net: log an error rather than asserting if send version is misused Also cleaned up the comments and moved from the header to the .cpp so that logging headers aren't needed from net.h --- src/net.cpp | 27 +++++++++++++++++++++++++++ src/net.h | 21 ++------------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 5626b9dbcead..7d121d742c49 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -719,6 +719,33 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete return true; } +void CNode::SetSendVersion(int nVersionIn) +{ + // Send version may only be changed in the version message, and + // only one version message is allowed per session. We can therefore + // treat this value as const and even atomic as long as it's only used + // once a version message has been successfully processed. Any attempt to + // set this twice is an error. + if (nSendVersion != 0) { + error("Send version already set for node: %i. Refusing to change from %i to %i", id, nSendVersion, nVersionIn); + } else { + nSendVersion = nVersionIn; + } +} + +int CNode::GetSendVersion() const +{ + // The send version should always be explicitly set to + // INIT_PROTO_VERSION rather than using this value until SetSendVersion + // has been called. + if (nSendVersion == 0) { + error("Requesting unset send version for node: %i. Using %i", id, INIT_PROTO_VERSION); + return INIT_PROTO_VERSION; + } + return nSendVersion; +} + + int CNetMessage::readHeader(const char *pch, unsigned int nBytes) { // copy data to temporary parsing buffer diff --git a/src/net.h b/src/net.h index 17771ea58b99..3d3568e4df60 100644 --- a/src/net.h +++ b/src/net.h @@ -748,25 +748,8 @@ class CNode BOOST_FOREACH(CNetMessage &msg, vRecvMsg) msg.SetVersion(nVersionIn); } - void SetSendVersion(int nVersionIn) - { - // Send version may only be changed in the version message, and - // only one version message is allowed per session. We can therefore - // treat this value as const and even atomic as long as it's only used - // once the handshake is complete. Any attempt to set this twice is an - // error. - assert(nSendVersion == 0); - nSendVersion = nVersionIn; - } - - int GetSendVersion() const - { - // The send version should always be explicitly set to - // INIT_PROTO_VERSION rather than using this value until the handshake - // is complete. See PushMessageWithVersion(). - assert(nSendVersion != 0); - return nSendVersion; - } + void SetSendVersion(int nVersionIn); + int GetSendVersion() const; CNode* AddRef() { From c8cfc1e1c25626f6d3196a09247eb024bc28eb84 Mon Sep 17 00:00:00 2001 From: Oleg Girko Date: Tue, 15 Aug 2017 20:52:45 +0100 Subject: [PATCH 6/7] Implement conditions for ForEachNode() and ForNode() methods of CConnman. A change making ForEachNode() and ForNode() methods ignore nodes that have not completed initial handshake have been backported from Bitcoin. Unfortunately, some Dash-specific code needs to iterate over all nodes. This change introduces additional condition argument to these methods. This argument is a functional object that should return true for nodes that should be taken into account, not ignored. Two functional objects are provided in CConnman namespace: * FullyConnectedOnly returns true for nodes that have handshake completed, * AllNodes returns true for all nodes. Overloads for ForEachNode() and ForNode() methods without condition argument are left for compatibility with non-Dash-specific code. They use FullyConnectedOnly functional object for condition. Signed-off-by: Oleg Girko --- src/net.cpp | 11 ++++--- src/net.h | 94 ++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 20 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7d121d742c49..cc3274716981 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -66,6 +66,9 @@ const static std::string NET_MESSAGE_COMMAND_OTHER = "*other*"; +constexpr const CConnman::CFullyConnectedOnly CConnman::FullyConnectedOnly; +constexpr const CConnman::CAllNodes CConnman::AllNodes; + // // Global state variables // @@ -2824,7 +2827,7 @@ void CConnman::PushMessage(CNode* pnode, CDataStream& strm, const std::string& s RecordBytesSent(nBytesSent); } -bool CConnman::ForNode(const CService& addr, std::function func) +bool CConnman::ForNode(const CService& addr, std::function cond, std::function func) { CNode* found = nullptr; LOCK(cs_vNodes); @@ -2834,10 +2837,10 @@ bool CConnman::ForNode(const CService& addr, std::function f break; } } - return found != nullptr && NodeFullyConnected(found) && func(found); + return found != nullptr && cond(found) && func(found); } -bool CConnman::ForNode(NodeId id, std::function func) +bool CConnman::ForNode(NodeId id, std::function cond, std::function func) { CNode* found = nullptr; LOCK(cs_vNodes); @@ -2847,7 +2850,7 @@ bool CConnman::ForNode(NodeId id, std::function func) break; } } - return found != nullptr && NodeFullyConnected(found) && func(found); + return found != nullptr && cond(found) && func(found); } int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) { diff --git a/src/net.h b/src/net.h index 3d3568e4df60..15a71c7b3063 100644 --- a/src/net.h +++ b/src/net.h @@ -143,8 +143,34 @@ class CConnman // because it's used in many Dash-specific places (masternode, privatesend). CNode* ConnectNode(CAddress addrConnect, const char *pszDest = NULL, bool fConnectToMasternode = false); - bool ForNode(NodeId id, std::function func); - bool ForNode(const CService& addr, std::function func); + struct CFullyConnectedOnly { + bool operator() (const CNode* pnode) const { + return NodeFullyConnected(pnode); + } + }; + + constexpr static const CFullyConnectedOnly FullyConnectedOnly{}; + + struct CAllNodes { + bool operator() (const CNode*) const {return true;} + }; + + constexpr static const CAllNodes AllNodes{}; + + bool ForNode(NodeId id, std::function cond, std::function func); + bool ForNode(const CService& addr, std::function cond, std::function func); + + template + bool ForNode(const CService& addr, Callable&& func) + { + return ForNode(addr, FullyConnectedOnly, func); + } + + template + bool ForNode(NodeId id, Callable&& func) + { + return ForNode(id, FullyConnectedOnly, func); + } template void PushMessageWithVersionAndFlag(CNode* pnode, int nVersion, int flag, const std::string& sCommand, Args&&... args) @@ -173,70 +199,106 @@ class CConnman PushMessageWithVersionAndFlag(pnode, 0, 0, sCommand, std::forward(args)...); } - template - bool ForEachNodeContinueIf(Callable&& func) + template + bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) { LOCK(cs_vNodes); for (auto&& node : vNodes) - if (NodeFullyConnected(node)) + if (cond(node)) if(!func(node)) return false; return true; }; template - bool ForEachNodeContinueIf(Callable&& func) const + bool ForEachNodeContinueIf(Callable&& func) + { + return ForEachNodeContinueIf(FullyConnectedOnly, func); + } + + template + bool ForEachNodeContinueIf(const Condition& cond, Callable&& func) const { LOCK(cs_vNodes); for (const auto& node : vNodes) - if (NodeFullyConnected(node)) + if (cond(node)) if(!func(node)) return false; return true; }; template - void ForEachNode(Callable&& func) + bool ForEachNodeContinueIf(Callable&& func) const + { + return ForEachNodeContinueIf(FullyConnectedOnly, func); + } + + template + void ForEachNode(const Condition& cond, Callable&& func) { LOCK(cs_vNodes); for (auto&& node : vNodes) { - if (NodeFullyConnected(node)) + if (cond(node)) func(node); } }; template - void ForEachNode(Callable&& func) const + void ForEachNode(Callable&& func) + { + ForEachNode(FullyConnectedOnly, func); + } + + template + void ForEachNode(const Condition& cond, Callable&& func) const { LOCK(cs_vNodes); for (auto&& node : vNodes) { - if (NodeFullyConnected(node)) + if (cond(node)) func(node); } }; - template - void ForEachNodeThen(Callable&& pre, CallableAfter&& post) + template + void ForEachNode(Callable&& func) const + { + ForEachNode(FullyConnectedOnly, func); + } + + template + void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) { LOCK(cs_vNodes); for (auto&& node : vNodes) { - if (NodeFullyConnected(node)) + if (cond(node)) pre(node); } post(); }; template - void ForEachNodeThen(Callable&& pre, CallableAfter&& post) const + void ForEachNodeThen(Callable&& pre, CallableAfter&& post) + { + ForEachNodeThen(FullyConnectedOnly, pre, post); + } + + template + void ForEachNodeThen(const Condition& cond, Callable&& pre, CallableAfter&& post) const { LOCK(cs_vNodes); for (auto&& node : vNodes) { - if (NodeFullyConnected(node)) + if (cond(node)) pre(node); } post(); }; + template + void ForEachNodeThen(Callable&& pre, CallableAfter&& post) const + { + ForEachNodeThen(FullyConnectedOnly, pre, post); + } + std::vector CopyNodeVector(); void ReleaseNodeVector(const std::vector& vecNodes); From 9d09881b73c3f7bd2b55b24b8593a9bd60ef06f3 Mon Sep 17 00:00:00 2001 From: Oleg Girko Date: Tue, 15 Aug 2017 21:07:08 +0100 Subject: [PATCH 7/7] Iterate over all nodes in Dash-specific code using AllNodes condition. Use AllNodes functional object as newly introduced condition argument for ForEachNode() and ForNode() methods of CConnman to iterate over all nodes where needed in Dash-specific code. Signed-off-by: Oleg Girko --- src/activemasternode.cpp | 2 +- src/masternode-sync.cpp | 4 ++-- src/masternodeman.cpp | 2 +- src/privatesend-client.cpp | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/activemasternode.cpp b/src/activemasternode.cpp index 5fd3e49b7d9d..b1d5bfd26095 100644 --- a/src/activemasternode.cpp +++ b/src/activemasternode.cpp @@ -156,7 +156,7 @@ void CActiveMasternode::ManageStateInitial() if(!fFoundLocal) { bool empty = true; // If we have some peers, let's try to find our local address from one of them - g_connman->ForEachNodeContinueIf([&fFoundLocal, &empty, this](CNode* pnode) { + g_connman->ForEachNodeContinueIf(CConnman::AllNodes, [&fFoundLocal, &empty, this](CNode* pnode) { empty = false; if (pnode->addr.IsIPv4()) fFoundLocal = GetLocal(service, &pnode->addr) && CMasternode::IsValidNetAddr(service); diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index 1b325e834537..d5c3322864b6 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -86,7 +86,7 @@ void CMasternodeSync::SwitchToNextAsset() // TRY_LOCK(cs_vNodes, lockRecv); // if(lockRecv) { ... } - g_connman->ForEachNode([](CNode* pnode) { + g_connman->ForEachNode(CConnman::AllNodes, [](CNode* pnode) { netfulfilledman.AddFulfilledRequest(pnode->addr, "full-sync"); }); LogPrintf("CMasternodeSync::SwitchToNextAsset -- Sync has finished\n"); @@ -132,7 +132,7 @@ void CMasternodeSync::ClearFulfilledRequests() // TRY_LOCK(cs_vNodes, lockRecv); // if(!lockRecv) return; - g_connman->ForEachNode([](CNode* pnode) { + g_connman->ForEachNode(CConnman::AllNodes, [](CNode* pnode) { netfulfilledman.RemoveFulfilledRequest(pnode->addr, "spork-sync"); netfulfilledman.RemoveFulfilledRequest(pnode->addr, "masternode-list-sync"); netfulfilledman.RemoveFulfilledRequest(pnode->addr, "masternode-payment-sync"); diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 26e03fe7af50..c4761e0f6861 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -763,7 +763,7 @@ void CMasternodeMan::ProcessMasternodeConnections() //we don't care about this for regtest if(Params().NetworkIDString() == CBaseChainParams::REGTEST) return; - g_connman->ForEachNode([](CNode* pnode) { + g_connman->ForEachNode(CConnman::AllNodes, [](CNode* pnode) { if(pnode->fMasternode) { if(privateSendClient.infoMixingMasternode.fInfoValid && pnode->addr == privateSendClient.infoMixingMasternode.addr) return true; LogPrintf("Closing Masternode connection: peer=%d, addr=%s\n", pnode->id, pnode->addr.ToString()); diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index cde47427bf02..bb9ecc2f9544 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -850,7 +850,7 @@ bool CPrivateSendClient::JoinExistingQueue(CAmount nBalanceNeedsAnonymized) CNode* pnodeFound = NULL; bool fDisconnect = false; - g_connman->ForNode(infoMn.addr, [&pnodeFound, &fDisconnect](CNode* pnode) { + g_connman->ForNode(infoMn.addr, CConnman::AllNodes, [&pnodeFound, &fDisconnect](CNode* pnode) { pnodeFound = pnode; if(pnodeFound->fDisconnect) { fDisconnect = true; @@ -925,7 +925,7 @@ bool CPrivateSendClient::StartNewQueue(CAmount nValueMin, CAmount nBalanceNeedsA CNode* pnodeFound = NULL; bool fDisconnect = false; - g_connman->ForNode(infoMn.addr, [&pnodeFound, &fDisconnect](CNode* pnode) { + g_connman->ForNode(infoMn.addr, CConnman::AllNodes, [&pnodeFound, &fDisconnect](CNode* pnode) { pnodeFound = pnode; if(pnodeFound->fDisconnect) { fDisconnect = true;