fix: ban peer if it sends bad liveness data #5844
Conversation
ghpbot-tari-project
added
P-acks_required
| level: debug, | ||
| level: warn, |
There was a problem hiding this comment.
Maybe info ?
From an outsiders perspective there's a high likelihood at any time they could receive data with unserailizable information but showing them big red warnings in the console makes them feel like they need to do something, when in fact this is a perfectly fine event to happen, and there's not actually any user required interaction that needs to occur.
There was a problem hiding this comment.
I'll meet you halfway with info. I think it is an indication that you are using the wrong build or network
| error!( target: LOG_TARGET, "Failed to handle liveness event because '{}'", e); | ||
| if let ChainMetadataSyncError::ReceivedInvalidChainMetadata(node_id,reason) = e { | ||
| log_if_error!( | ||
| level: warn, |
There was a problem hiding this comment.
Same here. Banning a peer is informational not deserving of anything alerting to an end user.
| Ok(_) => {} | ||
| Err(e) => { | ||
| error!( target: LOG_TARGET, "Failed to handle liveness event because '{}'", e); | ||
| if let ChainMetadataSyncError::ReceivedInvalidChainMetadata(node_id,reason) = e { |
There was a problem hiding this comment.
nit, i'm surprised rustfmt didn't fix this.
| if let ChainMetadataSyncError::ReceivedInvalidChainMetadata(node_id,reason) = e { | |
| if let ChainMetadataSyncError::ReceivedInvalidChainMetadata(node_id, reason) = e { |
There was a problem hiding this comment.
rustfmt can't seem to format anything in a tokio::select I've tried to make it neat manually, but it's the wild west of fmtland
There was a problem hiding this comment.
This issue on it was closed.
Description
Bans the peer if they sent invalid liveness data.
Motivation and Context
I think there was a change to the chainmetadata struct recently, and this resulted in a bunch of warnings on my igor node. This PR now bans the node if they sent data that doesn't deserialize.
How Has This Been Tested?
Tested with igor trying to connect to old nodes
What process can a PR reviewer use to test or verify this change?
Try to connect to an igor node running 0.52.0-pre.1
Breaking Changes