From 872f8fd0915c89573f75ebd85c9b10d667d791d1 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 8 Apr 2024 16:54:03 +1000 Subject: [PATCH] added richness to useless peer disconnect reasons (#6899) Signed-off-by: Sally MacFarlane Signed-off-by: amsmota --- .../hyperledger/besu/ethereum/eth/manager/EthPeers.java | 2 +- .../besu/ethereum/eth/manager/PeerReputation.java | 2 +- .../manager/task/AbstractRetryingSwitchingPeerTask.java | 2 +- .../besu/ethereum/eth/sync/ChainHeadTracker.java | 2 +- .../besu/ethereum/eth/sync/TrailingPeerLimiter.java | 2 +- .../ethereum/eth/sync/fastsync/SyncTargetManager.java | 2 +- .../eth/sync/fullsync/FullSyncTargetManager.java | 2 +- .../besu/ethereum/eth/manager/PeerReputationTest.java | 4 ++-- .../besu/ethereum/eth/sync/TrailingPeerLimiterTest.java | 2 +- .../ethereum/p2p/rlpx/connections/netty/DeFramer.java | 3 ++- .../p2p/rlpx/wire/messages/DisconnectMessage.java | 9 +++++++++ 11 files changed, 21 insertions(+), 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 index c2362c5dd16..67e012b9de1 100644 --- 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 index 687a93191c0..8a1a7b8839f 100644 --- 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 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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index 7db355646f0..9411fac081a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/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 index eb89ca12acc..5d563e012d7 100644 --- 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 index 79ea538b7e2..f5705e06731 100644 --- 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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index 548cce6eadc..b23297c8e17 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -133,7 +133,7 @@ private CompletableFuture> 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.empty()); } LOG.debug( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java index ee3d36a31ab..6bb9e3b7adc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java @@ -67,7 +67,7 @@ protected Optional 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 index 5025287a57b..99d94414716 100644 --- 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/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java index 9bcac6f0a9a..6f905ec4fe6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiterTest.java @@ -145,7 +145,7 @@ private void assertDisconnections(final EthPeer... disconnectedPeers) { final List 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/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java index c7c600d3fd3..2dc1e75d574 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java +++ b/ethereum/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/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java index 180889d21da..9cbbb310951 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java +++ b/ethereum/p2p/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),