Skip to content

Commit 635fc6a

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 635fc6a

File tree

1 file changed

+35
-13
lines changed

1 file changed

+35
-13
lines changed

src/net_processing.cpp

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -585,11 +585,19 @@ 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+
* @param[out] should_disconnect Set to true if the peer should be disconnected.
595+
* @return True if the peer was marked for disconnection in this function
591596
*/
592-
bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer);
597+
bool MaybeDiscourageAndDisconnect(Peer& peer, const CNetAddr& peer_addr,
598+
bool has_no_ban, bool is_manual,
599+
bool is_local_addr, bool is_inbound_onion,
600+
bool& should_disconnect);
593601

594602
/** Handle a transaction whose result was not MempoolAcceptResult::ResultType::VALID.
595603
* @param[in] first_time_failure Whether we should consider inserting into vExtraTxnForCompact, adding
@@ -4936,7 +4944,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
49364944
return;
49374945
}
49384946

4939-
bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer)
4947+
bool PeerManagerImpl::MaybeDiscourageAndDisconnect(Peer& peer, const CNetAddr& peer_addr,
4948+
bool has_no_ban, bool is_manual,
4949+
bool is_local_addr, bool is_inbound_onion,
4950+
bool& should_disconnect)
49404951
{
49414952
{
49424953
LOCK(peer.m_misbehavior_mutex);
@@ -4947,31 +4958,31 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer)
49474958
peer.m_should_discourage = false;
49484959
} // peer.m_misbehavior_mutex
49494960

4950-
if (pnode.HasPermission(NetPermissionFlags::NoBan)) {
4961+
if (has_no_ban) {
49514962
// We never disconnect or discourage peers for bad behavior if they have NetPermissionFlags::NoBan permission
49524963
LogPrintf("Warning: not punishing noban peer %d!\n", peer.m_id);
49534964
return false;
49544965
}
49554966

4956-
if (pnode.IsManualConn()) {
4967+
if (is_manual) {
49574968
// We never disconnect or discourage manual peers for bad behavior
49584969
LogPrintf("Warning: not punishing manually connected peer %d!\n", peer.m_id);
49594970
return false;
49604971
}
49614972

4962-
if (pnode.addr.IsLocal()) {
4973+
if (is_local_addr) {
49634974
// We disconnect local peers for bad behavior but don't discourage (since that would discourage
49644975
// all peers on the same local address)
49654976
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;
4977+
is_inbound_onion ? "inbound onion" : "local", peer.m_id);
4978+
should_disconnect = true;
49684979
return true;
49694980
}
49704981

49714982
// Normal case: Disconnect the peer and discourage all nodes sharing the address
49724983
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);
4984+
if (m_banman) m_banman->Discourage(peer_addr);
4985+
m_connman.DisconnectNode(peer_addr);
49754986
return true;
49764987
}
49774988

@@ -5473,7 +5484,18 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
54735484

54745485
// We must call MaybeDiscourageAndDisconnect first, to ensure that we'll
54755486
// disconnect misbehaving peers even before the version handshake is complete.
5476-
if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true;
5487+
bool should_disconnect = false;
5488+
if (MaybeDiscourageAndDisconnect(*peer, pto->addr,
5489+
pto->HasPermission(NetPermissionFlags::NoBan),
5490+
pto->IsManualConn(),
5491+
pto->addr.IsLocal(),
5492+
pto->m_inbound_onion,
5493+
should_disconnect)) {
5494+
if (should_disconnect) {
5495+
pto->fDisconnect = true;
5496+
}
5497+
return true;
5498+
}
54775499

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

0 commit comments

Comments
 (0)