From 67757cd4c7592970a5fface6e812b72193c2308f Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 12 Jun 2021 10:01:52 -0300 Subject: [PATCH 1/3] net: pass CConnman via pointer rather than reference There are a few too many edge-cases here to make this a scripted diff. The following commits will move a few functions into PeerLogicValidation, where the local connman instance can be used. This change prepares for that usage. Adapted from btc@28f11e9406b185dc87144f1f29af0d93eb115b4e --- src/net.cpp | 10 ++-- src/net.h | 6 +- src/net_processing.cpp | 130 ++++++++++++++++++++--------------------- src/net_processing.h | 4 +- 4 files changed, 75 insertions(+), 75 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d203c07bf382..dd869d19854e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1066,7 +1066,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, "", true); pnode->AddRef(); pnode->fWhitelisted = whitelisted; - GetNodeSignals().InitializeNode(pnode, *this); + GetNodeSignals().InitializeNode(pnode, this); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1828,7 +1828,7 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (fFeeler) pnode->fFeeler = true; - GetNodeSignals().InitializeNode(pnode, *this); + GetNodeSignals().InitializeNode(pnode, this); { LOCK(cs_vNodes); vNodes.push_back(pnode); @@ -1856,7 +1856,7 @@ void CConnman::ThreadMessageHandler() continue; // Receive messages - bool fMoreNodeWork = GetNodeSignals().ProcessMessages(pnode, *this, flagInterruptMsgProc); + bool fMoreNodeWork = GetNodeSignals().ProcessMessages(pnode, this, flagInterruptMsgProc); fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend); if (flagInterruptMsgProc) return; @@ -1864,7 +1864,7 @@ void CConnman::ThreadMessageHandler() // Send messages { LOCK(pnode->cs_sendProcessing); - GetNodeSignals().SendMessages(pnode, *this, flagInterruptMsgProc); + GetNodeSignals().SendMessages(pnode, this, flagInterruptMsgProc); } if (flagInterruptMsgProc) return; @@ -2105,7 +2105,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); pnodeLocalHost = new CNode(id, nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0, nonce); - GetNodeSignals().InitializeNode(pnodeLocalHost, *this); + GetNodeSignals().InitializeNode(pnodeLocalHost, this); } // diff --git a/src/net.h b/src/net.h index fa5f538fed70..c589ac15b18d 100644 --- a/src/net.h +++ b/src/net.h @@ -413,9 +413,9 @@ struct CombinerAll { // Signals for message handling struct CNodeSignals { - boost::signals2::signal&), CombinerAll> ProcessMessages; - boost::signals2::signal&), CombinerAll> SendMessages; - boost::signals2::signal InitializeNode; + boost::signals2::signal&), CombinerAll> ProcessMessages; + boost::signals2::signal&), CombinerAll> SendMessages; + boost::signals2::signal InitializeNode; boost::signals2::signal FinalizeNode; }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 872f15623aba..09f2f693796e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -262,7 +262,7 @@ void UpdatePreferredDownload(CNode* node, CNodeState* state) nPreferredDownload += state->fPreferredDownload; } -void PushNodeVersion(CNode* pnode, CConnman& connman, int64_t nTime) +void PushNodeVersion(CNode* pnode, CConnman* connman, int64_t nTime) { ServiceFlags nLocalNodeServices = pnode->GetLocalServices(); uint64_t nonce = pnode->GetLocalNonce(); @@ -273,7 +273,7 @@ void PushNodeVersion(CNode* pnode, CConnman& connman, int64_t nTime) CAddress addrYou = (addr.IsRoutable() && !IsProxy(addr) ? addr : CAddress(CService(), addr.nServices)); CAddress addrMe = CAddress(CService(), nLocalNodeServices); - connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, + connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, nonce, strSubVersion, nNodeStartingHeight, true)); if (fLogIPs) @@ -282,7 +282,7 @@ void PushNodeVersion(CNode* pnode, CConnman& connman, int64_t nTime) LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), nodeid); } -void InitializeNode(CNode *pnode, CConnman& connman) { +void InitializeNode(CNode *pnode, CConnman* connman) { CAddress addr = pnode->addr; std::string addrName = pnode->GetAddrName(); NodeId nodeid = pnode->GetId(); @@ -804,16 +804,16 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) return true; } -static void RelayTransaction(const CTransaction& tx, CConnman& connman) +static void RelayTransaction(const CTransaction& tx, CConnman* connman) { CInv inv(MSG_TX, tx.GetHash()); - connman.ForEachNode([&inv](CNode* pnode) + connman->ForEachNode([&inv](CNode* pnode) { pnode->PushInventory(inv); }); } -static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connman) +static void RelayAddress(const CAddress& addr, bool fReachable, CConnman* connman) { int nRelayNodes = fReachable ? 2 : 1; // limited relaying of addresses outside our network(s) @@ -822,7 +822,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma // at a time so the addrKnowns of the chosen nodes prevent repeats uint64_t hashAddr = addr.GetHash(); std::multimap mapMix; - const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60)); + const CSipHasher hasher = connman->GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60)); auto sortfunc = [&mapMix, &hasher](CNode* pnode) { if (pnode->nVersion >= CADDR_TIME_VERSION) { @@ -837,12 +837,12 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma mi->second->PushAddress(addr, insecure_rand); }; - connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); + connman->ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); } bool static PushTierTwoGetDataRequest(const CInv& inv, CNode* pfrom, - CConnman& connman, + CConnman* connman, CNetMsgMaker& msgMaker) { if (inv.type == MSG_SPORK) { @@ -850,7 +850,7 @@ bool static PushTierTwoGetDataRequest(const CInv& inv, CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(1000); ss << mapSporks[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, ss)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, ss)); return true; } } @@ -861,35 +861,35 @@ bool static PushTierTwoGetDataRequest(const CInv& inv, CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(1000); ss << masternodePayments.mapMasternodePayeeVotes[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNWINNER, ss)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::MNWINNER, ss)); return true; } } if (inv.type == MSG_BUDGET_VOTE) { if (g_budgetman.HaveSeenProposalVote(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETVOTE, g_budgetman.GetProposalVoteSerialized(inv.hash))); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETVOTE, g_budgetman.GetProposalVoteSerialized(inv.hash))); return true; } } if (inv.type == MSG_BUDGET_PROPOSAL) { if (g_budgetman.HaveProposal(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETPROPOSAL, g_budgetman.GetProposalSerialized(inv.hash))); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETPROPOSAL, g_budgetman.GetProposalSerialized(inv.hash))); return true; } } if (inv.type == MSG_BUDGET_FINALIZED_VOTE) { if (g_budgetman.HaveSeenFinalizedBudgetVote(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGETVOTE, g_budgetman.GetFinalizedBudgetVoteSerialized(inv.hash))); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGETVOTE, g_budgetman.GetFinalizedBudgetVoteSerialized(inv.hash))); return true; } } if (inv.type == MSG_BUDGET_FINALIZED) { if (g_budgetman.HaveFinalizedBudget(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGET, g_budgetman.GetFinalizedBudgetSerialized(inv.hash))); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGET, g_budgetman.GetFinalizedBudgetSerialized(inv.hash))); return true; } } @@ -900,7 +900,7 @@ bool static PushTierTwoGetDataRequest(const CInv& inv, CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(1000); ss << mnodeman.mapSeenMasternodeBroadcast[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNBROADCAST, ss)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::MNBROADCAST, ss)); return true; } } @@ -911,7 +911,7 @@ bool static PushTierTwoGetDataRequest(const CInv& inv, CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(1000); ss << mnodeman.mapSeenMasternodePing[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNPING, ss)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::MNPING, ss)); return true; } } @@ -920,7 +920,7 @@ bool static PushTierTwoGetDataRequest(const CInv& inv, return false; } -void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman, const std::atomic& interruptMsgProc) +void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman* connman, const std::atomic& interruptMsgProc) { LOCK(cs_main); CNetMsgMaker msgMaker(pfrom->GetSendVersion()); @@ -948,7 +948,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman if (!ReadBlockFromDisk(block, (*mi).second)) assert(!"cannot load block from disk"); if (inv.type == MSG_BLOCK) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); else // MSG_FILTERED_BLOCK) { bool send_ = false; @@ -961,7 +961,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman } } if (send_) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see // This avoids hurting performance by pointlessly requiring a round-trip // Note that there is currently no way for a node to request any single transactions we didnt send here - @@ -969,7 +969,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman // Thus, the protocol spec specified allows for us to provide duplicate txn here, // however we MUST always provide at least what the remote peer needs for (std::pair& pair : merkleBlock.vMatchedTxn) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *block.vtx[pair.first])); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *block.vtx[pair.first])); } // else // no response @@ -982,7 +982,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman // wait for other stuff first. std::vector vInv; vInv.emplace_back(MSG_BLOCK, chainActive.Tip()->GetBlockHash()); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); pfrom->hashContinue.SetNull(); } } @@ -1001,7 +1001,7 @@ bool static IsTierTwoInventoryTypeKnown(int type) type == MSG_MASTERNODE_PING; } -void static ProcessGetData(CNode* pfrom, CConnman& connman, const std::atomic& interruptMsgProc) +void static ProcessGetData(CNode* pfrom, CConnman* connman, const std::atomic& interruptMsgProc) { AssertLockNotHeld(cs_main); @@ -1029,7 +1029,7 @@ void static ProcessGetData(CNode* pfrom, CConnman& connman, const std::atomicPushMessage(pfrom, msgMaker.Make(NetMsgType::TX, ss)); pushed = true; } } @@ -1065,7 +1065,7 @@ void static ProcessGetData(CNode* pfrom, CConnman& connman, const std::atomicPushMessage(pfrom, msgMaker.Make(NetMsgType::NOTFOUND, vNotFound)); } } @@ -1104,7 +1104,7 @@ static void CheckBlockSpam(CValidationState& state, CNode* pfrom, const uint256& } bool fRequestedSporksIDB = false; -bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, int64_t nTimeReceived, CConnman& connman, std::atomic& interruptMsgProc) +bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, int64_t nTimeReceived, CConnman* connman, std::atomic& interruptMsgProc) { LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->id); if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0) { @@ -1115,7 +1115,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (strCommand == NetMsgType::VERSION) { // Each connection can only send one version message if (pfrom->nVersion != 0) { - connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message"))); + connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message"))); LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); return false; @@ -1136,11 +1136,11 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR nSendVersion = std::min(nVersion, PROTOCOL_VERSION); nServices = ServiceFlags(nServiceInt); if (!pfrom->fInbound) { - connman.SetServices(pfrom->addr, nServices); + connman->SetServices(pfrom->addr, nServices); } if (pfrom->nServicesExpected & ~nServices) { LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, nServices, pfrom->nServicesExpected); - connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, + connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, strprintf("Expected to offer services %08x", pfrom->nServicesExpected))); pfrom->fDisconnect = true; return false; @@ -1162,7 +1162,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } // Disconnect if we connected to ourself - if (pfrom->fInbound && !connman.CheckIncomingNonce(nNonce)) { + if (pfrom->fInbound && !connman->CheckIncomingNonce(nNonce)) { LogPrintf("connected to self at %s, disconnecting\n", pfrom->addr.ToString()); pfrom->fDisconnect = true; return true; @@ -1176,7 +1176,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (pfrom->fInbound) PushNodeVersion(pfrom, connman, GetAdjustedTime()); - connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); + connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); pfrom->nServices = nServices; pfrom->SetAddrLocal(addrMe); @@ -1223,11 +1223,11 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } // Get recent addresses - if (pfrom->fOneShot || pfrom->nVersion >= CADDR_TIME_VERSION || connman.GetAddressCount() < 1000) { - connman.PushMessage(pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); + if (pfrom->fOneShot || pfrom->nVersion >= CADDR_TIME_VERSION || connman->GetAddressCount() < 1000) { + connman->PushMessage(pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); pfrom->fGetAddr = true; } - connman.MarkAddressGood(pfrom->addr); + connman->MarkAddressGood(pfrom->addr); } std::string remoteAddr; @@ -1269,7 +1269,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (fMissingSporks || !fRequestedSporksIDB){ LogPrintf("asking peer for sporks\n"); - connman.PushMessage(pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETSPORKS)); + connman->PushMessage(pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETSPORKS)); fRequestedSporksIDB = true; } @@ -1307,7 +1307,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR vRecv >> vAddr; // Don't want addr from older versions unless seeding - if (pfrom->nVersion < CADDR_TIME_VERSION && connman.GetAddressCount() > 1000) + if (pfrom->nVersion < CADDR_TIME_VERSION && connman->GetAddressCount() > 1000) return true; if (vAddr.size() > 1000) { LOCK(cs_main); @@ -1338,7 +1338,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (fReachable) vAddrOk.push_back(addr); } - connman.AddNewAddresses(vAddrOk, pfrom->addr, 2 * 60 * 60); + connman->AddNewAddresses(vAddrOk, pfrom->addr, 2 * 60 * 60); if (vAddr.size() < 1000) pfrom->fGetAddr = false; if (pfrom->fOneShot) @@ -1392,7 +1392,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } if (!vToFetch.empty()) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETDATA, vToFetch)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETDATA, vToFetch)); } @@ -1493,7 +1493,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (--nLimit <= 0 || pindex->GetBlockHash() == hashStop) break; } - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); } @@ -1643,7 +1643,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR pfrom->id, pfrom->cleanSubVer, FormatStateMessage(state)); if (state.GetRejectCode() < REJECT_INTERNAL) // Never send AcceptToMemoryPool's internal codes over P2P - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); if (nDoS > 0) Misbehaving(pfrom->GetId(), nDoS); @@ -1703,7 +1703,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR // TODO: optimize: if pindexLast is an ancestor of chainActive.Tip or pindexBestHeader, continue // from there instead. LogPrintf("more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom->id, pfrom->nStartingHeight); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexLast), UINT256_ZERO)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexLast), UINT256_ZERO)); } } @@ -1720,11 +1720,11 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR CBlockLocator locator = WITH_LOCK(cs_main, return chainActive.GetLocator();); if (find(pfrom->vBlockRequested.begin(), pfrom->vBlockRequested.end(), hashBlock) != pfrom->vBlockRequested.end()) { // we already asked for this block, so lets work backwards and ask for the previous block - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, pblock->hashPrevBlock)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, pblock->hashPrevBlock)); pfrom->vBlockRequested.emplace_back(pblock->hashPrevBlock); } else { // ask to sync to this block - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, hashBlock)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, hashBlock)); pfrom->vBlockRequested.emplace_back(hashBlock); } } else { @@ -1744,7 +1744,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR int nDoS; if(state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); if(nDoS > 0) { TRY_LOCK(cs_main, lockMain); @@ -1766,7 +1766,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR // getaddr message mitigates the attack. else if ((strCommand == NetMsgType::GETADDR) && (pfrom->fInbound)) { pfrom->vAddrToSend.clear(); - std::vector vAddr = connman.GetAddresses(); + std::vector vAddr = connman->GetAddresses(); FastRandomContext insecure_rand; for (const CAddress& addr : vAddr) pfrom->PushAddress(addr, insecure_rand); @@ -1802,7 +1802,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR // it, if the remote node sends a ping once per second and this node takes 5 // seconds to respond to each, the 5th ping the remote sends would appear to // return very quickly. - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); } } @@ -1973,7 +1973,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } -bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interruptMsgProc) +bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interruptMsgProc) { // Message format // (4) message start @@ -2005,7 +2005,7 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interru // Just take one message msgs.splice(msgs.begin(), pfrom->vProcessMsg, pfrom->vProcessMsg.begin()); pfrom->nProcessQueueSize -= msgs.front().vRecv.size() + CMessageHeader::HEADER_SIZE; - pfrom->fPauseRecv = pfrom->nProcessQueueSize > connman.GetReceiveFloodSize(); + pfrom->fPauseRecv = pfrom->nProcessQueueSize > connman->GetReceiveFloodSize(); fMoreWork = !pfrom->vProcessMsg.empty(); } CNetMessage& msg(msgs.front()); @@ -2050,7 +2050,7 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interru if (!pfrom->vRecvGetData.empty()) fMoreWork = true; } catch (const std::ios_base::failure& e) { - connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message"))); + connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message"))); if (strstr(e.what(), "end of data")) { // Allow exceptions from under-length message on vRecv LogPrint(BCLog::NET, "ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", SanitizeString(strCommand), nMessageSize, e.what()); @@ -2089,7 +2089,7 @@ class CompareInvMempoolOrder } }; -bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsgProc) +bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interruptMsgProc) { { // Don't send anything until the version handshake is complete @@ -2120,11 +2120,11 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg pto->nPingUsecStart = GetTimeMicros(); if (pto->nVersion > BIP0031_VERSION) { pto->nPingNonceSent = nonce; - connman.PushMessage(pto, msgMaker.Make(NetMsgType::PING, nonce)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::PING, nonce)); } else { // Peer is too old to support ping command with nonce, pong will never arrive. pto->nPingNonceSent = 0; - connman.PushMessage(pto, msgMaker.Make(NetMsgType::PING)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::PING)); } } @@ -2135,7 +2135,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg CNodeState& state = *State(pto->GetId()); for (const CBlockReject& reject : state.rejects) - connman.PushMessage(pto, msgMaker.Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock)); state.rejects.clear(); if (state.fShouldBan) { @@ -2147,7 +2147,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg if (pto->addr.IsLocal()) LogPrintf("Warning: not banning local peer %s!\n", pto->addr.ToString()); else { - connman.Ban(pto->addr, BanReasonNodeMisbehaving); + connman->Ban(pto->addr, BanReasonNodeMisbehaving); } return true; } @@ -2173,14 +2173,14 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg vAddr.push_back(addr); // receiver rejects addr messages larger than 1000 if (vAddr.size() >= 1000) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr)); vAddr.clear(); } } } pto->vAddrToSend.clear(); if (!vAddr.empty()) - connman.PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr)); } // Start block sync @@ -2195,7 +2195,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg //CBlockIndex *pindexStart = pindexBestHeader->pprev ? pindexBestHeader->pprev : pindexBestHeader; //LogPrint(BCLog::NET, "initial getheaders (%d) to peer=%d (startheight:%d)\n", pindexStart->nHeight, pto->id, pto->nStartingHeight); //pto->PushMessage(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexStart), UINT256_ZERO); - connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(chainActive.Tip()), UINT256_ZERO)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(chainActive.Tip()), UINT256_ZERO)); } } @@ -2203,7 +2203,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg // Except during reindex, importing and IBD, when old wallet // transactions become unconfirmed and spams other nodes. if (!fReindex && !fImporting && !IsInitialBlockDownload()) { - GetMainSignals().Broadcast(&connman); + GetMainSignals().Broadcast(connman); } // @@ -2219,7 +2219,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg for (const uint256& hash : pto->vInventoryBlockToSend) { vInv.emplace_back(CInv(MSG_BLOCK, hash)); if (vInv.size() == MAX_INV_SZ) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } } @@ -2229,7 +2229,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg for (const CInv& tInv : pto->vInventoryTierTwoToSend) { vInv.emplace_back(tInv); if (vInv.size() == MAX_INV_SZ) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } } @@ -2267,7 +2267,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg pto->filterInventoryKnown.insert(hash); vInv.emplace_back(inv); if (vInv.size() == MAX_INV_SZ) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } } @@ -2313,7 +2313,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg vInv.emplace_back(CInv(MSG_TX, hash)); nRelayedTransactions++; if (vInv.size() == MAX_INV_SZ) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } pto->filterInventoryKnown.insert(hash); @@ -2321,7 +2321,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg } } if (!vInv.empty()) - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); // Detect whether we're stalling nNow = GetTimeMicros(); @@ -2375,7 +2375,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg LogPrint(BCLog::NET, "Requesting %s peer=%d\n", inv.ToString(), pto->id); vGetData.push_back(inv); if (vGetData.size() >= 1000) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); vGetData.clear(); } } else { @@ -2385,7 +2385,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg pto->mapAskFor.erase(pto->mapAskFor.begin()); } if (!vGetData.empty()) - connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); } return true; } diff --git a/src/net_processing.h b/src/net_processing.h index 3b20878b0c0f..de5e6ba5534f 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -62,7 +62,7 @@ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats); /** Increase a node's misbehavior score. */ void Misbehaving(NodeId nodeid, int howmuch) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Process protocol messages received from a given node */ -bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interrupt); +bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interrupt); /** * Send queued protocol messages to be sent to a give node. * @@ -71,7 +71,7 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interru * @param[in] interrupt Interrupt condition for processing threads * @return True if there is more work to be done */ -bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interrupt); +bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interrupt); #endif // BITCOIN_NET_PROCESSING_H From 50853a20f4ab0cccfe910d88d548b4c1d6343efb Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 12 Jun 2021 11:30:57 -0300 Subject: [PATCH 2/3] net: use an interface class rather than signals for message processing Drop boost signals in favor of a stateful class. This will allow the message processing loop to actually move to net_processing in a future step. Adapted from btc@8ad663c1fa88d68843e45580deced56112343183 --- src/init.cpp | 5 +-- src/net.cpp | 22 +++++----- src/net.h | 24 +++++------ src/net_processing.cpp | 93 ++++++++++++++++-------------------------- src/net_processing.h | 36 ++++++++-------- src/test/DoS_tests.cpp | 22 +++++----- src/test/test_pivx.cpp | 6 +-- src/test/test_pivx.h | 2 + 8 files changed, 93 insertions(+), 117 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 47dd50e99bf2..d963be55be9c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -243,13 +243,12 @@ void PrepareShutdown() // After the threads that potentially access these pointers have been stopped, // destruct and reset all to nullptr. - peerLogic.reset(); g_connman.reset(); + peerLogic.reset(); DumpMasternodes(); DumpBudgets(g_budgetman); DumpMasternodePayments(); - UnregisterNodeSignals(GetNodeSignals()); if (::mempool.IsLoaded() && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(::mempool); } @@ -1349,7 +1348,6 @@ bool AppInitMain() peerLogic.reset(new PeerLogicValidation(&connman)); RegisterValidationInterface(peerLogic.get()); - RegisterNodeSignals(GetNodeSignals()); // sanitize comments per BIP-0014, format user agent and check total size std::vector uacomments; @@ -1952,6 +1950,7 @@ bool AppInitMain() connOptions.nMaxFeeler = 1; connOptions.nBestHeight = chainActive.Height(); connOptions.uiInterface = &uiInterface; + connOptions.m_msgproc = peerLogic.get(); connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER); connOptions.nReceiveFloodSize = 1000*gArgs.GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER); diff --git a/src/net.cpp b/src/net.cpp index dd869d19854e..22524db9fd58 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -85,10 +85,6 @@ std::string strSubVersion; limitedmap mapAlreadyAskedFor(MAX_INV_SZ); -// Signals for message handling -static CNodeSignals g_signals; -CNodeSignals& GetNodeSignals() { return g_signals; } - void CConnman::AddOneShot(const std::string& strDest) { LOCK(cs_vOneShots); @@ -1066,7 +1062,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, "", true); pnode->AddRef(); pnode->fWhitelisted = whitelisted; - GetNodeSignals().InitializeNode(pnode, this); + m_msgproc->InitializeNode(pnode, this); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1828,7 +1824,7 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (fFeeler) pnode->fFeeler = true; - GetNodeSignals().InitializeNode(pnode, this); + m_msgproc->InitializeNode(pnode, this); { LOCK(cs_vNodes); vNodes.push_back(pnode); @@ -1856,7 +1852,7 @@ void CConnman::ThreadMessageHandler() continue; // Receive messages - bool fMoreNodeWork = GetNodeSignals().ProcessMessages(pnode, this, flagInterruptMsgProc); + bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, this, flagInterruptMsgProc); fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend); if (flagInterruptMsgProc) return; @@ -1864,8 +1860,9 @@ void CConnman::ThreadMessageHandler() // Send messages { LOCK(pnode->cs_sendProcessing); - GetNodeSignals().SendMessages(pnode, this, flagInterruptMsgProc); + m_msgproc->SendMessages(pnode, this, flagInterruptMsgProc); } + if (flagInterruptMsgProc) return; } @@ -2055,6 +2052,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c clientInterface = connOptions.uiInterface; if (clientInterface) clientInterface->InitMessage(_("Loading addresses...")); + m_msgproc = connOptions.m_msgproc; // Load addresses from peers.dat int64_t nStart = GetTimeMillis(); { @@ -2105,12 +2103,13 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); pnodeLocalHost = new CNode(id, nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0, nonce); - GetNodeSignals().InitializeNode(pnodeLocalHost, this); + m_msgproc->InitializeNode(pnodeLocalHost, this); } // // Start threads // + assert(m_msgproc); InterruptSocks5(false); interruptNet.reset(); flagInterruptMsgProc = false; @@ -2231,9 +2230,10 @@ void CConnman::DeleteNode(CNode* pnode) { assert(pnode); bool fUpdateConnectionTime = false; - GetNodeSignals().FinalizeNode(pnode->GetId(), fUpdateConnectionTime); - if(fUpdateConnectionTime) + m_msgproc->FinalizeNode(pnode->GetId(), fUpdateConnectionTime); + if (fUpdateConnectionTime) { addrman.Connected(pnode->addr); + } delete pnode; } diff --git a/src/net.h b/src/net.h index c589ac15b18d..22f4c1a72aba 100644 --- a/src/net.h +++ b/src/net.h @@ -34,8 +34,6 @@ #include #endif -#include - class CAddrMan; class CBlockIndex; class CScheduler; @@ -121,7 +119,7 @@ struct CSerializedNetMsg std::string command; }; - +class NetEventsInterface; class CConnman { public: @@ -142,6 +140,7 @@ class CConnman int nMaxFeeler = 0; int nBestHeight = 0; CClientUIInterface* uiInterface = nullptr; + NetEventsInterface* m_msgproc = nullptr; unsigned int nSendBufferMaxSize = 0; unsigned int nReceiveFloodSize = 0; }; @@ -368,6 +367,7 @@ class CConnman int nMaxFeeler{0}; std::atomic nBestHeight; CClientUIInterface* clientInterface{nullptr}; + NetEventsInterface* m_msgproc{nullptr}; /** SipHasher seeds for deterministic randomness */ const uint64_t nSeed0{0}, nSeed1{0}; @@ -410,19 +410,19 @@ struct CombinerAll { } }; -// Signals for message handling -struct CNodeSignals +/** + * Interface for message handling + */ +class NetEventsInterface { - boost::signals2::signal&), CombinerAll> ProcessMessages; - boost::signals2::signal&), CombinerAll> SendMessages; - boost::signals2::signal InitializeNode; - boost::signals2::signal FinalizeNode; +public: + virtual bool ProcessMessages(CNode* pnode, CConnman* connman, std::atomic& interrupt) = 0; + virtual bool SendMessages(CNode* pnode, CConnman* connman, std::atomic& interrupt) = 0; + virtual void InitializeNode(CNode* pnode, CConnman* connman) = 0; + virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0; }; -CNodeSignals& GetNodeSignals(); - - enum { LOCAL_NONE, // unknown LOCAL_IF, // address a local interface listens on diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 09f2f693796e..71cdb3db99ea 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -99,12 +99,6 @@ int nPreferredDownload = 0; } // anon namespace - -////////////////////////////////////////////////////////////////////////////// -// -// Registration of network node signals. -// - namespace { struct CBlockReject { @@ -282,39 +276,6 @@ void PushNodeVersion(CNode* pnode, CConnman* connman, int64_t nTime) LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), nodeid); } -void InitializeNode(CNode *pnode, CConnman* connman) { - CAddress addr = pnode->addr; - std::string addrName = pnode->GetAddrName(); - NodeId nodeid = pnode->GetId(); - { - LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName))); - } - if(!pnode->fInbound) - PushNodeVersion(pnode, connman, GetTime()); -} - -void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) -{ - fUpdateConnectionTime = false; - LOCK(cs_main); - CNodeState* state = State(nodeid); - - if (state->fSyncStarted) - nSyncStarted--; - - if (state->nMisbehavior == 0 && state->fCurrentlyConnected) { - fUpdateConnectionTime = true; - } - - for (const QueuedBlock& entry : state->vBlocksInFlight) - mapBlocksInFlight.erase(entry.hash); - EraseOrphansFor(nodeid); - nPreferredDownload -= state->fPreferredDownload; - - mapNodeState.erase(nodeid); -} - // Requires cs_main. void MarkBlockAsReceived(const uint256& hash) { @@ -466,6 +427,39 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectoraddr; + std::string addrName = pnode->GetAddrName(); + NodeId nodeid = pnode->GetId(); + { + LOCK(cs_main); + mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName))); + } + if(!pnode->fInbound) + PushNodeVersion(pnode, connman, GetTime()); +} + +void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) +{ + fUpdateConnectionTime = false; + LOCK(cs_main); + CNodeState* state = State(nodeid); + + if (state->fSyncStarted) + nSyncStarted--; + + if (state->nMisbehavior == 0 && state->fCurrentlyConnected) { + fUpdateConnectionTime = true; + } + + for (const QueuedBlock& entry : state->vBlocksInFlight) + mapBlocksInFlight.erase(entry.hash); + EraseOrphansFor(nodeid); + nPreferredDownload -= state->fPreferredDownload; + + mapNodeState.erase(nodeid); +} + bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) { LOCK(cs_main); @@ -482,23 +476,6 @@ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) return true; } -void RegisterNodeSignals(CNodeSignals& nodeSignals) -{ - nodeSignals.ProcessMessages.connect(&ProcessMessages); - nodeSignals.SendMessages.connect(&SendMessages); - nodeSignals.InitializeNode.connect(&InitializeNode); - nodeSignals.FinalizeNode.connect(&FinalizeNode); -} - -void UnregisterNodeSignals(CNodeSignals& nodeSignals) -{ - nodeSignals.ProcessMessages.disconnect(&ProcessMessages); - nodeSignals.SendMessages.disconnect(&SendMessages); - nodeSignals.InitializeNode.disconnect(&InitializeNode); - nodeSignals.FinalizeNode.disconnect(&FinalizeNode); -} - - ////////////////////////////////////////////////////////////////////////////// // // mapOrphanTransactions @@ -1973,7 +1950,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } -bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interruptMsgProc) +bool PeerLogicValidation::ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interruptMsgProc) { // Message format // (4) message start @@ -2089,7 +2066,7 @@ class CompareInvMempoolOrder } }; -bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interruptMsgProc) +bool PeerLogicValidation::SendMessages(CNode* pto, CConnman* connman, std::atomic& interruptMsgProc) { { // Don't send anything until the version handshake is complete diff --git a/src/net_processing.h b/src/net_processing.h index de5e6ba5534f..c76063458ab2 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -32,22 +32,32 @@ static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5; * Limits the impact of low-fee transaction floods. */ static const unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL; -/** Register with a network node to receive its signals */ -void RegisterNodeSignals(CNodeSignals& nodeSignals); -/** Unregister a network node */ -void UnregisterNodeSignals(CNodeSignals& nodeSignals); - -class PeerLogicValidation : public CValidationInterface { +class PeerLogicValidation : public CValidationInterface, public NetEventsInterface { private: CConnman* connman; public: - PeerLogicValidation(CConnman* connmanIn); + PeerLogicValidation(CConnman* connman); ~PeerLogicValidation() = default; void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; void BlockChecked(const CBlock& block, const CValidationState& state) override; + + + void InitializeNode(CNode* pnode, CConnman* connman) override; + void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) override; + /** Process protocol messages received from a given node */ + bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interrupt) override; + /** + * Send queued protocol messages to be sent to a give node. + * + * @param[in] pto The node which we are sending messages to. + * @param[in] connman The connection manager for that node. + * @param[in] interrupt Interrupt condition for processing threads + * @return True if there is more work to be done + */ + bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interrupt) override; }; struct CNodeStateStats { @@ -61,17 +71,5 @@ struct CNodeStateStats { bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats); /** Increase a node's misbehavior score. */ void Misbehaving(NodeId nodeid, int howmuch) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Process protocol messages received from a given node */ -bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interrupt); -/** - * Send queued protocol messages to be sent to a give node. - * - * @param[in] pto The node which we are sending messages to. - * @param[in] connman The connection manager for that node. - * @param[in] interrupt Interrupt condition for processing threads - * @return True if there is more work to be done - */ -bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interrupt); - #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 36146b6b47d8..915f51b2386d 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -60,26 +60,26 @@ BOOST_AUTO_TEST_CASE(DoS_banning) CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); - GetNodeSignals().InitializeNode(&dummyNode1, *connman); + peerLogic->InitializeNode(&dummyNode1, connman); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; misbehave(dummyNode1.GetId(), 100); // Should get banned - SendMessages(&dummyNode1, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, "", true); dummyNode2.SetSendVersion(PROTOCOL_VERSION); - GetNodeSignals().InitializeNode(&dummyNode2, *connman); + peerLogic->InitializeNode(&dummyNode2, connman); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; misbehave(dummyNode2.GetId(), 50); - SendMessages(&dummyNode2, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode2, connman, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be misbehave(dummyNode2.GetId(), 50); - SendMessages(&dummyNode2, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode2, connman, interruptDummy); BOOST_CHECK(connman->IsBanned(addr2)); } @@ -92,17 +92,17 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); - GetNodeSignals().InitializeNode(&dummyNode1, *connman); + peerLogic->InitializeNode(&dummyNode1, connman); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; misbehave(dummyNode1.GetId(), 100); - SendMessages(&dummyNode1, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); misbehave(dummyNode1.GetId(), 10); - SendMessages(&dummyNode1, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); misbehave(dummyNode1.GetId(), 1); - SendMessages(&dummyNode1, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); } @@ -118,12 +118,12 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) CAddress addr(ip(0xa0b0c001), NODE_NONE); CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, "", true); dummyNode.SetSendVersion(PROTOCOL_VERSION); - GetNodeSignals().InitializeNode(&dummyNode, *connman); + peerLogic->InitializeNode(&dummyNode, connman); dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; misbehave(dummyNode.GetId(), 100); - SendMessages(&dummyNode, *connman, interruptDummy); + peerLogic->SendMessages(&dummyNode, connman, interruptDummy); BOOST_CHECK(connman->IsBanned(addr)); SetMockTime(nStartTime+60*60); diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 20eb499ff1e4..13007091abd1 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -55,7 +55,6 @@ BasicTestingSetup::~BasicTestingSetup() { fs::remove_all(m_path_root); ECC_Stop(); - g_connman.reset(); deterministicMNManager.reset(); evoDb.reset(); } @@ -107,17 +106,18 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha nScriptCheckThreads = 3; for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - RegisterNodeSignals(GetNodeSignals()); + peerLogic.reset(new PeerLogicValidation(connman)); } TestingSetup::~TestingSetup() { - UnregisterNodeSignals(GetNodeSignals()); threadGroup.interrupt_all(); threadGroup.join_all(); GetMainSignals().FlushBackgroundCallbacks(); UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); + g_connman.reset(); + peerLogic.reset(); UnloadBlockIndex(); delete pEvoNotificationInterface; delete pcoinsTip; diff --git a/src/test/test_pivx.h b/src/test/test_pivx.h index 9f05ca95f536..7da9901cedd8 100644 --- a/src/test/test_pivx.h +++ b/src/test/test_pivx.h @@ -50,6 +50,7 @@ struct BasicTestingSetup { * and wallet (if enabled) setup. */ class CConnman; +class PeerLogicValidation; class EvoNotificationInterface; struct TestingSetup: public BasicTestingSetup { @@ -58,6 +59,7 @@ struct TestingSetup: public BasicTestingSetup CConnman* connman; EvoNotificationInterface* pEvoNotificationInterface; CScheduler scheduler; + std::unique_ptr peerLogic; TestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~TestingSetup(); From 21f05c18149bcea915a5f6e8f693c8b50ab5098f Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2017 14:08:23 -0400 Subject: [PATCH 3/3] net: drop unused connman param The copy in PeerLogicValidation can be used instead. --- src/net.cpp | 10 +++++----- src/net.h | 6 +++--- src/net_processing.cpp | 6 +++--- src/net_processing.h | 7 +++---- src/test/DoS_tests.cpp | 22 +++++++++++----------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 22524db9fd58..04d88feae937 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1062,7 +1062,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, "", true); pnode->AddRef(); pnode->fWhitelisted = whitelisted; - m_msgproc->InitializeNode(pnode, this); + m_msgproc->InitializeNode(pnode); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1824,7 +1824,7 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (fFeeler) pnode->fFeeler = true; - m_msgproc->InitializeNode(pnode, this); + m_msgproc->InitializeNode(pnode); { LOCK(cs_vNodes); vNodes.push_back(pnode); @@ -1852,7 +1852,7 @@ void CConnman::ThreadMessageHandler() continue; // Receive messages - bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, this, flagInterruptMsgProc); + bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, flagInterruptMsgProc); fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend); if (flagInterruptMsgProc) return; @@ -1860,7 +1860,7 @@ void CConnman::ThreadMessageHandler() // Send messages { LOCK(pnode->cs_sendProcessing); - m_msgproc->SendMessages(pnode, this, flagInterruptMsgProc); + m_msgproc->SendMessages(pnode, flagInterruptMsgProc); } if (flagInterruptMsgProc) @@ -2103,7 +2103,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); pnodeLocalHost = new CNode(id, nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0, nonce); - m_msgproc->InitializeNode(pnodeLocalHost, this); + m_msgproc->InitializeNode(pnodeLocalHost); } // diff --git a/src/net.h b/src/net.h index 22f4c1a72aba..ce9bc5a06e5d 100644 --- a/src/net.h +++ b/src/net.h @@ -416,9 +416,9 @@ struct CombinerAll { class NetEventsInterface { public: - virtual bool ProcessMessages(CNode* pnode, CConnman* connman, std::atomic& interrupt) = 0; - virtual bool SendMessages(CNode* pnode, CConnman* connman, std::atomic& interrupt) = 0; - virtual void InitializeNode(CNode* pnode, CConnman* connman) = 0; + virtual bool ProcessMessages(CNode* pnode, std::atomic& interrupt) = 0; + virtual bool SendMessages(CNode* pnode, std::atomic& interrupt) = 0; + virtual void InitializeNode(CNode* pnode) = 0; virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0; }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 71cdb3db99ea..85b16924b7bb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -427,7 +427,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectoraddr; std::string addrName = pnode->GetAddrName(); NodeId nodeid = pnode->GetId(); @@ -1950,7 +1950,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } -bool PeerLogicValidation::ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interruptMsgProc) +bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& interruptMsgProc) { // Message format // (4) message start @@ -2066,7 +2066,7 @@ class CompareInvMempoolOrder } }; -bool PeerLogicValidation::SendMessages(CNode* pto, CConnman* connman, std::atomic& interruptMsgProc) +bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic& interruptMsgProc) { { // Don't send anything until the version handshake is complete diff --git a/src/net_processing.h b/src/net_processing.h index c76063458ab2..7f32e7968d44 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -45,19 +45,18 @@ class PeerLogicValidation : public CValidationInterface, public NetEventsInterfa void BlockChecked(const CBlock& block, const CValidationState& state) override; - void InitializeNode(CNode* pnode, CConnman* connman) override; + void InitializeNode(CNode* pnode) override; void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) override; /** Process protocol messages received from a given node */ - bool ProcessMessages(CNode* pfrom, CConnman* connman, std::atomic& interrupt) override; + bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override; /** * Send queued protocol messages to be sent to a give node. * * @param[in] pto The node which we are sending messages to. - * @param[in] connman The connection manager for that node. * @param[in] interrupt Interrupt condition for processing threads * @return True if there is more work to be done */ - bool SendMessages(CNode* pto, CConnman* connman, std::atomic& interrupt) override; + bool SendMessages(CNode* pto, std::atomic& interrupt) override; }; struct CNodeStateStats { diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 915f51b2386d..d2b482ef8f37 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -60,26 +60,26 @@ BOOST_AUTO_TEST_CASE(DoS_banning) CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode1, connman); + peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; misbehave(dummyNode1.GetId(), 100); // Should get banned - peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, "", true); dummyNode2.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode2, connman); + peerLogic->InitializeNode(&dummyNode2); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; misbehave(dummyNode2.GetId(), 50); - peerLogic->SendMessages(&dummyNode2, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode2, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be misbehave(dummyNode2.GetId(), 50); - peerLogic->SendMessages(&dummyNode2, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode2, interruptDummy); BOOST_CHECK(connman->IsBanned(addr2)); } @@ -92,17 +92,17 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode1, connman); + peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; misbehave(dummyNode1.GetId(), 100); - peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); misbehave(dummyNode1.GetId(), 10); - peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); misbehave(dummyNode1.GetId(), 1); - peerLogic->SendMessages(&dummyNode1, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); } @@ -118,12 +118,12 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) CAddress addr(ip(0xa0b0c001), NODE_NONE); CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, "", true); dummyNode.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode, connman); + peerLogic->InitializeNode(&dummyNode); dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; misbehave(dummyNode.GetId(), 100); - peerLogic->SendMessages(&dummyNode, connman, interruptDummy); + peerLogic->SendMessages(&dummyNode, interruptDummy); BOOST_CHECK(connman->IsBanned(addr)); SetMockTime(nStartTime+60*60);