[net/refactor] Rework ThreadOpenConnections logic

Make the connection counts explicit and extract into interface functions around
m_conn_type. Using explicit counting and switch statements where possible
should help prevent counting bugs in the future.
This commit is contained in:
Amiti Uttarwar 2020-06-02 21:23:44 -07:00
parent 35839e963b
commit 7f7b83deb2
2 changed files with 41 additions and 12 deletions

View file

@ -1829,21 +1829,27 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
int nOutboundFullRelay = 0; int nOutboundFullRelay = 0;
int nOutboundBlockRelay = 0; int nOutboundBlockRelay = 0;
std::set<std::vector<unsigned char> > setConnected; std::set<std::vector<unsigned char> > setConnected;
{ {
LOCK(cs_vNodes); LOCK(cs_vNodes);
for (const CNode* pnode : vNodes) { for (const CNode* pnode : vNodes) {
if (!pnode->IsInboundConn() && (pnode->m_conn_type != ConnectionType::MANUAL)) { if (pnode->IsFullOutboundConn()) nOutboundFullRelay++;
// Netgroups for inbound and addnode peers are not excluded because our goal here if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++;
// is to not use multiple of our limited outbound slots on a single netgroup
// but inbound and addnode peers do not use our outbound slots. Inbound peers // Netgroups for inbound and manual peers are not excluded because our goal here
// also have the added issue that they're attacker controlled and could be used // is to not use multiple of our limited outbound slots on a single netgroup
// to prevent us from connecting to particular hosts if we used them here. // but inbound and manual peers do not use our outbound slots. Inbound peers
setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap)); // also have the added issue that they could be attacker controlled and used
if (pnode->m_tx_relay == nullptr) { // to prevent us from connecting to particular hosts if we used them here.
nOutboundBlockRelay++; switch(pnode->m_conn_type){
} else if (pnode->m_conn_type == ConnectionType::OUTBOUND) { case ConnectionType::INBOUND:
nOutboundFullRelay++; case ConnectionType::MANUAL:
} break;
case ConnectionType::OUTBOUND:
case ConnectionType::BLOCK_RELAY:
case ConnectionType::ADDR_FETCH:
case ConnectionType::FEELER:
setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
} }
} }
} }

View file

@ -789,10 +789,18 @@ public:
std::atomic_bool fPauseRecv{false}; std::atomic_bool fPauseRecv{false};
std::atomic_bool fPauseSend{false}; std::atomic_bool fPauseSend{false};
bool IsFullOutboundConn() const {
return m_conn_type == ConnectionType::OUTBOUND;
}
bool IsManualConn() const { bool IsManualConn() const {
return m_conn_type == ConnectionType::MANUAL; return m_conn_type == ConnectionType::MANUAL;
} }
bool IsBlockOnlyConn() const {
return m_conn_type == ConnectionType::BLOCK_RELAY;
}
bool IsFeelerConn() const { bool IsFeelerConn() const {
return m_conn_type == ConnectionType::FEELER; return m_conn_type == ConnectionType::FEELER;
} }
@ -805,6 +813,21 @@ public:
return m_conn_type == ConnectionType::INBOUND; return m_conn_type == ConnectionType::INBOUND;
} }
bool ExpectServicesFromConn() const {
switch(m_conn_type) {
case ConnectionType::INBOUND:
case ConnectionType::MANUAL:
case ConnectionType::FEELER:
return false;
case ConnectionType::OUTBOUND:
case ConnectionType::BLOCK_RELAY:
case ConnectionType::ADDR_FETCH:
return true;
}
assert(false);
}
protected: protected:
mapMsgCmdSize mapSendBytesPerMsgCmd; mapMsgCmdSize mapSendBytesPerMsgCmd;
mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv); mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);