[CHORE] Disable blacklisting on testnet internal; better logging of blacklist reasons (#696)

Author: Nicolás Tallar

src/main/scala/io/iohk/ethereum/network/PeerManagerActor.scala

@@ -120,7 +120,11 @@ class PeerManagerActor(
 
   def connections(pendingPeers: PeerMap, peers: PeerMap): Receive = {
     case PeerClosedConnection(peerAddress, reason) =>
-      blacklist(PeerAddress(peerAddress), getBlacklistDuration(reason), "error during tcp connection attempt")
+      blacklist(
+        PeerAddress(peerAddress),
+        getBlacklistDuration(reason),
+        s"peer disconnected due to: ${Disconnect.reasonToString(reason)}"
+      )
 
     case HandlePeerConnection(connection, remoteAddress) =>
       handleConnection(connection, remoteAddress, pendingPeers, peers)

src/main/scala/io/iohk/ethereum/network/p2p/messages/WireProtocol.scala

@@ -2,7 +2,6 @@ package io.iohk.ethereum.network.p2p.messages
 
 import akka.util.ByteString
 import io.iohk.ethereum.network.p2p.{Message, MessageSerializableImplicit}
-import io.iohk.ethereum.network.p2p.messages.WireProtocol.Disconnect.Reasons
 import io.iohk.ethereum.rlp.RLPImplicitConversions._
 import io.iohk.ethereum.rlp.RLPImplicits._
 import io.iohk.ethereum.rlp._
@@ -92,6 +91,23 @@ object WireProtocol {
       val Other = 0x10
     }
 
+    def reasonToString(reasonCode: Long): String =
+      reasonCode match {
+        case Reasons.DisconnectRequested => "Disconnect requested"
+        case Reasons.TcpSubsystemError => "TCP sub-system error"
+        case Reasons.UselessPeer => "Useless peer"
+        case Reasons.TooManyPeers => "Too many peers"
+        case Reasons.AlreadyConnected => "Already connected"
+        case Reasons.IncompatibleP2pProtocolVersion => "Incompatible P2P protocol version"
+        case Reasons.NullNodeIdentityReceived => "Null node identity received - this is automatically invalid"
+        case Reasons.ClientQuitting => "Client quitting"
+        case Reasons.UnexpectedIdentity => "Unexpected identity"
+        case Reasons.IdentityTheSame => "Identity is the same as this node"
+        case Reasons.TimeoutOnReceivingAMessage => "Timeout on receiving a message"
+        case Reasons.Other => "Some other reason specific to a subprotocol"
+        case other => s"unknown reason code: $other"
+      }
+
     val code = 0x01
 
     implicit class DisconnectEnc(val underlyingMsg: Disconnect) extends MessageSerializableImplicit[Disconnect](underlyingMsg) with RLPSerializable  {
@@ -111,26 +127,9 @@ object WireProtocol {
   case class Disconnect(reason: Long) extends Message {
     override val code: Int = Disconnect.code
 
-    override def toString: String = {
-
-      val message = reason match {
-        case Reasons.DisconnectRequested => "Disconnect requested"
-        case Reasons.TcpSubsystemError => "TCP sub-system error"
-        case Reasons.UselessPeer => "Useless peer"
-        case Reasons.TooManyPeers => "Too many peers"
-        case Reasons.AlreadyConnected => "Already connected"
-        case Reasons.IncompatibleP2pProtocolVersion => "Incompatible P2P protocol version"
-        case Reasons.NullNodeIdentityReceived => "Null node identity received - this is automatically invalid"
-        case Reasons.ClientQuitting => "Client quitting"
-        case Reasons.UnexpectedIdentity => "Unexpected identity"
-        case Reasons.IdentityTheSame => "Identity is the same as this node"
-        case Reasons.TimeoutOnReceivingAMessage => "Timeout on receiving a message"
-        case Reasons.Other => "Some other reason specific to a subprotocol"
-        case other => s"unknown reason code: $other"
-      }
+    override def toString: String =
+      s"Disconnect(${Disconnect.reasonToString(reason)}"
 
-      s"Disconnect($message)"
-    }
   }
 
   object Ping {

src/universal/conf/testnet-internal.conf

@@ -2,16 +2,15 @@ include "app.conf"
 
 mantis {
   sync {
-    # Whether to enable fast-sync
+    # Fast sync is disabled as it's an experimental feature for now
     do-fast-sync = false
 
-    # Duration for blacklisting a peer. Blacklisting reason include: invalid response from peer, response time-out, etc.
-    # 0 value is a valid duration and it will disable blacklisting completely (which can be useful when all nodes are
-    # are controlled by a single party, eg. private networks)
+    # All testnet members are assumed to be honest so blacklisting is turned off
     blacklist-duration = 0
   }
 
   pruning {
+    # Pruning is disabled as it's an experimental feature for now
     mode = "archive"
   }
 
@@ -34,6 +33,12 @@ mantis {
 
     }
 
+    peer {
+      # All testnet members are assumed to be honest so blacklisting is turned off
+      short-blacklist-duration = 0
+      long-blacklist-duration = 0
+    }
+
     rpc {
       http {
         # Listening address of JSON-RPC HTTP/HTTPS endpoint