Log reason of peer banning

[Eligioo]

What's in this pull request?

All paths to a CloseReason::MaliciousPeer as reason for closing the connection now contain a WARN log message to annotate the reason for banning a peer.

I chose for WARN as log level for now but can easily changed if one believes there is a reason for it.

This fixes #2661 .

Pull request checklist

• All tests pass. The project builds and runs.
• I have resolved any merge conflicts.
• I have resolved all clippy and rustfmt warnings.

[jsdanielh]

I'd have changed the CloseReason::MaliciousPeer variant to be CloseReason::MaliciousPeer(String) such that we enforce always the reason and then we log it in the network

[Eligioo]

@jsdanielh

I'd have changed the CloseReason::MaliciousPeer variant to be CloseReason::MaliciousPeer(String) such that we enforce always the reason and then we log it in the network

I thought about this but this will cause the log messages to double:

• the new log in the network layer as you suggested
• the existing tracing logs creating context at the caller location that initiates the closing connection e.g.:

log::warn!(
    given_checkpoint_epoch,
    expected_checkpoint_epoch,
    %peer_id,
    "Banning peer because requesting macro chain failed: invalid checkpoint",
);

Basically to remove the existing tracing logs but don't lose context, you would need to format the extra values into the String of CloseReason::MaliciousPeer(String). I'm not a fan of that.

[hrxi]

LGTM.

@Eligioo remarked that the peer banning in the testnet should already have a log for the ban unless it came from validator/src/proposal_buffer.rs.

[hrxi]

Rebased for merge.