Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

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

Merged
merged 5 commits into from
Mar 26, 2019
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f9d040
Synchroniser to wait for new peer if best is up to date
Mar 25, 2019
ae6d803
Rework following review
Mar 25, 2019
d4e48b8
update tests
Mar 26, 2019
6811ed8
Merge branch 'master' into uptodate
rain-on Mar 26, 2019
a212f4c
Updated to end fast sync when local chain after pivotblock
Mar 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/ChainDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private CompletableFuture<Void> checkSyncTarget() {
}

private boolean finishedSyncingToCurrentTarget() {
return syncTargetManager.isSyncTargetDisconnected()
return !syncTargetManager.syncTargetCanProvideMoreBlocks()
|| checkpointHeaderManager.checkpointsHaveTimedOut()
|| chainSegmentsHaveTimedOut();
}
Expand Down
11 changes: 11 additions & 0 deletions ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SyncTargetManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,15 @@ public void clearSyncTarget(final SyncTarget syncTarget) {
public abstract boolean shouldSwitchSyncTarget(final SyncTarget currentTarget);

public abstract boolean shouldContinueDownloading();

public abstract boolean isCaughtUpWithPeer(final EthPeer peer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and implementations) could probably be protected.


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() && !isCaughtUpWithPeer(currentSyncingPeer);
}
}
7 changes: 6 additions & 1 deletion ...src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncTargetManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
return completedFuture(Optional.empty());
} else {
final EthPeer bestPeer = maybeBestPeer.get();
if (bestPeer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber()) {
if (isCaughtUpWithPeer(bestPeer)) {
LOG.info("No sync target with sufficient chain height, wait for peers.");
return completedFuture(Optional.empty());
} else {
Expand Down Expand Up @@ -118,4 +118,9 @@ public boolean shouldSwitchSyncTarget(final SyncTarget currentTarget) {
public boolean shouldContinueDownloading() {
return !protocolContext.getBlockchain().getChainHeadHash().equals(pivotBlockHeader.getHash());
}

@Override
public boolean isCaughtUpWithPeer(final EthPeer peer) {
return peer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the right thing to extract here. It is the right condition for where it was (when fast syncing we can select any peer as our target as long as their chain height is above the pivot block). We've caught up with that peer when our chain height >= pivotBlockHeader.getNumber().

}
}
14 changes: 10 additions & 4 deletions ...src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,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 (isCaughtUpWithPeer(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());
Expand All @@ -94,6 +91,15 @@ protected CompletableFuture<Optional<EthPeer>> selectBestAvailableSyncTarget() {
}
}

@Override
public boolean isCaughtUpWithPeer(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();
Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down