Synchroniser to wait for new peer if best is up to date

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java

@@ -165,7 +165,7 @@ private CompletableFuture<Void> checkSyncTarget() {
   }
 
   private boolean finishedSyncingToCurrentTarget() {
-    return syncTargetManager.isSyncTargetDisconnected()
+    return !syncTargetManager.syncTargetCanProvideMoreBlocks()
         || checkpointHeaderManager.checkpointsHaveTimedOut()
         || chainSegmentsHaveTimedOut();
   }

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java

@@ -34,13 +34,15 @@
 public abstract class SyncTargetManager<C> {
 
   private static final Logger LOG = LogManager.getLogger();
+
+  protected final SyncState syncState;
+
   private volatile long syncTargetDisconnectListenerId;
   private volatile boolean syncTargetDisconnected = false;
   private final SynchronizerConfiguration config;
   private final ProtocolSchedule<C> protocolSchedule;
   private final ProtocolContext<C> protocolContext;
   private final EthContext ethContext;
-  private final SyncState syncState;
   private final MetricsSystem metricsSystem;
 
   public SyncTargetManager(
@@ -144,4 +146,15 @@ public void clearSyncTarget(final SyncTarget syncTarget) {
   public abstract boolean shouldSwitchSyncTarget(final SyncTarget currentTarget);
 
   public abstract boolean shouldContinueDownloading();
+
+  public abstract boolean isSyncTargetReached(final EthPeer peer);
+
+  public boolean syncTargetCanProvideMoreBlocks() {
+    if (!syncState.syncTarget().isPresent()) {
+      LOG.warn("SyncTarget should be set, but is not.");
+      return false;
+    }
+    final EthPeer currentSyncingPeer = syncState.syncTarget().get().peer();
+    return !isSyncTargetDisconnected() && !isSyncTargetReached(currentSyncingPeer);
+  }
 }

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncTargetManager.java

@@ -118,4 +118,9 @@ public boolean shouldSwitchSyncTarget(final SyncTarget currentTarget) {
   public boolean shouldContinueDownloading() {
     return !protocolContext.getBlockchain().getChainHeadHash().equals(pivotBlockHeader.getHash());
   }
+
+  @Override
+  public boolean isSyncTargetReached(final EthPeer peer) {
+    return syncState.chainHeadNumber() >= pivotBlockHeader.getNumber();
+  }
 }

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java

@@ -41,7 +41,6 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
   private final SynchronizerConfiguration config;
   private final ProtocolContext<C> protocolContext;
   private final EthContext ethContext;
-  private final SyncState syncState;
 
   FullSyncTargetManager(
       final SynchronizerConfiguration config,
@@ -54,7 +53,6 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
     this.config = config;
     this.protocolContext = protocolContext;
     this.ethContext = ethContext;
-    this.syncState = syncState;
   }
 
   @Override
@@ -82,10 +80,7 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
       return completedFuture(Optional.empty());
     } else {
       final EthPeer bestPeer = maybeBestPeer.get();
-      final long peerHeight = bestPeer.chainState().getEstimatedHeight();
-      final UInt256 peerTd = bestPeer.chainState().getBestBlock().getTotalDifficulty();
-      if (peerTd.compareTo(syncState.chainHeadTotalDifficulty()) <= 0
-          && peerHeight <= syncState.chainHeadNumber()) {
+      if (isSyncTargetReached(bestPeer)) {
         // We're caught up to our best peer, try again when a new peer connects
         LOG.debug("Caught up to best peer: " + bestPeer.chainState().getEstimatedHeight());
         return completedFuture(Optional.empty());
@@ -94,6 +89,15 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
     }
   }
 
+  @Override
+  public boolean isSyncTargetReached(final EthPeer peer) {
+    final long peerHeight = peer.chainState().getEstimatedHeight();
+    final UInt256 peerTd = peer.chainState().getBestBlock().getTotalDifficulty();
+
+    return peerTd.compareTo(syncState.chainHeadTotalDifficulty()) <= 0
+        && peerHeight <= syncState.chainHeadNumber();
+  }
+
   @Override
   public boolean shouldSwitchSyncTarget(final SyncTarget currentTarget) {
     final EthPeer currentPeer = currentTarget.peer();

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java

@@ -230,8 +230,8 @@ public void syncsToBetterChain_fromFork() {
     peer.respondWhileOtherThreadsWork(
         responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock);
 
-    assertThat(syncState.syncTarget()).isPresent();
-    assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer());
+    // Synctarget should not exist as chain has fully downloaded.
+    assertThat(syncState.syncTarget().isPresent()).isFalse();
     assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock);
   }