When picking fast sync pivot block, use the peer with the best total …

…difficulty, not the highest chain. (#899)
PegaSysEng · Feb 19, 2019 · dd8d338 · dd8d338
1 parent 78e0afd
commit dd8d338
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 1 deletion.
diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthPeers.java
@@ -33,6 +33,9 @@ public class EthPeers {
   public static final Comparator<EthPeer> CHAIN_HEIGHT =
       Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedHeight()));
 
+  private static final Comparator<EthPeer> HIGHEST_TOTAL_DIFFICULTY_PEER =
+      TOTAL_DIFFICULTY.thenComparing(CHAIN_HEIGHT);
+
   public static final Comparator<EthPeer> BEST_CHAIN = CHAIN_HEIGHT.thenComparing(TOTAL_DIFFICULTY);
 
   public static final Comparator<EthPeer> LEAST_TO_MOST_BUSY =
@@ -93,6 +96,10 @@ public Optional<EthPeer> bestPeer() {
     return availablePeers().max(BEST_CHAIN);
   }
 
+  public Optional<EthPeer> highestTotalDifficultyPeer() {
+    return availablePeers().max(HIGHEST_TOTAL_DIFFICULTY_PEER);
+  }
+
   public Optional<EthPeer> idlePeer() {
     return idlePeers().min(LEAST_TO_MOST_BUSY);
   }

diff --git a/...m/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/...m/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java
@@ -119,7 +119,7 @@ public CompletableFuture<FastSyncState> selectPivotBlock(final FastSyncState fas
   private CompletableFuture<FastSyncState> selectPivotBlockFromPeers() {
     return ethContext
         .getEthPeers()
-        .bestPeer()
+        .highestTotalDifficultyPeer()
         .filter(peer -> peer.chainState().hasEstimatedHeight())
         .map(
             peer -> {

diff --git a/...h/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java b/...h/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java
@@ -39,6 +39,7 @@
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.metrics.LabelledMetric;
 import tech.pegasys.pantheon.metrics.OperationTimer;
+import tech.pegasys.pantheon.util.uint.UInt256;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -150,6 +151,17 @@ public void selectPivotBlockShouldSelectBlockPivotDistanceFromBestPeer() {
     assertThat(result).isCompletedWithValue(expected);
   }
 
+  @Test
+  public void selectPivotBlockShouldConsiderTotalDifficultyWhenSelectingBestPeer() {
+    EthProtocolManagerTestUtil.createPeer(ethProtocolManager, UInt256.of(1000), 5500);
+    EthProtocolManagerTestUtil.createPeer(ethProtocolManager, UInt256.of(2000), 4000);
+
+    final CompletableFuture<FastSyncState> result =
+        fastSyncActions.selectPivotBlock(EMPTY_SYNC_STATE);
+    final FastSyncState expected = new FastSyncState(3000);
+    assertThat(result).isCompletedWithValue(expected);
+  }
+
   @Test
   public void selectPivotBlockShouldWaitAndRetryIfNoPeersAreAvailable() {
     final CompletableFuture<FastSyncState> result =