Useful response improves reputation (hyperledger#4130)

* useful response improves reputation Signed-off-by: Stefan <stefan.pingel@consensys.net>
noirgif · Jul 20, 2022 · b7877be · b7877be
1 parent 0ca45f3
commit b7877be
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 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
@@ -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
@@ -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
@@ -44,7 +44,7 @@ public class PeerReputation implements Comparable<PeerReputation> {
   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<DisconnectReason> recordRequestTimeout(final int requestCode) {
     final int newTimeoutCount = getOrCreateTimeoutCount(requestCode).incrementAndGet();
@@ -85,6 +85,10 @@ public Optional<DisconnectReason> 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/...src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java b/...src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java
@@ -105,7 +105,11 @@ private void handleMessage(
     }
     try {
       final Optional<R> 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
@@ -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
@@ -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);
+  }
 }