Peering - compare incoming connection

CHANGELOG.md

@@ -8,6 +8,7 @@
 ### Breaking Changes
 
 ### Additions and Improvements
+- Peering - compare incoming peer with current peers before deciding which to keep [#6968](https://github.com/hyperledger/besu/pull/6968)
 - Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569)
 
 ### Bug fixes

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java

@@ -431,21 +431,42 @@ private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes
         .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated())));
   }
 
-  public void disconnectWorstUselessPeer() {
-    streamAvailablePeers()
-        .filter(p -> !canExceedPeerLimits(p.getId()))
-        .min(getBestPeerComparator())
-        .ifPresent(
-            peer -> {
-              LOG.atDebug()
-                  .setMessage(
-                      "disconnecting peer {}. Waiting for better peers. Current {} of max {}")
-                  .addArgument(peer::getLoggableId)
-                  .addArgument(this::peerCount)
-                  .addArgument(this::getMaxPeers)
-                  .log();
-              peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER_BY_CHAIN_COMPARATOR);
-            });
+  public void disconnectWorstUselessPeerIfAtCapacityIncludingConnectingPeer(
+      final EthPeer connectingPeer) {
+    if (peerCount() >= getMaxPeers()) {
+      streamAvailablePeers()
+          .filter(p -> !canExceedPeerLimits(p.getId()))
+          .min(getBestPeerComparator())
+          .ifPresent(
+              worstCurrentPeer -> {
+                // TODO remove this debug log
+                LOG.atDebug()
+                    .setMessage("comparing worstCurrentPeer {} with connectingPeer {}")
+                    .addArgument(worstCurrentPeer)
+                    .addArgument(connectingPeer)
+                    .log();
+                if (getBestPeerComparator().compare(worstCurrentPeer, connectingPeer) < 0) {
+                  LOG.atDebug()
+                      .setMessage(
+                          "disconnecting current peer {}. Waiting for better peers. Current {} of max {}")
+                      .addArgument(worstCurrentPeer::getLoggableId)
+                      .addArgument(this::peerCount)
+                      .addArgument(this::getMaxPeers)
+                      .log();
+                  worstCurrentPeer.disconnect(
+                      DisconnectMessage.DisconnectReason.USELESS_PEER_BY_CHAIN_COMPARATOR);
+                } else {
+                  LOG.atDebug()
+                      .setMessage(
+                          "disconnecting connecting peer {}. Waiting for better peers. Current {} of max {}")
+                      .addArgument(connectingPeer::getLoggableId)
+                      .addArgument(this::peerCount)
+                      .addArgument(this::getMaxPeers)
+                      .log();
+                  connectingPeer.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS);
+                }
+              });
+    }
   }
 
   public void setChainHeadTracker(final ChainHeadTracker tracker) {
@@ -739,14 +760,8 @@ boolean addPeerToEthPeers(final EthPeer peer) {
                                     .log();
                               }));
         } else {
-          LOG.atTrace()
-              .setMessage(
-                  "Too many peers. Disconnect peer {} with connection: {}, max connections {}")
-              .addArgument(peer.getLoggableId())
-              .addArgument(connection)
-              .addArgument(peerUpperBound)
-              .log();
-          connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS);
+          // Compare the incoming peer to the worst current peer and disconnect worst of those
+          disconnectWorstUselessPeerIfAtCapacityIncludingConnectingPeer(peer);
           return false;
         }
       }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java

@@ -106,7 +106,6 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
             pivotBlockHeader.getNumber(),
             ethPeers.peerCount(),
             ethPeers.getMaxPeers());
-        ethPeers.disconnectWorstUselessPeer();
         return completedFuture(Optional.empty());
       } else {
         return confirmPivotBlockHeader(bestPeer);