diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/ChainHead.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/ChainHead.java index b1b977feb8..d2993502f0 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/ChainHead.java +++ b/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; + } } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/DefaultMutableBlockchain.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/DefaultMutableBlockchain.java index 32a013dac9..1ca176de88 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/DefaultMutableBlockchain.java +++ b/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 diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainState.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainState.java index 17ca77f633..73ae6154a0 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainState.java +++ b/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) 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 index a3deb47aa3..4a5dfc5fed 100644 --- 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 @@ -30,8 +30,7 @@ import java.util.stream.Stream; public class EthPeers { public static final Comparator TOTAL_DIFFICULTY = - Comparator.comparing( - ((final EthPeer p) -> p.chainState().getBestBlock().getTotalDifficulty())); + Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedTotalDifficulty())); public static final Comparator CHAIN_HEIGHT = Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedHeight())); diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java index c456d48ce6..1ffe4b33ab 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java +++ b/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; 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 8a9982e67e..97a90a223c 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 @@ -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 extends SyncTargetManager { } 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 diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncState.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncState.java index c11c7fe3fc..7f93d354b5 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncState.java +++ b/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() { diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncTarget.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncTarget.java index e56d25e090..ea78f0121a 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncTarget.java +++ b/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(); } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainStateTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainStateTest.java index d462ff891a..5a747ff0c5 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainStateTest.java +++ b/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); + } } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncStateTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncStateTest.java index d9b39bf979..f5caa86a94 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/state/SyncStateTest.java +++ b/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 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(); } } diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java index 79f7b58cc7..0770301288 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java +++ b/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 =