-
Notifications
You must be signed in to change notification settings - Fork 38.6k
net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers #20845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fa5436d to
fae5c7c
Compare
|
ACK fae5c7c92b012bb3ce6fe6d1edee4a2448fdf646 |
|
🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file. |
|
What I'd love to see is for #20576 to be implemented. |
vasild
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fae5c7c92b012bb3ce6fe6d1edee4a2448fdf646
As a further improvement maybe the following would make sense:
[patch] distinguish onion from local
Achieve two things:
- Recognize incoming onion peers even if the tor proxy is not running on
127.0.0.1 - Distinguish onion from normal local peers in the logging, if the tor proxy is running on
127.0.0.1
diff --git i/src/net_processing.cpp w/src/net_processing.cpp
index 413e3c034..ecd615995 100644
--- i/src/net_processing.cpp
+++ w/src/net_processing.cpp
@@ -3780,16 +3780,20 @@ bool PeerManager::MaybeDiscourageAndDisconnect(CNode& pnode)
if (pnode.IsManualConn()) {
// We never disconnect or discourage manual peers for bad behavior
LogPrintf("Warning: not punishing manually connected peer %d!\n", peer_id);
return false;
}
- if (pnode.addr.IsLocal()) {
- // We disconnect local peers for bad behavior but don't discourage (since that would discourage
- // all peers on the same local address)
- LogPrint(BCLog::NET, "Warning: disconnecting but not discouraging local peer %d!\n", peer_id);
+ if (pnode.addr.IsLocal() || pnode.IsInboundOnion()) {
+ // We disconnect local or onion peers for bad behavior but don't discourage
+ // (since that would discourage all peers on the same address)
+ if (pnode.IsInboundOnion()) {
+ LogPrint(BCLog::NET, "Warning: disconnecting but not discouraging onion peer %d!\n", peer_id);
+ } else {
+ LogPrintf("Warning: disconnecting but not discouraging local peer %d!\n", peer_id);
+ }
pnode.fDisconnect = true;
return true;
}
// Normal case: Disconnect the peer and discourage all nodes sharing the address
LogPrint(BCLog::NET, "Disconnecting and discouraging peer %d!\n", peer_id);…n and manual peers Github-Pull: bitcoin#20845 Rebased-From: fae5c7c92b012bb3ce6fe6d1edee4a2448fdf646
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
…n and manual peers
fae5c7c to
fa55159
Compare
|
Rebased and added a check for pnode.m_inbound_onion for local onion proxies. I'll leave non-local onion proxies for a follow-up. |
vasild
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa55159
|
ACK fa55159 Nice to see |
…nnect except for noban and manual peers fa55159 net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers (MarcoFalke) Pull request description: The goal is to avoid local peers (e.g. untrusted peers on the local network or inbound onion peers via a local onion proxy) filling the debug log (and thus the disk). ACKs for top commit: practicalswift: ACK fa55159 vasild: ACK fa55159 Tree-SHA512: de233bf57334580f9b91f369fafd131d71c5ae25db25b09cc8fa8cbf34c0648f083c52260a6a912238751467e3c3c5f5d2309c145710753058d44a0003f88f4f
Github-Pull: bitcoin#20845 Rebased-From: fa55159
The goal is to avoid local peers (e.g. untrusted peers on the local network or inbound onion peers via a local onion proxy) filling the debug log (and thus the disk).