added richness to useless peer disconnect reasons (hyperledger#6899)

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: amsmota <antonio.mota@citi.com>
Citi · Apr 16, 2024 · 872f8fd · 872f8fd
1 parent adac964
commit 872f8fd
Show file tree

Hide file tree

Showing 11 changed files with 21 additions and 11 deletions.
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java
@@ -393,7 +393,7 @@ public void disconnectWorstUselessPeer() {
                   .addArgument(this::peerCount)
                   .addArgument(this::getMaxPeers)
                   .log();
-              peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER);
+              peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER_BY_CHAIN_COMPARATOR);
             });
   }
 

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java
@@ -97,7 +97,7 @@ public Optional<DisconnectReason> recordUselessResponse(final long timestamp) {
     if (uselessResponseTimes.size() >= USELESS_RESPONSE_THRESHOLD) {
       score -= LARGE_ADJUSTMENT;
       LOG.debug("Disconnection triggered by exceeding useless response threshold");
-      return Optional.of(DisconnectReason.USELESS_PEER);
+      return Optional.of(DisconnectReason.USELESS_PEER_USELESS_RESPONSES);
     } else {
       score -= SMALL_ADJUSTMENT;
       return Optional.empty();

diff --git a/...ava/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/...ava/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java
@@ -149,7 +149,7 @@ private void refreshPeers() {
                     .addArgument(peers::peerCount)
                     .addArgument(peers::getMaxPeers)
                     .log();
-                peer.disconnect(DisconnectReason.USELESS_PEER);
+                peer.disconnect(DisconnectReason.USELESS_PEER_BY_REPUTATION);
               });
     }
   }

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java
@@ -100,7 +100,7 @@ public void onPeerConnected(final EthPeer peer) {
                     .addArgument(peer::getLoggableId)
                     .addArgument(error)
                     .log();
-                peer.disconnect(DisconnectReason.USELESS_PEER);
+                peer.disconnect(DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE);
               }
             });
   }

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiter.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiter.java
@@ -75,7 +75,7 @@ public void enforceTrailingPeerLimit() {
                   ? "(no chain state)"
                   : peerToDisconnect.chainState().getEstimatedHeight())
           .log();
-      peerToDisconnect.disconnect(DisconnectReason.USELESS_PEER);
+      peerToDisconnect.disconnect(DisconnectReason.USELESS_PEER_TRAILING_PEER);
     }
   }
 

diff --git a/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java
@@ -133,7 +133,7 @@ private CompletableFuture<Optional<EthPeer>> confirmPivotBlockHeader(final EthPe
                       pivotBlockHeader.getHash(),
                       result.size() == 1 ? result.get(0).getHash() : "invalid response",
                       bestPeer);
-                  bestPeer.disconnect(DisconnectReason.USELESS_PEER);
+                  bestPeer.disconnect(DisconnectReason.USELESS_PEER_MISMATCHED_PIVOT_BLOCK);
                   return CompletableFuture.completedFuture(Optional.<EthPeer>empty());
                 }
                 LOG.debug(

diff --git a/.../src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java b/.../src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java
@@ -67,7 +67,7 @@ protected Optional<SyncTarget> finalizeSelectedSyncTarget(final SyncTarget syncT
           syncTarget.peer(),
           commonAncestor.getNumber(),
           commonAncestor.getHash());
-      syncTarget.peer().disconnect(DisconnectReason.USELESS_PEER);
+      syncTarget.peer().disconnect(DisconnectReason.USELESS_PEER_WORLD_STATE_NOT_AVAILABLE);
       return Optional.empty();
     }
   }

diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java
@@ -16,7 +16,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason.TIMEOUT;
-import static org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason.USELESS_PEER;
+import static org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason.USELESS_PEER_USELESS_RESPONSES;
 
 import org.hyperledger.besu.ethereum.eth.messages.EthPV62;
 
@@ -62,7 +62,7 @@ public void shouldResetTimeoutCountForRequestType() {
   @Test
   public void shouldOnlyDisconnectWhenEmptyResponseThresholdReached() {
     sendUselessResponses(1001, PeerReputation.USELESS_RESPONSE_THRESHOLD - 1);
-    assertThat(reputation.recordUselessResponse(1005)).contains(USELESS_PEER);
+    assertThat(reputation.recordUselessResponse(1005)).contains(USELESS_PEER_USELESS_RESPONSES);
   }
 
   @Test

diff --git a/...eum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java b/...eum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java
@@ -145,7 +145,7 @@ private void assertDisconnections(final EthPeer... disconnectedPeers) {
     final List<EthPeer> disconnected = asList(disconnectedPeers);
     for (final EthPeer peer : peers) {
       if (disconnected.contains(peer)) {
-        verify(peer).disconnect(DisconnectReason.USELESS_PEER);
+        verify(peer).disconnect(DisconnectReason.USELESS_PEER_TRAILING_PEER);
       } else {
         verify(peer, never()).disconnect(any(DisconnectReason.class));
       }

diff --git a/.../p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java b/.../p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java
@@ -184,7 +184,8 @@ protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final L
           LOG.debug("Disconnecting because no capabilities are shared: {}", peerInfo);
           connectFuture.completeExceptionally(
               new IncompatiblePeerException("No shared capabilities"));
-          connection.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER);
+          connection.disconnect(
+              DisconnectMessage.DisconnectReason.USELESS_PEER_NO_SHARED_CAPABILITIES);
         }
 
         // Setup next stage

diff --git a/...src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java b/...src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java
@@ -110,6 +110,15 @@ public enum DisconnectReason {
     TCP_SUBSYSTEM_ERROR((byte) 0x01),
     BREACH_OF_PROTOCOL((byte) 0x02),
     USELESS_PEER((byte) 0x03),
+    USELESS_PEER_USELESS_RESPONSES((byte) 0x03, "Useless responses: exceeded threshold"),
+    USELESS_PEER_TRAILING_PEER((byte) 0x03, "Trailing peer requirement"),
+    USELESS_PEER_NO_SHARED_CAPABILITIES((byte) 0x03, "No shared capabilities"),
+    USELESS_PEER_WORLD_STATE_NOT_AVAILABLE((byte) 0x03, "World state not available"),
+    USELESS_PEER_MISMATCHED_PIVOT_BLOCK((byte) 0x03, "Mismatched pivot block"),
+    USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE(
+        (byte) 0x03, "Failed to retrieve header for chain state"),
+    USELESS_PEER_BY_REPUTATION((byte) 0x03, "Lowest reputation score"),
+    USELESS_PEER_BY_CHAIN_COMPARATOR((byte) 0x03, "Lowest by chain height comparator"),
     TOO_MANY_PEERS((byte) 0x04),
     ALREADY_CONNECTED((byte) 0x05),
     INCOMPATIBLE_P2P_PROTOCOL_VERSION((byte) 0x06),