From f3b8b04927bf7f6b03369fb8914e298e59570202 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 16 Oct 2019 18:45:25 -0400 Subject: [PATCH] Only set sync targets that have an estimated height value Signed-off-by: Meredith Baxter --- .../besu/ethereum/eth/manager/ChainState.java | 2 +- .../sync/fastsync/FastSyncTargetManager.java | 2 +- .../sync/fullsync/FullSyncTargetManager.java | 2 +- .../ethereum/eth/manager/ChainStateTest.java | 14 +++++++++ .../fullsync/FullSyncTargetManagerTest.java | 29 +++++++++++++++++++ 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ChainState.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ChainState.java index c8671509bee..33924780673 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ChainState.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ChainState.java @@ -103,8 +103,8 @@ public void updateForAnnouncedBlock( public void updateHeightEstimate(final long blockNumber) { synchronized (this) { - estimatedHeightKnown = true; if (blockNumber > estimatedHeight) { + estimatedHeightKnown = true; estimatedHeight = blockNumber; estimatedHeightListeners.forEach(e -> e.onEstimatedHeightChanged(estimatedHeight)); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncTargetManager.java index 1c64a18a6ac..60dbbe63be3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncTargetManager.java @@ -61,7 +61,7 @@ public FastSyncTargetManager( @Override protected CompletableFuture> selectBestAvailableSyncTarget() { - final Optional maybeBestPeer = ethContext.getEthPeers().bestPeer(); + final Optional maybeBestPeer = ethContext.getEthPeers().bestPeerWithHeightEstimate(); if (!maybeBestPeer.isPresent()) { LOG.info("No sync target, wait for peers."); return completedFuture(Optional.empty()); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java index e8a60419bee..9aa41a2a682 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManager.java @@ -70,7 +70,7 @@ protected Optional finalizeSelectedSyncTarget(final SyncTarget syncT @Override protected CompletableFuture> selectBestAvailableSyncTarget() { - final Optional maybeBestPeer = ethContext.getEthPeers().bestPeer(); + final Optional maybeBestPeer = ethContext.getEthPeers().bestPeerWithHeightEstimate(); if (!maybeBestPeer.isPresent()) { LOG.info("No sync target, wait for peers."); return completedFuture(Optional.empty()); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ChainStateTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ChainStateTest.java index ec2030d7f60..3f8a70af9db 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ChainStateTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ChainStateTest.java @@ -43,6 +43,20 @@ public void statusReceivedUpdatesBestBlock() { assertThat(chainState.getBestBlock().getTotalDifficulty()).isEqualTo(INITIAL_TOTAL_DIFFICULTY); } + @Test + public void updateHeightEstimate_toZero() { + chainState.updateHeightEstimate(0L); + assertThat(chainState.hasEstimatedHeight()).isFalse(); + assertThat(chainState.getEstimatedHeight()).isEqualTo(0L); + } + + @Test + public void updateHeightEstimate_toNonZeroValue() { + chainState.updateHeightEstimate(1L); + assertThat(chainState.hasEstimatedHeight()).isTrue(); + assertThat(chainState.getEstimatedHeight()).isEqualTo(1L); + } + @Test public void updateBestBlockAndHeightFromHashAndHeight() { final long blockNumber = 12; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManagerTest.java index 86f711f590e..9a3cf1428bb 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTargetManagerTest.java @@ -33,6 +33,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; +import org.hyperledger.besu.util.uint.UInt256; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -82,6 +83,34 @@ public void tearDown() { ethProtocolManager.stop(); } + @Test + public void findSyncTarget_withHeightEstimates() { + when(localWorldState.isWorldStateAvailable(localBlockchain.getChainHeadHeader().getStateRoot())) + .thenReturn(true); + final RespondingEthPeer bestPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, UInt256.MAX_VALUE, 1); + + final CompletableFuture result = syncTargetManager.findSyncTarget(Optional.empty()); + bestPeer.respond(responder); + + assertThat(result) + .isCompletedWithValue( + new SyncTarget(bestPeer.getEthPeer(), localBlockchain.getBlockHeader(4L).get())); + } + + @Test + public void findSyncTarget_noHeightEstimates() { + when(localWorldState.isWorldStateAvailable(localBlockchain.getChainHeadHeader().getStateRoot())) + .thenReturn(true); + final RespondingEthPeer bestPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, UInt256.MAX_VALUE, 0); + + final CompletableFuture result = syncTargetManager.findSyncTarget(Optional.empty()); + bestPeer.respond(responder); + + assertThat(result).isNotCompleted(); + } + @Test public void shouldDisconnectPeerIfWorldStateIsUnavailableForCommonAncestor() { when(localWorldState.isWorldStateAvailable(localBlockchain.getChainHeadHeader().getStateRoot()))