[PAN-2917] Rework "in-sync" checks (#1720)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/ChainHead.java

@@ -22,9 +22,12 @@ public final class ChainHead {
 
   private final UInt256 totalDifficulty;
 
-  public ChainHead(final Hash hash, final UInt256 totalDifficulty) {
+  private final long height;
+
+  public ChainHead(final Hash hash, final UInt256 totalDifficulty, final long height) {
     this.hash = hash;
     this.totalDifficulty = totalDifficulty;
+    this.height = height;
   }
 
   public Hash getHash() {
@@ -34,4 +37,8 @@ public final class ChainHead {
   public UInt256 getTotalDifficulty() {
     return totalDifficulty;
   }
+
+  public long getHeight() {
+    return height;
+  }
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/DefaultMutableBlockchain.java

@@ -114,7 +114,7 @@ public class DefaultMutableBlockchain implements MutableBlockchain {
 
   @Override
   public ChainHead getChainHead() {
-    return new ChainHead(chainHeader.getHash(), totalDifficulty);
+    return new ChainHead(chainHeader.getHash(), totalDifficulty, chainHeader.getNumber());
   }
 
   @Override

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainState.java

@@ -12,6 +12,7 @@
  */
 package tech.pegasys.pantheon.ethereum.eth.manager;
 
+import tech.pegasys.pantheon.ethereum.chain.ChainHead;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.Hash;
 import tech.pegasys.pantheon.util.Subscribers;
@@ -45,6 +46,10 @@ public class ChainState {
     return estimatedHeight;
   }
 
+  public UInt256 getEstimatedTotalDifficulty() {
+    return bestBlock.getTotalDifficulty();
+  }
+
   public BestBlock getBestBlock() {
     return bestBlock;
   }
@@ -99,6 +104,26 @@ public class ChainState {
     }
   }
 
+  /**
+   * Returns true if this chain state represents a chain that is "better" than the chain represented
+   * by the supplied {@link ChainHead}. "Better" currently means that this chain is longer or
+   * heavier than the supplied {@code chainToCheck}.
+   *
+   * @param chainToCheck The chain being compared.
+   * @return true if this {@link ChainState} represents a better chain than {@code chainToCheck}.
+   */
+  public boolean chainIsBetterThan(final ChainHead chainToCheck) {
+    return hasHigherDifficultyThan(chainToCheck) || hasLongerChainThan(chainToCheck);
+  }
+
+  private boolean hasHigherDifficultyThan(final ChainHead chainToCheck) {
+    return bestBlock.getTotalDifficulty().compareTo(chainToCheck.getTotalDifficulty()) > 0;
+  }
+
+  private boolean hasLongerChainThan(final ChainHead chainToCheck) {
+    return estimatedHeight > chainToCheck.getHeight();
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthPeers.java

@@ -30,8 +30,7 @@ import java.util.stream.Stream;
 
 public class EthPeers {
   public static final Comparator<EthPeer> TOTAL_DIFFICULTY =
-      Comparator.comparing(
-          ((final EthPeer p) -> p.chainState().getBestBlock().getTotalDifficulty()));
+      Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedTotalDifficulty()));
 
   public static final Comparator<EthPeer> CHAIN_HEIGHT =
       Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedHeight()));

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

@@ -47,8 +47,7 @@ public class BetterSyncTargetEvaluator {
               final ChainState bestPeerChainState = bestPeer.chainState();
               final UInt256 tdDifference =
                   bestPeerChainState
-                      .getBestBlock()
-                      .getTotalDifficulty()
+                      .getEstimatedTotalDifficulty()
                       .minus(currentPeerChainState.getBestBlock().getTotalDifficulty());
               if (tdDifference.compareTo(config.getDownloaderChangeTargetThresholdByTd()) > 0) {
                 return true;

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

@@ -25,7 +25,6 @@ import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncTarget;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason;
 import tech.pegasys.pantheon.metrics.MetricsSystem;
-import tech.pegasys.pantheon.util.uint.UInt256;
 
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
@@ -85,12 +84,9 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
   }
 
   private boolean isSyncTargetReached(final EthPeer peer) {
-    final long peerHeight = peer.chainState().getEstimatedHeight();
-    final UInt256 peerTd = peer.chainState().getBestBlock().getTotalDifficulty();
     final MutableBlockchain blockchain = protocolContext.getBlockchain();
-
-    return peerTd.compareTo(blockchain.getChainHead().getTotalDifficulty()) <= 0
-        && peerHeight <= blockchain.getChainHeadBlockNumber();
+    // We're in sync if the peer's chain is no better than our chain
+    return !peer.chainState().chainIsBetterThan(blockchain.getChainHead());
   }
 
   @Override

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncState.java

@@ -13,6 +13,7 @@
 package tech.pegasys.pantheon.ethereum.eth.sync.state;
 
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
+import tech.pegasys.pantheon.ethereum.chain.ChainHead;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.SyncStatus;
 import tech.pegasys.pantheon.ethereum.core.Synchronizer.SyncStatusListener;
@@ -84,11 +85,27 @@ public class SyncState {
   }
 
   public boolean isInSync(final long syncTolerance) {
+    // Sync target may be temporarily empty while we switch sync targets during a sync, so
+    // check both the sync target and our best peer to determine if we're in sync or not
+    return isInSyncWithTarget(syncTolerance) && isInSyncWithBestPeer(syncTolerance);
+  }
+
+  private boolean isInSyncWithTarget(final long syncTolerance) {
     return syncTarget
         .map(t -> t.estimatedTargetHeight() - blockchain.getChainHeadBlockNumber() <= syncTolerance)
         .orElse(true);
   }
 
+  private boolean isInSyncWithBestPeer(final long syncTolerance) {
+    final ChainHead chainHead = blockchain.getChainHead();
+    return ethPeers
+        .bestPeerWithHeightEstimate()
+        .filter(peer -> peer.chainState().chainIsBetterThan(chainHead))
+        .map(EthPeer::chainState)
+        .map(chainState -> chainState.getEstimatedHeight() - chainHead.getHeight() <= syncTolerance)
+        .orElse(true);
+  }
+
   public void clearSyncTarget() {
     replaceSyncTarget(Optional.empty());
   }
@@ -112,7 +129,10 @@ public class SyncState {
   public long bestChainHeight(final long localChainHeight) {
     return Math.max(
         localChainHeight,
-        ethPeers.bestPeer().map(p -> p.chainState().getEstimatedHeight()).orElse(localChainHeight));
+        ethPeers
+            .bestPeerWithHeightEstimate()
+            .map(p -> p.chainState().getEstimatedHeight())
+            .orElse(localChainHeight));
   }
 
   private synchronized void checkInSync() {

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncTarget.java

@@ -75,7 +75,7 @@ public class SyncTarget {
         .add(
             "height",
             (chainState.getEstimatedHeight() == 0 ? "?" : chainState.getEstimatedHeight()))
-        .add("td", chainState.getBestBlock().getTotalDifficulty())
+        .add("td", chainState.getEstimatedTotalDifficulty())
         .add("peer", peer)
         .toString();
   }

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainStateTest.java

@@ -17,6 +17,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
+import tech.pegasys.pantheon.ethereum.chain.ChainHead;
 import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture;
@@ -287,4 +288,65 @@ public class ChainStateTest {
 
     verifyNoMoreInteractions(listener);
   }
+
+  @Test
+  public void chainIsBetterThan_chainStateIsLighterAndShorter() {
+    final ChainState chainState = new ChainState();
+    updateChainState(chainState, UInt256.of(50), 50);
+    final ChainHead chainHead = new ChainHead(Hash.ZERO, UInt256.of(100), 100);
+
+    assertThat(chainState.chainIsBetterThan(chainHead)).isFalse();
+  }
+
+  @Test
+  public void chainIsBetterThan_chainStateIsHeavierAndShorter() {
+    final ChainState chainState = new ChainState();
+    updateChainState(chainState, UInt256.of(100), 50);
+    final ChainHead chainHead = new ChainHead(Hash.ZERO, UInt256.of(50), 100);
+
+    assertThat(chainState.chainIsBetterThan(chainHead)).isTrue();
+  }
+
+  @Test
+  public void chainIsBetterThan_chainStateIsLighterAndTaller() {
+    final ChainState chainState = new ChainState();
+    updateChainState(chainState, UInt256.of(50), 100);
+    final ChainHead chainHead = new ChainHead(Hash.ZERO, UInt256.of(100), 50);
+
+    assertThat(chainState.chainIsBetterThan(chainHead)).isTrue();
+  }
+
+  @Test
+  public void chainIsBetterThan_chainStateIsHeavierAndTaller() {
+    final ChainState chainState = new ChainState();
+    updateChainState(chainState, UInt256.of(100), 100);
+    final ChainHead chainHead = new ChainHead(Hash.ZERO, UInt256.of(50), 50);
+
+    assertThat(chainState.chainIsBetterThan(chainHead)).isTrue();
+  }
+
+  /**
+   * Updates the chain state, such that the peer will end up with an estimated height of {@code
+   * blockHeight} and an estimated total difficulty of {@code totalDifficulty}
+   *
+   * @param chainState The chain state to update
+   * @param totalDifficulty The total difficulty
+   * @param blockHeight The target estimated block height
+   */
+  private void updateChainState(
+      final ChainState chainState, final UInt256 totalDifficulty, final long blockHeight) {
+    // Chain state is updated based on the parent of the announced block
+    // So, increment block number by 1 and set block difficulty to zero
+    // in order to update to the values we want
+    final BlockHeader header =
+        new BlockHeaderTestFixture()
+            .number(blockHeight + 1L)
+            .difficulty(UInt256.ZERO)
+            .buildHeader();
+    chainState.updateForAnnouncedBlock(header, totalDifficulty);
+
+    // Sanity check this logic still holds
+    assertThat(chainState.getEstimatedHeight()).isEqualTo(blockHeight);
+    assertThat(chainState.getEstimatedTotalDifficulty()).isEqualTo(totalDifficulty);
+  }
 }

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncStateTest.java

@@ -13,8 +13,11 @@
 package tech.pegasys.pantheon.ethereum.eth.sync.state;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -22,16 +25,20 @@ import static org.mockito.Mockito.when;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedObserver;
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
+import tech.pegasys.pantheon.ethereum.chain.ChainHead;
 import tech.pegasys.pantheon.ethereum.core.Block;
 import tech.pegasys.pantheon.ethereum.core.BlockBody;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture;
+import tech.pegasys.pantheon.ethereum.core.Hash;
 import tech.pegasys.pantheon.ethereum.core.Synchronizer.SyncStatusListener;
 import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
 import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
 import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
+import tech.pegasys.pantheon.util.uint.UInt256;
 
 import java.util.Collections;
+import java.util.Optional;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -40,12 +47,18 @@ import org.mockito.ArgumentCaptor;
 public class SyncStateTest {
 
   private static final long OUR_CHAIN_HEAD_NUMBER = 500;
-  private static final long TARGET_SYNC_NUMBER = OUR_CHAIN_HEAD_NUMBER + 100;
+  private static final UInt256 OUR_CHAIN_DIFFICULTY = UInt256.of(500);
+  private static final long TARGET_CHAIN_DELTA = 100;
+  private static final long TARGET_CHAIN_HEIGHT = OUR_CHAIN_HEAD_NUMBER + TARGET_CHAIN_DELTA;
+  private static final UInt256 TARGET_DIFFICULTY = OUR_CHAIN_DIFFICULTY.plus(TARGET_CHAIN_DELTA);
+
   private final Blockchain blockchain = mock(Blockchain.class);
   private final EthPeers ethPeers = mock(EthPeers.class);
   private final SyncState.InSyncListener inSyncListener = mock(SyncState.InSyncListener.class);
-  private final EthPeer peer = mock(EthPeer.class);
-  private final ChainState peerChainHead = new ChainState();
+  private final EthPeer syncTargetPeer = mock(EthPeer.class);
+  private final ChainState syncTargetPeerChainState = spy(new ChainState());
+  private final EthPeer otherPeer = mock(EthPeer.class);
+  private final ChainState otherPeerChainState = spy(new ChainState());
   private SyncState syncState;
   private BlockAddedObserver blockAddedObserver;
 
@@ -53,27 +66,137 @@ public class SyncStateTest {
   public void setUp() {
     final ArgumentCaptor<BlockAddedObserver> captor =
         ArgumentCaptor.forClass(BlockAddedObserver.class);
+
+    final ChainHead ourChainHead =
+        new ChainHead(Hash.ZERO, OUR_CHAIN_DIFFICULTY, OUR_CHAIN_HEAD_NUMBER);
+
     when(blockchain.observeBlockAdded(captor.capture())).thenReturn(1L);
-    when(peer.chainState()).thenReturn(peerChainHead);
     when(blockchain.getChainHeadBlockNumber()).thenReturn(OUR_CHAIN_HEAD_NUMBER);
+    when(blockchain.getChainHead()).thenReturn(ourChainHead);
+    when(syncTargetPeer.chainState()).thenReturn(syncTargetPeerChainState);
+    when(otherPeer.chainState()).thenReturn(otherPeerChainState);
     syncState = new SyncState(blockchain, ethPeers);
     blockAddedObserver = captor.getValue();
     syncState.addInSyncListener(inSyncListener);
   }
 
   @Test
-  public void shouldBeInSyncWhenNoSyncTargetHasBeenSet() {
+  public void isInSync_noPeers() {
+    assertThat(syncState.isInSync()).isTrue();
+  }
+
+  @Test
+  public void isInSync_singlePeerWithWorseChainBetterHeight() {
+    updateChainState(otherPeerChainState, TARGET_CHAIN_HEIGHT, OUR_CHAIN_DIFFICULTY.minus(1L));
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(false).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.syncTarget()).isEmpty(); // Sanity check
+    assertThat(syncState.isInSync()).isTrue();
+    assertThat(syncState.isInSync(0)).isTrue();
+  }
+
+  @Test
+  public void isInSync_singlePeerWithWorseChainWorseHeight() {
+    updateChainState(
+        otherPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, OUR_CHAIN_DIFFICULTY.minus(1L));
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(false).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.syncTarget()).isEmpty(); // Sanity check
+    assertThat(syncState.isInSync()).isTrue();
+    assertThat(syncState.isInSync(0)).isTrue();
+  }
+
+  @Test
+  public void isInSync_singlePeerWithBetterChainWorseHeight() {
+    updateChainState(otherPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, TARGET_DIFFICULTY);
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(true).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.syncTarget()).isEmpty(); // Sanity check
     assertThat(syncState.isInSync()).isTrue();
+    assertThat(syncState.isInSync(0)).isTrue();
   }
 
   @Test
-  public void shouldSwitchToNotInSyncWhenSyncTargetWithBetterChainSet() {
-    final BlockHeader bestBlockHeader = targetBlockHeader();
-    peerChainHead.update(bestBlockHeader);
-    syncState.setSyncTarget(peer, bestBlockHeader);
+  public void isInSync_singlePeerWithBetterChainBetterHeight() {
+    updateChainState(otherPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(true).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.syncTarget()).isEmpty(); // Sanity check
     assertThat(syncState.isInSync()).isFalse();
-    verify(inSyncListener).onSyncStatusChanged(false);
-    verifyNoMoreInteractions(inSyncListener);
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA - 1)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA)).isTrue();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA + 1)).isTrue();
+  }
+
+  @Test
+  public void isInSync_syncTargetWithBetterHeight() {
+    updateChainState(syncTargetPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
+
+    assertThat(syncState.syncTarget()).isPresent(); // Sanity check
+    assertThat(syncState.isInSync()).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA - 1)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA)).isTrue();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA + 1)).isTrue();
+  }
+
+  @Test
+  public void isInSync_syncTargetWithWorseHeight() {
+    updateChainState(syncTargetPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, TARGET_DIFFICULTY);
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
+
+    assertThat(syncState.syncTarget()).isPresent(); // Sanity check
+    assertThat(syncState.isInSync()).isTrue();
+    assertThat(syncState.isInSync(0)).isTrue();
+  }
+
+  @Test
+  public void isInSync_outOfSyncWithTargetAndOutOfSyncWithBestPeer() {
+    updateChainState(syncTargetPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
+    updateChainState(otherPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(true).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.isInSync()).isFalse();
+    assertThat(syncState.isInSync(0)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA - 1)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA)).isTrue();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA + 1)).isTrue();
+  }
+
+  @Test
+  public void isInSync_inSyncWithTargetOutOfSyncWithBestPeer() {
+    updateChainState(
+        syncTargetPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, OUR_CHAIN_DIFFICULTY.minus(1L));
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
+    updateChainState(otherPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(true).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.isInSync()).isFalse();
+    assertThat(syncState.isInSync(0)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA - 1)).isFalse();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA)).isTrue();
+    assertThat(syncState.isInSync(TARGET_CHAIN_DELTA + 1)).isTrue();
+  }
+
+  @Test
+  public void isInSync_inSyncWithTargetInSyncWithBestPeer() {
+    updateChainState(
+        syncTargetPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, OUR_CHAIN_DIFFICULTY.minus(1L));
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
+    updateChainState(
+        otherPeerChainState, OUR_CHAIN_HEAD_NUMBER - 1L, OUR_CHAIN_DIFFICULTY.minus(1L));
+    when(ethPeers.bestPeerWithHeightEstimate()).thenReturn(Optional.of(otherPeer));
+    doReturn(false).when(otherPeerChainState).chainIsBetterThan(any());
+
+    assertThat(syncState.isInSync()).isTrue();
+    assertThat(syncState.isInSync(0)).isTrue();
   }
 
   @Test
@@ -90,7 +213,7 @@ public class SyncStateTest {
   public void shouldBecomeInSyncWhenOurBlockchainCatchesUp() {
     setupOutOfSyncState();
 
-    when(blockchain.getChainHeadBlockNumber()).thenReturn(TARGET_SYNC_NUMBER);
+    when(blockchain.getChainHeadBlockNumber()).thenReturn(TARGET_CHAIN_HEIGHT);
     blockAddedObserver.onBlockAdded(
         BlockAddedEvent.createForHeadAdvancement(
             new Block(
@@ -118,14 +241,42 @@ public class SyncStateTest {
   }
 
   private void setupOutOfSyncState() {
-    final BlockHeader bestBlockHeader = targetBlockHeader();
-    peerChainHead.update(bestBlockHeader);
-    syncState.setSyncTarget(peer, bestBlockHeader);
+    updateChainState(syncTargetPeerChainState, TARGET_CHAIN_HEIGHT, TARGET_DIFFICULTY);
+    syncState.setSyncTarget(syncTargetPeer, blockHeaderAt(0L));
     assertThat(syncState.isInSync()).isFalse();
     verify(inSyncListener).onSyncStatusChanged(false);
   }
 
+  /**
+   * Updates the chain state, such that the peer will end up with an estimated height of {@code
+   * blockHeight} and an estimated total difficulty of {@code totalDifficulty}
+   *
+   * @param chainState The chain state to update
+   * @param blockHeight The target estimated block height
+   * @param totalDifficulty The total difficulty
+   */
+  private void updateChainState(
+      final ChainState chainState, final long blockHeight, final UInt256 totalDifficulty) {
+    // Chain state is updated based on the parent of the announced block
+    // So, increment block number by 1 and set block difficulty to zero
+    // in order to update to the values we want
+    final BlockHeader header =
+        new BlockHeaderTestFixture()
+            .number(blockHeight + 1L)
+            .difficulty(UInt256.ZERO)
+            .buildHeader();
+    chainState.updateForAnnouncedBlock(header, totalDifficulty);
+
+    // Sanity check this logic still holds
+    assertThat(chainState.getEstimatedHeight()).isEqualTo(blockHeight);
+    assertThat(chainState.getEstimatedTotalDifficulty()).isEqualTo(totalDifficulty);
+  }
+
   private BlockHeader targetBlockHeader() {
-    return new BlockHeaderTestFixture().number(TARGET_SYNC_NUMBER).buildHeader();
+    return blockHeaderAt(TARGET_CHAIN_HEIGHT);
+  }
+
+  private BlockHeader blockHeaderAt(final long blockNumber) {
+    return new BlockHeaderTestFixture().number(blockNumber).buildHeader();
   }
 }

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java

@@ -58,7 +58,7 @@ public class AdminNodeInfoTest {
   private final BytesValue nodeId =
       BytesValue.fromHexString(
           "0x0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807");
-  private final ChainHead testChainHead = new ChainHead(Hash.EMPTY, UInt256.ONE);
+  private final ChainHead testChainHead = new ChainHead(Hash.EMPTY, UInt256.ONE, 1L);
   private final GenesisConfigOptions genesisConfigOptions =
       new StubGenesisConfigOptions().chainId(BigInteger.valueOf(2019));
   private final DefaultPeer defaultPeer =