diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java index 3b76aef0a3..92047ef893 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java @@ -165,7 +165,7 @@ private CompletableFuture checkSyncTarget() { } private boolean finishedSyncingToCurrentTarget() { - return syncTargetManager.isSyncTargetDisconnected() + return !syncTargetManager.syncTargetCanProvideMoreBlocks() || checkpointHeaderManager.checkpointsHaveTimedOut() || chainSegmentsHaveTimedOut(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java index 537d2ed959..0477456630 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java @@ -34,13 +34,15 @@ public abstract class SyncTargetManager { 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 protocolSchedule; private final ProtocolContext 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); + } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncTargetManager.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncTargetManager.java index e5c4b6d4d4..2752453169 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncTargetManager.java +++ b/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(); + } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java index df486d494f..43bbce0d55 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java @@ -41,7 +41,6 @@ class FullSyncTargetManager extends SyncTargetManager { private final SynchronizerConfiguration config; private final ProtocolContext protocolContext; private final EthContext ethContext; - private final SyncState syncState; FullSyncTargetManager( final SynchronizerConfiguration config, @@ -54,7 +53,6 @@ class FullSyncTargetManager extends SyncTargetManager { this.config = config; this.protocolContext = protocolContext; this.ethContext = ethContext; - this.syncState = syncState; } @Override @@ -82,10 +80,7 @@ protected CompletableFuture> 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> 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(); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index a16f244898..1b1603ff09 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/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); }