From b7877befdf7c5c8917752a88ea1ea30c3c3258b8 Mon Sep 17 00:00:00 2001 From: Stefan Pingel <16143240+pinges@users.noreply.github.com> Date: Wed, 20 Jul 2022 12:40:34 +1000 Subject: [PATCH] Useful response improves reputation (#4130) * useful response improves reputation Signed-off-by: Stefan --- .../hyperledger/besu/ethereum/eth/manager/EthPeer.java | 4 ++++ .../hyperledger/besu/ethereum/eth/manager/EthPeers.java | 7 +++++++ .../besu/ethereum/eth/manager/PeerReputation.java | 8 ++++++-- .../eth/manager/task/AbstractPeerRequestTask.java | 6 +++++- .../besu/ethereum/eth/manager/EthPeerTest.java | 8 ++++++++ .../besu/ethereum/eth/manager/PeerReputationTest.java | 6 ++++++ 6 files changed, 36 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 9df946a7ef4..260aec318cc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -215,6 +215,10 @@ public void recordUselessResponse(final String requestType) { reputation.recordUselessResponse(System.currentTimeMillis()).ifPresent(this::disconnect); } + public void recordUsefulResponse() { + reputation.recordUsefulResposne(); + } + public void disconnect(final DisconnectReason reason) { connection.disconnect(reason); } 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 9c6c0d16bc1..442471aadf7 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 @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.PeerInfo; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider; @@ -115,6 +116,12 @@ public void registerDisconnect(final PeerConnection connection) { disconnectCallbacks.forEach(callback -> callback.onDisconnect(peer)); peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); + final PeerInfo peerInfo = peer.getConnection().getPeerInfo(); + LOG.debug( + "Disconnected EthPeer {}, client ID: {}, {}", + peerInfo.getNodeId(), + peerInfo.getClientId(), + peer.getReputation()); } reattemptPendingPeerRequests(); } 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 8293f0dfa09..aa1c37f3218 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 @@ -44,7 +44,7 @@ public class PeerReputation implements Comparable { private static final int SMALL_ADJUSTMENT = 1; private static final int LARGE_ADJUSTMENT = 10; - private int score = DEFAULT_SCORE; + private long score = DEFAULT_SCORE; public Optional recordRequestTimeout(final int requestCode) { final int newTimeoutCount = getOrCreateTimeoutCount(requestCode).incrementAndGet(); @@ -85,6 +85,10 @@ public Optional recordUselessResponse(final long timestamp) { } } + public void recordUsefulResposne() { + score += SMALL_ADJUSTMENT; + } + private boolean shouldRemove(final Long timestamp, final long currentTimestamp) { return timestamp != null && timestamp + USELESS_RESPONSE_WINDOW_IN_MILLIS < currentTimestamp; } @@ -96,6 +100,6 @@ public String toString() { @Override public int compareTo(final @Nonnull PeerReputation otherReputation) { - return Integer.compare(this.score, otherReputation.score); + return Long.compare(this.score, otherReputation.score); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java index 39d4be28ad6..6dd87ae3b7a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java @@ -105,7 +105,11 @@ private void handleMessage( } try { final Optional result = processResponse(streamClosed, message, peer); - result.ifPresent(promise::complete); + result.ifPresent( + r -> { + promise.complete(r); + peer.recordUsefulResponse(); + }); } catch (final RLPException e) { // Peer sent us malformed data - disconnect LOG.debug("Disconnecting with BREACH_OF_PROTOCOL due to malformed message: {}", peer, e); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java index c9899ba0674..1da115f6920 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java @@ -362,6 +362,14 @@ public void compareTo_withDifferentNodeId() { assertThat(peer2.compareTo(peer1)).isEqualTo(-1); } + @Test + public void recordUsefullResponse() { + final EthPeer peer = createPeer(); + final EthPeer peer2 = createPeer(); + peer.recordUsefulResponse(); + assertThat(peer.getReputation().compareTo(peer2.getReputation())).isGreaterThan(0); + } + private void messageStream( final ResponseStreamSupplier getStream, final MessageData targetMessage, 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 1371f64c34a..e8a8c6d9a7d 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 @@ -80,4 +80,10 @@ public void shouldDiscardEmptyResponseRecordsAfterTimeWindowElapses() { 1001 + PeerReputation.USELESS_RESPONSE_WINDOW_IN_MILLIS + 1)) .isEmpty(); } + + @Test + public void shouldIncreaseScore() { + reputation.recordUsefulResposne(); + assertThat(reputation.compareTo(new PeerReputation())).isGreaterThan(0); + } }