From 8f3f01efb253c96a9fc16d68c46ff5dbe5bc3927 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Mon, 14 Feb 2022 13:45:26 +0000 Subject: [PATCH 1/9] FullSync Future should stop when total terminal difficulty is reached Signed-off-by: Jiri Peinlich --- .../controller/BesuControllerBuilder.java | 8 +- .../eth/sync/DefaultSynchronizer.java | 12 +- .../fullsync/FullSyncChainDownloader.java | 13 +- .../eth/sync/fullsync/FullSyncDownloader.java | 14 +- .../sync/fullsync/FullSyncTargetManager.java | 11 +- .../FullSyncChainDownloaderForkTest.java | 10 +- .../fullsync/FullSyncChainDownloaderTest.java | 8 +- ...DownloaderTotalTerminalDifficultyTest.java | 181 ++++++++++++++++++ .../sync/fullsync/FullSyncDownloaderTest.java | 9 +- .../fullsync/FullSyncTargetManagerTest.java | 4 +- 10 files changed, 257 insertions(+), 13 deletions(-) create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 336ea1c8ba2..66206652a6c 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -32,6 +32,7 @@ import org.hyperledger.besu.ethereum.chain.DefaultBlockchain; import org.hyperledger.besu.ethereum.chain.GenesisState; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.Synchronizer; @@ -371,7 +372,8 @@ public BesuController build() { syncState, dataDirectory, clock, - metricsSystem); + metricsSystem, + getTerminalTotalDifficulty()); final MiningCoordinator miningCoordinator = createMiningCoordinator( @@ -416,6 +418,10 @@ public BesuController build() { additionalPluginServices); } + protected Optional getTerminalTotalDifficulty() { + return genesisConfig.getConfigOptions().getTerminalTotalDifficulty().map(Difficulty::of); + } + protected void prepForBuild() {} protected JsonRpcMethods createAdditionalJsonRpcMethodFactory( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index 59440e65d4c..3d735574d9e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastDownloaderFactory; @@ -66,7 +67,8 @@ public DefaultSynchronizer( final SyncState syncState, final Path dataDirectory, final Clock clock, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Optional terminalTotalDifficulty) { this.maybePruner = maybePruner; this.syncState = syncState; @@ -91,7 +93,13 @@ public DefaultSynchronizer( this.fullSyncDownloader = new FullSyncDownloader( - syncConfig, protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + terminalTotalDifficulty); this.fastSyncDownloader = FastDownloaderFactory.create( syncConfig, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java index 9ae54249a11..08e089e86bb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PipelineChainDownloader; @@ -23,6 +24,8 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.util.Optional; + public class FullSyncChainDownloader { private FullSyncChainDownloader() {} @@ -32,11 +35,17 @@ public static ChainDownloader create( final ProtocolContext protocolContext, final EthContext ethContext, final SyncState syncState, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Optional terminalTotalDifficulty) { final FullSyncTargetManager syncTargetManager = new FullSyncTargetManager( - config, protocolSchedule, protocolContext, ethContext, metricsSystem); + config, + protocolSchedule, + protocolContext, + ethContext, + metricsSystem, + terminalTotalDifficulty); return new PipelineChainDownloader( syncState, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java index cdcbf861aec..0e3a39719df 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -23,6 +24,8 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.util.Optional; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,14 +43,21 @@ public FullSyncDownloader( final ProtocolContext protocolContext, final EthContext ethContext, final SyncState syncState, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Optional terminalTotalDifficulty) { this.syncConfig = syncConfig; this.protocolContext = protocolContext; this.syncState = syncState; this.chainDownloader = FullSyncChainDownloader.create( - syncConfig, protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + terminalTotalDifficulty); } public void start() { 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 404a595de8f..1671484f113 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 @@ -19,6 +19,7 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.sync.SyncTargetManager; @@ -39,16 +40,19 @@ class FullSyncTargetManager extends SyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(FullSyncTargetManager.class); private final ProtocolContext protocolContext; private final EthContext ethContext; + private final Optional terminalTotalDifficulty; FullSyncTargetManager( final SynchronizerConfiguration config, final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Optional terminalTotalDifficulty) { super(config, protocolSchedule, protocolContext, ethContext, metricsSystem); this.protocolContext = protocolContext; this.ethContext = ethContext; + this.terminalTotalDifficulty = terminalTotalDifficulty; } @Override @@ -105,6 +109,9 @@ private boolean isSyncTargetReached(final EthPeer peer) { @Override public boolean shouldContinueDownloading() { - return true; + return terminalTotalDifficulty.isEmpty() + || terminalTotalDifficulty + .get() + .greaterThan(protocolContext.getBlockchain().getChainHead().getTotalDifficulty()); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java index b87bea35690..0a1cb1c125f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java @@ -35,6 +35,8 @@ import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.util.Optional; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -80,7 +82,13 @@ public void tearDown() { private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { return FullSyncChainDownloader.create( - syncConfig, protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + Optional.empty()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java index 0fd530673ac..64d52989323 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java @@ -117,7 +117,13 @@ public void tearDown() { private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { return FullSyncChainDownloader.create( - syncConfig, protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + Optional.empty()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java new file mode 100644 index 00000000000..7168cb382e9 --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java @@ -0,0 +1,181 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.eth.sync.fullsync; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; +import org.hyperledger.besu.ethereum.core.Difficulty; +import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; +import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; +import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; +import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; +import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat; +import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; +import org.hyperledger.besu.plugin.services.MetricsSystem; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class FullSyncChainDownloaderTotalTerminalDifficultyTest { + + protected ProtocolSchedule protocolSchedule; + protected EthProtocolManager ethProtocolManager; + protected EthContext ethContext; + protected ProtocolContext protocolContext; + private SyncState syncState; + + private BlockchainSetupUtil localBlockchainSetup; + protected MutableBlockchain localBlockchain; + private BlockchainSetupUtil otherBlockchainSetup; + protected Blockchain otherBlockchain; + private final MetricsSystem metricsSystem = new NoOpMetricsSystem(); + private static final Difficulty TARGET_TERMINAL_DIFFICULTY = Difficulty.of(1_000_000L); + + @Parameters + public static Collection data() { + return Arrays.asList(new Object[][] {{DataStorageFormat.BONSAI}, {DataStorageFormat.FOREST}}); + } + + private final DataStorageFormat storageFormat; + + public FullSyncChainDownloaderTotalTerminalDifficultyTest(final DataStorageFormat storageFormat) { + this.storageFormat = storageFormat; + } + + @Before + public void setupTest() { + localBlockchainSetup = BlockchainSetupUtil.forTesting(storageFormat); + localBlockchain = localBlockchainSetup.getBlockchain(); + otherBlockchainSetup = BlockchainSetupUtil.forTesting(storageFormat); + otherBlockchain = otherBlockchainSetup.getBlockchain(); + + protocolSchedule = localBlockchainSetup.getProtocolSchedule(); + protocolContext = localBlockchainSetup.getProtocolContext(); + ethProtocolManager = + EthProtocolManagerTestUtil.create( + localBlockchain, + new EthScheduler(1, 1, 1, 1, new NoOpMetricsSystem()), + localBlockchainSetup.getWorldArchive(), + localBlockchainSetup.getTransactionPool(), + EthProtocolConfiguration.defaultConfig()); + ethContext = ethProtocolManager.ethContext(); + syncState = new SyncState(protocolContext.getBlockchain(), ethContext.getEthPeers()); + } + + @After + public void tearDown() { + ethProtocolManager.stop(); + } + + private ChainDownloader downloader( + final SynchronizerConfiguration syncConfig, + final Optional targetTerminalDifficulty1) { + return FullSyncChainDownloader.create( + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + targetTerminalDifficulty1); + } + + private SynchronizerConfiguration.Builder syncConfigBuilder() { + return SynchronizerConfiguration.builder(); + } + + @Test + public void syncsFullyAndStopsWhenTTDReached() { + otherBlockchainSetup.importFirstBlocks(30); + final long targetBlock = otherBlockchain.getChainHeadBlockNumber(); + // Sanity check + assertThat(targetBlock).isGreaterThan(localBlockchain.getChainHeadBlockNumber()); + + final RespondingEthPeer peer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, otherBlockchain); + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(otherBlockchain); + + final SynchronizerConfiguration syncConfig = + syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); + final ChainDownloader downloader = + downloader(syncConfig, Optional.of(TARGET_TERMINAL_DIFFICULTY)); + final CompletableFuture future = downloader.start(); + + assertThat(future.isDone()).isFalse(); + + peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); + assertThat(syncState.syncTarget()).isPresent(); + assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); + + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); + + assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); + + assertThat(future.isDone()).isTrue(); + } + + @Test + public void syncsFullyAndContinuesWhenTTDNotSpecified() { + otherBlockchainSetup.importFirstBlocks(30); + final long targetBlock = otherBlockchain.getChainHeadBlockNumber(); + // Sanity check + assertThat(targetBlock).isGreaterThan(localBlockchain.getChainHeadBlockNumber()); + + final RespondingEthPeer peer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, otherBlockchain); + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(otherBlockchain); + + final SynchronizerConfiguration syncConfig = + syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); + final ChainDownloader downloader = downloader(syncConfig, Optional.empty()); + final CompletableFuture future = downloader.start(); + + assertThat(future.isDone()).isFalse(); + + peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); + assertThat(syncState.syncTarget()).isPresent(); + assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); + + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); + + assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); + + assertThat(future.isDone()).isFalse(); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index bab67345bd3..494abd37603 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java @@ -35,6 +35,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Optional; import org.junit.After; import org.junit.Before; @@ -92,7 +93,13 @@ public void tearDown() { private FullSyncDownloader downloader(final SynchronizerConfiguration syncConfig) { return new FullSyncDownloader( - syncConfig, protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + metricsSystem, + Optional.empty()); } @Test 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 e99fff3a1f9..12324396b26 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 @@ -40,6 +40,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.junit.After; @@ -97,7 +98,8 @@ public void setup() { protocolSchedule, protocolContext, ethContext, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + Optional.empty()); } @After From 4663e5014f22542961599e0c3d04476a1be67bba Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Fri, 18 Feb 2022 10:58:54 +0000 Subject: [PATCH 2/9] stopping related services when full sync stops Signed-off-by: Jiri Peinlich --- .../besu/ethereum/core/Synchronizer.java | 3 +- .../ethereum/eth/manager/EthMessages.java | 12 ++- .../eth/sync/BlockPropagationManager.java | 38 ++++++++-- .../eth/sync/DefaultSynchronizer.java | 75 ++++++++++--------- .../eth/sync/fullsync/FullSyncDownloader.java | 5 +- .../FullSyncTerminationCondition.java | 2 + .../ethereum/retesteth/DummySynchronizer.java | 5 +- 7 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java index 24f60f0ae80..72f9de06dc5 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.plugin.services.BesuEvents; import java.util.Optional; +import java.util.concurrent.CompletableFuture; /** Provides an interface to block synchronization processes. */ public interface Synchronizer { @@ -25,7 +26,7 @@ public interface Synchronizer { // Default tolerance used to determine whether or not this node is "in sync" long DEFAULT_IN_SYNC_TOLERANCE = 5; - void start(); + CompletableFuture start(); void stop(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthMessages.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthMessages.java index ae4cee41a0a..f1eca234bc0 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthMessages.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthMessages.java @@ -41,8 +41,16 @@ public Optional dispatch(final EthMessage ethMessage) { messageResponseConstructor.response(ethMessage.getData())); } - public void subscribe(final int messageCode, final MessageCallback callback) { - listenersByCode.computeIfAbsent(messageCode, key -> Subscribers.create()).subscribe(callback); + public long subscribe(final int messageCode, final MessageCallback callback) { + return listenersByCode + .computeIfAbsent(messageCode, key -> Subscribers.create()) + .subscribe(callback); + } + + public void unsubsribe(final long id) { + for (Subscribers subscribers : listenersByCode.values()) { + subscribers.unsubscribe(id); + } } public void registerResponseConstructor( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java index bcde21b4944..8eacb2a1ccc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java @@ -82,6 +82,9 @@ public class BlockPropagationManager { private final Set requestedNonAnnouncedBlocks = Collections.newSetFromMap(new ConcurrentHashMap<>()); private final PendingBlocksManager pendingBlocksManager; + private Optional onBlockAddedSId = Optional.empty(); + private Optional newBlockSId; + private Optional newBlockHashesSId; BlockPropagationManager( final SynchronizerConfiguration config, @@ -111,12 +114,37 @@ public void start() { } } + public void stop() { + if (started.get()) { + clearListeners(); + started.set(false); + } else { + LOG.info("Attempted to stop when we are not even running..."); + } + } + private void setupListeners() { - protocolContext.getBlockchain().observeBlockAdded(this::onBlockAdded); - ethContext.getEthMessages().subscribe(EthPV62.NEW_BLOCK, this::handleNewBlockFromNetwork); - ethContext - .getEthMessages() - .subscribe(EthPV62.NEW_BLOCK_HASHES, this::handleNewBlockHashesFromNetwork); + onBlockAddedSId = + Optional.of(protocolContext.getBlockchain().observeBlockAdded(this::onBlockAdded)); + newBlockSId = + Optional.of( + ethContext + .getEthMessages() + .subscribe(EthPV62.NEW_BLOCK, this::handleNewBlockFromNetwork)); + newBlockHashesSId = + Optional.of( + ethContext + .getEthMessages() + .subscribe(EthPV62.NEW_BLOCK_HASHES, this::handleNewBlockHashesFromNetwork)); + } + + private void clearListeners() { + onBlockAddedSId.ifPresent(id -> protocolContext.getBlockchain().removeObserver(id)); + newBlockSId.ifPresent(id -> ethContext.getEthMessages().unsubsribe(id)); + newBlockHashesSId.ifPresent(id -> ethContext.getEthMessages().unsubsribe(id)); + onBlockAddedSId = Optional.empty(); + newBlockSId = Optional.empty(); + newBlockHashesSId = Optional.empty(); } private void onBlockAdded(final BlockAddedEvent blockAddedEvent) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index 3d735574d9e..dedc8a67c91 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -22,7 +22,6 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastDownloaderFactory; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncDownloader; -import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncException; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncDownloader; import org.hyperledger.besu.ethereum.eth.sync.state.PendingBlocksManager; @@ -34,11 +33,11 @@ import org.hyperledger.besu.plugin.data.SyncStatus; import org.hyperledger.besu.plugin.services.BesuEvents.SyncStatusListener; import org.hyperledger.besu.plugin.services.MetricsSystem; -import org.hyperledger.besu.util.ExceptionUtils; import java.nio.file.Path; import java.time.Clock; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; @@ -131,25 +130,25 @@ private TrailingPeerRequirements calculateTrailingPeerRequirements() { } @Override - public void start() { + public CompletableFuture start() { if (running.compareAndSet(false, true)) { LOG.info("Starting synchronizer."); blockPropagationManager.start(); + CompletableFuture future; if (fastSyncDownloader.isPresent()) { - fastSyncDownloader - .get() - .start() - .whenComplete(this::handleFastSyncResult) - .exceptionally( - ex -> { - LOG.warn("Exiting FastSync process"); - System.exit(0); - return null; - }); + future = fastSyncDownloader.get().start().thenCompose(this::handleFastSyncResult); } else { - startFullSync(); + future = startFullSync(); } + future = + future.thenApply( + unused -> { + blockPropagationManager.stop(); + running.set(false); + return null; + }); + return future; } else { throw new IllegalStateException("Attempt to start an already started synchronizer."); } @@ -162,6 +161,7 @@ public void stop() { fastSyncDownloader.ifPresent(FastSyncDownloader::stop); fullSyncDownloader.stop(); maybePruner.ifPresent(Pruner::stop); + blockPropagationManager.stop(); } } @@ -172,36 +172,37 @@ public void awaitStop() throws InterruptedException { } } - private void handleFastSyncResult(final FastSyncState result, final Throwable error) { + private CompletableFuture handleFastSyncResult(final FastSyncState result) { if (!running.get()) { // We've been shutdown which will have triggered the fast sync future to complete - return; + return CompletableFuture.completedFuture(null); } fastSyncDownloader.ifPresent(FastSyncDownloader::deleteFastSyncState); - final Throwable rootCause = ExceptionUtils.rootCause(error); - if (rootCause instanceof FastSyncException) { - LOG.error( - "Fast sync failed ({}), please try again.", ((FastSyncException) rootCause).getError()); - throw new FastSyncException(rootCause); - } else if (error != null) { - LOG.error("Fast sync failed, please try again.", error); - throw new FastSyncException(error); - } else { - result - .getPivotBlockHeader() - .ifPresent( - blockHeader -> - protocolContext.getWorldStateArchive().setArchiveStateUnSafe(blockHeader)); - LOG.info( - "Fast sync completed successfully with pivot block {}", - result.getPivotBlockNumber().getAsLong()); - } - startFullSync(); + result + .getPivotBlockHeader() + .ifPresent( + blockHeader -> + protocolContext.getWorldStateArchive().setArchiveStateUnSafe(blockHeader)); + LOG.info( + "Fast sync completed successfully with pivot block {}", + result.getPivotBlockNumber().getAsLong()); + return startFullSync(); } - private void startFullSync() { + private CompletableFuture startFullSync() { maybePruner.ifPresent(Pruner::start); - fullSyncDownloader.start(); + return fullSyncDownloader + .start() + .thenCompose( + unused -> { + maybePruner.ifPresent(Pruner::stop); + return null; + }) + .thenApply( + o -> { + maybePruner.ifPresent(Pruner::stop); + return null; + }); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java index 0e3a39719df..8911f5eb37b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java @@ -25,6 +25,7 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,9 +61,9 @@ public FullSyncDownloader( terminalTotalDifficulty); } - public void start() { + public CompletableFuture start() { LOG.info("Starting full sync."); - chainDownloader.start(); + return chainDownloader.start(); } public void stop() { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java new file mode 100644 index 00000000000..4154ff2ade8 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java @@ -0,0 +1,2 @@ +package org.hyperledger.besu.ethereum.eth.sync.fullsync;public interface FullSyncTerminationCondition { +} diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java index 857df96d158..1f94d49e246 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.plugin.services.BesuEvents; import java.util.Optional; +import java.util.concurrent.CompletableFuture; /** * Naive implementation of Synchronizer used by retesteth. Because retesteth is not implemented in @@ -28,7 +29,9 @@ */ public class DummySynchronizer implements Synchronizer { @Override - public void start() {} + public CompletableFuture start() { + return CompletableFuture.completedFuture(null); + } @Override public void stop() {} From aed91a9b7ef97e6787d2f588050c327f365d252d Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Fri, 18 Feb 2022 11:37:57 +0000 Subject: [PATCH 3/9] Generalising the full sync termination condition in the future we want to be able to specify a specific block hash as a termination condition. The FullSyncTerminationCondition interface should allow it Signed-off-by: Jiri Peinlich --- .../controller/BesuControllerBuilder.java | 12 ++++++---- .../eth/sync/DefaultSynchronizer.java | 6 ++--- .../fullsync/FullSyncChainDownloader.java | 7 ++---- .../eth/sync/fullsync/FullSyncDownloader.java | 6 ++--- .../sync/fullsync/FullSyncTargetManager.java | 12 ++++------ .../FullSyncTerminationCondition.java | 22 ++++++++++++++++++- .../FullSyncChainDownloaderForkTest.java | 4 +--- .../fullsync/FullSyncChainDownloaderTest.java | 2 +- ...DownloaderTotalTerminalDifficultyTest.java | 11 +++++----- .../sync/fullsync/FullSyncDownloaderTest.java | 3 +-- .../fullsync/FullSyncTargetManagerTest.java | 3 +-- 11 files changed, 49 insertions(+), 39 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 66206652a6c..1bac2c102e6 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -32,7 +32,6 @@ import org.hyperledger.besu.ethereum.chain.DefaultBlockchain; import org.hyperledger.besu.ethereum.chain.GenesisState; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; -import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.Synchronizer; @@ -52,6 +51,7 @@ import org.hyperledger.besu.ethereum.eth.sync.DefaultSynchronizer; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncTerminationCondition; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -373,7 +373,7 @@ public BesuController build() { dataDirectory, clock, metricsSystem, - getTerminalTotalDifficulty()); + getFullSyncTerminationCondition()); final MiningCoordinator miningCoordinator = createMiningCoordinator( @@ -418,8 +418,12 @@ public BesuController build() { additionalPluginServices); } - protected Optional getTerminalTotalDifficulty() { - return genesisConfig.getConfigOptions().getTerminalTotalDifficulty().map(Difficulty::of); + protected FullSyncTerminationCondition getFullSyncTerminationCondition() { + return genesisConfig + .getConfigOptions() + .getTerminalTotalDifficulty() + .map(FullSyncTerminationCondition::difficulty) + .orElse(FullSyncTerminationCondition.never()); } protected void prepForBuild() {} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index dedc8a67c91..560cf2eb20a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -17,13 +17,13 @@ import static com.google.common.base.Preconditions.checkNotNull; import org.hyperledger.besu.ethereum.ProtocolContext; -import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastDownloaderFactory; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncDownloader; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncDownloader; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncTerminationCondition; import org.hyperledger.besu.ethereum.eth.sync.state.PendingBlocksManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -67,7 +67,7 @@ public DefaultSynchronizer( final Path dataDirectory, final Clock clock, final MetricsSystem metricsSystem, - final Optional terminalTotalDifficulty) { + final FullSyncTerminationCondition terminationCondition) { this.maybePruner = maybePruner; this.syncState = syncState; @@ -98,7 +98,7 @@ public DefaultSynchronizer( ethContext, syncState, metricsSystem, - terminalTotalDifficulty); + terminationCondition); this.fastSyncDownloader = FastDownloaderFactory.create( syncConfig, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java index 08e089e86bb..6b46336c7f7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.ethereum.ProtocolContext; -import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PipelineChainDownloader; @@ -24,8 +23,6 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Optional; - public class FullSyncChainDownloader { private FullSyncChainDownloader() {} @@ -36,7 +33,7 @@ public static ChainDownloader create( final EthContext ethContext, final SyncState syncState, final MetricsSystem metricsSystem, - final Optional terminalTotalDifficulty) { + final FullSyncTerminationCondition terminationCondition) { final FullSyncTargetManager syncTargetManager = new FullSyncTargetManager( @@ -45,7 +42,7 @@ public static ChainDownloader create( protocolContext, ethContext, metricsSystem, - terminalTotalDifficulty); + terminationCondition); return new PipelineChainDownloader( syncState, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java index 8911f5eb37b..32c7dd64294 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.ethereum.ProtocolContext; -import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -24,7 +23,6 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; @@ -45,7 +43,7 @@ public FullSyncDownloader( final EthContext ethContext, final SyncState syncState, final MetricsSystem metricsSystem, - final Optional terminalTotalDifficulty) { + final FullSyncTerminationCondition terminationCondition) { this.syncConfig = syncConfig; this.protocolContext = protocolContext; this.syncState = syncState; @@ -58,7 +56,7 @@ public FullSyncDownloader( ethContext, syncState, metricsSystem, - terminalTotalDifficulty); + terminationCondition); } public CompletableFuture start() { 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 1671484f113..89f3e0f8004 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 @@ -19,7 +19,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.sync.SyncTargetManager; @@ -40,7 +39,7 @@ class FullSyncTargetManager extends SyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(FullSyncTargetManager.class); private final ProtocolContext protocolContext; private final EthContext ethContext; - private final Optional terminalTotalDifficulty; + private final FullSyncTerminationCondition terminationCondition; FullSyncTargetManager( final SynchronizerConfiguration config, @@ -48,11 +47,11 @@ class FullSyncTargetManager extends SyncTargetManager { final ProtocolContext protocolContext, final EthContext ethContext, final MetricsSystem metricsSystem, - final Optional terminalTotalDifficulty) { + final FullSyncTerminationCondition terminationCondition) { super(config, protocolSchedule, protocolContext, ethContext, metricsSystem); this.protocolContext = protocolContext; this.ethContext = ethContext; - this.terminalTotalDifficulty = terminalTotalDifficulty; + this.terminationCondition = terminationCondition; } @Override @@ -109,9 +108,6 @@ private boolean isSyncTargetReached(final EthPeer peer) { @Override public boolean shouldContinueDownloading() { - return terminalTotalDifficulty.isEmpty() - || terminalTotalDifficulty - .get() - .greaterThan(protocolContext.getBlockchain().getChainHead().getTotalDifficulty()); + return terminationCondition.test(protocolContext.getBlockchain()); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java index 4154ff2ade8..15e4fc59cb0 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java @@ -1,2 +1,22 @@ -package org.hyperledger.besu.ethereum.eth.sync.fullsync;public interface FullSyncTerminationCondition { +package org.hyperledger.besu.ethereum.eth.sync.fullsync; + +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.core.Difficulty; + +import java.util.function.Predicate; + +import org.apache.tuweni.units.bigints.UInt256; + +public interface FullSyncTerminationCondition extends Predicate { + static FullSyncTerminationCondition never() { + return blockchain -> false; + } + + static FullSyncTerminationCondition difficulty(final UInt256 difficulty) { + return difficulty(Difficulty.of(difficulty)); + } + + static FullSyncTerminationCondition difficulty(final Difficulty difficulty) { + return blockchain -> difficulty.greaterThan(blockchain.getChainHead().getTotalDifficulty()); + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java index 0a1cb1c125f..237e7d7e536 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java @@ -35,8 +35,6 @@ import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Optional; - import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -88,7 +86,7 @@ private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { ethContext, syncState, metricsSystem, - Optional.empty()); + FullSyncTerminationCondition.never()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java index 64d52989323..09f0f60c3ae 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java @@ -123,7 +123,7 @@ private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { ethContext, syncState, metricsSystem, - Optional.empty()); + FullSyncTerminationCondition.never()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java index 7168cb382e9..6557d00e6d6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java @@ -37,7 +37,6 @@ import java.util.Arrays; import java.util.Collection; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.junit.After; @@ -101,7 +100,7 @@ public void tearDown() { private ChainDownloader downloader( final SynchronizerConfiguration syncConfig, - final Optional targetTerminalDifficulty1) { + final FullSyncTerminationCondition terminalCondition) { return FullSyncChainDownloader.create( syncConfig, protocolSchedule, @@ -109,7 +108,7 @@ private ChainDownloader downloader( ethContext, syncState, metricsSystem, - targetTerminalDifficulty1); + terminalCondition); } private SynchronizerConfiguration.Builder syncConfigBuilder() { @@ -131,12 +130,12 @@ public void syncsFullyAndStopsWhenTTDReached() { final SynchronizerConfiguration syncConfig = syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); final ChainDownloader downloader = - downloader(syncConfig, Optional.of(TARGET_TERMINAL_DIFFICULTY)); + downloader(syncConfig, FullSyncTerminationCondition.difficulty(TARGET_TERMINAL_DIFFICULTY)); final CompletableFuture future = downloader.start(); assertThat(future.isDone()).isFalse(); - peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); + peer.respondWhileOtherThreadsWork(responder, () -> syncState.syncTarget().isEmpty()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); @@ -162,7 +161,7 @@ public void syncsFullyAndContinuesWhenTTDNotSpecified() { final SynchronizerConfiguration syncConfig = syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); - final ChainDownloader downloader = downloader(syncConfig, Optional.empty()); + final ChainDownloader downloader = downloader(syncConfig, FullSyncTerminationCondition.never()); final CompletableFuture future = downloader.start(); assertThat(future.isDone()).isFalse(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index 494abd37603..d39f8e1a051 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java @@ -35,7 +35,6 @@ import java.util.Arrays; import java.util.Collection; -import java.util.Optional; import org.junit.After; import org.junit.Before; @@ -99,7 +98,7 @@ private FullSyncDownloader downloader(final SynchronizerConfiguration syncConfig ethContext, syncState, metricsSystem, - Optional.empty()); + FullSyncTerminationCondition.never()); } @Test 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 12324396b26..7473d231d05 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 @@ -40,7 +40,6 @@ import java.util.Arrays; import java.util.Collection; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.junit.After; @@ -99,7 +98,7 @@ public void setup() { protocolContext, ethContext, new NoOpMetricsSystem(), - Optional.empty()); + FullSyncTerminationCondition.never()); } @After From 313b558f0d0c41136812db2364a5fd0a4ff3743e Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Fri, 18 Feb 2022 12:56:11 +0000 Subject: [PATCH 4/9] adding missing header Signed-off-by: Jiri Peinlich --- .../fullsync/FullSyncTerminationCondition.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java index 15e4fc59cb0..5e63c23cc5e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java @@ -1,3 +1,17 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.ethereum.chain.Blockchain; From d56ed1dbbf3321c0c28b981aeb921ec41ea8c829 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Mon, 21 Feb 2022 16:31:08 +0000 Subject: [PATCH 5/9] Full sync should stop when required Signed-off-by: Jiri Peinlich --- .../besu/ethereum/eth/sync/PipelineChainDownloader.java | 3 +++ .../ethereum/eth/sync/PipelineChainDownloaderTest.java | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloader.java index f32804ba7f4..aaeb16f2c1b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloader.java @@ -153,6 +153,9 @@ private synchronized CompletionStage startDownloadForSyncTarget(final Sync return CompletableFuture.failedFuture( new CancellationException("Chain download was cancelled")); } + if (!syncTargetManager.shouldContinueDownloading()) { + return CompletableFuture.completedFuture(null); + } syncState.setSyncTarget(target.peer(), target.commonAncestor()); currentDownloadPipeline = downloadPipelineFactory.createDownloadPipelineForSyncTarget(target); debugLambda( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java index 60297938bbf..0505bc7509c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java @@ -46,6 +46,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) @@ -91,6 +92,7 @@ public void shouldSelectSyncTargetWhenStarted() { public void shouldStartChainDownloadWhenTargetSelected() { final CompletableFuture selectTargetFuture = new CompletableFuture<>(); when(syncTargetManager.findSyncTarget()).thenReturn(selectTargetFuture); + when(syncTargetManager.shouldContinueDownloading()).thenReturn(true); expectPipelineCreation(syncTarget, downloadPipeline); when(scheduler.startPipeline(downloadPipeline)).thenReturn(new CompletableFuture<>()); chainDownloader.start(); @@ -106,11 +108,11 @@ public void shouldStartChainDownloadWhenTargetSelected() { public void shouldUpdateSyncStateWhenTargetSelected() { final CompletableFuture selectTargetFuture = new CompletableFuture<>(); when(syncTargetManager.findSyncTarget()).thenReturn(selectTargetFuture); + when(syncTargetManager.shouldContinueDownloading()).thenReturn(true); expectPipelineCreation(syncTarget, downloadPipeline); when(scheduler.startPipeline(downloadPipeline)).thenReturn(new CompletableFuture<>()); chainDownloader.start(); verifyNoInteractions(downloadPipelineFactory); - selectTargetFuture.complete(syncTarget); verify(syncState).setSyncTarget(peer1, commonAncestor); @@ -159,7 +161,7 @@ public void shouldBeCompleteWhenPipelineCompletesAndSyncTargetManagerShouldNotCo when(syncTargetManager.shouldContinueDownloading()).thenReturn(false); pipelineFuture.complete(null); - verify(syncTargetManager).shouldContinueDownloading(); + verify(syncTargetManager, Mockito.times(2)).shouldContinueDownloading(); verify(syncState).clearSyncTarget(); verifyNoMoreInteractions(syncTargetManager); assertThat(result).isCompleted(); @@ -187,6 +189,7 @@ public void shouldSelectNewSyncTargetWhenPipelineCompletesIfSyncTargetManagerSho @Test public void shouldNotNestExceptionHandling() { when(syncTargetManager.shouldContinueDownloading()) + .thenReturn(true) .thenReturn(true) // Allow continuing after first successful download .thenReturn(false); // But not after finding the second sync target fails @@ -213,7 +216,7 @@ public void shouldNotNestExceptionHandling() { // Should only need to check if it should continue twice. // We'll wind up doing this check more than necessary if we keep wrapping additional exception // handlers when restarting the sequence which wastes memory. - verify(syncTargetManager, times(2)).shouldContinueDownloading(); + verify(syncTargetManager, times(3)).shouldContinueDownloading(); } @Test From bea07129a10c5834c360eabdbf0a2128ff8314a9 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Wed, 23 Feb 2022 12:00:16 +0000 Subject: [PATCH 6/9] Terminal Condition is now a Boolean Provider Signed-off-by: Jiri Peinlich --- .../controller/BesuControllerBuilder.java | 7 +++--- .../FlexibleBlockHashTerminalCondition.java | 23 +++++++++++++++++++ .../sync/fullsync/FullSyncTargetManager.java | 2 +- .../FullSyncTerminationCondition.java | 22 ++++++++++++------ ...DownloaderTotalTerminalDifficultyTest.java | 4 +++- 5 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 1bac2c102e6..9f30c7045ac 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -373,7 +373,7 @@ public BesuController build() { dataDirectory, clock, metricsSystem, - getFullSyncTerminationCondition()); + getFullSyncTerminationCondition(protocolContext.getBlockchain())); final MiningCoordinator miningCoordinator = createMiningCoordinator( @@ -418,11 +418,12 @@ public BesuController build() { additionalPluginServices); } - protected FullSyncTerminationCondition getFullSyncTerminationCondition() { + protected FullSyncTerminationCondition getFullSyncTerminationCondition( + final Blockchain blockchain) { return genesisConfig .getConfigOptions() .getTerminalTotalDifficulty() - .map(FullSyncTerminationCondition::difficulty) + .map(difficulty -> FullSyncTerminationCondition.difficulty(difficulty, blockchain)) .orElse(FullSyncTerminationCondition.never()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java new file mode 100644 index 00000000000..f974fc31ce6 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java @@ -0,0 +1,23 @@ +package org.hyperledger.besu.ethereum.eth.sync.fullsync; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.chain.Blockchain; + +public class FlexibleBlockHashTerminalCondition implements FullSyncTerminationCondition { + private Hash blockHash; + private final Blockchain blockchain; + + public FlexibleBlockHashTerminalCondition(final Hash blockHash, final Blockchain blockchain) { + this.blockHash = blockHash; + this.blockchain = blockchain; + } + + public synchronized void setBlockHash(final Hash blockHash) { + this.blockHash = blockHash; + } + + @Override + public synchronized boolean getAsBoolean() { + return blockchain.contains(blockHash); + } +} 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 89f3e0f8004..b2986f636bd 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 @@ -108,6 +108,6 @@ private boolean isSyncTargetReached(final EthPeer peer) { @Override public boolean shouldContinueDownloading() { - return terminationCondition.test(protocolContext.getBlockchain()); + return terminationCondition.getAsBoolean(); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java index 5e63c23cc5e..d753439d55c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java @@ -14,23 +14,31 @@ */ package org.hyperledger.besu.ethereum.eth.sync.fullsync; +import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Difficulty; -import java.util.function.Predicate; +import java.util.function.BooleanSupplier; import org.apache.tuweni.units.bigints.UInt256; -public interface FullSyncTerminationCondition extends Predicate { +public interface FullSyncTerminationCondition extends BooleanSupplier { static FullSyncTerminationCondition never() { - return blockchain -> false; + return () -> true; } - static FullSyncTerminationCondition difficulty(final UInt256 difficulty) { - return difficulty(Difficulty.of(difficulty)); + static FullSyncTerminationCondition difficulty( + final UInt256 difficulty, final Blockchain blockchain) { + return difficulty(Difficulty.of(difficulty), blockchain); } - static FullSyncTerminationCondition difficulty(final Difficulty difficulty) { - return blockchain -> difficulty.greaterThan(blockchain.getChainHead().getTotalDifficulty()); + static FullSyncTerminationCondition difficulty( + final Difficulty difficulty, final Blockchain blockchain) { + return () -> difficulty.greaterThan(blockchain.getChainHead().getTotalDifficulty()); + } + + static FlexibleBlockHashTerminalCondition blockHash( + final Hash blockHash, final Blockchain blockchain) { + return new FlexibleBlockHashTerminalCondition(blockHash, blockchain); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java index 6557d00e6d6..d8fdbc38273 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java @@ -130,7 +130,9 @@ public void syncsFullyAndStopsWhenTTDReached() { final SynchronizerConfiguration syncConfig = syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); final ChainDownloader downloader = - downloader(syncConfig, FullSyncTerminationCondition.difficulty(TARGET_TERMINAL_DIFFICULTY)); + downloader( + syncConfig, + FullSyncTerminationCondition.difficulty(TARGET_TERMINAL_DIFFICULTY, localBlockchain)); final CompletableFuture future = downloader.start(); assertThat(future.isDone()).isFalse(); From 09d0cd065c852cea9f338c5b36633b3f094130d3 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Wed, 23 Feb 2022 13:07:07 +0000 Subject: [PATCH 7/9] Terminating at target and not a block later Signed-off-by: Jiri Peinlich --- .../FlexibleBlockHashTerminalCondition.java | 14 ++++++ .../sync/fullsync/FullImportBlockStep.java | 9 +++- .../fullsync/FullSyncChainDownloader.java | 7 ++- .../FullSyncDownloadPipelineFactory.java | 15 ++++-- .../sync/fullsync/FullSyncTargetManager.java | 2 +- .../FullSyncTerminationCondition.java | 49 +++++++++++++++++-- .../eth/sync/PipelineChainDownloaderTest.java | 1 - .../fullsync/FullImportBlockStepTest.java | 4 +- 8 files changed, 86 insertions(+), 15 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java index f974fc31ce6..faa322e8c70 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java @@ -1,3 +1,17 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ package org.hyperledger.besu.ethereum.eth.sync.fullsync; import org.hyperledger.besu.datatypes.Hash; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java index ac3faf6d332..6c17f8d5f1d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java @@ -35,18 +35,25 @@ public class FullImportBlockStep implements Consumer { private final EthContext ethContext; private long gasAccumulator = 0; private long lastReportMillis = 0; + private final FullSyncTerminationCondition fullSyncTerminationCondition; public FullImportBlockStep( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, - final EthContext ethContext) { + final EthContext ethContext, + final FullSyncTerminationCondition fullSyncTerminationCondition) { this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; + this.fullSyncTerminationCondition = fullSyncTerminationCondition; } @Override public void accept(final Block block) { + if (fullSyncTerminationCondition.shouldStopDownload()) { + LOG.info("Not importing another block, because terminal condition was reached."); + return; + } final long blockNumber = block.getHeader().getNumber(); final String blockHash = block.getHash().toHexString(); final BlockImporter importer = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java index 6b46336c7f7..c4165dbba0f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java @@ -48,7 +48,12 @@ public static ChainDownloader create( syncState, syncTargetManager, new FullSyncDownloadPipelineFactory( - config, protocolSchedule, protocolContext, ethContext, metricsSystem), + config, + protocolSchedule, + protocolContext, + ethContext, + metricsSystem, + terminationCondition), ethContext.getScheduler(), metricsSystem); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java index ac12ddbb494..e9503304db6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java @@ -48,18 +48,21 @@ public class FullSyncDownloadPipelineFactory implements DownloadPipelineFactory private final ValidationPolicy detachedValidationPolicy = () -> HeaderValidationMode.DETACHED_ONLY; private final BetterSyncTargetEvaluator betterSyncTargetEvaluator; + private final FullSyncTerminationCondition fullSyncTerminationCondition; public FullSyncDownloadPipelineFactory( final SynchronizerConfiguration syncConfig, final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final FullSyncTerminationCondition fullSyncTerminationCondition) { this.syncConfig = syncConfig; this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; this.metricsSystem = metricsSystem; + this.fullSyncTerminationCondition = fullSyncTerminationCondition; betterSyncTargetEvaluator = new BetterSyncTargetEvaluator(syncConfig, ethContext.getEthPeers()); } @@ -91,7 +94,8 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncTarget target) new DownloadBodiesStep(protocolSchedule, ethContext, metricsSystem); final ExtractTxSignaturesStep extractTxSignaturesStep = new ExtractTxSignaturesStep(); final FullImportBlockStep importBlockStep = - new FullImportBlockStep(protocolSchedule, protocolContext, ethContext); + new FullImportBlockStep( + protocolSchedule, protocolContext, ethContext, fullSyncTerminationCondition); return PipelineBuilder.createPipelineFrom( "fetchCheckpoints", @@ -115,18 +119,19 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncTarget target) private boolean shouldContinueDownloadingFromPeer( final EthPeer peer, final BlockHeader lastCheckpointHeader) { + final boolean shouldTerminate = fullSyncTerminationCondition.getAsBoolean(); final boolean caughtUpToPeer = peer.chainState().getEstimatedHeight() <= lastCheckpointHeader.getNumber(); final boolean isDisconnected = peer.isDisconnected(); final boolean shouldSwitchSyncTarget = betterSyncTargetEvaluator.shouldSwitchSyncTarget(peer); - LOG.debug( - "shouldContinueDownloadingFromPeer? {}, disconnected {}, caughtUp {}, shouldSwitchSyncTarget {}", + "shouldTerminate {}, shouldContinueDownloadingFromPeer? {}, disconnected {}, caughtUp {}, shouldSwitchSyncTarget {}", + shouldTerminate, peer, isDisconnected, caughtUpToPeer, shouldSwitchSyncTarget); - return !isDisconnected && !caughtUpToPeer && !shouldSwitchSyncTarget; + return !shouldTerminate && !isDisconnected && !caughtUpToPeer && !shouldSwitchSyncTarget; } } 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 b2986f636bd..33142778fdc 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 @@ -108,6 +108,6 @@ private boolean isSyncTargetReached(final EthPeer peer) { @Override public boolean shouldContinueDownloading() { - return terminationCondition.getAsBoolean(); + return terminationCondition.shouldContinueDownload(); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java index d753439d55c..1a851413049 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java @@ -22,21 +22,60 @@ import org.apache.tuweni.units.bigints.UInt256; +/** return true when termination condition is fullfilled and the full sync should stop */ public interface FullSyncTerminationCondition extends BooleanSupplier { + + default boolean shouldContinueDownload() { + return !shouldStopDownload(); + } + + default boolean shouldStopDownload() { + return getAsBoolean(); + } + + /** + * When we want full sync to continue forever (for instance when we don't want to merge) + * + * @return always false therefore continues forever * + */ static FullSyncTerminationCondition never() { - return () -> true; + return () -> false; } + /** + * When we want full sync to finish after reaching a difficulty. For instance when we merge on + * total terminal difficulty. + * + * @param targetDifficulty target difficulty to reach + * @param blockchain blockchain to reach the difficulty on + * @return true when blockchain reaches difficulty + */ static FullSyncTerminationCondition difficulty( - final UInt256 difficulty, final Blockchain blockchain) { - return difficulty(Difficulty.of(difficulty), blockchain); + final UInt256 targetDifficulty, final Blockchain blockchain) { + return difficulty(Difficulty.of(targetDifficulty), blockchain); } + /** + * When we want full sync to finish after reaching a difficulty. For instance when we merge on + * total terminal difficulty. + * + * @param targetDifficulty target difficulty to reach + * @param blockchain blockchain to reach the difficulty on* + * @return true when blockchain reaches difficulty + */ static FullSyncTerminationCondition difficulty( - final Difficulty difficulty, final Blockchain blockchain) { - return () -> difficulty.greaterThan(blockchain.getChainHead().getTotalDifficulty()); + final Difficulty targetDifficulty, final Blockchain blockchain) { + return () -> blockchain.getChainHead().getTotalDifficulty().greaterThan(targetDifficulty); } + /** + * When we want the full sync to finish on a target hash. For instance when we reach a merge + * checkpoint. + * + * @param blockHash target hash to look for + * @param blockchain blockchain to reach the difficulty on + * @return true when blockchain contains target hash (target hash can be changed) + */ static FlexibleBlockHashTerminalCondition blockHash( final Hash blockHash, final Blockchain blockchain) { return new FlexibleBlockHashTerminalCondition(blockHash, blockchain); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java index 0505bc7509c..b06b9f8ec69 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java @@ -158,7 +158,6 @@ public void shouldBeCompleteWhenPipelineCompletesAndSyncTargetManagerShouldNotCo verify(syncTargetManager).findSyncTarget(); - when(syncTargetManager.shouldContinueDownloading()).thenReturn(false); pipelineFuture.complete(null); verify(syncTargetManager, Mockito.times(2)).shouldContinueDownloading(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java index 91e28984841..b512f1a8fe6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java @@ -50,7 +50,9 @@ public void setUp() { when(protocolSchedule.getByBlockNumber(anyLong())).thenReturn(protocolSpec); when(protocolSpec.getBlockImporter()).thenReturn(blockImporter); - importBlocksStep = new FullImportBlockStep(protocolSchedule, protocolContext, null); + importBlocksStep = + new FullImportBlockStep( + protocolSchedule, protocolContext, null, FullSyncTerminationCondition.never()); } @Test From ff0a998eee0392dbde487a891e187c0cdb1e3028 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Tue, 1 Mar 2022 13:34:28 +0000 Subject: [PATCH 8/9] Fixing unit tests not stopping when syncing is done Signed-off-by: Jiri Peinlich --- .../controller/BesuControllerBuilder.java | 9 ++++---- .../eth/sync/CheckpointRangeSource.java | 21 ++++++++++++------- .../eth/sync/DefaultSynchronizer.java | 4 ++-- .../FastSyncDownloadPipelineFactory.java | 4 +++- .../FlexibleBlockHashTerminalCondition.java | 2 +- .../sync/fullsync/FullImportBlockStep.java | 6 +++--- .../fullsync/FullSyncChainDownloader.java | 2 +- .../FullSyncDownloadPipelineFactory.java | 11 +++++----- .../eth/sync/fullsync/FullSyncDownloader.java | 2 +- .../sync/fullsync/FullSyncTargetManager.java | 4 ++-- ...ion.java => SyncTerminationCondition.java} | 8 +++---- .../eth/sync/CheckpointRangeSourceTest.java | 4 +++- .../fullsync/FullImportBlockStepTest.java | 2 +- .../FullSyncChainDownloaderForkTest.java | 2 +- .../fullsync/FullSyncChainDownloaderTest.java | 2 +- ...DownloaderTotalTerminalDifficultyTest.java | 12 +++++------ .../sync/fullsync/FullSyncDownloaderTest.java | 2 +- .../fullsync/FullSyncTargetManagerTest.java | 2 +- 18 files changed, 55 insertions(+), 44 deletions(-) rename ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/{FullSyncTerminationCondition.java => SyncTerminationCondition.java} (92%) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 9f30c7045ac..680ad14ef4d 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -51,7 +51,7 @@ import org.hyperledger.besu.ethereum.eth.sync.DefaultSynchronizer; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; -import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncTerminationCondition; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -418,13 +418,12 @@ public BesuController build() { additionalPluginServices); } - protected FullSyncTerminationCondition getFullSyncTerminationCondition( - final Blockchain blockchain) { + protected SyncTerminationCondition getFullSyncTerminationCondition(final Blockchain blockchain) { return genesisConfig .getConfigOptions() .getTerminalTotalDifficulty() - .map(difficulty -> FullSyncTerminationCondition.difficulty(difficulty, blockchain)) - .orElse(FullSyncTerminationCondition.never()); + .map(difficulty -> SyncTerminationCondition.difficulty(difficulty, blockchain)) + .orElse(SyncTerminationCondition.never()); } protected void prepForBuild() {} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSource.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSource.java index 523d8b71f38..977c189b0ad 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSource.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSource.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition; import java.time.Duration; import java.util.ArrayDeque; @@ -45,6 +46,7 @@ public class CheckpointRangeSource implements Iterator { private final EthScheduler ethScheduler; private final int checkpointTimeoutsPermitted; private final Duration newHeaderWaitDuration; + private final SyncTerminationCondition terminationCondition; private final Queue retrievedRanges = new ArrayDeque<>(); private BlockHeader lastRangeEnd; @@ -59,7 +61,8 @@ public CheckpointRangeSource( final EthScheduler ethScheduler, final EthPeer peer, final BlockHeader commonAncestor, - final int checkpointTimeoutsPermitted) { + final int checkpointTimeoutsPermitted, + final SyncTerminationCondition terminationCondition) { this( checkpointFetcher, syncTargetChecker, @@ -67,7 +70,8 @@ public CheckpointRangeSource( peer, commonAncestor, checkpointTimeoutsPermitted, - Duration.ofSeconds(5)); + Duration.ofSeconds(5), + terminationCondition); } CheckpointRangeSource( @@ -77,7 +81,8 @@ public CheckpointRangeSource( final EthPeer peer, final BlockHeader commonAncestor, final int checkpointTimeoutsPermitted, - final Duration newHeaderWaitDuration) { + final Duration newHeaderWaitDuration, + final SyncTerminationCondition terminationCondition) { this.checkpointFetcher = checkpointFetcher; this.syncTargetChecker = syncTargetChecker; this.ethScheduler = ethScheduler; @@ -85,14 +90,16 @@ public CheckpointRangeSource( this.lastRangeEnd = commonAncestor; this.checkpointTimeoutsPermitted = checkpointTimeoutsPermitted; this.newHeaderWaitDuration = newHeaderWaitDuration; + this.terminationCondition = terminationCondition; } @Override public boolean hasNext() { - return !retrievedRanges.isEmpty() - || (requestFailureCount < checkpointTimeoutsPermitted - && syncTargetChecker.shouldContinueDownloadingFromSyncTarget(peer, lastRangeEnd) - && !reachedEndOfCheckpoints); + return terminationCondition.shouldContinueDownload() + && (!retrievedRanges.isEmpty() + || (requestFailureCount < checkpointTimeoutsPermitted + && syncTargetChecker.shouldContinueDownloadingFromSyncTarget(peer, lastRangeEnd) + && !reachedEndOfCheckpoints)); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index 560cf2eb20a..76a3ef734eb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -23,7 +23,7 @@ import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncDownloader; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncDownloader; -import org.hyperledger.besu.ethereum.eth.sync.fullsync.FullSyncTerminationCondition; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition; import org.hyperledger.besu.ethereum.eth.sync.state.PendingBlocksManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -67,7 +67,7 @@ public DefaultSynchronizer( final Path dataDirectory, final Clock clock, final MetricsSystem metricsSystem, - final FullSyncTerminationCondition terminationCondition) { + final SyncTerminationCondition terminationCondition) { this.maybePruner = maybePruner; this.syncState = syncState; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloadPipelineFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloadPipelineFactory.java index 11bd55c6f55..39e0f297c3b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloadPipelineFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloadPipelineFactory.java @@ -33,6 +33,7 @@ import org.hyperledger.besu.ethereum.eth.sync.DownloadHeadersStep; import org.hyperledger.besu.ethereum.eth.sync.DownloadPipelineFactory; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition; import org.hyperledger.besu.ethereum.eth.sync.state.SyncTarget; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.metrics.BesuMetricCategory; @@ -105,7 +106,8 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncT ethContext.getScheduler(), target.peer(), target.commonAncestor(), - syncConfig.getDownloaderCheckpointTimeoutsPermitted()); + syncConfig.getDownloaderCheckpointTimeoutsPermitted(), + SyncTerminationCondition.never()); final DownloadHeadersStep downloadHeadersStep = new DownloadHeadersStep( protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java index faa322e8c70..d0ef7c64fcb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FlexibleBlockHashTerminalCondition.java @@ -17,7 +17,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.chain.Blockchain; -public class FlexibleBlockHashTerminalCondition implements FullSyncTerminationCondition { +public class FlexibleBlockHashTerminalCondition implements SyncTerminationCondition { private Hash blockHash; private final Blockchain blockchain; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java index 6c17f8d5f1d..e9557ac238e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java @@ -35,17 +35,17 @@ public class FullImportBlockStep implements Consumer { private final EthContext ethContext; private long gasAccumulator = 0; private long lastReportMillis = 0; - private final FullSyncTerminationCondition fullSyncTerminationCondition; + private final SyncTerminationCondition fullSyncTerminationCondition; public FullImportBlockStep( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final FullSyncTerminationCondition fullSyncTerminationCondition) { + final SyncTerminationCondition syncTerminationCondition) { this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; - this.fullSyncTerminationCondition = fullSyncTerminationCondition; + this.fullSyncTerminationCondition = syncTerminationCondition; } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java index c4165dbba0f..f4a70116269 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloader.java @@ -33,7 +33,7 @@ public static ChainDownloader create( final EthContext ethContext, final SyncState syncState, final MetricsSystem metricsSystem, - final FullSyncTerminationCondition terminationCondition) { + final SyncTerminationCondition terminationCondition) { final FullSyncTargetManager syncTargetManager = new FullSyncTargetManager( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java index e9503304db6..85b3bbbe94c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloadPipelineFactory.java @@ -48,7 +48,7 @@ public class FullSyncDownloadPipelineFactory implements DownloadPipelineFactory private final ValidationPolicy detachedValidationPolicy = () -> HeaderValidationMode.DETACHED_ONLY; private final BetterSyncTargetEvaluator betterSyncTargetEvaluator; - private final FullSyncTerminationCondition fullSyncTerminationCondition; + private final SyncTerminationCondition fullSyncTerminationCondition; public FullSyncDownloadPipelineFactory( final SynchronizerConfiguration syncConfig, @@ -56,13 +56,13 @@ public FullSyncDownloadPipelineFactory( final ProtocolContext protocolContext, final EthContext ethContext, final MetricsSystem metricsSystem, - final FullSyncTerminationCondition fullSyncTerminationCondition) { + final SyncTerminationCondition syncTerminationCondition) { this.syncConfig = syncConfig; this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; this.metricsSystem = metricsSystem; - this.fullSyncTerminationCondition = fullSyncTerminationCondition; + this.fullSyncTerminationCondition = syncTerminationCondition; betterSyncTargetEvaluator = new BetterSyncTargetEvaluator(syncConfig, ethContext.getEthPeers()); } @@ -78,7 +78,8 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncTarget target) ethContext.getScheduler(), target.peer(), target.commonAncestor(), - syncConfig.getDownloaderCheckpointTimeoutsPermitted()); + syncConfig.getDownloaderCheckpointTimeoutsPermitted(), + fullSyncTerminationCondition); final DownloadHeadersStep downloadHeadersStep = new DownloadHeadersStep( protocolSchedule, @@ -119,7 +120,7 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncTarget target) private boolean shouldContinueDownloadingFromPeer( final EthPeer peer, final BlockHeader lastCheckpointHeader) { - final boolean shouldTerminate = fullSyncTerminationCondition.getAsBoolean(); + final boolean shouldTerminate = fullSyncTerminationCondition.shouldStopDownload(); final boolean caughtUpToPeer = peer.chainState().getEstimatedHeight() <= lastCheckpointHeader.getNumber(); final boolean isDisconnected = peer.isDisconnected(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java index 32c7dd64294..4484c194460 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloader.java @@ -43,7 +43,7 @@ public FullSyncDownloader( final EthContext ethContext, final SyncState syncState, final MetricsSystem metricsSystem, - final FullSyncTerminationCondition terminationCondition) { + final SyncTerminationCondition terminationCondition) { this.syncConfig = syncConfig; this.protocolContext = protocolContext; this.syncState = syncState; 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 33142778fdc..5c61caa17e4 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 @@ -39,7 +39,7 @@ class FullSyncTargetManager extends SyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(FullSyncTargetManager.class); private final ProtocolContext protocolContext; private final EthContext ethContext; - private final FullSyncTerminationCondition terminationCondition; + private final SyncTerminationCondition terminationCondition; FullSyncTargetManager( final SynchronizerConfiguration config, @@ -47,7 +47,7 @@ class FullSyncTargetManager extends SyncTargetManager { final ProtocolContext protocolContext, final EthContext ethContext, final MetricsSystem metricsSystem, - final FullSyncTerminationCondition terminationCondition) { + final SyncTerminationCondition terminationCondition) { super(config, protocolSchedule, protocolContext, ethContext, metricsSystem); this.protocolContext = protocolContext; this.ethContext = ethContext; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/SyncTerminationCondition.java similarity index 92% rename from ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java rename to ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/SyncTerminationCondition.java index 1a851413049..ae1184fc144 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncTerminationCondition.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/SyncTerminationCondition.java @@ -23,7 +23,7 @@ import org.apache.tuweni.units.bigints.UInt256; /** return true when termination condition is fullfilled and the full sync should stop */ -public interface FullSyncTerminationCondition extends BooleanSupplier { +public interface SyncTerminationCondition extends BooleanSupplier { default boolean shouldContinueDownload() { return !shouldStopDownload(); @@ -38,7 +38,7 @@ default boolean shouldStopDownload() { * * @return always false therefore continues forever * */ - static FullSyncTerminationCondition never() { + static SyncTerminationCondition never() { return () -> false; } @@ -50,7 +50,7 @@ static FullSyncTerminationCondition never() { * @param blockchain blockchain to reach the difficulty on * @return true when blockchain reaches difficulty */ - static FullSyncTerminationCondition difficulty( + static SyncTerminationCondition difficulty( final UInt256 targetDifficulty, final Blockchain blockchain) { return difficulty(Difficulty.of(targetDifficulty), blockchain); } @@ -63,7 +63,7 @@ static FullSyncTerminationCondition difficulty( * @param blockchain blockchain to reach the difficulty on* * @return true when blockchain reaches difficulty */ - static FullSyncTerminationCondition difficulty( + static SyncTerminationCondition difficulty( final Difficulty targetDifficulty, final Blockchain blockchain) { return () -> blockchain.getChainHead().getTotalDifficulty().greaterThan(targetDifficulty); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSourceTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSourceTest.java index 3493a94885d..ea1d85dd1c2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSourceTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/CheckpointRangeSourceTest.java @@ -32,6 +32,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; +import org.hyperledger.besu.ethereum.eth.sync.fullsync.SyncTerminationCondition; import java.time.Duration; import java.util.List; @@ -60,7 +61,8 @@ public class CheckpointRangeSourceTest { peer, commonAncestor, CHECKPOINT_TIMEOUTS_PERMITTED, - Duration.ofMillis(1)); + Duration.ofMillis(1), + SyncTerminationCondition.never()); @Test public void shouldHaveNextWhenNoCheckpointsLoadedButSyncTargetCheckerSaysToContinue() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java index b512f1a8fe6..eac021d0dff 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStepTest.java @@ -52,7 +52,7 @@ public void setUp() { importBlocksStep = new FullImportBlockStep( - protocolSchedule, protocolContext, null, FullSyncTerminationCondition.never()); + protocolSchedule, protocolContext, null, SyncTerminationCondition.never()); } @Test diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java index 237e7d7e536..3c877bd2309 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java @@ -86,7 +86,7 @@ private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { ethContext, syncState, metricsSystem, - FullSyncTerminationCondition.never()); + SyncTerminationCondition.never()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java index 09f0f60c3ae..24d9fa2e305 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java @@ -123,7 +123,7 @@ private ChainDownloader downloader(final SynchronizerConfiguration syncConfig) { ethContext, syncState, metricsSystem, - FullSyncTerminationCondition.never()); + SyncTerminationCondition.never()); } private ChainDownloader downloader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java index d8fdbc38273..b80e94450d5 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTotalTerminalDifficultyTest.java @@ -100,7 +100,7 @@ public void tearDown() { private ChainDownloader downloader( final SynchronizerConfiguration syncConfig, - final FullSyncTerminationCondition terminalCondition) { + final SyncTerminationCondition terminalCondition) { return FullSyncChainDownloader.create( syncConfig, protocolSchedule, @@ -132,7 +132,7 @@ public void syncsFullyAndStopsWhenTTDReached() { final ChainDownloader downloader = downloader( syncConfig, - FullSyncTerminationCondition.difficulty(TARGET_TERMINAL_DIFFICULTY, localBlockchain)); + SyncTerminationCondition.difficulty(TARGET_TERMINAL_DIFFICULTY, localBlockchain)); final CompletableFuture future = downloader.start(); assertThat(future.isDone()).isFalse(); @@ -141,10 +141,10 @@ public void syncsFullyAndStopsWhenTTDReached() { assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); - peer.respondWhileOtherThreadsWork( - responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); + peer.respondWhileOtherThreadsWork(responder, () -> !future.isDone()); - assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); + assertThat(localBlockchain.getChainHead().getTotalDifficulty()) + .isGreaterThan(TARGET_TERMINAL_DIFFICULTY); assertThat(future.isDone()).isTrue(); } @@ -163,7 +163,7 @@ public void syncsFullyAndContinuesWhenTTDNotSpecified() { final SynchronizerConfiguration syncConfig = syncConfigBuilder().downloaderChainSegmentSize(1).downloaderParallelism(1).build(); - final ChainDownloader downloader = downloader(syncConfig, FullSyncTerminationCondition.never()); + final ChainDownloader downloader = downloader(syncConfig, SyncTerminationCondition.never()); final CompletableFuture future = downloader.start(); assertThat(future.isDone()).isFalse(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index d39f8e1a051..1214c5da1e6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java @@ -98,7 +98,7 @@ private FullSyncDownloader downloader(final SynchronizerConfiguration syncConfig ethContext, syncState, metricsSystem, - FullSyncTerminationCondition.never()); + SyncTerminationCondition.never()); } @Test 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 7473d231d05..4a766c5d288 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 @@ -98,7 +98,7 @@ public void setup() { protocolContext, ethContext, new NoOpMetricsSystem(), - FullSyncTerminationCondition.never()); + SyncTerminationCondition.never()); } @After From aec582b0f4afc5efe6b1bdcfb38f84c9f3901db6 Mon Sep 17 00:00:00 2001 From: Jiri Peinlich Date: Tue, 1 Mar 2022 14:24:54 +0000 Subject: [PATCH 9/9] Addressing PR comments Signed-off-by: Jiri Peinlich --- .../besu/ethereum/eth/sync/BlockPropagationManager.java | 2 +- .../besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java index 8eacb2a1ccc..2b82ddc8837 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java @@ -119,7 +119,7 @@ public void stop() { clearListeners(); started.set(false); } else { - LOG.info("Attempted to stop when we are not even running..."); + LOG.warn("Attempted to stop when we are not even running..."); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java index e9557ac238e..fc37ecaac78 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java @@ -51,7 +51,7 @@ public FullImportBlockStep( @Override public void accept(final Block block) { if (fullSyncTerminationCondition.shouldStopDownload()) { - LOG.info("Not importing another block, because terminal condition was reached."); + LOG.debug("Not importing another block, because terminal condition was reached."); return; } final long blockNumber = block.getHeader().getNumber();