confirm pivot block - removed disconnect based on chain height estimate

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

@@ -629,8 +629,9 @@ public boolean hasSupportForMessage(final int messageCode) {
   @Override
   public String toString() {
     return String.format(
-        "PeerId: %s %s, validated? %s, disconnected? %s, client: %s, %s, %s",
+        "PeerId: %s height (est): %s %s, validated? %s, disconnected? %s, client: %s, %s, %s",
         getLoggableId(),
+        chainState().getEstimatedHeight(),
         reputation,
         isFullyValidated(),
         isDisconnected(),

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

@@ -198,7 +198,7 @@ private boolean registerDisconnect(final EthPeer peer, final PeerConnection conn
         peer.handleDisconnect();
         abortPendingRequestsAssignedToDisconnectedPeers();
         if (peer.getReputation().getScore() > USEFULL_PEER_SCORE_THRESHOLD) {
-          LOG.debug("Disconnected USEFULL peer {}", peer);
+          LOG.info("Disconnected USEFULL peer {}", peer);
         } else {
           LOG.debug("Disconnected EthPeer {}", peer.getLoggableId());
         }
@@ -335,7 +335,7 @@ public Optional<EthPeer> bestPeerMatchingCriteria(final Predicate<EthPeer> match
   }
 
   public void setBestChainComparator(final Comparator<EthPeer> comparator) {
-    LOG.info("Updating the default best peer comparator");
+    LOG.info("Updating the default best peer comparator to " + comparator.toString());
     bestPeerComparator = comparator;
   }
 
@@ -380,23 +380,6 @@ public boolean shouldConnect(final Peer peer, final boolean inbound) {
     return true;
   }
 
-  public void disconnectWorstUselessPeer() {
-    streamAvailablePeers()
-        .sorted(getBestChainComparator())
-        .findFirst()
-        .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);
-            });
-  }
-
   @FunctionalInterface
   public interface ConnectCallback {
     void onPeerConnected(EthPeer newPeer);

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractGetHeadersFromPeerTask.java

@@ -143,7 +143,6 @@ private void updatePeerChainState(final EthPeer peer, final BlockHeader blockHea
           .log();
       peer.chainState().update(blockHeader);
     }
-    LOG.trace("Peer chain state {}", peer.chainState());
   }
 
   protected abstract boolean matchesFirstHeader(BlockHeader firstHeader);

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerLimiter.java

@@ -66,7 +66,7 @@ public void enforceTrailingPeerLimit() {
       final EthPeer peerToDisconnect = trailingPeers.remove(0);
       LOG.atDebug()
           .setMessage(
-              "Enforcing trailing peers limit (min height {}, max trailing peers {}) by disconnecting {}... with height {}")
+              "Enforcing trailing peers limit (min height {}, max trailing peers {}) by disconnecting {} with height {}")
           .addArgument(minimumHeightToBeUpToDate)
           .addArgument(maxTrailingPeers)
           .addArgument(peerToDisconnect::getLoggableId)

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/TrailingPeerRequirements.java

@@ -19,8 +19,11 @@
 import java.util.Objects;
 
 import com.google.common.base.MoreObjects;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class TrailingPeerRequirements {
+  private static final Logger LOG = LoggerFactory.getLogger(TrailingPeerRequirements.class);
   public static TrailingPeerRequirements UNRESTRICTED =
       new TrailingPeerRequirements(BlockHeader.GENESIS_BLOCK_NUMBER, Long.MAX_VALUE);
   private final long minimumHeightToBeUpToDate;
@@ -30,6 +33,7 @@ public TrailingPeerRequirements(
       final long minimumHeightToBeUpToDate, final long maxTrailingPeers) {
     this.minimumHeightToBeUpToDate = minimumHeightToBeUpToDate;
     this.maxTrailingPeers = maxTrailingPeers;
+    LOG.info("trailing peer requirements {}", this);
   }
 
   public long getMinimumHeightToBeUpToDate() {

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

@@ -163,7 +163,7 @@ public void deleteFastSyncState() {
   protected FastSyncState updateMaxTrailingPeers(final FastSyncState state) {
     if (state.getPivotBlockNumber().isPresent()) {
       trailingPeerRequirements =
-          Optional.of(new TrailingPeerRequirements(state.getPivotBlockNumber().getAsLong(), 0));
+          Optional.of(new TrailingPeerRequirements(state.getPivotBlockNumber().getAsLong(), 5));
     } else {
       trailingPeerRequirements = Optional.empty();
     }

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

@@ -93,19 +93,12 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
       return completedFuture(Optional.empty());
     } else {
       final EthPeer bestPeer = maybeBestPeer.get();
-      if (bestPeer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber()) {
-        LOG.info(
-            "Best peer {} has chain height {} below pivotBlock height {}. Waiting for better peers. Current {} of max {}",
-            maybeBestPeer.map(EthPeer::getLoggableId).orElse("none"),
-            maybeBestPeer.map(p -> p.chainState().getEstimatedHeight()).orElse(-1L),
-            pivotBlockHeader.getNumber(),
-            ethPeers.peerCount(),
-            ethPeers.getMaxPeers());
-        ethPeers.disconnectWorstUselessPeer();
-        return completedFuture(Optional.empty());
-      } else {
-        return confirmPivotBlockHeader(bestPeer);
-      }
+      LOG.debug(
+          "attempting to confirm pivot block {} with best peer {}",
+          pivotBlockHeader.getNumber(),
+          bestPeer);
+
+      return confirmPivotBlockHeader(bestPeer);
     }
   }
 
@@ -142,6 +135,11 @@ private CompletableFuture<Optional<EthPeer>> confirmPivotBlockHeader(final EthPe
                     pivotBlockHeader.toLogString());
                 return confirmPivotBlockHeader(bestPeer);
               } else {
+                LOG.debug(
+                    "Confirmed pivot block {} with peer {}",
+                    pivotBlockHeader.getNumber(),
+                    bestPeer.getLoggableId());
+                bestPeer.chainState().updateHeightEstimate(pivotBlockHeader.getNumber());
                 return CompletableFuture.completedFuture(Optional.of(bestPeer));
               }
             })

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java

@@ -564,7 +564,7 @@ public void shouldNotAllowPeersBeforePivotBlockOnceSelected(
 
     downloader.start();
     Assertions.assertThat(downloader.calculateTrailingPeerRequirements())
-        .contains(new TrailingPeerRequirements(50, 0));
+        .contains(new TrailingPeerRequirements(50, 5));
   }
 
   @ParameterizedTest

ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java

@@ -118,7 +118,6 @@ public enum DisconnectReason {
     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),