-
Notifications
You must be signed in to change notification settings - Fork 643
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
substrate: peer_store: log warn on disconnecting because of reputation #1299
Conversation
Disconnecting and banning a peer because of negative reputation is usually an indicative of one of two things: 1. We've got a bug that forces disconnects. 2. We've got malicious peers that try to attack us. We both cases I don't think we should hide this behind a trace log and we should log errors, so that things are easy to notice and debug/mitigated. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
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.
let's see what we previously didn't see 🍿
@@ -222,7 +222,7 @@ impl PeerStoreInner { | |||
if peer_info.reputation < BANNED_THRESHOLD { | |||
self.protocols.iter().for_each(|handle| handle.disconnect_peer(peer_id)); | |||
|
|||
log::trace!( | |||
log::error!( |
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.
I would prefer warning instead of error.
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.
Agree! Done.
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
#1299) * substrate: peer_store: log error on disconnecting because of reputation Disconnecting and banning a peer because of negative reputation is usually an indicative of one of two things: 1. We've got a bug that forces disconnects. 2. We've got malicious peers that try to attack us. We both cases I don't think we should hide this behind a trace log and we should log errors, so that things are easy to notice and debug/mitigated. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> * Move from error to warn Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> --------- Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
paritytech#1299) * substrate: peer_store: log error on disconnecting because of reputation Disconnecting and banning a peer because of negative reputation is usually an indicative of one of two things: 1. We've got a bug that forces disconnects. 2. We've got malicious peers that try to attack us. We both cases I don't think we should hide this behind a trace log and we should log errors, so that things are easy to notice and debug/mitigated. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> * Move from error to warn Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> --------- Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
…h#1299) * use raw balance value if tokenDecimals property is missing * fmt
* use raw balance value if tokenDecimals property is missing * fmt
Disconnecting and banning a peer because of negative reputation is usually an indicative of one of two things:
In both cases I don't think we should hide this behind a trace log and we should log errors, so that things are easy to notice and debug/mitigated.