net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers#20845
Merged
maflcko merged 1 commit intobitcoin:masterfrom Feb 22, 2021
Merged
Conversation
fa5436d to
fae5c7c
Compare
vasild
reviewed
Jan 4, 2021
Contributor
|
ACK fae5c7c92b012bb3ce6fe6d1edee4a2448fdf646 |
Contributor
|
🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file. |
Member
|
What I'd love to see is for #20576 to be implemented. |
vasild
approved these changes
Jan 15, 2021
Contributor
vasild
left a comment
There was a problem hiding this comment.
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);
Contributor
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Jan 28, 2021
…n and manual peers Github-Pull: bitcoin#20845 Rebased-From: fae5c7c92b012bb3ce6fe6d1edee4a2448fdf646
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
This was referenced Feb 12, 2021
…n and manual peers
fae5c7c to
fa55159
Compare
Member
Author
|
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
approved these changes
Feb 16, 2021
Contributor
|
ACK fa55159 Nice to see |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 22, 2021
…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
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Jun 27, 2021
Github-Pull: bitcoin#20845 Rebased-From: fa55159
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).