Skip to content

Commit 15ce12c

Browse files
committed
refactor/p2p: Decouple CNode from PeerManagerImpl::MaybeDiscourageAndDisconnect
Refactor MaybeDiscourageAndDisconnect to accept individual parameters (has_no_ban, is_manual, is_local_addr, is_inbound_onion, peer_addr) instead of taking a CNode& reference directly. This decouples the function from CNode while preserving all original logic paths and behavior.
1 parent e249ea7 commit 15ce12c

File tree

1 file changed

+24
-13
lines changed

1 file changed

+24
-13
lines changed

src/net_processing.cpp

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -585,11 +585,17 @@ class PeerManagerImpl final : public PeerManager
585585

586586
/** Maybe disconnect a peer and discourage future connections from its address.
587587
*
588-
* @param[in] pnode The node to check.
589-
* @param[in] peer The peer object to check.
590-
* @return True if the peer was marked for disconnection in this function
588+
* @param[in] peer The peer object to check.
589+
* @param[in] peer_addr The peer's address.
590+
* @param[in] has_no_ban Whether the peer has NoBan permission.
591+
* @param[in] is_manual Whether the peer is manually connected.
592+
* @param[in] is_local_addr Whether the peer's address is local.
593+
* @param[in] is_inbound_onion Whether the peer is an inbound onion connection.
594+
* @return True if the peer was marked for disconnection in this function
591595
*/
592-
bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer);
596+
bool MaybeDiscourageAndDisconnect(Peer& peer, const CNetAddr& peer_addr,
597+
bool has_no_ban, bool is_manual,
598+
bool is_local_addr, bool is_inbound_onion);
593599

594600
/** Handle a transaction whose result was not MempoolAcceptResult::ResultType::VALID.
595601
* @param[in] first_time_failure Whether we should consider inserting into vExtraTxnForCompact, adding
@@ -4936,7 +4942,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
49364942
return;
49374943
}
49384944

4939-
bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer)
4945+
bool PeerManagerImpl::MaybeDiscourageAndDisconnect(Peer& peer, const CNetAddr& peer_addr,
4946+
bool has_no_ban, bool is_manual,
4947+
bool is_local_addr, bool is_inbound_onion)
49404948
{
49414949
{
49424950
LOCK(peer.m_misbehavior_mutex);
@@ -4947,31 +4955,30 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer)
49474955
peer.m_should_discourage = false;
49484956
} // peer.m_misbehavior_mutex
49494957

4950-
if (pnode.HasPermission(NetPermissionFlags::NoBan)) {
4958+
if (has_no_ban) {
49514959
// We never disconnect or discourage peers for bad behavior if they have NetPermissionFlags::NoBan permission
49524960
LogPrintf("Warning: not punishing noban peer %d!\n", peer.m_id);
49534961
return false;
49544962
}
49554963

4956-
if (pnode.IsManualConn()) {
4964+
if (is_manual) {
49574965
// We never disconnect or discourage manual peers for bad behavior
49584966
LogPrintf("Warning: not punishing manually connected peer %d!\n", peer.m_id);
49594967
return false;
49604968
}
49614969

4962-
if (pnode.addr.IsLocal()) {
4970+
if (is_local_addr) {
49634971
// We disconnect local peers for bad behavior but don't discourage (since that would discourage
49644972
// all peers on the same local address)
49654973
LogDebug(BCLog::NET, "Warning: disconnecting but not discouraging %s peer %d!\n",
4966-
pnode.m_inbound_onion ? "inbound onion" : "local", peer.m_id);
4967-
pnode.fDisconnect = true;
4974+
is_inbound_onion ? "inbound onion" : "local", peer.m_id);
49684975
return true;
49694976
}
49704977

49714978
// Normal case: Disconnect the peer and discourage all nodes sharing the address
49724979
LogDebug(BCLog::NET, "Disconnecting and discouraging peer %d!\n", peer.m_id);
4973-
if (m_banman) m_banman->Discourage(pnode.addr);
4974-
m_connman.DisconnectNode(pnode.addr);
4980+
if (m_banman) m_banman->Discourage(peer_addr);
4981+
m_connman.DisconnectNode(peer_addr);
49754982
return true;
49764983
}
49774984

@@ -5473,7 +5480,11 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
54735480

54745481
// We must call MaybeDiscourageAndDisconnect first, to ensure that we'll
54755482
// disconnect misbehaving peers even before the version handshake is complete.
5476-
if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true;
5483+
if (MaybeDiscourageAndDisconnect(*peer, pto->addr,
5484+
pto->HasPermission(NetPermissionFlags::NoBan),
5485+
pto->IsManualConn(),
5486+
pto->addr.IsLocal(),
5487+
pto->m_inbound_onion)) return true;
54775488

54785489
// Initiate version handshake for outbound connections
54795490
if (!pto->IsInboundConn() && !peer->m_outbound_version_message_sent) {

0 commit comments

Comments
 (0)