From 657effffd4a9629589be68a614e941aa00e2a62a Mon Sep 17 00:00:00 2001 From: Matilda-Clerke Date: Wed, 11 Dec 2024 16:03:31 +1100 Subject: [PATCH] 7311 add get headers from peer task (#7781) * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Fix broken BesuCommandTest Signed-off-by: Matilda Clerke * 7311: add class Signed-off-by: Matilda Clerke * 7311: Move PeerTaskFeatureToggle to more appropriate location Signed-off-by: Matilda Clerke * 7311: add X prefix to peertask-system-enabled Signed-off-by: Matilda Clerke * 7311: Move --Xpeertask-system-enabled out of BesuCommand and make hidden Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Add GetReceiptsFromPeerTask Signed-off-by: Matilda Clerke * 7311: Move isPeerTaskSystemEnabled to SynchronizerOptions Signed-off-by: Matilda Clerke * 7311: Fix javadoc issue Signed-off-by: Matilda Clerke * 7311: Fix javadoc issue Signed-off-by: Matilda Clerke * 7311: Move PeerTaskFeatureToggleTestHelper to TestUtil and fix RunnerTest Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Remove PeerTaskFeatureToggle in favor of including isPeerTaskSystemEnabled in SynchronizerConfiguration Signed-off-by: Matilda Clerke * 7311: Adjust to the removal of PeerTaskFeatureToggle and use SynchronizerConfiguration to get the toggle instead Signed-off-by: Matilda Clerke * 7311: Reduce timeout in PeerTaskRequestSender to 5s Signed-off-by: Matilda Clerke * 7311: Refactor PeerManager to be an interface Signed-off-by: Matilda Clerke * 7311: Fix up compile errors after merge Signed-off-by: Matilda Clerke * 7311: Fix MetricsAcceptanceTest Signed-off-by: Matilda Clerke * 7311: Fix MetricsAcceptanceTest Signed-off-by: Matilda Clerke * 7311: Fix DownloadReceiptsStep when using peer task system Signed-off-by: Matilda Clerke * 7311: Rename PeerManager to PeerSelector Signed-off-by: Matilda Clerke * 7311: Reword PeerSelector javadoc to avoid implementation details Signed-off-by: Matilda Clerke * 7311: Use ConcurrentHashMap in DefaultPeerSelector Signed-off-by: Matilda Clerke * 7311: Reword trace log in DefaultPeerSelector Signed-off-by: Matilda Clerke * 7311: Remove unused imports Signed-off-by: Matilda Clerke * 7311: Use a 1 second delay between retries in PeerTaskExecutor to match old implementation Signed-off-by: Matilda Clerke * 7311: Add testGetPeerButNoPeerMatchesFilter to DefaultPeerSelectorTest Signed-off-by: Matilda Clerke * 7311: Add testGetPeerButNoPeerMatchesFilter to DefaultPeerSelectorTest Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Fix MetricsAcceptanceTest Signed-off-by: Matilda Clerke * 7311: Fix MetricsAcceptanceTest Signed-off-by: Matilda Clerke * 7311: Modify PeerTaskExecutor metric to include response time from peer Signed-off-by: Matilda Clerke * 7311: Use SubProtocol instead of subprotocol name string in PeerTask Signed-off-by: Matilda Clerke * 7311: rename timing context to ignored to prevent intellij warnings Signed-off-by: Matilda Clerke * 7311: Use constants for number of retries Signed-off-by: Matilda Clerke * 7311: Convert PeerTaskExecutorResult to a record Signed-off-by: Matilda Clerke * 7311: Rename PeerTaskBehavior to PeerTaskRetryBehavior Signed-off-by: Matilda Clerke * 7311: Move peer selection logic to PeerSelector Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Fix up everything broken after merge Signed-off-by: Matilda Clerke * 7311: Attempt to improve performance of peer task system in pipeline Signed-off-by: Matilda Clerke * 7311: fix compile check Signed-off-by: Matilda Clerke * 7311: Fix broken workflow Signed-off-by: Matilda Clerke * 7311: Reduce logging in JsonRpcExecutor to trace level Signed-off-by: Matilda Clerke * 7311: More changes in DownloadReceiptsStep Signed-off-by: Matilda Clerke * 7311: Rework DownloadReceiptsStep Signed-off-by: Matilda Clerke * 7311: Make changes as discussed in walkthrough meeting Remove DefaultPeerSelector, make EthPeers implement PeerSelector interface, and add PeerTask.getPeerRequirementFilter Signed-off-by: Matilda Clerke * 7311: Update after merge and make discussed changes from walkthrough discussion Signed-off-by: Matilda Clerke * 7311: Change to regular HashMap Signed-off-by: Matilda Clerke * 7311: Remove runtime exception again Signed-off-by: Matilda Clerke * 7311: Rename getPeerTaskBehavior to getPeerTaskRetryBehavior Signed-off-by: Matilda Clerke * 7311: Rename getPeerTaskBehavior to getPeerTaskRetryBehavior Signed-off-by: Matilda Clerke * 7311: Rework PeerTaskExecutor retry system to be 0-based Signed-off-by: Matilda Clerke * 7311: Fix up compile errors after merge Signed-off-by: Matilda Clerke * 7311: Fix broken DownloadReceiptsStepTest test Signed-off-by: Matilda Clerke * 7311: Move GetReceipts to services worker for parallelism Signed-off-by: Matilda Clerke * 7311: Refactor peer task system usage in DownloadReceiptsStep to better match old system Signed-off-by: Matilda Clerke * 7311: Remove unused async methods in PeerTaskExecutor Signed-off-by: Matilda Clerke * 7311: Return Optional in PeerSelector.getPeer and utilise existing peer selection behavior in EthPeers Signed-off-by: Matilda Clerke * 7311: Update after merge Signed-off-by: Matilda Clerke * 7311: Redo getPeer again to include hasAvailableRequestCapacity check Signed-off-by: Matilda Clerke * 7311: Add protocol spec supplier to GetReceiptsFromPeerTask Signed-off-by: Matilda Clerke * 7311: Rework getPeer again to use LEAST_TO_MOST_BUSY comparator Signed-off-by: Matilda Clerke * 7311: Import PeerNotConnected class instead of using fully qualified class name Signed-off-by: Matilda Clerke * 7311: Change to specifying retry counts in PeerTask instead of behavior enums Signed-off-by: Matilda Clerke * 7311: clean up after merge Signed-off-by: Matilda Clerke * 7311: clean up after merge Signed-off-by: Matilda Clerke * 7311: Fix up javadoc Signed-off-by: Matilda Clerke * 7311: Add additional metrics to PeerTaskExecutor Signed-off-by: Matilda Clerke * 7311: Add Predicate to PeerTask to check for partial success Signed-off-by: Matilda Clerke * 7311: Fix incorrect name on isPartialSuccessTest Signed-off-by: Matilda Clerke * 7311: Implement isPartialSuccess and add unit tests Signed-off-by: Matilda Clerke * 7311: Add partialSuccessCounter and inflightRequestGauge in PeerTaskExecutor Signed-off-by: Matilda Clerke * 7311: Also filter by whether a peer is fully validated Signed-off-by: Matilda Clerke * 7311: Remove unneeded throws in RunnerTest Signed-off-by: Matilda Clerke * 7311: Fix up inflight requests gauge in PeerTaskExecutor Signed-off-by: Matilda Clerke * 7311: Update plugin api hash Signed-off-by: Matilda Clerke * 7311: Update plugin api hash Signed-off-by: Matilda Clerke * 7311: Add javadoc to LabelledGauge.isLabelsObserved Signed-off-by: Matilda Clerke * 7311: Update plugin-api hash Signed-off-by: Matilda Clerke * 7311: Update changelog Signed-off-by: Matilda Clerke * 7311: Implement GetHeadersFromPeerTask and use in DetermineCommonAncestorTask Signed-off-by: Matilda Clerke * 7311: Handle headers with no receipts as a special case in DownloadReceiptsStep Signed-off-by: Matilda Clerke * 7311: Complete merge Signed-off-by: Matilda Clerke * 7311: Get DetermineCommonAncestorTask working with peer task system Signed-off-by: Matilda Clerke * 7311: Use taskName instead of className for labelNames Signed-off-by: Matilda Clerke * 7311: Use snake_case for metric names Signed-off-by: Matilda Clerke * 7311: Use _total metric name suffix Signed-off-by: Matilda Clerke * 7311: rework partial success handling Signed-off-by: Matilda Clerke * 7311: Update GetReceiptsFromPeerTask with partialSuccess changes Signed-off-by: Matilda Clerke * 7311: Update GetHeadersFromPeerTask with partialSuccess changes Signed-off-by: Matilda Clerke * 7311: Add default implementation to LabelledGauge.isLabelsObserved Signed-off-by: Matilda Clerke * 7311: Use Peer task systems GetHeadersFromPeerTask in GetBlockFromPeerTask Signed-off-by: Matilda Clerke * 7311: Fix broken unit test Signed-off-by: Matilda Clerke * 7311: Remove unused constructor from AbstractPeerBlockValidator Signed-off-by: Matilda Clerke * 7311: Use GetHeadersFromPeerTask in AbstractPeerBlockValidator Signed-off-by: Matilda Clerke * 7311: Use peer task executor in SyncTargetManager Signed-off-by: Matilda Clerke * 7311: Fix javadoc on BesuControllerBuilder Signed-off-by: Matilda Clerke * 7311: Remove logs used to confirm operation Signed-off-by: Matilda Clerke * 7311: Implement GetHeadersFromPeerTask in FastSyncActions and PivotBlockConfirmer Signed-off-by: Matilda Clerke * 7311: Rename parseResponse to processResponse Signed-off-by: Matilda Clerke * 7311: Wrap peer task system usage in ethScheduler call to match other usages Signed-off-by: Matilda Clerke * 7311: apply spotless Signed-off-by: Matilda Clerke * 7311: Move check for empty trie hash into GetReceiptsFromPeerTask and update unit test to test for this functionality Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Fix compile issue after merge Signed-off-by: Matilda Clerke * 7311: Fix compile issue after merge Signed-off-by: Matilda Clerke * 7311: Remove BodyValidator and update code and test to match Signed-off-by: Matilda Clerke * 7311: Implement GetHeadersForPeerTask usage in DownloadHeadersStep Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: remove unneeded logs Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Fix up pre-fill and add test to test failure scenario Signed-off-by: Matilda Clerke * 7311: Use ProtocolSchedule.anyMatch to find if any ProtocolSpecs are PoS, remove new usages of currentProtocolSpecSupplier Signed-off-by: Matilda Clerke * 7311: Only attempt to remove headers on successful requests Signed-off-by: Matilda Clerke * 7311: clean up after merge Signed-off-by: Matilda Clerke * 7311: clean up after merge Signed-off-by: Matilda Clerke * 7311: Use peer task system in RangeHeadersFetcher Signed-off-by: Matilda Clerke * 7311: Use peer task system in DownloadHeaderSequenceTask Signed-off-by: Matilda Clerke * 7311: Fix GetHeadersFromPeerTask mocking in CheckPointSyncChainDownloaderTest Signed-off-by: Matilda Clerke * 7311: Extract peer task executor answer for getHeaders to separate class for reuse in tests Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Implement peer task system usage in BackwardSyncStep Signed-off-by: Matilda Clerke * 7311: Implement peer task system usage in ChainHeadTracker Signed-off-by: Matilda Clerke * 7311: Implement peer task system usage in PivotSelectorFromSafeBlock and improve logging Signed-off-by: Matilda Clerke * 7311: Implement unit test for GetHeadersFromPeerTask Signed-off-by: Matilda Clerke * 7311: Fix up merge compile error Signed-off-by: Matilda Clerke * 7311: Ensure FastSyncActions and PivotSelectorFromSafeBlock retry getting headers for all peers, matching RetryingGetHeaderFromPeerByHashTask Signed-off-by: Matilda Clerke * 7311: Change PeerTaskExecutorResult.ethPeer to an Optional Signed-off-by: Matilda Clerke * 7311: Use CancellationException instead of InterruptedException in PivotBlockConfirmer Signed-off-by: Matilda Clerke * 7311: Use PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER to set retries for GetHeadersFromPeerTask Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Add PeerTask.shouldDisconnectPeer and ensure functionality matches old code Signed-off-by: Matilda Clerke * 7311: Remove old info logs Signed-off-by: Matilda Clerke * 7311: Fix broken test by correctly including peer in PeerTaskExecutorResults in test classes Signed-off-by: Matilda Clerke * 7311: Fix incorrect equality tests Signed-off-by: Matilda Clerke * 7311: Fix broken test Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Move PeerTaskExecutor into EthContext to reduce plumbing changes Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Remove protocol check from GetHeadersFromPeerTask.getPeerRequirementFilter Signed-off-by: Matilda Clerke * 7311: Fix broken test Signed-off-by: Matilda Clerke * 7311: Fix broken integration test Signed-off-by: Matilda Clerke * 7311: Refactor peer task validation Signed-off-by: Matilda Clerke * 7311: Refactor peer task validation Signed-off-by: Matilda Clerke * 7311: Use peer count for retry count when getting headers in BackwardSyncStep, FastSyncActions, and PivotSelectorFromSafeBlock Signed-off-by: Matilda Clerke * 7311: spotless Signed-off-by: Matilda Clerke * 7311: Move chainstate update into GetHeadersFromPeerTask.postProcessResult Signed-off-by: Matilda Clerke * 7311: Fix compile errors Signed-off-by: Matilda Clerke * 7311: Update after merge Signed-off-by: Matilda Clerke --------- Signed-off-by: Matilda Clerke Signed-off-by: Matilda-Clerke Co-authored-by: Sally MacFarlane --- .../controller/BesuControllerBuilder.java | 30 +- .../MergeBesuControllerBuilder.java | 9 +- .../TransitionBesuControllerBuilder.java | 1 + .../TransitionControllerBuilderTest.java | 2 + .../merge/TransitionBackwardSyncContext.java | 3 + .../besu/ethereum/eth/manager/EthContext.java | 17 +- .../InvalidPeerTaskResponseException.java | 4 + .../eth/manager/peertask/PeerTask.java | 10 +- .../manager/peertask/PeerTaskExecutor.java | 47 +++- .../peertask/PeerTaskExecutorResult.java | 4 +- .../peertask/PeerTaskValidationResponse.java | 37 +++ .../peertask/task/GetHeadersFromPeerTask.java | 263 ++++++++++++++++++ .../task/GetReceiptsFromPeerTask.java | 14 +- .../manager/task/GetBlockFromPeerTask.java | 127 +++++++-- .../task/RetryingGetBlockFromPeersTask.java | 20 +- .../AbstractPeerBlockValidator.java | 111 +++++--- .../CheckpointBlocksPeerValidator.java | 23 +- .../ClassicForkPeerValidator.java | 22 +- .../peervalidation/DaoForkPeerValidator.java | 22 +- .../RequiredBlocksPeerValidator.java | 22 +- .../eth/sync/AbstractSyncTargetManager.java | 1 + .../eth/sync/BlockPropagationManager.java | 1 + .../ethereum/eth/sync/ChainHeadTracker.java | 111 ++++++-- .../eth/sync/DefaultSynchronizer.java | 4 +- .../eth/sync/DownloadHeadersStep.java | 49 +++- .../backwardsync/BackwardSyncContext.java | 10 + .../sync/backwardsync/BackwardSyncStep.java | 78 ++++-- .../eth/sync/backwardsync/SyncStepStep.java | 1 + .../CheckpointDownloadBlockStep.java | 7 +- .../CheckpointDownloaderFactory.java | 4 - .../checkpointsync/CheckpointSyncActions.java | 4 - .../CheckpointSyncChainDownloader.java | 10 +- ...CheckpointSyncDownloadPipelineFactory.java | 13 +- .../sync/fastsync/DownloadReceiptsStep.java | 6 +- .../eth/sync/fastsync/FastSyncActions.java | 72 ++++- .../fastsync/FastSyncChainDownloader.java | 10 +- .../FastSyncDownloadPipelineFactory.java | 8 +- .../eth/sync/fastsync/FastSyncDownloader.java | 7 + .../sync/fastsync/PivotBlockConfirmer.java | 74 ++++- .../sync/fastsync/PivotBlockRetriever.java | 7 + .../fastsync/PivotSelectorFromSafeBlock.java | 64 ++++- .../eth/sync/fastsync/SyncTargetManager.java | 61 +++- .../worldstate/FastDownloaderFactory.java | 3 - .../FullSyncDownloadPipelineFactory.java | 1 + .../eth/sync/range/RangeHeadersFetcher.java | 87 ++++-- .../sync/snapsync/SnapDownloaderFactory.java | 3 - .../tasks/DetermineCommonAncestorTask.java | 96 ++++++- .../tasks/DownloadHeaderSequenceTask.java | 195 ++++++++----- .../EthProtocolManagerTestBuilder.java | 14 +- .../ethtaskutils/AbstractMessageTaskTest.java | 6 +- .../peertask/PeerTaskExecutorTest.java | 47 +++- .../GetHeadersFromPeerTaskExecutorAnswer.java | 88 ++++++ .../task/GetHeadersFromPeerTaskTest.java | 172 ++++++++++++ .../task/GetReceiptsFromPeerTaskTest.java | 12 +- .../task/GetBlockFromPeerTaskTest.java | 5 + .../RetryingGetBlockFromPeersTaskTest.java | 2 + .../DaoForkPeerValidatorTest.java | 29 +- .../RequiredBlocksPeerValidatorTest.java | 20 +- .../AbstractBlockPropagationManagerTest.java | 6 +- .../eth/sync/ChainHeadTrackerTest.java | 79 +++++- .../eth/sync/DownloadHeadersStepTest.java | 100 ++++++- .../eth/sync/RangeHeadersFetcherTest.java | 132 ++++++++- .../backwardsync/BackwardSyncContextTest.java | 5 + .../backwardsync/BackwardSyncStepTest.java | 143 +++++++++- .../CheckPointSyncChainDownloaderTest.java | 73 ++--- .../fastsync/DownloadReceiptsStepTest.java | 5 +- .../fastsync/FastDownloaderFactoryTest.java | 7 - .../sync/fastsync/FastSyncActionsTest.java | 83 +++--- .../fastsync/FastSyncChainDownloaderTest.java | 2 - .../fastsync/PivotBlockConfirmerTest.java | 164 +++++++++-- .../fastsync/PivotBlockRetrieverTest.java | 2 + ...neCommonAncestorTaskParameterizedTest.java | 48 +++- .../DetermineCommonAncestorTaskTest.java | 195 +++++++++++++ .../tasks/DownloadHeaderSequenceTaskTest.java | 141 ++++++++++ .../ethereum/eth/transactions/TestNode.java | 2 +- 75 files changed, 2812 insertions(+), 545 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskValidationResponse.java create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTask.java create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskExecutorAnswer.java create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskTest.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 086850fa415..258cf95ca35 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -690,9 +690,10 @@ public BesuController build() { .build()); } - final EthContext ethContext = new EthContext(ethPeers, ethMessages, snapMessages, scheduler); final PeerTaskExecutor peerTaskExecutor = new PeerTaskExecutor(ethPeers, new PeerTaskRequestSender(), metricsSystem); + final EthContext ethContext = + new EthContext(ethPeers, ethMessages, snapMessages, scheduler, peerTaskExecutor); final boolean fullSyncDisabled = !SyncMode.isFullSync(syncConfig.getSyncMode()); final SyncState syncState = new SyncState(blockchain, ethPeers, fullSyncDisabled, checkpoint); @@ -718,7 +719,8 @@ public BesuController build() { besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()), miningConfiguration); - final List peerValidators = createPeerValidators(protocolSchedule); + final List peerValidators = + createPeerValidators(protocolSchedule, peerTaskExecutor); final EthProtocolManager ethProtocolManager = createEthProtocolManager( @@ -947,6 +949,7 @@ private PivotBlockSelector createPivotSelector( ethContext, metricsSystem, genesisConfigOptions, + syncConfig, unverifiedForkchoiceSupplier, unsubscribeForkchoiceListener); } else { @@ -1179,29 +1182,42 @@ private ChainDataPruner createChainPruner(final BlockchainStorage blockchainStor * Create peer validators list. * * @param protocolSchedule the protocol schedule + * @param peerTaskExecutor the peer task executor * @return the list */ - protected List createPeerValidators(final ProtocolSchedule protocolSchedule) { + protected List createPeerValidators( + final ProtocolSchedule protocolSchedule, final PeerTaskExecutor peerTaskExecutor) { final List validators = new ArrayList<>(); final OptionalLong daoBlock = genesisConfigOptions.getDaoForkBlock(); if (daoBlock.isPresent()) { // Setup dao validator validators.add( - new DaoForkPeerValidator(protocolSchedule, metricsSystem, daoBlock.getAsLong())); + new DaoForkPeerValidator( + protocolSchedule, peerTaskExecutor, syncConfig, metricsSystem, daoBlock.getAsLong())); } final OptionalLong classicBlock = genesisConfigOptions.getClassicForkBlock(); // setup classic validator if (classicBlock.isPresent()) { validators.add( - new ClassicForkPeerValidator(protocolSchedule, metricsSystem, classicBlock.getAsLong())); + new ClassicForkPeerValidator( + protocolSchedule, + peerTaskExecutor, + syncConfig, + metricsSystem, + classicBlock.getAsLong())); } for (final Map.Entry requiredBlock : requiredBlocks.entrySet()) { validators.add( new RequiredBlocksPeerValidator( - protocolSchedule, metricsSystem, requiredBlock.getKey(), requiredBlock.getValue())); + protocolSchedule, + peerTaskExecutor, + syncConfig, + metricsSystem, + requiredBlock.getKey(), + requiredBlock.getValue())); } final CheckpointConfigOptions checkpointConfigOptions = @@ -1210,6 +1226,8 @@ protected List createPeerValidators(final ProtocolSchedule protoc validators.add( new CheckpointBlocksPeerValidator( protocolSchedule, + peerTaskExecutor, + syncConfig, metricsSystem, checkpointConfigOptions.getNumber().orElseThrow(), checkpointConfigOptions.getHash().map(Hash::fromHexString).orElseThrow())); diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java index e7c1a5c40d8..2352f9d3965 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java @@ -34,6 +34,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.eth.manager.MergePeerFilter; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.eth.peervalidation.RequiredBlocksPeerValidator; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -79,6 +80,7 @@ protected MiningCoordinator createMiningCoordinator( new BackwardSyncContext( protocolContext, protocolSchedule, + syncConfig, metricsSystem, ethProtocolManager.ethContext(), syncState, @@ -235,14 +237,17 @@ protected PluginServiceFactory createAdditionalPluginServices( } @Override - protected List createPeerValidators(final ProtocolSchedule protocolSchedule) { - List retval = super.createPeerValidators(protocolSchedule); + protected List createPeerValidators( + final ProtocolSchedule protocolSchedule, final PeerTaskExecutor peerTaskExecutor) { + List retval = super.createPeerValidators(protocolSchedule, peerTaskExecutor); final OptionalLong powTerminalBlockNumber = genesisConfigOptions.getTerminalBlockNumber(); final Optional powTerminalBlockHash = genesisConfigOptions.getTerminalBlockHash(); if (powTerminalBlockHash.isPresent() && powTerminalBlockNumber.isPresent()) { retval.add( new RequiredBlocksPeerValidator( protocolSchedule, + peerTaskExecutor, + syncConfig, metricsSystem, powTerminalBlockNumber.getAsLong(), powTerminalBlockHash.get(), diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java index 75abca6a574..4f078fda286 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java @@ -119,6 +119,7 @@ protected MiningCoordinator createMiningCoordinator( new TransitionBackwardSyncContext( protocolContext, transitionProtocolSchedule, + syncConfig, metricsSystem, ethProtocolManager.ethContext(), syncState, diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java index 24b0133a0f1..da409482e03 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java @@ -40,6 +40,7 @@ import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; import org.hyperledger.besu.ethereum.core.MiningConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator; @@ -73,6 +74,7 @@ public class TransitionControllerBuilderTest { @Mock ProtocolContext protocolContext; @Mock MutableBlockchain mockBlockchain; @Mock TransactionPool transactionPool; + @Mock PeerTaskExecutor peerTaskExecutor; @Mock SyncState syncState; @Mock(answer = Answers.RETURNS_DEEP_STUBS) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java index 4f0531a1bc4..37eb53cc58b 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardChain; import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; @@ -43,6 +44,7 @@ public class TransitionBackwardSyncContext extends BackwardSyncContext { public TransitionBackwardSyncContext( final ProtocolContext protocolContext, final TransitionProtocolSchedule transitionProtocolSchedule, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final EthContext ethContext, final SyncState syncState, @@ -50,6 +52,7 @@ public TransitionBackwardSyncContext( super( protocolContext, transitionProtocolSchedule, + synchronizerConfiguration, metricsSystem, ethContext, syncState, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthContext.java index b33fda3bc64..88e1b64e64b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthContext.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.ethereum.eth.manager; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; + import java.util.Optional; public class EthContext { @@ -22,24 +24,31 @@ public class EthContext { private final EthMessages ethMessages; private final Optional snapMessages; private final EthScheduler scheduler; + private final PeerTaskExecutor peerTaskExecutor; public EthContext( final EthPeers ethPeers, final EthMessages ethMessages, final EthMessages snapMessages, - final EthScheduler scheduler) { + final EthScheduler scheduler, + final PeerTaskExecutor peerTaskExecutor) { this.ethPeers = ethPeers; this.ethMessages = ethMessages; this.snapMessages = Optional.of(snapMessages); this.scheduler = scheduler; + this.peerTaskExecutor = peerTaskExecutor; } public EthContext( - final EthPeers ethPeers, final EthMessages ethMessages, final EthScheduler scheduler) { + final EthPeers ethPeers, + final EthMessages ethMessages, + final EthScheduler scheduler, + final PeerTaskExecutor peerTaskExecutor) { this.ethPeers = ethPeers; this.ethMessages = ethMessages; this.snapMessages = Optional.empty(); this.scheduler = scheduler; + this.peerTaskExecutor = peerTaskExecutor; } public EthPeers getEthPeers() { @@ -57,4 +66,8 @@ public Optional getSnapMessages() { public EthScheduler getScheduler() { return scheduler; } + + public PeerTaskExecutor getPeerTaskExecutor() { + return peerTaskExecutor; + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/InvalidPeerTaskResponseException.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/InvalidPeerTaskResponseException.java index 824c0860d70..3cea6088e16 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/InvalidPeerTaskResponseException.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/InvalidPeerTaskResponseException.java @@ -20,6 +20,10 @@ public InvalidPeerTaskResponseException() { super(); } + public InvalidPeerTaskResponseException(final String message) { + super(message); + } + public InvalidPeerTaskResponseException(final Throwable cause) { super(cause); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java index fed671d38d2..5439da7deab 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java @@ -75,9 +75,13 @@ default int getRetriesWithSamePeer() { Predicate getPeerRequirementFilter(); /** - * Checks if the supplied result is considered a success + * Performs a high level check of the results, returning a PeerTaskValidationResponse to describe + * the result of the check * - * @return true if the supplied result is considered a success + * @param result The results of the PeerTask, as returned by processResponse + * @return a PeerTaskValidationResponse to describe the result of the check */ - boolean isSuccess(T result); + PeerTaskValidationResponse validateResult(T result); + + default void postProcessResult(final PeerTaskExecutorResult result) {} } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java index c8eed6e1f1f..8f5e411f6c5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java @@ -29,12 +29,15 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** Manages the execution of PeerTasks, respecting their PeerTaskRetryBehavior */ public class PeerTaskExecutor { + private static final Logger LOG = LoggerFactory.getLogger(PeerTaskExecutor.class); private final PeerSelector peerSelector; private final PeerTaskRequestSender requestSender; @@ -98,8 +101,8 @@ public PeerTaskExecutorResult execute(final PeerTask peerTask) { if (peer.isEmpty()) { executorResult = new PeerTaskExecutorResult<>( - Optional.empty(), PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE); - continue; + Optional.empty(), PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE, Optional.empty()); + break; } usedEthPeers.add(peer.get()); executorResult = executeAgainstPeer(peerTask, peer.get()); @@ -138,43 +141,59 @@ public PeerTaskExecutorResult executeAgainstPeer( inflightRequestCountForThisTaskClass.decrementAndGet(); } - if (peerTask.isSuccess(result)) { + PeerTaskValidationResponse validationResponse = peerTask.validateResult(result); + if (validationResponse == PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD) { peer.recordUsefulResponse(); executorResult = new PeerTaskExecutorResult<>( - Optional.ofNullable(result), PeerTaskExecutorResponseCode.SUCCESS); + Optional.ofNullable(result), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(peer)); + peerTask.postProcessResult(executorResult); } else { - // At this point, the result is most likely empty. Technically, this is a valid result, so - // we don't penalise the peer, but it's also a useless result, so we return - // INVALID_RESPONSE code + LOG.debug( + "Invalid response found for {} from peer {}", taskClassName, peer.getLoggableId()); + validationResponse + .getDisconnectReason() + .ifPresent((disconnectReason) -> peer.disconnect(disconnectReason)); executorResult = new PeerTaskExecutorResult<>( - Optional.ofNullable(result), PeerTaskExecutorResponseCode.INVALID_RESPONSE); + Optional.ofNullable(result), + PeerTaskExecutorResponseCode.INVALID_RESPONSE, + Optional.of(peer)); } } catch (PeerNotConnected e) { executorResult = new PeerTaskExecutorResult<>( - Optional.empty(), PeerTaskExecutorResponseCode.PEER_DISCONNECTED); + Optional.empty(), + PeerTaskExecutorResponseCode.PEER_DISCONNECTED, + Optional.of(peer)); } catch (InterruptedException | TimeoutException e) { peer.recordRequestTimeout(requestMessageData.getCode()); timeoutCounter.labels(taskClassName).inc(); executorResult = - new PeerTaskExecutorResult<>(Optional.empty(), PeerTaskExecutorResponseCode.TIMEOUT); + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.TIMEOUT, Optional.of(peer)); } catch (InvalidPeerTaskResponseException e) { peer.recordUselessResponse(e.getMessage()); invalidResponseCounter.labels(taskClassName).inc(); + LOG.debug( + "Invalid response found for {} from peer {}", taskClassName, peer.getLoggableId(), e); executorResult = new PeerTaskExecutorResult<>( - Optional.empty(), PeerTaskExecutorResponseCode.INVALID_RESPONSE); + Optional.empty(), PeerTaskExecutorResponseCode.INVALID_RESPONSE, Optional.of(peer)); - } catch (ExecutionException e) { + } catch (Exception e) { internalExceptionCounter.labels(taskClassName).inc(); + LOG.error("Server error found for {} from peer {}", taskClassName, peer.getLoggableId(), e); executorResult = new PeerTaskExecutorResult<>( - Optional.empty(), PeerTaskExecutorResponseCode.INTERNAL_SERVER_ERROR); + Optional.empty(), + PeerTaskExecutorResponseCode.INTERNAL_SERVER_ERROR, + Optional.of(peer)); } } while (retriesRemaining-- > 0 && executorResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java index 86dec85c295..b3874011c85 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java @@ -14,7 +14,9 @@ */ package org.hyperledger.besu.ethereum.eth.manager.peertask; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; + import java.util.Optional; public record PeerTaskExecutorResult( - Optional result, PeerTaskExecutorResponseCode responseCode) {} + Optional result, PeerTaskExecutorResponseCode responseCode, Optional ethPeer) {} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskValidationResponse.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskValidationResponse.java new file mode 100644 index 00000000000..5fd4e6a98b8 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskValidationResponse.java @@ -0,0 +1,37 @@ +/* + * Copyright contributors to Besu. + * + * 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.manager.peertask; + +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; + +import java.util.Optional; + +public enum PeerTaskValidationResponse { + NO_RESULTS_RETURNED(null), + TOO_MANY_RESULTS_RETURNED(null), + RESULTS_DO_NOT_MATCH_QUERY(null), + NON_SEQUENTIAL_HEADERS_RETURNED( + DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS), + RESULTS_VALID_AND_GOOD(null); + private final Optional disconnectReason; + + PeerTaskValidationResponse(final DisconnectMessage.DisconnectReason disconnectReason) { + this.disconnectReason = Optional.ofNullable(disconnectReason); + } + + public Optional getDisconnectReason() { + return disconnectReason; + } +} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTask.java new file mode 100644 index 00000000000..4cde0a8b776 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTask.java @@ -0,0 +1,263 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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.manager.peertask.task; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.EthProtocol; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; +import org.hyperledger.besu.ethereum.eth.messages.BlockHeadersMessage; +import org.hyperledger.besu.ethereum.eth.messages.GetBlockHeadersMessage; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; + +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class GetHeadersFromPeerTask implements PeerTask> { + private static final Logger LOG = LoggerFactory.getLogger(GetHeadersFromPeerTask.class); + private final long blockNumber; + private final Hash blockHash; + private final int maxHeaders; + private final int skip; + private final Direction direction; + private final int maximumRetriesAgainstDifferentPeers; + private final ProtocolSchedule protocolSchedule; + private final long requiredBlockchainHeight; + + public GetHeadersFromPeerTask( + final long blockNumber, + final int maxHeaders, + final int skip, + final Direction direction, + final ProtocolSchedule protocolSchedule) { + this(blockNumber, maxHeaders, skip, direction, 5, protocolSchedule); + } + + public GetHeadersFromPeerTask( + final long blockNumber, + final int maxHeaders, + final int skip, + final Direction direction, + final int maximumRetriesAgainstDifferentPeers, + final ProtocolSchedule protocolSchedule) { + this( + null, + blockNumber, + maxHeaders, + skip, + direction, + maximumRetriesAgainstDifferentPeers, + protocolSchedule); + } + + public GetHeadersFromPeerTask( + final Hash blockHash, + final long blockNumber, + final int maxHeaders, + final int skip, + final Direction direction, + final ProtocolSchedule protocolSchedule) { + this(blockHash, blockNumber, maxHeaders, skip, direction, 5, protocolSchedule); + } + + public GetHeadersFromPeerTask( + final Hash blockHash, + final long blockNumber, + final int maxHeaders, + final int skip, + final Direction direction, + final int maximumRetriesAgainstDifferentPeers, + final ProtocolSchedule protocolSchedule) { + LOG.debug( + "Constructing GetHeadersFromPeerTask with hash {}, number {}, maxHeaders {}, skip {}, direction {}", + blockHash, + blockNumber, + maxHeaders, + skip, + direction); + this.blockHash = blockHash; + this.blockNumber = blockNumber; + this.maxHeaders = maxHeaders; + this.skip = skip; + this.direction = direction; + this.maximumRetriesAgainstDifferentPeers = maximumRetriesAgainstDifferentPeers; + this.protocolSchedule = protocolSchedule; + + requiredBlockchainHeight = + direction == Direction.FORWARD + ? blockNumber + (long) (maxHeaders - 1) * (skip + 1) + : blockNumber; + } + + @Override + public SubProtocol getSubProtocol() { + return EthProtocol.get(); + } + + @Override + public MessageData getRequestMessage() { + if (blockHash != null) { + return GetBlockHeadersMessage.create( + blockHash, maxHeaders, skip, direction == Direction.REVERSE); + } else { + return GetBlockHeadersMessage.create( + blockNumber, maxHeaders, skip, direction == Direction.REVERSE); + } + } + + @Override + public List processResponse(final MessageData messageData) + throws InvalidPeerTaskResponseException { + if (messageData == null) { + throw new InvalidPeerTaskResponseException("Response MessageData is null"); + } + return BlockHeadersMessage.readFrom(messageData).getHeaders(protocolSchedule); + } + + @Override + public Predicate getPeerRequirementFilter() { + return (ethPeer) -> + protocolSchedule.anyMatch((ps) -> ps.spec().isPoS()) + || ethPeer.chainState().getEstimatedHeight() >= requiredBlockchainHeight; + } + + @Override + public PeerTaskValidationResponse validateResult(final List blockHeaders) { + if (blockHeaders.isEmpty()) { + // Message contains no data - nothing to do + LOG.debug( + "No blockheaders returned for query starting at {}", + blockHash != null ? blockHash : blockNumber); + return PeerTaskValidationResponse.NO_RESULTS_RETURNED; + } + + if (blockHeaders.size() > maxHeaders) { + // Too many headers - this isn't our response + LOG.debug( + "Too many blockheaders returned for query starting at {}", + blockHash != null ? blockHash : blockNumber); + return PeerTaskValidationResponse.TOO_MANY_RESULTS_RETURNED; + } + + if ((blockHash != null && !blockHeaders.getFirst().getHash().equals(blockHash)) + || (blockHash == null && blockHeaders.getFirst().getNumber() != blockNumber)) { + // This isn't our message - nothing to do + LOG.debug( + "First header returned doesn't match query starting at {}", + blockHash != null ? blockHash : blockNumber); + return PeerTaskValidationResponse.RESULTS_DO_NOT_MATCH_QUERY; + } + + if (!isBlockHeadersMatchingRequest(blockHeaders)) { + LOG.debug( + "Blockheaders do not match expected headers from request for query starting at {}", + blockHash != null ? blockHash : blockNumber); + return PeerTaskValidationResponse.RESULTS_DO_NOT_MATCH_QUERY; + } + + if (blockHeaders.size() >= 2 && skip == 0) { + // headers are supposed to be sequential and at least 2 have been returned, check if a chain + // is formed + for (int i = 0; i < blockHeaders.size() - 1; i++) { + BlockHeader parentHeader = null; + BlockHeader childHeader = null; + switch (direction) { + case FORWARD: + parentHeader = blockHeaders.get(i); + childHeader = blockHeaders.get(i + 1); + break; + case REVERSE: + childHeader = blockHeaders.get(i); + parentHeader = blockHeaders.get(i + 1); + break; + } + if (!parentHeader.getHash().equals(childHeader.getParentHash())) { + LOG.warn( + "Blockheaders were non-sequential for query starting at {}", + blockHash != null ? blockHash : blockNumber); + return PeerTaskValidationResponse.NON_SEQUENTIAL_HEADERS_RETURNED; + } + } + } + return PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD; + } + + @Override + public void postProcessResult(final PeerTaskExecutorResult> result) { + final AtomicReference highestBlockHeader = + new AtomicReference<>(result.result().get().getFirst()); + for (BlockHeader blockHeader : result.result().get()) { + if (highestBlockHeader.get().getNumber() < blockHeader.getNumber()) { + highestBlockHeader.set(blockHeader); + } + } + result.ethPeer().ifPresent((ethPeer) -> ethPeer.chainState().update(highestBlockHeader.get())); + } + + @Override + public int getRetriesWithOtherPeer() { + return maximumRetriesAgainstDifferentPeers; + } + + public Long getBlockNumber() { + return blockNumber; + } + + public Hash getBlockHash() { + return blockHash; + } + + public int getMaxHeaders() { + return maxHeaders; + } + + public int getSkip() { + return skip; + } + + public Direction getDirection() { + return direction; + } + + public enum Direction { + FORWARD, + REVERSE + } + + private boolean isBlockHeadersMatchingRequest(final List blockHeaders) { + BlockHeader prevBlockHeader = blockHeaders.getFirst(); + final int expectedDelta = direction == Direction.REVERSE ? -(skip + 1) : (skip + 1); + BlockHeader header; + for (int i = 1; i < blockHeaders.size(); i++) { + header = blockHeaders.get(i); + if (header.getNumber() != prevBlockHeader.getNumber() + expectedDelta) { + // Skip doesn't match, this isn't our data + return false; + } + prevBlockHeader = header; + } + return true; + } +} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTask.java index b4b018b173c..850adb85c28 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTask.java @@ -23,6 +23,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; import org.hyperledger.besu.ethereum.eth.messages.GetReceiptsMessage; import org.hyperledger.besu.ethereum.eth.messages.ReceiptsMessage; import org.hyperledger.besu.ethereum.mainnet.BodyValidation; @@ -128,7 +129,16 @@ public Predicate getPeerRequirementFilter() { } @Override - public boolean isSuccess(final Map> result) { - return !result.isEmpty(); + public PeerTaskValidationResponse validateResult( + final Map> result) { + if (result.isEmpty()) { + return PeerTaskValidationResponse.NO_RESULTS_RETURNED; + } + + return PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD; + } + + public Collection getBlockHeaders() { + return blockHeaders; } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTask.java index c352fbc3009..ddb0271378d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTask.java @@ -20,6 +20,12 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.PeerDisconnectedException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -35,6 +41,7 @@ public class GetBlockFromPeerTask extends AbstractPeerTask { private static final Logger LOG = LoggerFactory.getLogger(GetBlockFromPeerTask.class); private final ProtocolSchedule protocolSchedule; + private final SynchronizerConfiguration synchronizerConfiguration; private final Optional hash; private final long blockNumber; private final MetricsSystem metricsSystem; @@ -42,10 +49,12 @@ public class GetBlockFromPeerTask extends AbstractPeerTask { protected GetBlockFromPeerTask( final ProtocolSchedule protocolSchedule, final EthContext ethContext, + final SynchronizerConfiguration synchronizerConfiguration, final Optional hash, final long blockNumber, final MetricsSystem metricsSystem) { super(ethContext, metricsSystem); + this.synchronizerConfiguration = synchronizerConfiguration; this.blockNumber = blockNumber; this.metricsSystem = metricsSystem; this.protocolSchedule = protocolSchedule; @@ -55,10 +64,12 @@ protected GetBlockFromPeerTask( public static GetBlockFromPeerTask create( final ProtocolSchedule protocolSchedule, final EthContext ethContext, + final SynchronizerConfiguration synchronizerConfiguration, final Optional hash, final long blockNumber, final MetricsSystem metricsSystem) { - return new GetBlockFromPeerTask(protocolSchedule, ethContext, hash, blockNumber, metricsSystem); + return new GetBlockFromPeerTask( + protocolSchedule, ethContext, synchronizerConfiguration, hash, blockNumber, metricsSystem); } @Override @@ -68,31 +79,22 @@ protected void executeTask() { "Downloading block {} from peer {}.", blockIdentifier, assignedPeer.map(EthPeer::toString).orElse("")); - downloadHeader() - .thenCompose(this::completeBlock) - .whenComplete( - (r, t) -> { - if (t != null) { - LOG.debug( - "Failed to download block {} from peer {} with message '{}' and cause '{}'", - blockIdentifier, - assignedPeer.map(EthPeer::toString).orElse(""), - t.getMessage(), - t.getCause()); - result.completeExceptionally(t); - } else if (r.getResult().isEmpty()) { - r.getPeer().recordUselessResponse("Download block returned an empty result"); - LOG.debug( - "Failed to download block {} from peer {} with empty result.", - blockIdentifier, - r.getPeer()); - result.completeExceptionally(new IncompleteResultsException()); - } else { - LOG.debug( - "Successfully downloaded block {} from peer {}.", blockIdentifier, r.getPeer()); - result.complete(new PeerTaskResult<>(r.getPeer(), r.getResult().get(0))); - } - }); + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + downloadHeaderUsingPeerTaskSystem() + .thenCompose(this::completeBlock) + .whenComplete((r, t) -> completeTask(r, t, blockIdentifier)); + }); + } else { + downloadHeader() + .thenCompose( + (peerTaskResult) -> CompletableFuture.completedFuture(peerTaskResult.getResult())) + .thenCompose(this::completeBlock) + .whenComplete((r, t) -> completeTask(r, t, blockIdentifier)); + } } private CompletableFuture>> downloadHeader() { @@ -113,9 +115,45 @@ private CompletableFuture>> downloadHeader() { }); } + private CompletableFuture> downloadHeaderUsingPeerTaskSystem() { + GetHeadersFromPeerTask task = + hash.map( + (h) -> + new GetHeadersFromPeerTask( + h, blockNumber, 1, 0, Direction.FORWARD, protocolSchedule)) + .orElseGet( + () -> + new GetHeadersFromPeerTask( + blockNumber, 1, 0, Direction.FORWARD, protocolSchedule)); + PeerTaskExecutorResult> taskResult; + if (assignedPeer.isPresent()) { + taskResult = ethContext.getPeerTaskExecutor().executeAgainstPeer(task, assignedPeer.get()); + } else { + taskResult = ethContext.getPeerTaskExecutor().execute(task); + } + + CompletableFuture> returnValue = new CompletableFuture>(); + if (taskResult.responseCode() == PeerTaskExecutorResponseCode.PEER_DISCONNECTED + && taskResult.ethPeer().isPresent()) { + returnValue.completeExceptionally(new PeerDisconnectedException(taskResult.ethPeer().get())); + } else if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + String logMessage = + "Peer " + + taskResult.ethPeer().map(EthPeer::getLoggableId).orElse("UNKNOWN") + + " failed to successfully return requested block headers. Response code was " + + taskResult.responseCode(); + returnValue.completeExceptionally(new RuntimeException(logMessage)); + LOG.debug(logMessage); + } else { + returnValue.complete(taskResult.result().get()); + } + return returnValue; + } + private CompletableFuture>> completeBlock( - final PeerTaskResult> headerResult) { - if (headerResult.getResult().isEmpty()) { + final List headers) { + if (headers.isEmpty()) { LOG.debug("header result is empty."); return CompletableFuture.failedFuture(new IncompleteResultsException()); } @@ -124,9 +162,38 @@ private CompletableFuture>> completeBlock( () -> { final GetBodiesFromPeerTask task = GetBodiesFromPeerTask.forHeaders( - protocolSchedule, ethContext, headerResult.getResult(), metricsSystem); - task.assignPeer(headerResult.getPeer()); + protocolSchedule, ethContext, headers, metricsSystem); + assignedPeer.ifPresent(task::assignPeer); return task.run(); }); } + + private void completeTask( + final PeerTaskResult> blockTaskResult, + final Throwable throwable, + final String blockIdentifier) { + if (throwable != null) { + LOG.debug( + "Failed to download block {} from peer {} with message '{}' and cause '{}'", + blockIdentifier, + assignedPeer.map(EthPeer::toString).orElse(""), + throwable.getMessage(), + throwable.getCause()); + result.completeExceptionally(throwable); + } else if (blockTaskResult.getResult().isEmpty()) { + blockTaskResult.getPeer().recordUselessResponse("Download block returned an empty result"); + LOG.debug( + "Failed to download block {} from peer {} with empty result.", + blockIdentifier, + blockTaskResult.getPeer()); + result.completeExceptionally(new IncompleteResultsException()); + } else { + LOG.debug( + "Successfully downloaded block {} from peer {}.", + blockIdentifier, + blockTaskResult.getPeer()); + result.complete( + new PeerTaskResult<>(blockTaskResult.getPeer(), blockTaskResult.getResult().get(0))); + } + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java index e4e472f4f4c..592e2786acc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -36,18 +37,21 @@ public class RetryingGetBlockFromPeersTask private static final Logger LOG = LoggerFactory.getLogger(RetryingGetBlockFromPeersTask.class); private final ProtocolSchedule protocolSchedule; + private final SynchronizerConfiguration synchronizerConfiguration; private final Optional maybeBlockHash; private final long blockNumber; protected RetryingGetBlockFromPeersTask( final EthContext ethContext, final ProtocolSchedule protocolSchedule, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final int maxRetries, final Optional maybeBlockHash, final long blockNumber) { super(ethContext, metricsSystem, Objects::isNull, maxRetries); this.protocolSchedule = protocolSchedule; + this.synchronizerConfiguration = synchronizerConfiguration; this.maybeBlockHash = maybeBlockHash; this.blockNumber = blockNumber; } @@ -55,12 +59,19 @@ protected RetryingGetBlockFromPeersTask( public static RetryingGetBlockFromPeersTask create( final ProtocolSchedule protocolSchedule, final EthContext ethContext, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final int maxRetries, final Optional maybeHash, final long blockNumber) { return new RetryingGetBlockFromPeersTask( - ethContext, protocolSchedule, metricsSystem, maxRetries, maybeHash, blockNumber); + ethContext, + protocolSchedule, + synchronizerConfiguration, + metricsSystem, + maxRetries, + maybeHash, + blockNumber); } @Override @@ -68,7 +79,12 @@ protected CompletableFuture> executeTaskOnCurrentPeer( final EthPeer currentPeer) { final GetBlockFromPeerTask getBlockTask = GetBlockFromPeerTask.create( - protocolSchedule, getEthContext(), maybeBlockHash, blockNumber, getMetricsSystem()); + protocolSchedule, + getEthContext(), + synchronizerConfiguration, + maybeBlockHash, + blockNumber, + getMetricsSystem()); getBlockTask.assignPeer(currentPeer); return executeSubTask(getBlockTask::run) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidator.java index dfc5b49d9bc..1e86f62b472 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidator.java @@ -19,8 +19,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByNumberTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -36,6 +41,8 @@ abstract class AbstractPeerBlockValidator implements PeerValidator { static long DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER = 10L; private final ProtocolSchedule protocolSchedule; + private final PeerTaskExecutor peerTaskExecutor; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; final long blockNumber; @@ -44,11 +51,15 @@ abstract class AbstractPeerBlockValidator implements PeerValidator { AbstractPeerBlockValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long blockNumber, final long chainHeightEstimationBuffer) { checkArgument(chainHeightEstimationBuffer >= 0); this.protocolSchedule = protocolSchedule; + this.peerTaskExecutor = peerTaskExecutor; + this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; this.blockNumber = blockNumber; this.chainHeightEstimationBuffer = chainHeightEstimationBuffer; @@ -57,44 +68,76 @@ abstract class AbstractPeerBlockValidator implements PeerValidator { @Override public CompletableFuture validatePeer( final EthContext ethContext, final EthPeer ethPeer) { - final AbstractPeerTask> getHeaderTask = - GetHeadersFromPeerByNumberTask.forSingleNumber( - protocolSchedule, ethContext, blockNumber, metricsSystem) - .setTimeout(Duration.ofSeconds(20)) - .assignPeer(ethPeer); - return getHeaderTask - .run() - .handle( - (res, err) -> { - if (err != null) { - // Mark peer as invalid on error - LOG.debug( - "Peer {} is invalid because required block ({}) is unavailable: {}", - ethPeer, - blockNumber, - err.toString()); - return false; - } - final List headers = res.getResult(); - if (headers.size() == 0) { - if (blockIsRequired()) { - // If no headers are returned, fail - LOG.debug( - "Peer {} is invalid because required block ({}) is unavailable.", - ethPeer, - blockNumber); - return false; + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + return ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + blockNumber, + 1, + 0, + GetHeadersFromPeerTask.Direction.FORWARD, + protocolSchedule); + PeerTaskExecutorResult> taskResult = + peerTaskExecutor.executeAgainstPeer(task, ethPeer); + CompletableFuture resultFuture; + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + resultFuture = CompletableFuture.completedFuture(false); } else { + resultFuture = + CompletableFuture.completedFuture( + validateBlockHeaders(ethPeer, taskResult.result().get())); + } + return resultFuture; + }); + } else { + final AbstractPeerTask> getHeaderTask = + GetHeadersFromPeerByNumberTask.forSingleNumber( + protocolSchedule, ethContext, blockNumber, metricsSystem) + .setTimeout(Duration.ofSeconds(20)) + .assignPeer(ethPeer); + return getHeaderTask + .run() + .handle( + (res, err) -> { + if (err != null) { + // Mark peer as invalid on error LOG.debug( - "Peer {} deemed valid because unavailable block ({}) is not required.", + "Peer {} is invalid because required block ({}) is unavailable: {}", ethPeer, - blockNumber); - return true; + blockNumber, + err.toString()); + return false; } - } - final BlockHeader header = headers.get(0); - return validateBlockHeader(ethPeer, header); - }); + final List headers = res.getResult(); + return validateBlockHeaders(ethPeer, headers); + }); + } + } + + private Boolean validateBlockHeaders(final EthPeer ethPeer, final List headers) { + boolean isValid; + if (headers.isEmpty()) { + if (blockIsRequired()) { + // If no headers are returned, fail + LOG.debug( + "Peer {} is invalid because required block ({}) is unavailable.", ethPeer, blockNumber); + isValid = false; + } else { + LOG.debug( + "Peer {} deemed valid because unavailable block ({}) is not required.", + ethPeer, + blockNumber); + isValid = true; + } + } else { + final BlockHeader header = headers.getFirst(); + isValid = validateBlockHeader(ethPeer, header); + } + return isValid; } abstract boolean validateBlockHeader(EthPeer ethPeer, BlockHeader header); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/CheckpointBlocksPeerValidator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/CheckpointBlocksPeerValidator.java index a66cc16785d..734017d1a6f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/CheckpointBlocksPeerValidator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/CheckpointBlocksPeerValidator.java @@ -17,6 +17,8 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -24,20 +26,37 @@ public class CheckpointBlocksPeerValidator extends RequiredBlocksPeerValidator { public CheckpointBlocksPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long blockNumber, final Hash hash, final long chainHeightEstimationBuffer) { - super(protocolSchedule, metricsSystem, blockNumber, hash, chainHeightEstimationBuffer); + super( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + blockNumber, + hash, + chainHeightEstimationBuffer); } public CheckpointBlocksPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long blockNumber, final Hash hash) { this( - protocolSchedule, metricsSystem, blockNumber, hash, DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + blockNumber, + hash, + DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/ClassicForkPeerValidator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/ClassicForkPeerValidator.java index 2958ec8e551..2ce969b8cd3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/ClassicForkPeerValidator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/ClassicForkPeerValidator.java @@ -16,6 +16,8 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -28,17 +30,33 @@ public class ClassicForkPeerValidator extends AbstractPeerBlockValidator { ClassicForkPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long daoBlockNumber, final long chainHeightEstimationBuffer) { - super(protocolSchedule, metricsSystem, daoBlockNumber, chainHeightEstimationBuffer); + super( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + daoBlockNumber, + chainHeightEstimationBuffer); } public ClassicForkPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long daoBlockNumber) { - this(protocolSchedule, metricsSystem, daoBlockNumber, DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); + this( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + daoBlockNumber, + DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidator.java index 01ce6144ee0..89bb36ec82f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidator.java @@ -16,6 +16,8 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -28,17 +30,33 @@ public class DaoForkPeerValidator extends AbstractPeerBlockValidator { DaoForkPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long daoBlockNumber, final long chainHeightEstimationBuffer) { - super(protocolSchedule, metricsSystem, daoBlockNumber, chainHeightEstimationBuffer); + super( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + daoBlockNumber, + chainHeightEstimationBuffer); } public DaoForkPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long daoBlockNumber) { - this(protocolSchedule, metricsSystem, daoBlockNumber, DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); + this( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + daoBlockNumber, + DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidator.java index bf8716c328b..aa423a56cd1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidator.java @@ -17,6 +17,8 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -30,21 +32,37 @@ public class RequiredBlocksPeerValidator extends AbstractPeerBlockValidator { public RequiredBlocksPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long blockNumber, final Hash hash, final long chainHeightEstimationBuffer) { - super(protocolSchedule, metricsSystem, blockNumber, chainHeightEstimationBuffer); + super( + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + blockNumber, + chainHeightEstimationBuffer); this.hash = hash; } public RequiredBlocksPeerValidator( final ProtocolSchedule protocolSchedule, + final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final long blockNumber, final Hash hash) { this( - protocolSchedule, metricsSystem, blockNumber, hash, DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); + protocolSchedule, + peerTaskExecutor, + synchronizerConfiguration, + metricsSystem, + blockNumber, + hash, + DEFAULT_CHAIN_HEIGHT_ESTIMATION_BUFFER); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/AbstractSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/AbstractSyncTargetManager.java index f5b5f978cb3..a60892f1574 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/AbstractSyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/AbstractSyncTargetManager.java @@ -73,6 +73,7 @@ public CompletableFuture findSyncTarget() { ethContext, bestPeer, config.getDownloaderHeaderRequestSize(), + config, metricsSystem) .run() .handle( 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 aee8ec48ea7..ab7e65f103e 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 @@ -536,6 +536,7 @@ private CompletableFuture scheduleGetBlockFromPeers( RetryingGetBlockFromPeersTask.create( protocolSchedule, ethContext, + config, metricsSystem, Math.max(1, ethContext.getEthPeers().peerCount()), maybeBlockHash, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index 67681acf6a7..08284e88fff 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -19,6 +19,10 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -38,28 +42,32 @@ public class ChainHeadTracker { private final EthContext ethContext; private final ProtocolSchedule protocolSchedule; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; public ChainHeadTracker( final EthContext ethContext, final ProtocolSchedule protocolSchedule, - final TrailingPeerLimiter trailingPeerLimiter, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { this.ethContext = ethContext; this.protocolSchedule = protocolSchedule; + this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; } public static void trackChainHeadForPeers( final EthContext ethContext, final ProtocolSchedule protocolSchedule, + final SynchronizerConfiguration synchronizerConfiguration, final Blockchain blockchain, final Supplier trailingPeerRequirementsCalculator, final MetricsSystem metricsSystem) { final TrailingPeerLimiter trailingPeerLimiter = new TrailingPeerLimiter(ethContext.getEthPeers(), trailingPeerRequirementsCalculator); final ChainHeadTracker tracker = - new ChainHeadTracker(ethContext, protocolSchedule, trailingPeerLimiter, metricsSystem); + new ChainHeadTracker( + ethContext, protocolSchedule, synchronizerConfiguration, metricsSystem); ethContext.getEthPeers().setChainHeadTracker(tracker); blockchain.observeBlockAdded(trailingPeerLimiter); } @@ -69,33 +77,78 @@ public CompletableFuture getBestHeaderFromPeer(final EthPeer peer) .setMessage("Requesting chain head info from {}...") .addArgument(peer::getLoggableId) .log(); - final CompletableFuture>> - bestHeaderFromPeerCompletableFuture = getBestHeaderFromPeerCompletableFuture(peer); - final CompletableFuture future = new CompletableFuture<>(); - bestHeaderFromPeerCompletableFuture.whenComplete( - (peerResult, error) -> { - if (peerResult != null && !peerResult.getResult().isEmpty()) { - final BlockHeader chainHeadHeader = peerResult.getResult().get(0); - peer.chainState().update(chainHeadHeader); - future.complete(chainHeadHeader); - LOG.atDebug() - .setMessage("Retrieved chain head info {} from {}...") - .addArgument( - () -> chainHeadHeader.getNumber() + " (" + chainHeadHeader.getBlockHash() + ")") - .addArgument(peer::getLoggableId) - .log(); - } else { - LOG.atDebug() - .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") - .addArgument(peer::getLoggableId) - .addArgument(error != null ? error : "Empty Response") - .log(); - peer.disconnect( - DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); - future.complete(null); - } - }); - return future; + + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + return ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + Hash.wrap(peer.chainState().getBestBlock().getHash()), + 0, + 1, + 0, + Direction.FORWARD, + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, peer); + if (taskResult.responseCode() == PeerTaskExecutorResponseCode.SUCCESS + && taskResult.result().isPresent()) { + BlockHeader chainHeadHeader = taskResult.result().get().getFirst(); + LOG.atDebug() + .setMessage("Retrieved chain head info {} from {}...") + .addArgument( + () -> + chainHeadHeader.getNumber() + + " (" + + chainHeadHeader.getBlockHash() + + ")") + .addArgument(peer::getLoggableId) + .log(); + return CompletableFuture.completedFuture(chainHeadHeader); + } else { + LOG.atDebug() + .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") + .addArgument(peer::getLoggableId) + .addArgument(taskResult.responseCode()) + .log(); + peer.disconnect( + DisconnectMessage.DisconnectReason + .USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); + return CompletableFuture.completedFuture(null); + } + }); + } else { + final CompletableFuture>> + bestHeaderFromPeerCompletableFuture = getBestHeaderFromPeerCompletableFuture(peer); + final CompletableFuture future = new CompletableFuture<>(); + bestHeaderFromPeerCompletableFuture.whenComplete( + (peerResult, error) -> { + if (peerResult != null && !peerResult.getResult().isEmpty()) { + final BlockHeader chainHeadHeader = peerResult.getResult().get(0); + peer.chainState().update(chainHeadHeader); + future.complete(chainHeadHeader); + LOG.atDebug() + .setMessage("Retrieved chain head info {} from {}...") + .addArgument( + () -> + chainHeadHeader.getNumber() + " (" + chainHeadHeader.getBlockHash() + ")") + .addArgument(peer::getLoggableId) + .log(); + } else { + LOG.atDebug() + .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") + .addArgument(peer::getLoggableId) + .addArgument(error != null ? error : "Empty Response") + .log(); + peer.disconnect( + DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); + future.complete(null); + } + }); + return future; + } } public CompletableFuture>> 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 66684ab7873..43dfa275eba 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 @@ -99,6 +99,7 @@ public DefaultSynchronizer( ChainHeadTracker.trackChainHeadForPeers( ethContext, protocolSchedule, + syncConfig, protocolContext.getBlockchain(), this::calculateTrailingPeerRequirements, metricsSystem); @@ -149,7 +150,6 @@ public DefaultSynchronizer( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -166,7 +166,6 @@ public DefaultSynchronizer( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -183,7 +182,6 @@ public DefaultSynchronizer( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStep.java index ae568491587..240a7502a64 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStep.java @@ -20,6 +20,9 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; import org.hyperledger.besu.ethereum.eth.sync.range.RangeHeaders; @@ -44,6 +47,7 @@ public class DownloadHeadersStep private final ProtocolContext protocolContext; private final EthContext ethContext; private final ValidationPolicy validationPolicy; + private final SynchronizerConfiguration synchronizerConfiguration; private final int headerRequestSize; private final MetricsSystem metricsSystem; @@ -52,12 +56,14 @@ public DownloadHeadersStep( final ProtocolContext protocolContext, final EthContext ethContext, final ValidationPolicy validationPolicy, + final SynchronizerConfiguration synchronizerConfiguration, final int headerRequestSize, final MetricsSystem metricsSystem) { this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; this.validationPolicy = validationPolicy; + this.synchronizerConfiguration = synchronizerConfiguration; this.headerRequestSize = headerRequestSize; this.metricsSystem = metricsSystem; } @@ -85,6 +91,7 @@ private CompletableFuture> downloadHeaders(final SyncTargetRan protocolSchedule, protocolContext, ethContext, + synchronizerConfiguration, range.getEnd(), range.getSegmentLengthExclusive(), validationPolicy, @@ -92,15 +99,39 @@ private CompletableFuture> downloadHeaders(final SyncTargetRan .run(); } else { LOG.debug("Downloading headers starting from {}", range.getStart().getNumber()); - return GetHeadersFromPeerByHashTask.startingAtHash( - protocolSchedule, - ethContext, - range.getStart().getHash(), - range.getStart().getNumber(), - headerRequestSize, - metricsSystem) - .run() - .thenApply(PeerTaskResult::getResult); + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + return ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + range.getStart().getHash(), + range.getStart().getNumber(), + headerRequestSize, + 0, + GetHeadersFromPeerTask.Direction.FORWARD, + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().execute(task); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + return CompletableFuture.failedFuture( + new RuntimeException("Unable to download headers for range " + range)); + } + return CompletableFuture.completedFuture(taskResult.result().get()); + }); + } else { + return GetHeadersFromPeerByHashTask.startingAtHash( + protocolSchedule, + ethContext, + range.getStart().getHash(), + range.getStart().getNumber(), + headerRequestSize, + metricsSystem) + .run() + .thenApply(PeerTaskResult::getResult); + } } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index bef62ec927e..ab0b94ab616 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -52,6 +53,7 @@ public class BackwardSyncContext { protected final ProtocolContext protocolContext; private final ProtocolSchedule protocolSchedule; private final EthContext ethContext; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; private final SyncState syncState; private final AtomicReference currentBackwardSyncStatus = new AtomicReference<>(); @@ -65,6 +67,7 @@ public class BackwardSyncContext { public BackwardSyncContext( final ProtocolContext protocolContext, final ProtocolSchedule protocolSchedule, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final EthContext ethContext, final SyncState syncState, @@ -72,6 +75,7 @@ public BackwardSyncContext( this( protocolContext, protocolSchedule, + synchronizerConfiguration, metricsSystem, ethContext, syncState, @@ -83,6 +87,7 @@ public BackwardSyncContext( public BackwardSyncContext( final ProtocolContext protocolContext, final ProtocolSchedule protocolSchedule, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem, final EthContext ethContext, final SyncState syncState, @@ -93,6 +98,7 @@ public BackwardSyncContext( this.protocolContext = protocolContext; this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; + this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; this.syncState = syncState; this.backwardChain = backwardChain; @@ -431,6 +437,10 @@ private void logBlockImportProgress(final long currImportedHeight) { } } + public SynchronizerConfiguration getSynchronizerConfiguration() { + return synchronizerConfiguration; + } + class Status { private final CompletableFuture currentFuture; private final long initialChainHeight; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStep.java index 748c71fc6f6..60baee2b640 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStep.java @@ -16,6 +16,10 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; import org.hyperledger.besu.ethereum.eth.manager.task.RetryingGetHeadersEndingAtFromPeerByHashTask; import java.util.List; @@ -69,29 +73,57 @@ protected CompletableFuture> requestHeaders(final Hash hash) { final int batchSize = context.getBatchSize(); LOG.trace("Requesting headers for hash {}, with batch size {}", hash, batchSize); - final RetryingGetHeadersEndingAtFromPeerByHashTask - retryingGetHeadersEndingAtFromPeerByHashTask = - RetryingGetHeadersEndingAtFromPeerByHashTask.endingAtHash( - context.getProtocolSchedule(), - context.getEthContext(), - hash, - 0, - batchSize, - context.getMetricsSystem(), - context.getEthContext().getEthPeers().peerCount()); - return context - .getEthContext() - .getScheduler() - .scheduleSyncWorkerTask(retryingGetHeadersEndingAtFromPeerByHashTask::run) - .thenApply( - blockHeaders -> { - LOG.atDebug() - .setMessage("Got headers {} -> {}") - .addArgument(blockHeaders.get(0)::getNumber) - .addArgument(blockHeaders.get(blockHeaders.size() - 1)::getNumber) - .log(); - return blockHeaders; - }); + CompletableFuture> headersResult; + if (context.getSynchronizerConfiguration().isPeerTaskSystemEnabled()) { + headersResult = + context + .getEthContext() + .getScheduler() + .scheduleSyncWorkerTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + hash, + 0, + batchSize, + 0, + Direction.REVERSE, + context.getEthContext().getEthPeers().peerCount(), + context.getProtocolSchedule()); + PeerTaskExecutorResult> taskResult = + context.getEthContext().getPeerTaskExecutor().execute(task); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + throw new RuntimeException("Unable to retrieve headers"); + } + return CompletableFuture.completedFuture(taskResult.result().get()); + }); + } else { + final RetryingGetHeadersEndingAtFromPeerByHashTask + retryingGetHeadersEndingAtFromPeerByHashTask = + RetryingGetHeadersEndingAtFromPeerByHashTask.endingAtHash( + context.getProtocolSchedule(), + context.getEthContext(), + hash, + 0, + batchSize, + context.getMetricsSystem(), + context.getEthContext().getEthPeers().peerCount()); + headersResult = + context + .getEthContext() + .getScheduler() + .scheduleSyncWorkerTask(retryingGetHeadersEndingAtFromPeerByHashTask::run); + } + return headersResult.thenApply( + blockHeaders -> { + LOG.atDebug() + .setMessage("Got headers {} -> {}") + .addArgument(blockHeaders.get(0)::getNumber) + .addArgument(blockHeaders.get(blockHeaders.size() - 1)::getNumber) + .log(); + return blockHeaders; + }); } @VisibleForTesting diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/SyncStepStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/SyncStepStep.java index 1f7bc8979b9..890d51ee82e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/SyncStepStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/SyncStepStep.java @@ -50,6 +50,7 @@ private CompletableFuture requestBlock(final Hash targetHash) { RetryingGetBlockFromPeersTask.create( context.getProtocolSchedule(), context.getEthContext(), + context.getSynchronizerConfiguration(), context.getMetricsSystem(), context.getEthContext().getEthPeers().peerCount(), Optional.of(targetHash), diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloadBlockStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloadBlockStep.java index 72f1fae764f..303a728cd2b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloadBlockStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloadBlockStep.java @@ -20,7 +20,6 @@ import org.hyperledger.besu.ethereum.core.BlockWithReceipts; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetReceiptsFromPeerTask; @@ -40,7 +39,6 @@ public class CheckpointDownloadBlockStep { private final ProtocolSchedule protocolSchedule; private final EthContext ethContext; - private final PeerTaskExecutor peerTaskExecutor; private final Checkpoint checkpoint; private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; @@ -48,13 +46,11 @@ public class CheckpointDownloadBlockStep { public CheckpointDownloadBlockStep( final ProtocolSchedule protocolSchedule, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final Checkpoint checkpoint, final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; - this.peerTaskExecutor = peerTaskExecutor; this.checkpoint = checkpoint; this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; @@ -65,6 +61,7 @@ public CompletableFuture> downloadBlock(final Hash h GetBlockFromPeerTask.create( protocolSchedule, ethContext, + synchronizerConfiguration, Optional.of(hash), checkpoint.blockNumber(), metricsSystem); @@ -85,7 +82,7 @@ private CompletableFuture> downloadReceipts( GetReceiptsFromPeerTask task = new GetReceiptsFromPeerTask(List.of(block.getHeader()), protocolSchedule); PeerTaskExecutorResult>> executorResult = - peerTaskExecutor.execute(task); + ethContext.getPeerTaskExecutor().execute(task); if (executorResult.responseCode() == PeerTaskExecutorResponseCode.SUCCESS) { List transactionReceipts = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java index 30134d9f6c5..03df47e4407 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java @@ -17,7 +17,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -62,7 +61,6 @@ public static Optional> createCheckpointDownloader( final ProtocolContext protocolContext, final MetricsSystem metricsSystem, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final WorldStateStorageCoordinator worldStateStorageCoordinator, final SyncState syncState, final Clock clock, @@ -112,7 +110,6 @@ public static Optional> createCheckpointDownloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, pivotBlockSelector, metricsSystem); @@ -130,7 +127,6 @@ public static Optional> createCheckpointDownloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, pivotBlockSelector, metricsSystem); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncActions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncActions.java index 61b997e6c53..5096b74e24f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncActions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncActions.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -35,7 +34,6 @@ public CheckpointSyncActions( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final SyncState syncState, final PivotBlockSelector pivotBlockSelector, final MetricsSystem metricsSystem) { @@ -45,7 +43,6 @@ public CheckpointSyncActions( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, pivotBlockSelector, metricsSystem); @@ -60,7 +57,6 @@ public ChainDownloader createChainDownloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, metricsSystem, currentState, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java index 2590e4736ae..5450b9e5a49 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PipelineChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -37,7 +36,6 @@ public static ChainDownloader create( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final SyncState syncState, final MetricsSystem metricsSystem, final FastSyncState fastSyncState, @@ -57,13 +55,7 @@ public static ChainDownloader create( syncState, syncTargetManager, new CheckpointSyncDownloadPipelineFactory( - config, - protocolSchedule, - protocolContext, - ethContext, - peerTaskExecutor, - fastSyncState, - metricsSystem), + config, protocolSchedule, protocolContext, ethContext, fastSyncState, metricsSystem), ethContext.getScheduler(), metricsSystem, syncDurationMetrics); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncDownloadPipelineFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncDownloadPipelineFactory.java index 0be10869861..27dc032b915 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncDownloadPipelineFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncDownloadPipelineFactory.java @@ -19,7 +19,6 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncDownloadPipelineFactory; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; @@ -41,17 +40,9 @@ public CheckpointSyncDownloadPipelineFactory( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final FastSyncState fastSyncState, final MetricsSystem metricsSystem) { - super( - syncConfig, - protocolSchedule, - protocolContext, - ethContext, - peerTaskExecutor, - fastSyncState, - metricsSystem); + super(syncConfig, protocolSchedule, protocolContext, ethContext, fastSyncState, metricsSystem); } @Override @@ -86,7 +77,7 @@ protected Pipeline createDownloadCheckPointPipeline( final CheckpointDownloadBlockStep checkPointDownloadBlockStep = new CheckpointDownloadBlockStep( - protocolSchedule, ethContext, peerTaskExecutor, checkpoint, syncConfig, metricsSystem); + protocolSchedule, ethContext, checkpoint, syncConfig, metricsSystem); return PipelineBuilder.createPipelineFrom( "fetchCheckpoints", diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStep.java index 876e96d1072..c2b7ee56901 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStep.java @@ -22,7 +22,6 @@ import org.hyperledger.besu.ethereum.core.BlockWithReceipts; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetReceiptsFromPeerTask; @@ -42,19 +41,16 @@ public class DownloadReceiptsStep private final ProtocolSchedule protocolSchedule; private final EthContext ethContext; - private final PeerTaskExecutor peerTaskExecutor; private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; public DownloadReceiptsStep( final ProtocolSchedule protocolSchedule, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; - this.peerTaskExecutor = peerTaskExecutor; this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; } @@ -81,7 +77,7 @@ public CompletableFuture> apply(final List blocks do { GetReceiptsFromPeerTask task = new GetReceiptsFromPeerTask(headers, protocolSchedule); PeerTaskExecutorResult>> getReceiptsResult = - peerTaskExecutor.execute(task); + ethContext.getPeerTaskExecutor().execute(task); if (getReceiptsResult.responseCode() == PeerTaskExecutorResponseCode.SUCCESS && getReceiptsResult.result().isPresent()) { Map> taskResult = getReceiptsResult.result().get(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActions.java index 58a64bd562a..0ce6d2f2ff5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActions.java @@ -18,8 +18,12 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.WaitForPeersTask; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; @@ -34,6 +38,7 @@ import org.hyperledger.besu.plugin.services.metrics.Counter; import java.time.Duration; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -49,7 +54,6 @@ public class FastSyncActions { protected final ProtocolSchedule protocolSchedule; protected final ProtocolContext protocolContext; protected final EthContext ethContext; - protected final PeerTaskExecutor peerTaskExecutor; protected final SyncState syncState; protected final PivotBlockSelector pivotBlockSelector; protected final MetricsSystem metricsSystem; @@ -62,7 +66,6 @@ public FastSyncActions( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final SyncState syncState, final PivotBlockSelector pivotBlockSelector, final MetricsSystem metricsSystem) { @@ -71,7 +74,6 @@ public FastSyncActions( this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; - this.peerTaskExecutor = peerTaskExecutor; this.syncState = syncState; this.pivotBlockSelector = pivotBlockSelector; this.metricsSystem = metricsSystem; @@ -103,7 +105,6 @@ public CompletableFuture selectPivotBlock(final FastSyncState fas } private CompletableFuture selectNewPivotBlock() { - return pivotBlockSelector .selectNewPivotBlock() .map(CompletableFuture::completedFuture) @@ -130,7 +131,7 @@ public CompletableFuture downloadPivotBlockHeader( private CompletableFuture internalDownloadPivotBlockHeader( final FastSyncState currentState) { if (currentState.hasPivotBlockHeader()) { - LOG.debug("Initial sync state {} already contains the block header", currentState); + LOG.info("Initial sync state {} already contains the block header", currentState); return completedFuture(currentState); } @@ -146,6 +147,7 @@ private CompletableFuture internalDownloadPivotBlockHeader( protocolSchedule, ethContext, metricsSystem, + syncConfig, currentState.getPivotBlockNumber().getAsLong(), syncConfig.getSyncMinimumPeerCount(), syncConfig.getSyncPivotDistance()) @@ -168,7 +170,6 @@ public ChainDownloader createChainDownloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, metricsSystem, currentState, @@ -177,13 +178,56 @@ public ChainDownloader createChainDownloader( private CompletableFuture downloadPivotBlockHeader(final Hash hash) { LOG.debug("Downloading pivot block header by hash {}", hash); - return RetryingGetHeaderFromPeerByHashTask.byHash( - protocolSchedule, - ethContext, - hash, - pivotBlockSelector.getMinRequiredBlockNumber(), - metricsSystem) - .getHeader() + CompletableFuture blockHeaderFuture; + if (syncConfig.isPeerTaskSystemEnabled()) { + blockHeaderFuture = + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + hash, + pivotBlockSelector.getMinRequiredBlockNumber(), + 1, + 0, + GetHeadersFromPeerTask.Direction.FORWARD, + ethContext.getEthPeers().peerCount(), + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().execute(task); + if (taskResult.responseCode() == PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE + || taskResult.responseCode() + == PeerTaskExecutorResponseCode.PEER_DISCONNECTED) { + LOG.error( + "Failed to download pivot block header. Response Code was {}", + taskResult.responseCode()); + return CompletableFuture.failedFuture(new NoAvailablePeersException()); + } else if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + LOG.error( + "Failed to download pivot block header. Response Code was {}", + taskResult.responseCode()); + return CompletableFuture.failedFuture( + new RuntimeException( + "Failed to download pivot block header. Response Code was " + + taskResult.responseCode())); + } else { + return CompletableFuture.completedFuture( + taskResult.result().get().getFirst()); + } + }); + } else { + blockHeaderFuture = + RetryingGetHeaderFromPeerByHashTask.byHash( + protocolSchedule, + ethContext, + hash, + pivotBlockSelector.getMinRequiredBlockNumber(), + metricsSystem) + .getHeader(); + } + return blockHeaderFuture .whenComplete( (blockHeader, throwable) -> { if (throwable != null) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java index 1bf55a3811a..c36ff7cb482 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.PipelineChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -36,7 +35,6 @@ public static ChainDownloader create( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final SyncState syncState, final MetricsSystem metricsSystem, final FastSyncState fastSyncState, @@ -55,13 +53,7 @@ public static ChainDownloader create( syncState, syncTargetManager, new FastSyncDownloadPipelineFactory( - config, - protocolSchedule, - protocolContext, - ethContext, - peerTaskExecutor, - fastSyncState, - metricsSystem), + config, protocolSchedule, protocolContext, ethContext, fastSyncState, metricsSystem), ethContext.getScheduler(), metricsSystem, syncDurationMetrics); 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 67085252e8f..707fc6aa13f 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 @@ -26,7 +26,6 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.DownloadBodiesStep; import org.hyperledger.besu.ethereum.eth.sync.DownloadHeadersStep; import org.hyperledger.besu.ethereum.eth.sync.DownloadPipelineFactory; @@ -58,7 +57,6 @@ public class FastSyncDownloadPipelineFactory implements DownloadPipelineFactory protected final ProtocolSchedule protocolSchedule; protected final ProtocolContext protocolContext; protected final EthContext ethContext; - protected final PeerTaskExecutor peerTaskExecutor; protected final FastSyncState fastSyncState; protected final MetricsSystem metricsSystem; protected final FastSyncValidationPolicy attachedValidationPolicy; @@ -70,14 +68,12 @@ public FastSyncDownloadPipelineFactory( final ProtocolSchedule protocolSchedule, final ProtocolContext protocolContext, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final FastSyncState fastSyncState, final MetricsSystem metricsSystem) { this.syncConfig = syncConfig; this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; - this.peerTaskExecutor = peerTaskExecutor; this.fastSyncState = fastSyncState; this.metricsSystem = metricsSystem; final LabelledMetric fastSyncValidationCounter = @@ -138,6 +134,7 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncT protocolContext, ethContext, detachedValidationPolicy, + syncConfig, headerRequestSize, metricsSystem); final RangeHeadersValidationStep validateHeadersJoinUpStep = @@ -145,8 +142,7 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncT final DownloadBodiesStep downloadBodiesStep = new DownloadBodiesStep(protocolSchedule, ethContext, metricsSystem); final DownloadReceiptsStep downloadReceiptsStep = - new DownloadReceiptsStep( - protocolSchedule, ethContext, peerTaskExecutor, syncConfig, metricsSystem); + new DownloadReceiptsStep(protocolSchedule, ethContext, syncConfig, metricsSystem); final ImportBlocksStep importBlockStep = new ImportBlocksStep( protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java index 0aaeefc6d6e..da3f12390ab 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java @@ -17,6 +17,7 @@ import static org.hyperledger.besu.util.FutureUtils.exceptionallyCompose; import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.TrailingPeerRequirements; import org.hyperledger.besu.ethereum.eth.sync.worldstate.StalledDownloadException; @@ -132,6 +133,12 @@ protected CompletableFuture handleFailure(final Throwable error) LOG.debug( "A download operation reached the max number of retries, re-pivoting to newer block"); return start(FastSyncState.EMPTY_SYNC_STATE); + } else if (rootCause instanceof NoAvailablePeersException) { + LOG.debug( + "No peers available for sync. Restarting sync in {} seconds", + FAST_SYNC_RETRY_DELAY.getSeconds()); + return fastSyncActions.scheduleFutureTask( + () -> start(FastSyncState.EMPTY_SYNC_STATE), FAST_SYNC_RETRY_DELAY); } else { LOG.error( "Encountered an unexpected error during sync. Restarting sync in " diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java index c5c586ea77c..e8a07c92050 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java @@ -17,8 +17,12 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.manager.task.WaitForPeerTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByNumberTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -26,8 +30,12 @@ import java.time.Duration; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; @@ -52,6 +60,7 @@ class PivotBlockConfirmer { private final EthContext ethContext; private final MetricsSystem metricsSystem; private final ProtocolSchedule protocolSchedule; + private final SynchronizerConfiguration synchronizerConfiguration; // The number of peers we need to query to confirm our pivot block private final int numberOfPeersToQuery; @@ -64,6 +73,7 @@ class PivotBlockConfirmer { private final Collection> runningQueries = new ConcurrentLinkedQueue<>(); private final Map pivotBlockQueriesByPeerId = new ConcurrentHashMap<>(); + private final Set peerIdsUsed = Collections.synchronizedSet(new HashSet<>()); private final Map pivotBlockVotes = new ConcurrentHashMap<>(); private final AtomicBoolean isStarted = new AtomicBoolean(false); @@ -73,12 +83,14 @@ class PivotBlockConfirmer { final ProtocolSchedule protocolSchedule, final EthContext ethContext, final MetricsSystem metricsSystem, + final SynchronizerConfiguration synchronizerConfiguration, final long pivotBlockNumber, final int numberOfPeersToQuery, final int numberOfRetriesPerPeer) { this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; this.metricsSystem = metricsSystem; + this.synchronizerConfiguration = synchronizerConfiguration; this.pivotBlockNumber = pivotBlockNumber; this.numberOfPeersToQuery = numberOfPeersToQuery; this.numberOfRetriesPerPeer = numberOfRetriesPerPeer; @@ -97,8 +109,14 @@ public CompletableFuture confirmPivotBlock() { private void queryPeers(final long blockNumber) { synchronized (runningQueries) { for (int i = 0; i < numberOfPeersToQuery; i++) { - final CompletableFuture query = - executePivotQuery(blockNumber).whenComplete(this::processReceivedHeader); + final CompletableFuture query; + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + query = + executePivotQueryWithPeerTaskSystem(blockNumber) + .whenComplete(this::processReceivedHeader); + } else { + query = executePivotQuery(blockNumber).whenComplete(this::processReceivedHeader); + } runningQueries.add(query); } } @@ -178,6 +196,58 @@ private CompletableFuture executePivotQuery(final long blockNumber) return pivotHeaderFuture; } + private CompletableFuture executePivotQueryWithPeerTaskSystem( + final long blockNumber) { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + blockNumber, 1, 0, GetHeadersFromPeerTask.Direction.FORWARD, protocolSchedule); + Optional maybeEthPeer; + final EthPeer ethPeer; + try { + do { + if (isCancelled.get()) { + return CompletableFuture.failedFuture( + new CancellationException("Pivot block confirmation has been cancelled")); + } + maybeEthPeer = + ethContext + .getEthPeers() + .getPeer( + (p) -> + task.getPeerRequirementFilter().test(p) + && !peerIdsUsed.contains(p.nodeId())); + } while (maybeEthPeer.isEmpty() && waitForPeer()); + ethPeer = maybeEthPeer.get(); + peerIdsUsed.add(ethPeer.nodeId()); + } catch (InterruptedException e) { + return CompletableFuture.failedFuture(e); + } + + return ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, ethPeer); + if (taskResult.responseCode() == PeerTaskExecutorResponseCode.INTERNAL_SERVER_ERROR) { + // something is probably wrong with the request, so we won't retry as below + return CompletableFuture.failedFuture( + new RuntimeException("Unexpected internal issue")); + } else if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + // recursively call executePivotQueryWithPeerTaskSystem to retry with a different + // peer. + return executePivotQueryWithPeerTaskSystem(blockNumber); + } + return CompletableFuture.completedFuture(taskResult.result().get().getFirst()); + }); + } + + private boolean waitForPeer() throws InterruptedException { + Thread.sleep(Duration.ofSeconds(5)); + return true; + } + private Optional createPivotQuery(final long blockNumber) { return ethContext .getEthPeers() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java index d8ff3627101..6198c2c85b9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.util.ExceptionUtils; @@ -46,6 +47,7 @@ public class PivotBlockRetriever { private final EthContext ethContext; private final MetricsSystem metricsSystem; private final ProtocolSchedule protocolSchedule; + private final SynchronizerConfiguration synchronizerConfiguration; // The number of peers we need to query to confirm our pivot block private final int peersToQuery; @@ -66,6 +68,7 @@ public class PivotBlockRetriever { final ProtocolSchedule protocolSchedule, final EthContext ethContext, final MetricsSystem metricsSystem, + final SynchronizerConfiguration synchronizerConfiguration, final long pivotBlockNumber, final int peersToQuery, final long pivotBlockNumberResetDelta, @@ -73,6 +76,7 @@ public class PivotBlockRetriever { this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; this.metricsSystem = metricsSystem; + this.synchronizerConfiguration = synchronizerConfiguration; this.pivotBlockNumber = new AtomicLong(pivotBlockNumber); this.peersToQuery = peersToQuery; this.pivotBlockNumberResetDelta = pivotBlockNumberResetDelta; @@ -83,6 +87,7 @@ public PivotBlockRetriever( final ProtocolSchedule protocolSchedule, final EthContext ethContext, final MetricsSystem metricsSystem, + final SynchronizerConfiguration synchronizerConfiguration, final long pivotBlockNumber, final int peersToQuery, final long pivotBlockNumberResetDelta) { @@ -90,6 +95,7 @@ public PivotBlockRetriever( protocolSchedule, ethContext, metricsSystem, + synchronizerConfiguration, pivotBlockNumber, peersToQuery, pivotBlockNumberResetDelta, @@ -111,6 +117,7 @@ private void confirmBlock(final long blockNumber) { protocolSchedule, ethContext, metricsSystem, + synchronizerConfiguration, pivotBlockNumber.get(), peersToQuery, MAX_QUERY_RETRIES_PER_PEER); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotSelectorFromSafeBlock.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotSelectorFromSafeBlock.java index c3bc9a5f903..bfbba248c1f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotSelectorFromSafeBlock.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotSelectorFromSafeBlock.java @@ -20,12 +20,17 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.WaitForPeersTask; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByHashTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -42,6 +47,7 @@ public class PivotSelectorFromSafeBlock implements PivotBlockSelector { private final EthContext ethContext; private final MetricsSystem metricsSystem; private final GenesisConfigOptions genesisConfig; + private final SynchronizerConfiguration synchronizerConfiguration; private final Supplier> forkchoiceStateSupplier; private final Runnable cleanupAction; @@ -55,6 +61,7 @@ public PivotSelectorFromSafeBlock( final EthContext ethContext, final MetricsSystem metricsSystem, final GenesisConfigOptions genesisConfig, + final SynchronizerConfiguration synchronizerConfiguration, final Supplier> forkchoiceStateSupplier, final Runnable cleanupAction) { this.protocolContext = protocolContext; @@ -62,6 +69,7 @@ public PivotSelectorFromSafeBlock( this.ethContext = ethContext; this.metricsSystem = metricsSystem; this.genesisConfig = genesisConfig; + this.synchronizerConfiguration = synchronizerConfiguration; this.forkchoiceStateSupplier = forkchoiceStateSupplier; this.cleanupAction = cleanupAction; } @@ -142,20 +150,48 @@ public long getBestChainHeight() { } private CompletableFuture downloadBlockHeader(final Hash hash) { - return RetryingGetHeaderFromPeerByHashTask.byHash( - protocolSchedule, ethContext, hash, 0, metricsSystem) - .getHeader() - .whenComplete( - (blockHeader, throwable) -> { - if (throwable != null) { - LOG.debug("Error downloading block header by hash {}", hash); - } else { - LOG.atDebug() - .setMessage("Successfully downloaded pivot block header by hash {}") - .addArgument(blockHeader::toLogString) - .log(); - } - }); + CompletableFuture resultFuture; + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + resultFuture = + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + hash, + 0, + 1, + 0, + GetHeadersFromPeerTask.Direction.FORWARD, + ethContext.getEthPeers().peerCount(), + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().execute(task); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + return CompletableFuture.failedFuture( + new RuntimeException("Unable to retrieve header")); + } + return CompletableFuture.completedFuture(taskResult.result().get().getFirst()); + }); + } else { + resultFuture = + RetryingGetHeaderFromPeerByHashTask.byHash( + protocolSchedule, ethContext, hash, 0, metricsSystem) + .getHeader(); + } + return resultFuture.whenComplete( + (blockHeader, throwable) -> { + if (throwable != null) { + LOG.debug("Error downloading block header by hash {}", hash); + } else { + LOG.atDebug() + .setMessage("Successfully downloaded pivot block header by hash {}") + .addArgument(blockHeader::toLogString) + .log(); + } + }); } private CompletableFuture waitForPeers(final int count) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index c4ac358b806..16f996605eb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.eth.sync.fastsync; import static java.util.concurrent.CompletableFuture.completedFuture; -import static org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER; import static org.hyperledger.besu.util.log.LogUtil.throttledLog; import org.hyperledger.besu.ethereum.ProtocolContext; @@ -23,6 +22,9 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthPeers; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.sync.AbstractSyncTargetManager; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByNumberTask; @@ -120,18 +122,51 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { private CompletableFuture> confirmPivotBlockHeader(final EthPeer bestPeer) { final BlockHeader pivotBlockHeader = fastSyncState.getPivotBlockHeader().get(); - final RetryingGetHeaderFromPeerByNumberTask task = - RetryingGetHeaderFromPeerByNumberTask.forSingleNumber( - protocolSchedule, - ethContext, - metricsSystem, - pivotBlockHeader.getNumber(), - MAX_QUERY_RETRIES_PER_PEER); - task.assignPeer(bestPeer); - return ethContext - .getScheduler() - // Task is a retrying task. Make sure that the timeout is long enough to allow for retries. - .timeout(task, Duration.ofSeconds(MAX_QUERY_RETRIES_PER_PEER * SECONDS_PER_REQUEST + 2)) + CompletableFuture> headersFuture; + if (config.isPeerTaskSystemEnabled()) { + headersFuture = + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + pivotBlockHeader.getNumber(), + 1, + 0, + GetHeadersFromPeerTask.Direction.FORWARD, + PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER, + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, bestPeer); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + return CompletableFuture.failedFuture( + new RuntimeException("Unable to retrieve requested header from peer")); + } + return CompletableFuture.completedFuture(taskResult.result().get()); + }); + + } else { + final RetryingGetHeaderFromPeerByNumberTask task = + RetryingGetHeaderFromPeerByNumberTask.forSingleNumber( + protocolSchedule, + ethContext, + metricsSystem, + pivotBlockHeader.getNumber(), + PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER); + task.assignPeer(bestPeer); + headersFuture = + ethContext + .getScheduler() + // Task is a retrying task. Make sure that the timeout is long enough to allow for + // retries. + .timeout( + task, + Duration.ofSeconds( + PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER * SECONDS_PER_REQUEST + 2)); + } + return headersFuture .thenCompose( result -> { if (peerHasDifferentPivotBlock(result)) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/FastDownloaderFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/FastDownloaderFactory.java index 1d775cc80fd..8b71a57885d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/FastDownloaderFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/FastDownloaderFactory.java @@ -17,7 +17,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -60,7 +59,6 @@ public static Optional> create( final ProtocolContext protocolContext, final MetricsSystem metricsSystem, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final WorldStateStorageCoordinator worldStateStorageCoordinator, final SyncState syncState, final Clock clock, @@ -128,7 +126,6 @@ public static Optional> create( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, pivotBlockSelector, 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 b822236b8f0..570ef303779 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 @@ -99,6 +99,7 @@ public Pipeline createDownloadPipelineForSyncTarget(final SyncTarget target) protocolContext, ethContext, detachedValidationPolicy, + syncConfig, headerRequestSize, metricsSystem); final RangeHeadersValidationStep validateHeadersJoinUpStep = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java index e73c5aa9f5f..cf5e6025182 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java @@ -21,6 +21,10 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -116,31 +120,64 @@ private CompletableFuture> requestHeaders( .addArgument(referenceHeader.getNumber()) .addArgument(skip) .log(); - return GetHeadersFromPeerByHashTask.startingAtHash( - protocolSchedule, - ethContext, - referenceHeader.getHash(), - referenceHeader.getNumber(), - // + 1 because lastHeader will be returned as well. - headerCount + 1, - skip, - metricsSystem) - .assignPeer(peer) - .run() - .thenApply(PeerTaskResult::getResult) - .thenApply( - headers -> { - if (headers.size() < headerCount) { - LOG.atTrace() - .setMessage( - "Peer {} returned fewer headers than requested. Expected: {}, Actual: {}") - .addArgument(peer) - .addArgument(headerCount) - .addArgument(headers.size()) - .log(); - } - return stripExistingRangeHeaders(referenceHeader, headers); - }); + CompletableFuture> headersFuture; + if (syncConfig.isPeerTaskSystemEnabled()) { + headersFuture = + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + referenceHeader.getHash(), + referenceHeader.getNumber(), + // + 1 because lastHeader will be returned as well. + headerCount + 1, + skip, + Direction.FORWARD, + protocolSchedule); + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, peer); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + LOG.warn( + "Unsuccessfully used peer task system to fetch headers. Response code was {}", + taskResult.responseCode()); + return CompletableFuture.failedFuture( + new RuntimeException( + "Unable to retrieve headers. Response code was " + + taskResult.responseCode())); + } + return CompletableFuture.completedFuture(taskResult.result().get()); + }); + } else { + headersFuture = + GetHeadersFromPeerByHashTask.startingAtHash( + protocolSchedule, + ethContext, + referenceHeader.getHash(), + referenceHeader.getNumber(), + // + 1 because lastHeader will be returned as well. + headerCount + 1, + skip, + metricsSystem) + .assignPeer(peer) + .run() + .thenApply(PeerTaskResult::getResult); + } + return headersFuture.thenApply( + headers -> { + if (headers.size() < headerCount) { + LOG.atTrace() + .setMessage( + "Peer {} returned fewer headers than requested. Expected: {}, Actual: {}") + .addArgument(peer) + .addArgument(headerCount) + .addArgument(headers.size()) + .log(); + } + return stripExistingRangeHeaders(referenceHeader, headers); + }); } private List stripExistingRangeHeaders( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java index 6c5ce0b04e9..5de8ceb9843 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java @@ -17,7 +17,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -58,7 +57,6 @@ public static Optional> createSnapDownloader( final ProtocolContext protocolContext, final MetricsSystem metricsSystem, final EthContext ethContext, - final PeerTaskExecutor peerTaskExecutor, final WorldStateStorageCoordinator worldStateStorageCoordinator, final SyncState syncState, final Clock clock, @@ -123,7 +121,6 @@ public static Optional> createSnapDownloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, pivotBlockSelector, metricsSystem), diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTask.java index 91aa6e5ff8d..2f622e4470e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTask.java @@ -18,9 +18,15 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.PeerDisconnectedException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractEthTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByNumberTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.util.BlockchainUtil; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -46,6 +52,7 @@ public class DetermineCommonAncestorTask extends AbstractEthTask { private final ProtocolContext protocolContext; private final EthPeer peer; private final int headerRequestSize; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; private long maximumPossibleCommonAncestorNumber; @@ -59,6 +66,7 @@ private DetermineCommonAncestorTask( final EthContext ethContext, final EthPeer peer, final int headerRequestSize, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { super(metricsSystem); this.protocolSchedule = protocolSchedule; @@ -66,6 +74,7 @@ private DetermineCommonAncestorTask( this.protocolContext = protocolContext; this.peer = peer; this.headerRequestSize = headerRequestSize; + this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; maximumPossibleCommonAncestorNumber = @@ -83,9 +92,16 @@ public static DetermineCommonAncestorTask create( final EthContext ethContext, final EthPeer peer, final int headerRequestSize, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { return new DetermineCommonAncestorTask( - protocolSchedule, protocolContext, ethContext, peer, headerRequestSize, metricsSystem); + protocolSchedule, + protocolContext, + ethContext, + peer, + headerRequestSize, + synchronizerConfiguration, + metricsSystem); } @Override @@ -100,16 +116,50 @@ protected void executeTask() { result.completeExceptionally(new IllegalStateException("No common ancestor.")); return; } - requestHeaders() - .thenCompose(this::processHeaders) - .whenComplete( - (peerResult, error) -> { - if (error != null) { - result.completeExceptionally(error); - } else if (!result.isDone()) { - executeTaskTimed(); - } - }); + + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + do { + PeerTaskExecutorResult> taskResult = + requestHeadersUsingPeerTaskSystem(); + if (taskResult.responseCode() == PeerTaskExecutorResponseCode.PEER_DISCONNECTED) { + result.completeExceptionally(new PeerDisconnectedException(peer)); + continue; + } else if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + result.completeExceptionally( + new RuntimeException( + "Peer failed to successfully return requested block headers")); + continue; + } + taskResult.ethPeer().ifPresent((peer) -> taskResult.result().get().getFirst()); + processHeaders(taskResult.result().get()); + if (maximumPossibleCommonAncestorNumber == minimumPossibleCommonAncestorNumber) { + // Bingo, we found our common ancestor. + result.complete(commonAncestorCandidate); + } else if (maximumPossibleCommonAncestorNumber < BlockHeader.GENESIS_BLOCK_NUMBER + && !result.isDone()) { + result.completeExceptionally(new IllegalStateException("No common ancestor.")); + } + } while (!result.isDone()); + }); + + } else { + requestHeaders() + .thenApply(AbstractPeerTask.PeerTaskResult::getResult) + .thenCompose(this::processHeaders) + .whenComplete( + (peerResult, error) -> { + if (error != null) { + result.completeExceptionally(error); + } else if (!result.isDone()) { + executeTaskTimed(); + } + }); + } } @VisibleForTesting @@ -136,6 +186,26 @@ CompletableFuture>> requestHea .run()); } + PeerTaskExecutorResult> requestHeadersUsingPeerTaskSystem() { + final long range = maximumPossibleCommonAncestorNumber - minimumPossibleCommonAncestorNumber; + final int skipInterval = initialQuery ? 0 : calculateSkipInterval(range, headerRequestSize); + final int count = + initialQuery ? headerRequestSize : calculateCount((double) range, skipInterval); + LOG.debug( + "Searching for common ancestor with {} between {} and {}", + peer, + minimumPossibleCommonAncestorNumber, + maximumPossibleCommonAncestorNumber); + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + maximumPossibleCommonAncestorNumber, + count, + skipInterval, + Direction.REVERSE, + protocolSchedule); + return ethContext.getPeerTaskExecutor().executeAgainstPeer(task, peer); + } + /** * In the case where the remote chain contains 100 blocks, the initial count work out to 11, and * the skip interval would be 9. This would yield the headers (0, 10, 20, 30, 40, 50, 60, 70, 80, @@ -151,10 +221,8 @@ static int calculateCount(final double range, final int skipInterval) { return Math.toIntExact((long) Math.ceil(range / (skipInterval + 1)) + 1); } - private CompletableFuture processHeaders( - final AbstractPeerTask.PeerTaskResult> headersResult) { + private CompletableFuture processHeaders(final List headers) { initialQuery = false; - final List headers = headersResult.getResult(); if (headers.isEmpty()) { // Nothing to do return CompletableFuture.completedFuture(null); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java index 8ca376902e4..5eee6495ebd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java @@ -25,11 +25,15 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractGetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetBodiesFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.ValidationPolicy; import org.hyperledger.besu.ethereum.eth.sync.tasks.exceptions.InvalidBlockException; import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator; @@ -61,6 +65,7 @@ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask> executePeerTask( final Optional assignedPeer) { LOG.debug( "Downloading headers from {} to {}.", startingBlockNumber, referenceHeader.getNumber()); - final CompletableFuture> task = - downloadHeaders(assignedPeer).thenCompose(this::processHeaders); - return task.whenComplete( + final CompletableFuture> headersFuture; + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { + headersFuture = + downloadHeadersUsingPeerTaskSystem(assignedPeer) + .thenCompose(this::processHeadersUsingPeerTask); + } else { + headersFuture = downloadHeaders(assignedPeer).thenCompose(this::processHeaders); + } + return headersFuture.whenComplete( (r, t) -> { // We're done if we've filled all requested headers if (lastFilledHeaderIndex == 0) { @@ -179,73 +196,127 @@ private CompletableFuture>> downloadHeaders( }); } + private CompletableFuture>> + downloadHeadersUsingPeerTaskSystem(final Optional ethPeer) { + return ethContext + .getScheduler() + .scheduleServiceTask( + () -> { + // Figure out parameters for our headers request + final boolean partiallyFilled = lastFilledHeaderIndex < segmentLength; + final BlockHeader referenceHeaderForNextRequest = + partiallyFilled ? headers[lastFilledHeaderIndex] : referenceHeader; + final Hash referenceHash = referenceHeaderForNextRequest.getHash(); + final int count = partiallyFilled ? lastFilledHeaderIndex : segmentLength; + + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + referenceHash, + referenceHeaderForNextRequest.getNumber(), + count + 1, + 0, + GetHeadersFromPeerTask.Direction.REVERSE, + protocolSchedule); + PeerTaskExecutorResult> taskResult; + if (ethPeer.isPresent()) { + taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, ethPeer.get()); + } else { + taskResult = ethContext.getPeerTaskExecutor().execute(task); + } + + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + return CompletableFuture.failedFuture( + new RuntimeException( + "Failed to download headers. Response code was " + + taskResult.responseCode())); + } + return CompletableFuture.completedFuture(taskResult); + }); + } + @VisibleForTesting CompletableFuture> processHeaders( final PeerTaskResult> headersResult) { return executeWorkerSubTask( ethContext.getScheduler(), () -> { - final CompletableFuture> future = new CompletableFuture<>(); - BlockHeader child = null; - boolean firstSkipped = false; - final int previousHeaderIndex = lastFilledHeaderIndex; - for (final BlockHeader header : headersResult.getResult()) { - final int headerIndex = - Ints.checkedCast( - segmentLength - (referenceHeader.getNumber() - header.getNumber())); - if (!firstSkipped) { - // Skip over reference header - firstSkipped = true; - continue; - } - if (child == null) { - child = - (headerIndex == segmentLength - 1) ? referenceHeader : headers[headerIndex + 1]; - } + return processHeaders(headersResult.getResult(), headersResult.getPeer()); + }); + } - final boolean foundChild = child != null; - final boolean headerInRange = checkHeaderInRange(header); - final boolean headerInvalid = foundChild && !validateHeader(child, header); - if (!headerInRange || !foundChild || headerInvalid) { - final BlockHeader invalidHeader = child; - final CompletableFuture badBlockHandled = - headerInvalid - ? markBadBlock(invalidHeader, headersResult.getPeer()) - : CompletableFuture.completedFuture(null); - badBlockHandled.whenComplete( - (res, err) -> { - LOG.debug( - "Received invalid headers from peer (BREACH_OF_PROTOCOL), disconnecting from: {}", - headersResult.getPeer()); - headersResult - .getPeer() - .disconnect(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_HEADERS); - final InvalidBlockException exception; - if (invalidHeader == null) { - final String msg = - String.format( - "Received misordered blocks. Missing child of %s", - header.toLogString()); - exception = InvalidBlockException.create(msg); - } else { - final String errorMsg = - headerInvalid - ? "Header failed validation" - : "Out-of-range header received from peer"; - exception = InvalidBlockException.fromInvalidBlock(errorMsg, invalidHeader); - } - future.completeExceptionally(exception); - }); + private CompletableFuture> processHeadersUsingPeerTask( + final PeerTaskExecutorResult> headersResult) { + final List blockHeaders = + headersResult + .result() + .orElseThrow( + () -> new RuntimeException("Expected blockHeaders in PeerTaskExecutorResult")); + final EthPeer ethPeer = + headersResult + .ethPeer() + .orElseThrow(() -> new RuntimeException("Expected a peer in PeerTaskExecutorResult")); + return processHeaders(blockHeaders, ethPeer); + } - return future; - } - headers[headerIndex] = header; - lastFilledHeaderIndex = headerIndex; - child = header; - } - future.complete(asList(headers).subList(lastFilledHeaderIndex, previousHeaderIndex)); - return future; - }); + private CompletableFuture> processHeaders( + final List blockHeaders, final EthPeer ethPeer) { + final CompletableFuture> future = new CompletableFuture<>(); + BlockHeader child = null; + boolean firstSkipped = false; + final int previousHeaderIndex = lastFilledHeaderIndex; + for (final BlockHeader header : blockHeaders) { + final int headerIndex = + Ints.checkedCast(segmentLength - (referenceHeader.getNumber() - header.getNumber())); + if (!firstSkipped) { + // Skip over reference header + firstSkipped = true; + continue; + } + if (child == null) { + child = (headerIndex == segmentLength - 1) ? referenceHeader : headers[headerIndex + 1]; + } + + final boolean foundChild = child != null; + final boolean headerInRange = checkHeaderInRange(header); + final boolean headerInvalid = foundChild && !validateHeader(child, header); + if (!headerInRange || !foundChild || headerInvalid) { + final BlockHeader invalidHeader = child; + final CompletableFuture badBlockHandled = + headerInvalid + ? markBadBlock(invalidHeader, ethPeer) + : CompletableFuture.completedFuture(null); + badBlockHandled.whenComplete( + (res, err) -> { + LOG.debug( + "Received invalid headers from peer (BREACH_OF_PROTOCOL), disconnecting from: {}", + ethPeer); + ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_HEADERS); + final InvalidBlockException exception; + if (invalidHeader == null) { + final String msg = + String.format( + "Received misordered blocks. Missing child of %s", header.toLogString()); + exception = InvalidBlockException.create(msg); + } else { + final String errorMsg = + headerInvalid + ? "Header failed validation" + : "Out-of-range header received from peer"; + exception = InvalidBlockException.fromInvalidBlock(errorMsg, invalidHeader); + } + future.completeExceptionally(exception); + }); + + return future; + } + headers[headerIndex] = header; + lastFilledHeaderIndex = headerIndex; + child = header; + } + future.complete(asList(headers).subList(lastFilledHeaderIndex, previousHeaderIndex)); + return future; } private CompletableFuture markBadBlock(final BlockHeader badHeader, final EthPeer badPeer) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestBuilder.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestBuilder.java index 0a267022b95..90c69925818 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestBuilder.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestBuilder.java @@ -23,6 +23,7 @@ import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.core.ProtocolScheduleFixture; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -63,6 +64,7 @@ public class EthProtocolManagerTestBuilder { private List peerValidators; private Optional mergePeerFilter; private SynchronizerConfiguration synchronizerConfiguration; + private PeerTaskExecutor peerTaskExecutor; public static EthProtocolManagerTestBuilder builder() { return new EthProtocolManagerTestBuilder(); @@ -159,6 +161,12 @@ public EthProtocolManagerTestBuilder setEthScheduler(final EthScheduler ethSched return this; } + public EthProtocolManagerTestBuilder setPeerTaskExecutor( + final PeerTaskExecutor peerTaskExecutor) { + this.peerTaskExecutor = peerTaskExecutor; + return this; + } + public EthProtocolManager build() { if (protocolSchedule == null) { protocolSchedule = DEFAULT_PROTOCOL_SCHEDULE; @@ -215,8 +223,12 @@ public EthProtocolManager build() { ethScheduler = new DeterministicEthScheduler(DeterministicEthScheduler.TimeoutPolicy.NEVER_TIMEOUT); } + if (peerTaskExecutor == null) { + peerTaskExecutor = mock(PeerTaskExecutor.class); + } if (ethContext == null) { - ethContext = new EthContext(ethPeers, ethMessages, snapMessages, ethScheduler); + ethContext = + new EthContext(ethPeers, ethMessages, snapMessages, ethScheduler, peerTaskExecutor); } if (peerValidators == null) { peerValidators = Collections.emptyList(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index 9d6f63df283..79638fee41e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -37,6 +37,7 @@ 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.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; @@ -64,6 +65,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; /** * @param The type of data being requested from the network @@ -92,6 +94,7 @@ public abstract class AbstractMessageTaskTest { protected EthContext ethContext; protected EthPeers ethPeers; protected TransactionPool transactionPool; + protected PeerTaskExecutor peerTaskExecutor; protected AtomicBoolean peersDoTimeout; protected AtomicInteger peerCountToTimeout; @@ -131,7 +134,8 @@ public void setupTest() { final EthScheduler ethScheduler = new DeterministicEthScheduler( () -> peerCountToTimeout.getAndDecrement() > 0 || peersDoTimeout.get()); - ethContext = new EthContext(ethPeers, ethMessages, ethScheduler); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); + ethContext = new EthContext(ethPeers, ethMessages, ethScheduler, peerTaskExecutor); final SyncState syncState = new SyncState(blockchain, ethContext.getEthPeers()); transactionPool = TransactionPoolFactory.createTransactionPool( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java index 9639de154d7..d0ee3f163ec 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import java.util.Optional; @@ -73,7 +74,8 @@ public void testExecuteAgainstPeerWithNoRetriesAndSuccessfulFlow() Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); - Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD); PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); @@ -85,6 +87,37 @@ public void testExecuteAgainstPeerWithNoRetriesAndSuccessfulFlow() Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.responseCode()); } + @Test + public void testExecuteAgainstPeerWithNoRetriesAndPeerShouldBeDisconnected() + throws PeerConnection.PeerNotConnected, + ExecutionException, + InterruptedException, + TimeoutException, + InvalidPeerTaskResponseException { + + Object responseObject = new Object(); + + Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); + Mockito.when(peerTask.getRetriesWithSamePeer()).thenReturn(0); + Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); + Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) + .thenReturn(responseMessageData); + Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.NON_SEQUENTIAL_HEADERS_RETURNED); + + PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); + + Mockito.verify(ethPeer) + .disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS); + + Assertions.assertNotNull(result); + Assertions.assertTrue(result.result().isPresent()); + Assertions.assertSame(responseObject, result.result().get()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.INVALID_RESPONSE, result.responseCode()); + } + @Test public void testExecuteAgainstPeerWithNoRetriesAndPartialSuccessfulFlow() throws PeerConnection.PeerNotConnected, @@ -102,7 +135,8 @@ public void testExecuteAgainstPeerWithNoRetriesAndPartialSuccessfulFlow() Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); - Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(false); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.NO_RESULTS_RETURNED); PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); @@ -131,7 +165,8 @@ public void testExecuteAgainstPeerWithRetriesAndSuccessfulFlowAfterFirstFailure( .thenReturn(responseMessageData); Mockito.when(requestMessageData.getCode()).thenReturn(requestMessageDataCode); Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); - Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD); PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); @@ -237,7 +272,8 @@ public void testExecuteWithNoRetriesAndSuccessFlow() Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); - Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD); PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); @@ -275,7 +311,8 @@ public void testExecuteWithPeerSwitchingAndSuccessFlow() Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, peer2)) .thenReturn(responseMessageData); Mockito.when(peerTask.processResponse(responseMessageData)).thenReturn(responseObject); - Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true); + Mockito.when(peerTask.validateResult(responseObject)) + .thenReturn(PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD); PeerTaskExecutorResult result = peerTaskExecutor.execute(peerTask); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskExecutorAnswer.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskExecutorAnswer.java new file mode 100644 index 00000000000..9437265efba --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskExecutorAnswer.java @@ -0,0 +1,88 @@ +/* + * Copyright contributors to Besu. + * + * 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.manager.peertask.task; + +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.manager.EthPeers; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +public class GetHeadersFromPeerTaskExecutorAnswer + implements Answer>> { + private final Blockchain otherBlockchain; + private final EthPeers ethPeers; + + public GetHeadersFromPeerTaskExecutorAnswer( + final Blockchain otherBlockchain, final EthPeers ethPeers) { + this.otherBlockchain = otherBlockchain; + this.ethPeers = ethPeers; + } + + @Override + public PeerTaskExecutorResult> answer(final InvocationOnMock invocationOnMock) + throws Throwable { + GetHeadersFromPeerTask task = invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + List getHeadersFromPeerTaskResult = new ArrayList<>(); + BlockHeader initialHeader; + if (task.getBlockHash() != null) { + initialHeader = otherBlockchain.getBlockHeader(task.getBlockHash()).orElse(null); + } else { + initialHeader = otherBlockchain.getBlockHeader(task.getBlockNumber()).orElse(null); + } + getHeadersFromPeerTaskResult.add(initialHeader); + + if (initialHeader != null && task.getMaxHeaders() > 1) { + if (task.getDirection() == GetHeadersFromPeerTask.Direction.FORWARD) { + int skip = task.getSkip() + 1; + long nextHeaderNumber = initialHeader.getNumber() + skip; + long getLimit = nextHeaderNumber + ((task.getMaxHeaders() - 1) * skip); + for (long i = nextHeaderNumber; i < getLimit; i += skip) { + Optional header = otherBlockchain.getBlockHeader(i); + if (header.isPresent()) { + getHeadersFromPeerTaskResult.add(header.get()); + } else { + break; + } + } + + } else { + int skip = task.getSkip() + 1; + long nextHeaderNumber = initialHeader.getNumber() - skip; + long getLimit = nextHeaderNumber - ((task.getMaxHeaders() - 1) * skip); + for (long i = initialHeader.getNumber() - 1; i > getLimit; i -= skip) { + Optional header = otherBlockchain.getBlockHeader(i); + if (header.isPresent()) { + getHeadersFromPeerTaskResult.add(header.get()); + } else { + break; + } + } + } + } + + return new PeerTaskExecutorResult<>( + Optional.of(getHeadersFromPeerTaskResult), + PeerTaskExecutorResponseCode.SUCCESS, + ethPeers.bestPeer()); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskTest.java new file mode 100644 index 00000000000..4fd66a8dd88 --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetHeadersFromPeerTaskTest.java @@ -0,0 +1,172 @@ +/* + * Copyright contributors to Besu. + * + * 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.manager.peertask.task; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; +import org.hyperledger.besu.ethereum.eth.EthProtocol; +import org.hyperledger.besu.ethereum.eth.manager.ChainState; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask.Direction; +import org.hyperledger.besu.ethereum.eth.messages.BlockHeadersMessage; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; + +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +public class GetHeadersFromPeerTaskTest { + + @Test + public void testGetSubProtocol() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(0, 1, 0, Direction.FORWARD, null); + Assertions.assertEquals(EthProtocol.get(), task.getSubProtocol()); + } + + @Test + public void testGetRequestMessageForHash() { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask(Hash.ZERO, 0, 1, 0, Direction.FORWARD, null); + MessageData requestMessageData = task.getRequestMessage(); + Assertions.assertEquals( + "0xe4a00000000000000000000000000000000000000000000000000000000000000000018080", + requestMessageData.getData().toHexString()); + } + + @Test + public void testGetRequestMessageForBlockNumber() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(123, 1, 0, Direction.FORWARD, null); + MessageData requestMessageData = task.getRequestMessage(); + Assertions.assertEquals("0xc47b018080", requestMessageData.getData().toHexString()); + } + + @Test + public void testGetRequestMessageForHashWhenBlockNumberAlsoProvided() { + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask(Hash.ZERO, 123, 1, 0, Direction.FORWARD, null); + MessageData requestMessageData = task.getRequestMessage(); + Assertions.assertEquals( + "0xe4a00000000000000000000000000000000000000000000000000000000000000000018080", + requestMessageData.getData().toHexString()); + } + + @Test + public void testProcessResponseWithNullMessageData() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(0, 1, 0, Direction.FORWARD, null); + Assertions.assertThrows( + InvalidPeerTaskResponseException.class, + () -> task.processResponse(null), + "Response MessageData is null"); + } + + @Test + public void testProcessResponse() throws InvalidPeerTaskResponseException { + final BlockchainSetupUtil blockchainSetupUtil = + BlockchainSetupUtil.forTesting(DataStorageFormat.FOREST); + blockchainSetupUtil.importAllBlocks(); + Blockchain blockchain = blockchainSetupUtil.getBlockchain(); + BlockHeader blockHeader = blockchain.getChainHeadHeader(); + BlockHeadersMessage responseMessage = BlockHeadersMessage.create(blockHeader); + + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask( + blockHeader.getBlockHash(), + 0, + 1, + 0, + Direction.FORWARD, + blockchainSetupUtil.getProtocolSchedule()); + + Assertions.assertEquals( + List.of(blockchain.getChainHeadHeader()), task.processResponse(responseMessage)); + } + + @Test + public void testGetPeerRequirementFilter() { + ProtocolSchedule protocolSchedule = Mockito.mock(ProtocolSchedule.class); + Mockito.when(protocolSchedule.anyMatch(Mockito.any())).thenReturn(false); + + GetHeadersFromPeerTask task = + new GetHeadersFromPeerTask(5, 1, 0, Direction.FORWARD, protocolSchedule); + + EthPeer failForShortChainHeight = mockPeer(1); + EthPeer successfulCandidate = mockPeer(5); + + Assertions.assertFalse(task.getPeerRequirementFilter().test(failForShortChainHeight)); + Assertions.assertTrue(task.getPeerRequirementFilter().test(successfulCandidate)); + } + + @Test + public void testValidateResultForEmptyResult() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(5, 1, 0, Direction.FORWARD, null); + Assertions.assertEquals( + PeerTaskValidationResponse.NO_RESULTS_RETURNED, + task.validateResult(Collections.emptyList())); + } + + @Test + public void testShouldDisconnectPeerForTooManyHeadersReturned() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(5, 1, 1, Direction.FORWARD, null); + + BlockHeader header1 = Mockito.mock(BlockHeader.class); + BlockHeader header2 = Mockito.mock(BlockHeader.class); + BlockHeader header3 = Mockito.mock(BlockHeader.class); + + Assertions.assertEquals( + PeerTaskValidationResponse.TOO_MANY_RESULTS_RETURNED, + task.validateResult(List.of(header1, header2, header3))); + } + + @Test + public void testValidateResultForNonSequentialHeaders() { + GetHeadersFromPeerTask task = new GetHeadersFromPeerTask(1, 3, 0, Direction.FORWARD, null); + + Hash block1Hash = Hash.fromHexStringLenient("01"); + Hash block2Hash = Hash.fromHexStringLenient("02"); + BlockHeader header1 = Mockito.mock(BlockHeader.class); + Mockito.when(header1.getNumber()).thenReturn(1L); + Mockito.when(header1.getHash()).thenReturn(block1Hash); + BlockHeader header2 = Mockito.mock(BlockHeader.class); + Mockito.when(header2.getNumber()).thenReturn(2L); + Mockito.when(header2.getHash()).thenReturn(block2Hash); + Mockito.when(header2.getParentHash()).thenReturn(block1Hash); + BlockHeader header3 = Mockito.mock(BlockHeader.class); + Mockito.when(header3.getNumber()).thenReturn(3L); + Mockito.when(header3.getParentHash()).thenReturn(Hash.ZERO); + + Assertions.assertEquals( + PeerTaskValidationResponse.NON_SEQUENTIAL_HEADERS_RETURNED, + task.validateResult(List.of(header1, header2, header3))); + } + + private EthPeer mockPeer(final long chainHeight) { + EthPeer ethPeer = Mockito.mock(EthPeer.class); + ChainState chainState = Mockito.mock(ChainState.class); + + Mockito.when(ethPeer.chainState()).thenReturn(chainState); + Mockito.when(chainState.getEstimatedHeight()).thenReturn(chainHeight); + + return ethPeer; + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTaskTest.java index 243db8462b3..8b8530aa02e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetReceiptsFromPeerTaskTest.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.eth.manager.ChainState; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; import org.hyperledger.besu.ethereum.eth.messages.EthPV63; import org.hyperledger.besu.ethereum.eth.messages.GetReceiptsMessage; import org.hyperledger.besu.ethereum.eth.messages.ReceiptsMessage; @@ -222,20 +223,23 @@ public void testGetPeerRequirementFilter() { } @Test - public void testIsSuccessForPartialSuccess() { + public void testValidateResultForPartialSuccess() { GetReceiptsFromPeerTask task = new GetReceiptsFromPeerTask(Collections.emptyList(), null); - Assertions.assertFalse(task.isSuccess(Collections.emptyMap())); + Assertions.assertEquals( + PeerTaskValidationResponse.NO_RESULTS_RETURNED, + task.validateResult(Collections.emptyMap())); } @Test - public void testIsSuccessForFullSuccess() { + public void testValidateResultForFullSuccess() { GetReceiptsFromPeerTask task = new GetReceiptsFromPeerTask(Collections.emptyList(), null); Map> map = new HashMap<>(); map.put(mockBlockHeader(1), null); - Assertions.assertTrue(task.isSuccess(map)); + Assertions.assertEquals( + PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD, task.validateResult(map)); } private BlockHeader mockBlockHeader(final long blockNumber) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTaskTest.java index 99ea4ad06cb..ff2aeee2ba4 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBlockFromPeerTaskTest.java @@ -24,6 +24,8 @@ import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.AbstractMessageTaskTest; import org.hyperledger.besu.ethereum.eth.manager.exceptions.EthTaskException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.util.ExceptionUtils; import java.util.Optional; @@ -32,6 +34,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; public class GetBlockFromPeerTaskTest extends AbstractMessageTaskTest> { @@ -47,9 +50,11 @@ protected Block generateDataToBeRequested() { @Override protected EthTask> createTask(final Block requestedData) { + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); return GetBlockFromPeerTask.create( protocolSchedule, ethContext, + SynchronizerConfiguration.builder().build(), Optional.of(requestedData.getHash()), BLOCK_NUMBER, metricsSystem); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java index d1594d46f07..3d8734c4a5c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java @@ -22,6 +22,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.RetryingSwitchingPeerMessageTaskTest; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import java.util.Optional; import java.util.concurrent.ExecutionException; @@ -51,6 +52,7 @@ protected RetryingGetBlockFromPeersTask createTask(final PeerTaskResult r return RetryingGetBlockFromPeersTask.create( protocolSchedule, ethContext, + SynchronizerConfiguration.builder().build(), metricsSystem, maxRetries, Optional.of(requestedData.getResult().getHash()), diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidatorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidatorTest.java index c27b656df8d..27b4a7b4597 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidatorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/DaoForkPeerValidatorTest.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; @@ -38,7 +39,12 @@ public class DaoForkPeerValidatorTest extends AbstractPeerBlockValidatorTest { @Override AbstractPeerBlockValidator createValidator(final long blockNumber, final long buffer) { return new DaoForkPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), blockNumber, buffer); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + blockNumber, + buffer); } @Test @@ -54,7 +60,12 @@ public void validatePeer_responsivePeerOnRightSideOfFork() { final PeerValidator validator = new DaoForkPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), daoBlockNumber, 0); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + daoBlockNumber, + 0); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, daoBlockNumber); @@ -82,7 +93,12 @@ public void validatePeer_responsivePeerOnWrongSideOfFork() { final PeerValidator validator = new DaoForkPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), daoBlockNumber, 0); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + daoBlockNumber, + 0); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, daoBlockNumber); @@ -107,7 +123,12 @@ public void validatePeer_responsivePeerDoesNotHaveBlockWhenPastForkHeight() { final PeerValidator validator = new DaoForkPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), daoBlockNumber, 0); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + daoBlockNumber, + 0); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, daoBlockNumber); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidatorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidatorTest.java index 38486570d5f..198ee990b2a 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidatorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/RequiredBlocksPeerValidatorTest.java @@ -25,6 +25,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import java.util.concurrent.CompletableFuture; @@ -37,7 +38,13 @@ public class RequiredBlocksPeerValidatorTest extends AbstractPeerBlockValidatorT @Override AbstractPeerBlockValidator createValidator(final long blockNumber, final long buffer) { return new RequiredBlocksPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), blockNumber, Hash.ZERO, buffer); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + blockNumber, + Hash.ZERO, + buffer); } @Test @@ -54,6 +61,8 @@ public void validatePeer_responsivePeerWithRequiredBlock() { final PeerValidator validator = new RequiredBlocksPeerValidator( ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), new NoOpMetricsSystem(), requiredBlockNumber, requiredBlock.getHash(), @@ -86,6 +95,8 @@ public void validatePeer_responsivePeerWithBadRequiredBlock() { final PeerValidator validator = new RequiredBlocksPeerValidator( ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), new NoOpMetricsSystem(), requiredBlockNumber, Hash.ZERO, @@ -113,7 +124,12 @@ public void validatePeer_responsivePeerDoesNotHaveBlockWhenPastForkHeight() { final PeerValidator validator = new RequiredBlocksPeerValidator( - ProtocolScheduleFixture.MAINNET, new NoOpMetricsSystem(), 1, Hash.ZERO); + ProtocolScheduleFixture.MAINNET, + null, + SynchronizerConfiguration.builder().build(), + new NoOpMetricsSystem(), + 1, + Hash.ZERO); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java index c06d1cda2a8..8a6f4c4ecc0 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java @@ -656,7 +656,8 @@ public void shouldNotImportBlocksThatAreAlreadyBeingImported() { new ForkIdManager( blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), - ethScheduler); + ethScheduler, + null); final BlockPropagationManager blockPropagationManager = new BlockPropagationManager( syncConfig, @@ -797,7 +798,8 @@ public Object answer(final InvocationOnMock invocation) throws Throwable { new ForkIdManager( blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), - ethScheduler); + ethScheduler, + null); final BlockPropagationManager blockPropagationManager = new BlockPropagationManager( syncConfig, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java index 6fa8a6ec9d1..dfad68a65ee 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.eth.sync; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.datatypes.Hash; @@ -26,10 +25,14 @@ import org.hyperledger.besu.ethereum.core.MiningConfiguration; import org.hyperledger.besu.ethereum.difficulty.fixed.FixedDifficultyProtocolSchedule; import org.hyperledger.besu.ethereum.eth.manager.ChainState; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer.Responder; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTaskExecutorAnswer; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.evm.internal.EvmConfiguration; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; @@ -44,6 +47,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; +import org.mockito.Mockito; public class ChainHeadTrackerTest { @@ -51,6 +55,10 @@ public class ChainHeadTrackerTest { private MutableBlockchain blockchain; private EthProtocolManager ethProtocolManager; private RespondingEthPeer respondingPeer; + + private PeerTaskExecutor peerTaskExecutor; + private SynchronizerConfiguration synchronizerConfiguration; + private ChainHeadTracker chainHeadTracker; private final ProtocolSchedule protocolSchedule = @@ -63,8 +71,6 @@ public class ChainHeadTrackerTest { false, new NoOpMetricsSystem()); - private final TrailingPeerLimiter trailingPeerLimiter = mock(TrailingPeerLimiter.class); - static class ChainHeadTrackerTestArguments implements ArgumentsProvider { @Override public Stream provideArguments(final ExtensionContext context) { @@ -73,10 +79,15 @@ public Stream provideArguments(final ExtensionContext conte } } - public void setup(final DataStorageFormat storageFormat) { + public void setup(final DataStorageFormat storageFormat, final boolean isPeerTaskSystemEnabled) { blockchainSetupUtil = BlockchainSetupUtil.forTesting(storageFormat); blockchain = blockchainSetupUtil.getBlockchain(); - ethProtocolManager = EthProtocolManagerTestBuilder.builder().setBlockchain(blockchain).build(); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); + ethProtocolManager = + EthProtocolManagerTestBuilder.builder() + .setBlockchain(blockchain) + .setPeerTaskExecutor(peerTaskExecutor) + .build(); respondingPeer = RespondingEthPeer.builder() .ethProtocolManager(ethProtocolManager) @@ -84,11 +95,24 @@ public void setup(final DataStorageFormat storageFormat) { .totalDifficulty(blockchain.getChainHead().getTotalDifficulty()) .estimatedHeight(0) .build(); + GetHeadersFromPeerTaskExecutorAnswer getHeadersAnswer = + new GetHeadersFromPeerTaskExecutorAnswer( + blockchain, ethProtocolManager.ethContext().getEthPeers()); + Mockito.when(peerTaskExecutor.execute(Mockito.any(GetHeadersFromPeerTask.class))) + .thenAnswer(getHeadersAnswer); + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.any(EthPeer.class))) + .thenAnswer(getHeadersAnswer); + synchronizerConfiguration = + SynchronizerConfiguration.builder() + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) + .build(); chainHeadTracker = new ChainHeadTracker( ethProtocolManager.ethContext(), protocolSchedule, - trailingPeerLimiter, + synchronizerConfiguration, new NoOpMetricsSystem()); } @@ -96,7 +120,7 @@ public void setup(final DataStorageFormat storageFormat) { @ArgumentsSource(ChainHeadTrackerTestArguments.class) public void shouldRequestHeaderChainHeadWhenNewPeerConnects( final DataStorageFormat storageFormat) { - setup(storageFormat); + setup(storageFormat, false); final Responder responder = RespondingEthPeer.blockchainResponder( blockchainSetupUtil.getBlockchain(), @@ -112,11 +136,23 @@ public void shouldRequestHeaderChainHeadWhenNewPeerConnects( .isEqualTo(blockchain.getChainHeadBlockNumber()); } + @ParameterizedTest + @ArgumentsSource(ChainHeadTrackerTestArguments.class) + public void shouldRequestHeaderChainHeadWhenNewPeerConnectsUsingPeerTaskSystem( + final DataStorageFormat storageFormat) { + setup(storageFormat, true); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); + + Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); + Assertions.assertThat(chainHeadState().getEstimatedHeight()) + .isEqualTo(blockchain.getChainHeadBlockNumber()); + } + @ParameterizedTest @ArgumentsSource(ChainHeadTrackerTestArguments.class) public void shouldIgnoreHeadersIfChainHeadHasAlreadyBeenUpdatedWhileWaiting( final DataStorageFormat storageFormat) { - setup(storageFormat); + setup(storageFormat, false); final Responder responder = RespondingEthPeer.blockchainResponder( blockchainSetupUtil.getBlockchain(), @@ -132,10 +168,23 @@ public void shouldIgnoreHeadersIfChainHeadHasAlreadyBeenUpdatedWhileWaiting( Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); } + @ParameterizedTest + @ArgumentsSource(ChainHeadTrackerTestArguments.class) + public void shouldIgnoreHeadersIfChainHeadHasAlreadyBeenUpdatedWhileWaitingUsingPeerTaskSystem( + final DataStorageFormat storageFormat) { + setup(storageFormat, true); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); + + // Change the hash of the current known head + respondingPeer.getEthPeer().chainState().statusReceived(Hash.EMPTY_TRIE_HASH, Difficulty.ONE); + + Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); + } + @ParameterizedTest @ArgumentsSource(ChainHeadTrackerTestArguments.class) public void shouldCheckTrialingPeerLimits(final DataStorageFormat storageFormat) { - setup(storageFormat); + setup(storageFormat, false); final Responder responder = RespondingEthPeer.blockchainResponder( blockchainSetupUtil.getBlockchain(), @@ -151,6 +200,18 @@ public void shouldCheckTrialingPeerLimits(final DataStorageFormat storageFormat) .isEqualTo(blockchain.getChainHeadBlockNumber()); } + @ParameterizedTest + @ArgumentsSource(ChainHeadTrackerTestArguments.class) + public void shouldCheckTrialingPeerLimitsUsingPeerTaskSystem( + final DataStorageFormat storageFormat) { + setup(storageFormat, true); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); + + Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); + Assertions.assertThat(chainHeadState().getEstimatedHeight()) + .isEqualTo(blockchain.getChainHeadBlockNumber()); + } + private ChainState chainHeadState() { return respondingPeer.getEthPeer().chainState(); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStepTest.java index dfc2986610a..e2dee4c2c53 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/DownloadHeadersStepTest.java @@ -27,6 +27,10 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.sync.range.RangeHeaders; import org.hyperledger.besu.ethereum.eth.sync.range.SyncTargetRange; import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; @@ -37,11 +41,13 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; public class DownloadHeadersStepTest { @@ -52,6 +58,7 @@ public class DownloadHeadersStepTest { private final EthPeer syncTarget = mock(EthPeer.class); private EthProtocolManager ethProtocolManager; + private PeerTaskExecutor peerTaskExecutor; private DownloadHeadersStep downloader; private SyncTargetRange checkpointRange; @@ -66,15 +73,12 @@ public static void setUpClass() { @BeforeEach public void setUp() { - ethProtocolManager = EthProtocolManagerTestBuilder.builder().setBlockchain(blockchain).build(); - downloader = - new DownloadHeadersStep( - protocolSchedule, - protocolContext, - ethProtocolManager.ethContext(), - () -> HeaderValidationMode.DETACHED_ONLY, - HEADER_REQUEST_SIZE, - new NoOpMetricsSystem()); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); + ethProtocolManager = + EthProtocolManagerTestBuilder.builder() + .setBlockchain(blockchain) + .setPeerTaskExecutor(peerTaskExecutor) + .build(); checkpointRange = new SyncTargetRange( @@ -83,6 +87,15 @@ public void setUp() { @Test public void shouldRetrieveHeadersForCheckpointRange() { + downloader = + new DownloadHeadersStep( + protocolSchedule, + protocolContext, + ethProtocolManager.ethContext(), + () -> HeaderValidationMode.DETACHED_ONLY, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + HEADER_REQUEST_SIZE, + new NoOpMetricsSystem()); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); final CompletableFuture result = downloader.apply(checkpointRange); @@ -95,6 +108,15 @@ public void shouldRetrieveHeadersForCheckpointRange() { @Test public void shouldCancelRequestToPeerWhenReturnedFutureIsCancelled() { + downloader = + new DownloadHeadersStep( + protocolSchedule, + protocolContext, + ethProtocolManager.ethContext(), + () -> HeaderValidationMode.DETACHED_ONLY, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + HEADER_REQUEST_SIZE, + new NoOpMetricsSystem()); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); final CompletableFuture result = this.downloader.apply(checkpointRange); @@ -110,6 +132,15 @@ public void shouldCancelRequestToPeerWhenReturnedFutureIsCancelled() { @Test public void shouldReturnOnlyEndHeaderWhenCheckpointRangeHasLengthOfOne() { + downloader = + new DownloadHeadersStep( + protocolSchedule, + protocolContext, + ethProtocolManager.ethContext(), + () -> HeaderValidationMode.DETACHED_ONLY, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + HEADER_REQUEST_SIZE, + new NoOpMetricsSystem()); final SyncTargetRange checkpointRange = new SyncTargetRange( syncTarget, blockchain.getBlockHeader(3).get(), blockchain.getBlockHeader(4).get()); @@ -122,6 +153,15 @@ public void shouldReturnOnlyEndHeaderWhenCheckpointRangeHasLengthOfOne() { @Test public void shouldGetRemainingHeadersWhenRangeHasNoEnd() { + downloader = + new DownloadHeadersStep( + protocolSchedule, + protocolContext, + ethProtocolManager.ethContext(), + () -> HeaderValidationMode.DETACHED_ONLY, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + HEADER_REQUEST_SIZE, + new NoOpMetricsSystem()); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); final SyncTargetRange checkpointRange = new SyncTargetRange(peer.getEthPeer(), blockchain.getBlockHeader(3).get()); @@ -134,6 +174,48 @@ public void shouldGetRemainingHeadersWhenRangeHasNoEnd() { .isCompletedWithValue(new RangeHeaders(checkpointRange, headersFromChain(4, 19))); } + @Test + public void shouldGetRemainingHeadersWhenRangeHasNoEndUsingPeerTaskSystem() { + downloader = + new DownloadHeadersStep( + protocolSchedule, + protocolContext, + ethProtocolManager.ethContext(), + () -> HeaderValidationMode.DETACHED_ONLY, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + HEADER_REQUEST_SIZE, + new NoOpMetricsSystem()); + final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + final SyncTargetRange checkpointRange = + new SyncTargetRange(peer.getEthPeer(), blockchain.getBlockHeader(3).get()); + + Mockito.when(peerTaskExecutor.execute(Mockito.any(GetHeadersFromPeerTask.class))) + .thenAnswer( + (invocationOnMock) -> { + GetHeadersFromPeerTask task = + invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + List result = new ArrayList<>(); + for (long i = task.getBlockNumber() + 1; i < task.getMaxHeaders(); i++) { + Optional blockHeader = blockchain.getBlockHeader(i); + if (blockHeader.isPresent()) { + result.add(blockHeader.get()); + } else { + // we have reached the end of the blockchain, do nothing and break out of the loop + break; + } + } + return new PeerTaskExecutorResult>( + Optional.of(result), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty()); + }); + + final CompletableFuture result = this.downloader.apply(checkpointRange); + + peer.respond(blockchainResponder(blockchain)); + + assertThat(result) + .isCompletedWithValue(new RangeHeaders(checkpointRange, headersFromChain(4, 19))); + } + private List headersFromChain(final long startNumber, final long endNumber) { final List headers = new ArrayList<>(); for (long i = startNumber; i <= endNumber; i++) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersFetcherTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersFetcherTest.java index 27a4650c549..c255fb7b241 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersFetcherTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersFetcherTest.java @@ -30,6 +30,10 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer.Responder; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.range.RangeHeadersFetcher; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; @@ -39,14 +43,18 @@ import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; import org.hyperledger.besu.testutil.DeterministicEthScheduler; +import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.stubbing.Answer; @ExtendWith(MockitoExtension.class) public class RangeHeadersFetcherTest { @@ -57,7 +65,27 @@ public class RangeHeadersFetcherTest { private static ProtocolContext protocolContext; private static final MetricsSystem metricsSystem = new NoOpMetricsSystem(); private static TransactionPool transactionPool; + + private static final Answer>> executeAgainstPeerAnswer = + (invocationOnMock) -> { + List resultList = new ArrayList<>(); + GetHeadersFromPeerTask task = invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + for (long i = task.getBlockNumber(); + i < task.getBlockNumber() + task.getMaxHeaders() * (task.getSkip() + 1); + i += task.getSkip() + 1) { + Optional blockHeader = blockchain.getBlockHeader(i); + if (blockHeader.isPresent()) { + resultList.add(blockHeader.get()); + } else { + break; + } + } + return new PeerTaskExecutorResult>( + Optional.of(resultList), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty()); + }; + private EthProtocolManager ethProtocolManager; + private PeerTaskExecutor peerTaskExecutor; private Responder responder; private RespondingEthPeer respondingPeer; @@ -74,6 +102,7 @@ public static void setUpClass() { @BeforeEach public void setUpTest() { + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); ethProtocolManager = EthProtocolManagerTestBuilder.builder() .setProtocolSchedule(protocolSchedule) @@ -82,6 +111,7 @@ public void setUpTest() { .setWorldStateArchive(protocolContext.getWorldStateArchive()) .setTransactionPool(transactionPool) .setEthereumWireProtocolConfiguration(EthProtocolConfiguration.defaultConfig()) + .setPeerTaskExecutor(peerTaskExecutor) .build(); responder = RespondingEthPeer.blockchainResponder( @@ -93,7 +123,7 @@ public void setUpTest() { @Test public void shouldRequestHeadersFromPeerAndExcludeExistingHeader() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(false); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); @@ -105,9 +135,44 @@ public void shouldRequestHeadersFromPeerAndExcludeExistingHeader() { assertThat(result).isCompletedWithValue(asList(header(6), header(11), header(16))); } + @Test + public void shouldRequestHeadersFromPeerAndExcludeExistingHeaderUsingPeerTaskSystem() { + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(true); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingPeer.getEthPeer()))) + .thenAnswer(executeAgainstPeerAnswer); + + final CompletableFuture> result = + rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); + + respondingPeer.respond(responder); + + assertThat(result).isCompletedWithValue(asList(header(6), header(11), header(16))); + } + @Test public void shouldNotRequestHeadersBeyondTargetWhenTargetIsMultipleOfSegmentSize() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(11)); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(11), false); + + final CompletableFuture> result = + rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); + + respondingPeer.respond(responder); + + assertThat(result).isCompletedWithValue(asList(header(6), header(11))); + } + + @Test + public void + shouldNotRequestHeadersBeyondTargetWhenTargetIsMultipleOfSegmentSizeWithPeerTaskSystem() { + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(11), true); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingPeer.getEthPeer()))) + .thenAnswer(executeAgainstPeerAnswer); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); @@ -119,7 +184,25 @@ public void shouldNotRequestHeadersBeyondTargetWhenTargetIsMultipleOfSegmentSize @Test public void shouldNotRequestHeadersBeyondTargetWhenTargetIsNotAMultipleOfSegmentSize() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15)); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15), false); + + final CompletableFuture> result = + rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); + + respondingPeer.respond(responder); + + assertThat(result).isCompletedWithValue(asList(header(6), header(11))); + } + + @Test + public void + shouldNotRequestHeadersBeyondTargetWhenTargetIsNotAMultipleOfSegmentSizeWithPeerTaskSystem() { + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15), true); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingPeer.getEthPeer()))) + .thenAnswer(executeAgainstPeerAnswer); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(1)); @@ -131,7 +214,7 @@ public void shouldNotRequestHeadersBeyondTargetWhenTargetIsNotAMultipleOfSegment @Test public void shouldReturnOnlyTargetHeaderWhenLastHeaderIsTheRangeBeforeTarget() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15)); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15), false); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(11)); @@ -141,7 +224,7 @@ public void shouldReturnOnlyTargetHeaderWhenLastHeaderIsTheRangeBeforeTarget() { @Test public void shouldReturnEmptyListWhenLastHeaderIsTarget() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15)); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15), false); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(15)); @@ -150,7 +233,7 @@ public void shouldReturnEmptyListWhenLastHeaderIsTarget() { @Test public void shouldReturnEmptyListWhenLastHeaderIsAfterTarget() { - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15)); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(header(15), false); final CompletableFuture> result = rangeHeaderFetcher.getNextRangeHeaders(respondingPeer.getEthPeer(), header(16)); @@ -160,7 +243,7 @@ public void shouldReturnEmptyListWhenLastHeaderIsAfterTarget() { @Test public void nextRangeShouldEndAtChainHeadWhenNextRangeHeaderIsAfterHead() { final long remoteChainHeight = blockchain.getChainHeadBlockNumber(); - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(false); assertThat( rangeHeaderFetcher.nextRangeEndsAtChainHead( @@ -172,7 +255,7 @@ public void nextRangeShouldEndAtChainHeadWhenNextRangeHeaderIsAfterHead() { public void nextRangeShouldNotEndAtChainHeadWhenAFinalRangeHeaderIsSpecified() { final long remoteChainHeight = blockchain.getChainHeadBlockNumber(); final RangeHeadersFetcher rangeHeaderFetcher = - createRangeHeaderFetcher(header(remoteChainHeight)); + createRangeHeaderFetcher(header(remoteChainHeight), false); assertThat( rangeHeaderFetcher.nextRangeEndsAtChainHead( @@ -183,7 +266,31 @@ public void nextRangeShouldNotEndAtChainHeadWhenAFinalRangeHeaderIsSpecified() { @Test public void shouldReturnRemoteChainHeadWhenNextRangeHeaderIsTheRemoteHead() { final long remoteChainHeight = blockchain.getChainHeadBlockNumber(); - final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(false); + + assertThat( + rangeHeaderFetcher.nextRangeEndsAtChainHead( + respondingPeer.getEthPeer(), header(remoteChainHeight - SEGMENT_SIZE))) + .isFalse(); + + final CompletableFuture> result = + rangeHeaderFetcher.getNextRangeHeaders( + respondingPeer.getEthPeer(), header(remoteChainHeight - SEGMENT_SIZE)); + + respondingPeer.respond(responder); + + assertThat(result).isCompletedWithValue(singletonList(header(remoteChainHeight))); + } + + @Test + public void shouldReturnRemoteChainHeadWhenNextRangeHeaderIsTheRemoteHeadWithPeerTaskSystem() { + final long remoteChainHeight = blockchain.getChainHeadBlockNumber(); + final RangeHeadersFetcher rangeHeaderFetcher = createRangeHeaderFetcher(true); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingPeer.getEthPeer()))) + .thenAnswer(executeAgainstPeerAnswer); assertThat( rangeHeaderFetcher.nextRangeEndsAtChainHead( @@ -199,24 +306,27 @@ public void shouldReturnRemoteChainHeadWhenNextRangeHeaderIsTheRemoteHead() { assertThat(result).isCompletedWithValue(singletonList(header(remoteChainHeight))); } - private RangeHeadersFetcher createRangeHeaderFetcher() { + private RangeHeadersFetcher createRangeHeaderFetcher(final boolean isPeerTaskSystemEnabled) { final EthContext ethContext = ethProtocolManager.ethContext(); return new RangeHeadersFetcher( SynchronizerConfiguration.builder() .downloaderChainSegmentSize(SEGMENT_SIZE) .downloaderHeadersRequestSize(3) + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) .build(), protocolSchedule, ethContext, metricsSystem); } - private RangeHeadersFetcher createRangeHeaderFetcher(final BlockHeader targetHeader) { + private RangeHeadersFetcher createRangeHeaderFetcher( + final BlockHeader targetHeader, final boolean isPeerTaskSystemEnabled) { final EthContext ethContext = ethProtocolManager.ethContext(); return new RangeHeadersFetcher( SynchronizerConfiguration.builder() .downloaderChainSegmentSize(SEGMENT_SIZE) .downloaderHeadersRequestSize(3) + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) .build(), protocolSchedule, ethContext, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java index 66cfc211303..fdb3102af0c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java @@ -43,6 +43,8 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.MainnetProtocolSchedule; @@ -113,6 +115,7 @@ public class BackwardSyncContextTest { @Mock private BlockValidator blockValidator; @Mock private SyncState syncState; + @Mock private PeerTaskExecutor peerTaskExecutor; private BackwardChain backwardChain; private Block uncle; private Block genesisBlock; @@ -149,6 +152,7 @@ public void setup() { EthProtocolManagerTestBuilder.builder() .setProtocolSchedule(protocolSchedule) .setBlockchain(localBlockchain) + .setPeerTaskExecutor(peerTaskExecutor) .build(); peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager); @@ -183,6 +187,7 @@ public void setup() { new BackwardSyncContext( protocolContext, protocolSchedule, + SynchronizerConfiguration.builder().build(), metricsSystem, ethContext, syncState, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStepTest.java index 76f1e8ea62d..8a07a9647da 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStepTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -32,10 +33,17 @@ import org.hyperledger.besu.ethereum.core.MiningConfiguration; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTaskExecutorAnswer; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.MainnetProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -45,6 +53,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import javax.annotation.Nonnull; @@ -57,6 +66,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import org.mockito.stubbing.Answer; @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -82,6 +92,7 @@ public class BackwardSyncStepTest { private MutableBlockchain localBlockchain; private MutableBlockchain remoteBlockchain; private RespondingEthPeer peer; + @Mock private PeerTaskExecutor peerTaskExecutor; GenericKeyValueStorageFacade headersStorage; GenericKeyValueStorageFacade blocksStorage; GenericKeyValueStorageFacade chainStorage; @@ -131,7 +142,10 @@ public void setup() { when(context.getBatchSize()).thenReturn(5); EthProtocolManager ethProtocolManager = - EthProtocolManagerTestBuilder.builder().setEthScheduler(ethScheduler).build(); + EthProtocolManagerTestBuilder.builder() + .setEthScheduler(ethScheduler) + .setPeerTaskExecutor(peerTaskExecutor) + .build(); peer = RespondingEthPeer.builder() @@ -140,6 +154,12 @@ public void setup() { .build(); EthContext ethContext = ethProtocolManager.ethContext(); when(context.getEthContext()).thenReturn(ethContext); + + Answer>> getHeadersAnswer = + new GetHeadersFromPeerTaskExecutorAnswer(remoteBlockchain, ethContext.getEthPeers()); + when(peerTaskExecutor.execute(any(GetHeadersFromPeerTask.class))).thenAnswer(getHeadersAnswer); + when(peerTaskExecutor.executeAgainstPeer(any(GetHeadersFromPeerTask.class), any(EthPeer.class))) + .thenAnswer(getHeadersAnswer); } @Test @@ -157,6 +177,23 @@ public void shouldFindHeaderWhenRequested() throws Exception { future.get(); } + @Test + public void shouldFindHeaderWhenRequestedUsingPeerTaskSystem() throws Exception { + final BackwardChain backwardChain = createBackwardChain(LOCAL_HEIGHT + 3); + when(context.getBatchSize()).thenReturn(5); + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + BackwardSyncStep step = spy(new BackwardSyncStep(context, backwardChain)); + + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(remoteBlockchain); + + final CompletableFuture future = + step.executeAsync(backwardChain.getFirstAncestorHeader().orElseThrow()); + peer.respondWhileOtherThreadsWork(responder, () -> !future.isDone()); + future.get(); + } + @Test public void shouldFindHashToSync() { @@ -167,6 +204,17 @@ public void shouldFindHashToSync() { assertThat(hash).isEqualTo(getBlockByNumber(REMOTE_HEIGHT - 4).getHeader().getParentHash()); } + @Test + public void shouldFindHashToSyncUsingPeerTaskSystem() { + final BackwardChain backwardChain = createBackwardChain(REMOTE_HEIGHT - 4, REMOTE_HEIGHT); + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + BackwardSyncStep step = new BackwardSyncStep(context, backwardChain); + final Hash hash = + step.possibleRestoreOldNodes(backwardChain.getFirstAncestorHeader().orElseThrow()); + assertThat(hash).isEqualTo(getBlockByNumber(REMOTE_HEIGHT - 4).getHeader().getParentHash()); + } + @Test public void shouldRequestHeaderWhenAsked() throws Exception { BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); @@ -183,6 +231,24 @@ public void shouldRequestHeaderWhenAsked() throws Exception { assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); } + @Test + public void shouldRequestHeaderWhenAskedUsingPeerTaskSystem() throws Exception { + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); + final Block lookingForBlock = getBlockByNumber(REMOTE_HEIGHT - 2); + + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(remoteBlockchain); + + final CompletableFuture> future = + step.requestHeaders(lookingForBlock.getHeader().getHash()); + peer.respondWhileOtherThreadsWork(responder, () -> !future.isDone()); + + final BlockHeader blockHeader = future.get().get(0); + assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); + } + @Test public void shouldNotRequestHeaderIfAlreadyPresent() throws Exception { BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); @@ -197,6 +263,22 @@ public void shouldNotRequestHeaderIfAlreadyPresent() throws Exception { assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); } + @Test + public void shouldNotRequestHeaderIfAlreadyPresentUsingPeerTaskSystem() throws Exception { + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); + final Block lookingForBlock = getBlockByNumber(LOCAL_HEIGHT); + + final CompletableFuture> future = + step.requestHeaders(lookingForBlock.getHeader().getHash()); + + verify(localBlockchain).getBlockHeader(lookingForBlock.getHash()); + verify(context, never()).getEthContext(); + final BlockHeader blockHeader = future.get().get(0); + assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); + } + @Test public void shouldRequestHeaderBeforeCurrentHeight() throws Exception { extendBlockchain(REMOTE_HEIGHT + 1, context.getProtocolContext().getBlockchain()); @@ -215,6 +297,26 @@ public void shouldRequestHeaderBeforeCurrentHeight() throws Exception { assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); } + @Test + public void shouldRequestHeaderBeforeCurrentHeightUsingPeerTaskSystem() throws Exception { + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + extendBlockchain(REMOTE_HEIGHT + 1, context.getProtocolContext().getBlockchain()); + + BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); + final Block lookingForBlock = getBlockByNumber(REMOTE_HEIGHT - 2); + + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(remoteBlockchain); + + final CompletableFuture> future = + step.requestHeaders(lookingForBlock.getHeader().getHash()); + peer.respondWhileOtherThreadsWork(responder, () -> !future.isDone()); + + final BlockHeader blockHeader = future.get().get(0); + assertThat(blockHeader).isEqualTo(lookingForBlock.getHeader()); + } + @Test public void shouldThrowWhenResponseIsEmptyWhenRequestingHeader() { BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); @@ -229,6 +331,31 @@ public void shouldThrowWhenResponseIsEmptyWhenRequestingHeader() { assertThatThrownBy(future::get).cause().isInstanceOf(MaxRetriesReachedException.class); } + @Test + public void shouldThrowWhenResponseIsEmptyWhenRequestingHeaderUsingPeerTaskSystem() { + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + Mockito.reset(peerTaskExecutor); + when(peerTaskExecutor.execute(any(GetHeadersFromPeerTask.class))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty())); + when(peerTaskExecutor.executeAgainstPeer(any(GetHeadersFromPeerTask.class), any(EthPeer.class))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty())); + BackwardSyncStep step = new BackwardSyncStep(context, createBackwardChain(REMOTE_HEIGHT - 1)); + final Block lookingForBlock = getBlockByNumber(REMOTE_HEIGHT - 2); + + final RespondingEthPeer.Responder responder = RespondingEthPeer.emptyResponder(); + + final CompletableFuture> future = + step.requestHeaders(lookingForBlock.getHeader().getHash()); + peer.respondWhileOtherThreadsWork(responder, () -> !future.isDone()); + + assertThatThrownBy(future::get).cause().isInstanceOf(RuntimeException.class); + } + @Test public void shouldSaveHeaderDelegatesProperly() { final BackwardChain chain = Mockito.mock(BackwardChain.class); @@ -241,6 +368,20 @@ public void shouldSaveHeaderDelegatesProperly() { verify(chain).prependAncestorsHeader(header); } + @Test + public void shouldSaveHeaderDelegatesProperlyUsingPeerTaskSystem() { + when(context.getSynchronizerConfiguration()) + .thenReturn(SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build()); + final BackwardChain chain = Mockito.mock(BackwardChain.class); + final BlockHeader header = Mockito.mock(BlockHeader.class); + + BackwardSyncStep step = new BackwardSyncStep(context, chain); + + step.saveHeader(header); + + verify(chain).prependAncestorsHeader(header); + } + private BackwardChain createBackwardChain(final int from, final int until) { BackwardChain chain = createBackwardChain(until); for (int i = until; i > from; --i) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckPointSyncChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckPointSyncChainDownloaderTest.java index e4039aee49d..e73d286fb0b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckPointSyncChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckPointSyncChainDownloaderTest.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; @@ -35,6 +36,8 @@ import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTaskExecutorAnswer; import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetReceiptsFromPeerTask; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -51,15 +54,11 @@ import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; -import java.lang.reflect.Field; -import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeoutException; import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; @@ -69,8 +68,6 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; -import org.junit.platform.commons.util.ReflectionUtils; -import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; public class CheckPointSyncChainDownloaderTest { @@ -121,11 +118,13 @@ public void setup(final DataStorageFormat dataStorageFormat) { otherBlockchainSetup.importFirstBlocks(30); protocolSchedule = localBlockchainSetup.getProtocolSchedule(); protocolContext = localBlockchainSetup.getProtocolContext(); + peerTaskExecutor = mock(PeerTaskExecutor.class); ethProtocolManager = EthProtocolManagerTestBuilder.builder() .setProtocolSchedule(protocolSchedule) .setBlockchain(localBlockchain) .setEthScheduler(new EthScheduler(1, 1, 1, 1, new NoOpMetricsSystem())) + .setPeerTaskExecutor(peerTaskExecutor) .build(); ethContext = ethProtocolManager.ethContext(); @@ -144,40 +143,30 @@ public void setup(final DataStorageFormat dataStorageFormat) { true, Optional.of(checkpoint)); - peerTaskExecutor = mock(PeerTaskExecutor.class); - when(peerTaskExecutor.execute(any(GetReceiptsFromPeerTask.class))) .thenAnswer( - new Answer>>>() { - @Override - public PeerTaskExecutorResult>> answer( - final InvocationOnMock invocationOnMock) throws Throwable { - GetReceiptsFromPeerTask task = - invocationOnMock.getArgument(0, GetReceiptsFromPeerTask.class); - - return processTask(task); - } + (invocationOnMock) -> { + GetReceiptsFromPeerTask task = + invocationOnMock.getArgument(0, GetReceiptsFromPeerTask.class); + Map> getReceiptsFromPeerTaskResult = + new HashMap<>(); + task.getBlockHeaders() + .forEach( + (bh) -> + getReceiptsFromPeerTaskResult.put( + bh, otherBlockchain.getTxReceipts(bh.getHash()).get())); + + return new PeerTaskExecutorResult<>( + Optional.of(getReceiptsFromPeerTaskResult), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.empty()); }); - } - @SuppressWarnings("unchecked") - private PeerTaskExecutorResult>> processTask( - final GetReceiptsFromPeerTask task) throws IllegalAccessException { - Map> getReceiptsFromPeerTaskResult = new HashMap<>(); - List fields = - ReflectionUtils.findFields( - task.getClass(), - (field) -> field.getName().equals("blockHeaders"), - ReflectionUtils.HierarchyTraversalMode.TOP_DOWN); - fields.forEach((f) -> f.setAccessible(true)); - Collection blockHeaders = (Collection) fields.getFirst().get(task); - blockHeaders.forEach( - (bh) -> - getReceiptsFromPeerTaskResult.put( - bh, otherBlockchain.getTxReceipts(bh.getHash()).get())); - - return new PeerTaskExecutorResult<>( - Optional.of(getReceiptsFromPeerTaskResult), PeerTaskExecutorResponseCode.SUCCESS); + final Answer>> getHeadersAnswer = + new GetHeadersFromPeerTaskExecutorAnswer(otherBlockchain, ethContext.getEthPeers()); + when(peerTaskExecutor.execute(any(GetHeadersFromPeerTask.class))).thenAnswer(getHeadersAnswer); + when(peerTaskExecutor.executeAgainstPeer(any(GetHeadersFromPeerTask.class), any(EthPeer.class))) + .thenAnswer(getHeadersAnswer); } @AfterEach @@ -195,7 +184,6 @@ private ChainDownloader downloader( protocolSchedule, protocolContext, ethContext, - peerTaskExecutor, syncState, new NoOpMetricsSystem(), new FastSyncState(otherBlockchain.getBlockHeader(pivotBlockNumber).get()), @@ -204,8 +192,7 @@ private ChainDownloader downloader( @ParameterizedTest @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) - public void shouldSyncToPivotBlockInMultipleSegments(final DataStorageFormat storageFormat) - throws IllegalAccessException { + public void shouldSyncToPivotBlockInMultipleSegments(final DataStorageFormat storageFormat) { setup(storageFormat); final RespondingEthPeer peer = @@ -241,8 +228,7 @@ public void shouldSyncToPivotBlockInMultipleSegments(final DataStorageFormat sto @ParameterizedTest @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) - public void shouldSyncToPivotBlockInSingleSegment(final DataStorageFormat storageFormat) - throws IllegalAccessException { + public void shouldSyncToPivotBlockInSingleSegment(final DataStorageFormat storageFormat) { setup(storageFormat); final RespondingEthPeer peer = @@ -275,8 +261,7 @@ public void shouldSyncToPivotBlockInSingleSegment(final DataStorageFormat storag @ParameterizedTest @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) public void shouldSyncToPivotBlockInMultipleSegmentsWithPeerTaskSystem( - final DataStorageFormat storageFormat) - throws IllegalAccessException, ExecutionException, InterruptedException, TimeoutException { + final DataStorageFormat storageFormat) { setup(storageFormat); final RespondingEthPeer peer = @@ -313,7 +298,7 @@ public void shouldSyncToPivotBlockInMultipleSegmentsWithPeerTaskSystem( @ParameterizedTest @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) public void shouldSyncToPivotBlockInSingleSegmentWithPeerTaskSystem( - final DataStorageFormat storageFormat) throws IllegalAccessException { + final DataStorageFormat storageFormat) { setup(storageFormat); final RespondingEthPeer peer = diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStepTest.java index 97cc268d6ba..597ccea8e04 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadReceiptsStepTest.java @@ -87,6 +87,7 @@ public void setUp() { .setWorldStateArchive(protocolContext.getWorldStateArchive()) .setTransactionPool(transactionPool) .setEthereumWireProtocolConfiguration(EthProtocolConfiguration.defaultConfig()) + .setPeerTaskExecutor(peerTaskExecutor) .build(); } @@ -96,7 +97,6 @@ public void shouldDownloadReceiptsForBlocks() { new DownloadReceiptsStep( protocolSchedule, ethProtocolManager.ethContext(), - peerTaskExecutor, SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), new NoOpMetricsSystem()); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); @@ -122,7 +122,6 @@ public void shouldDownloadReceiptsForBlocksUsingPeerTaskSystem() new DownloadReceiptsStep( protocolSchedule, ethProtocolManager.ethContext(), - peerTaskExecutor, SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), new NoOpMetricsSystem()); @@ -132,7 +131,7 @@ public void shouldDownloadReceiptsForBlocksUsingPeerTaskSystem() (b) -> receiptsMap.put(b.getHeader(), List.of(Mockito.mock(TransactionReceipt.class)))); PeerTaskExecutorResult>> peerTaskResult = new PeerTaskExecutorResult<>( - Optional.of(receiptsMap), PeerTaskExecutorResponseCode.SUCCESS); + Optional.of(receiptsMap), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty()); Mockito.when(peerTaskExecutor.execute(Mockito.any(GetReceiptsFromPeerTask.class))) .thenReturn(peerTaskResult); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastDownloaderFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastDownloaderFactoryTest.java index bc493ebd036..37ca5be2e99 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastDownloaderFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastDownloaderFactoryTest.java @@ -25,7 +25,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; @@ -72,7 +71,6 @@ public class FastDownloaderFactoryTest { @Mock private ProtocolContext protocolContext; @Mock private MetricsSystem metricsSystem; @Mock private EthContext ethContext; - @Mock private PeerTaskExecutor peerTaskExecutor; @Mock private SyncState syncState; @Mock private Clock clock; @Mock private Path dataDirectory; @@ -116,7 +114,6 @@ public void shouldThrowIfSyncModeChangedWhileFastSyncIncomplete( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -142,7 +139,6 @@ public void shouldNotThrowIfSyncModeChangedWhileFastSyncComplete( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -171,7 +167,6 @@ public void shouldNotThrowWhenFastSyncModeRequested(final DataStorageFormat data protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -207,7 +202,6 @@ public void shouldClearWorldStateDuringFastSyncWhenStateQueDirectoryExists( protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, @@ -245,7 +239,6 @@ public void shouldCrashWhenStateQueueIsNotDirectory(final DataStorageFormat data protocolContext, metricsSystem, ethContext, - peerTaskExecutor, worldStateStorageCoordinator, syncState, clock, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActionsTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActionsTest.java index a1003fe4113..ffce23ee2c9 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActionsTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncActionsTest.java @@ -35,7 +35,6 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.eth.sync.PivotBlockSelector; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; @@ -64,14 +63,10 @@ import org.junit.jupiter.params.provider.ArgumentsSource; public class FastSyncActionsTest { - - private final SynchronizerConfiguration.Builder syncConfigBuilder = - new SynchronizerConfiguration.Builder().syncMode(SyncMode.FAST).syncPivotDistance(1000); - private final WorldStateStorageCoordinator worldStateStorageCoordinator = mock(WorldStateStorageCoordinator.class); private final AtomicInteger timeoutCount = new AtomicInteger(0); - private SynchronizerConfiguration syncConfig = syncConfigBuilder.build(); + private SynchronizerConfiguration syncConfig; private FastSyncActions fastSyncActions; private EthProtocolManager ethProtocolManager; private EthContext ethContext; @@ -89,7 +84,21 @@ public Stream provideArguments(final ExtensionContext conte } } - public void setUp(final DataStorageFormat storageFormat) { + public void setUp(final DataStorageFormat storageFormat, final boolean isPeerTaskSystemEnabled) { + setUp(storageFormat, isPeerTaskSystemEnabled, Optional.empty()); + } + + public void setUp( + final DataStorageFormat storageFormat, + final boolean isPeerTaskSystemEnabled, + final Optional syncMinimumPeers) { + SynchronizerConfiguration.Builder syncConfigBuilder = + new SynchronizerConfiguration.Builder() + .syncMode(SyncMode.FAST) + .syncPivotDistance(1000) + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled); + syncMinimumPeers.ifPresent(syncConfigBuilder::syncMinimumPeerCount); + syncConfig = syncConfigBuilder.build(); when(worldStateStorageCoordinator.getDataStorageFormat()).thenReturn(storageFormat); blockchainSetupUtil = BlockchainSetupUtil.forTesting(storageFormat); blockchainSetupUtil.importAllBlocks(); @@ -118,7 +127,7 @@ public void setUp(final DataStorageFormat storageFormat) { @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void waitForPeersShouldSucceedIfEnoughPeersAreFound( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false); for (int i = 0; i < syncConfig.getSyncMinimumPeerCount(); i++) { EthProtocolManagerTestUtil.createPeer( ethProtocolManager, syncConfig.getSyncPivotDistance() + i + 1); @@ -131,7 +140,7 @@ public void waitForPeersShouldSucceedIfEnoughPeersAreFound( @ParameterizedTest @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void returnTheSamePivotBlockIfAlreadySelected(final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false); final BlockHeader pivotHeader = new BlockHeaderTestFixture().number(1024).buildHeader(); final FastSyncState fastSyncState = new FastSyncState(pivotHeader); final CompletableFuture result = fastSyncActions.selectPivotBlock(fastSyncState); @@ -143,7 +152,7 @@ public void returnTheSamePivotBlockIfAlreadySelected(final DataStorageFormat sto @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldUseExistingPivotBlockIfAvailable( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false); final BlockHeader pivotHeader = new BlockHeaderTestFixture().number(1024).buildHeader(); EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 5000); @@ -157,10 +166,8 @@ public void selectPivotBlockShouldUseExistingPivotBlockIfAvailable( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldSelectBlockPivotDistanceFromBestPeer( final DataStorageFormat storageFormat) { - setUp(storageFormat); - final int minPeers = 1; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); + setUp(storageFormat, false, Optional.of(1)); + fastSyncActions = createFastSyncActions( syncConfig, @@ -178,10 +185,7 @@ public void selectPivotBlockShouldSelectBlockPivotDistanceFromBestPeer( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldConsiderTotalDifficultyWhenSelectingBestPeer( final DataStorageFormat storageFormat) { - setUp(storageFormat); - final int minPeers = 1; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); + setUp(storageFormat, false, Optional.of(1)); fastSyncActions = createFastSyncActions( syncConfig, @@ -200,11 +204,8 @@ public void selectPivotBlockShouldConsiderTotalDifficultyWhenSelectingBestPeer( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldWaitAndRetryUntilMinHeightEstimatesAreAvailable( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false, Optional.of(2)); EthProtocolManagerTestUtil.disableEthSchedulerAutoRun(ethProtocolManager); - final int minPeers = 2; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); fastSyncActions = createFastSyncActions( syncConfig, @@ -232,10 +233,8 @@ public void selectPivotBlockShouldWaitAndRetryUntilMinHeightEstimatesAreAvailabl @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldWaitAndRetryIfSufficientChainHeightEstimatesAreUnavailable( final DataStorageFormat storageFormat) { - setUp(storageFormat); - final int minPeers = 3; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); + int minPeers = 3; + setUp(storageFormat, false, Optional.of(minPeers)); fastSyncActions = createFastSyncActions( syncConfig, @@ -282,11 +281,9 @@ public void selectPivotBlockShouldWaitAndRetryIfSufficientChainHeightEstimatesAr @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldWaitAndRetryIfSufficientValidatedPeersUnavailable( final DataStorageFormat storageFormat) { - setUp(storageFormat); final int minPeers = 3; + setUp(storageFormat, false, Optional.of(minPeers)); final PeerValidator validator = mock(PeerValidator.class); - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); fastSyncActions = createFastSyncActions( syncConfig, @@ -334,14 +331,14 @@ public void selectPivotBlockShouldWaitAndRetryIfSufficientValidatedPeersUnavaila @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockUsesBestPeerWithHeightEstimate( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false, Optional.of(3)); selectPivotBlockUsesBestPeerMatchingRequiredCriteria(true, false); } @ParameterizedTest @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockUsesBestPeerThatIsValidated(final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false, Optional.of(3)); selectPivotBlockUsesBestPeerMatchingRequiredCriteria(false, true); } @@ -349,16 +346,13 @@ public void selectPivotBlockUsesBestPeerThatIsValidated(final DataStorageFormat @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockUsesBestPeerThatIsValidatedAndHasHeightEstimate( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false, Optional.of(3)); selectPivotBlockUsesBestPeerMatchingRequiredCriteria(true, true); } private void selectPivotBlockUsesBestPeerMatchingRequiredCriteria( final boolean bestMissingHeight, final boolean bestNotValidated) { - final int minPeers = 3; - final int peerCount = minPeers + 1; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); + final int peerCount = 4; fastSyncActions = createFastSyncActions( syncConfig, @@ -407,10 +401,7 @@ private void selectPivotBlockUsesBestPeerMatchingRequiredCriteria( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldWaitAndRetryIfBestPeerChainIsShorterThanPivotDistance( final DataStorageFormat storageFormat) { - setUp(storageFormat); - final int minPeers = 1; - syncConfigBuilder.syncMinimumPeerCount(minPeers); - syncConfig = syncConfigBuilder.build(); + setUp(storageFormat, false, Optional.of(1)); fastSyncActions = createFastSyncActions( syncConfig, @@ -437,7 +428,7 @@ public void selectPivotBlockShouldWaitAndRetryIfBestPeerChainIsShorterThanPivotD @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void selectPivotBlockShouldRetryIfBestPeerChainIsEqualToPivotDistance( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false); final long pivotDistance = syncConfig.getSyncPivotDistance(); EthProtocolManagerTestUtil.disableEthSchedulerAutoRun(ethProtocolManager); // Create peers with chains that are too short @@ -462,7 +453,7 @@ public void selectPivotBlockShouldRetryIfBestPeerChainIsEqualToPivotDistance( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void downloadPivotBlockHeaderShouldUseExistingPivotBlockHeaderIfPresent( final DataStorageFormat storageFormat) { - setUp(storageFormat); + setUp(storageFormat, false); final BlockHeader pivotHeader = new BlockHeaderTestFixture().number(1024).buildHeader(); final FastSyncState expected = new FastSyncState(pivotHeader); assertThat(fastSyncActions.downloadPivotBlockHeader(expected)).isCompletedWithValue(expected); @@ -472,8 +463,7 @@ public void downloadPivotBlockHeaderShouldUseExistingPivotBlockHeaderIfPresent( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void downloadPivotBlockHeaderShouldRetrievePivotBlockHeader( final DataStorageFormat storageFormat) { - setUp(storageFormat); - syncConfig = SynchronizerConfiguration.builder().syncMinimumPeerCount(1).build(); + setUp(storageFormat, false, Optional.of(1)); fastSyncActions = createFastSyncActions( syncConfig, @@ -494,8 +484,7 @@ public void downloadPivotBlockHeaderShouldRetrievePivotBlockHeader( @ArgumentsSource(FastSyncActionsTest.FastSyncActionsTestArguments.class) public void downloadPivotBlockHeaderShouldRetrievePivotBlockHash( final DataStorageFormat storageFormat) { - setUp(storageFormat); - syncConfig = SynchronizerConfiguration.builder().syncMinimumPeerCount(1).build(); + setUp(storageFormat, false, Optional.of(1)); GenesisConfigOptions genesisConfig = mock(GenesisConfigOptions.class); when(genesisConfig.getTerminalBlockNumber()).thenReturn(OptionalLong.of(10L)); @@ -515,6 +504,7 @@ public void downloadPivotBlockHeaderShouldRetrievePivotBlockHash( ethContext, metricsSystem, genesisConfig, + syncConfig, () -> finalizedEvent, () -> {})); @@ -541,7 +531,6 @@ private FastSyncActions createFastSyncActions( protocolSchedule, protocolContext, ethContext, - new PeerTaskExecutor(null, null, new NoOpMetricsSystem()), new SyncState(blockchain, ethContext.getEthPeers(), true, Optional.empty()), pivotBlockSelector, new NoOpMetricsSystem()); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloaderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloaderTest.java index 9865d4f23cf..911a9234d4c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloaderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloaderTest.java @@ -30,7 +30,6 @@ 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.manager.peertask.PeerTaskExecutor; import org.hyperledger.besu.ethereum.eth.messages.EthPV62; import org.hyperledger.besu.ethereum.eth.messages.GetBlockHeadersMessage; import org.hyperledger.besu.ethereum.eth.sync.ChainDownloader; @@ -113,7 +112,6 @@ private ChainDownloader downloader( protocolSchedule, protocolContext, ethContext, - new PeerTaskExecutor(null, null, new NoOpMetricsSystem()), syncState, new NoOpMetricsSystem(), new FastSyncState(otherBlockchain.getBlockHeader(pivotBlockNumber).get()), diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmerTest.java index ab81fc68eda..eb60398589e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmerTest.java @@ -25,11 +25,17 @@ import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer.Responder; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotBlockConfirmer.ContestedPivotBlockException; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -38,6 +44,7 @@ import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; import org.hyperledger.besu.testutil.DeterministicEthScheduler; +import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; @@ -50,6 +57,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; +import org.mockito.Mockito; public class PivotBlockConfirmerTest { @@ -62,8 +70,8 @@ public class PivotBlockConfirmerTest { private EthProtocolManager ethProtocolManager; private MutableBlockchain blockchain; private TransactionPool transactionPool; - private PivotBlockConfirmer pivotBlockConfirmer; private ProtocolSchedule protocolSchedule; + private PeerTaskExecutor peerTaskExecutor; static class PivotBlockConfirmerTestArguments implements ArgumentsProvider { @Override @@ -80,6 +88,7 @@ public void setUp(final DataStorageFormat storageFormat) { transactionPool = blockchainSetupUtil.getTransactionPool(); protocolSchedule = blockchainSetupUtil.getProtocolSchedule(); protocolContext = blockchainSetupUtil.getProtocolContext(); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); ethProtocolManager = EthProtocolManagerTestBuilder.builder() .setProtocolSchedule(protocolSchedule) @@ -88,28 +97,30 @@ public void setUp(final DataStorageFormat storageFormat) { .setWorldStateArchive(blockchainSetupUtil.getWorldArchive()) .setTransactionPool(transactionPool) .setEthereumWireProtocolConfiguration(EthProtocolConfiguration.defaultConfig()) + .setPeerTaskExecutor(peerTaskExecutor) .build(); - pivotBlockConfirmer = createPivotBlockConfirmer(3, 2); } private PivotBlockConfirmer createPivotBlockConfirmer( - final int peersToQuery, final int maxRetries) { - return pivotBlockConfirmer = - spy( - new PivotBlockConfirmer( - protocolSchedule, - ethProtocolManager.ethContext(), - metricsSystem, - PIVOT_BLOCK_NUMBER, - peersToQuery, - maxRetries)); + final int peersToQuery, final int maxRetries, final boolean isPeerTaskSystemEnabled) { + return spy( + new PivotBlockConfirmer( + protocolSchedule, + ethProtocolManager.ethContext(), + metricsSystem, + SynchronizerConfiguration.builder() + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) + .build(), + PIVOT_BLOCK_NUMBER, + peersToQuery, + maxRetries)); } @ParameterizedTest @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void completeSuccessfully(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(2, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, false); final Responder responder = RespondingEthPeer.blockchainResponder( @@ -136,11 +147,51 @@ public void completeSuccessfully(final DataStorageFormat storageFormat) { new FastSyncState(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())); } + @ParameterizedTest + @ArgumentsSource(PivotBlockConfirmerTestArguments.class) + public void completeSuccessfullyWithPeerTask(final DataStorageFormat storageFormat) { + setUp(storageFormat); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, true); + + final RespondingEthPeer respondingPeerA = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + final RespondingEthPeer respondingPeerB = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingPeerA.getEthPeer()))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeerA.getEthPeer()))); + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingPeerB.getEthPeer()))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeerB.getEthPeer()))); + + // Execute task + final CompletableFuture future = pivotBlockConfirmer.confirmPivotBlock(); + + future.join(); + assertThat(future) + .isCompletedWithValue( + new FastSyncState(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())); + } + @ParameterizedTest @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void delayedResponse(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(2, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, false); final Responder responder = RespondingEthPeer.blockchainResponder( @@ -175,7 +226,7 @@ public void delayedResponse(final DataStorageFormat storageFormat) { @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void peerTimesOutThenIsUnresponsive(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(2, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, false); final Responder responder = RespondingEthPeer.blockchainResponder( @@ -217,7 +268,7 @@ public void peerTimesOutThenIsUnresponsive(final DataStorageFormat storageFormat @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void peerTimesOut(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(2, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, false); final Responder responder = RespondingEthPeer.blockchainResponder( @@ -255,11 +306,45 @@ public void peerTimesOut(final DataStorageFormat storageFormat) { new FastSyncState(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())); } + @ParameterizedTest + @ArgumentsSource(PivotBlockConfirmerTestArguments.class) + public void peerTimesOutUsingPeerTaskSystem(final DataStorageFormat storageFormat) { + setUp(storageFormat); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, true); + + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + when(peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.any(EthPeer.class))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.empty())) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.TIMEOUT, Optional.empty())) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.empty())); + + // Execute task + final CompletableFuture future = pivotBlockConfirmer.confirmPivotBlock(); + + assertThat(future) + .isCompletedWithValue( + new FastSyncState(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())); + } + @ParameterizedTest @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void peerUnresponsive(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(2, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(2, 2, false); final Responder responder = RespondingEthPeer.blockchainResponder( @@ -303,7 +388,7 @@ public void peerUnresponsive(final DataStorageFormat storageFormat) { @ArgumentsSource(PivotBlockConfirmerTestArguments.class) public void headerMismatch(final DataStorageFormat storageFormat) { setUp(storageFormat); - pivotBlockConfirmer = createPivotBlockConfirmer(3, 2); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(3, 2, false); final Responder responderA = RespondingEthPeer.blockchainResponder( @@ -331,6 +416,49 @@ public void headerMismatch(final DataStorageFormat storageFormat) { assertThat(extraPeer.hasOutstandingRequests()).isFalse(); } + @ParameterizedTest + @ArgumentsSource(PivotBlockConfirmerTestArguments.class) + public void headerMismatchUsingPeerTaskSystem(final DataStorageFormat storageFormat) { + setUp(storageFormat); + PivotBlockConfirmer pivotBlockConfirmer = createPivotBlockConfirmer(3, 2, true); + + final RespondingEthPeer respondingPeerA = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + final RespondingEthPeer respondingPeerB = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingPeerA.getEthPeer()))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(blockchain.getBlockHeader(PIVOT_BLOCK_NUMBER).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeerA.getEthPeer()))); + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingPeerB.getEthPeer()))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of( + List.of( + new BlockHeaderTestFixture() + .number(PIVOT_BLOCK_NUMBER) + .extraData(Bytes.of(1)) + .buildHeader())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeerB.getEthPeer()))); + + // Execute task and wait for response + final CompletableFuture future = pivotBlockConfirmer.confirmPivotBlock(); + + assertThat(future).isCompletedExceptionally(); + assertThatThrownBy(future::get).hasRootCauseInstanceOf(ContestedPivotBlockException.class); + } + private Responder responderForFakeBlocks(final long... blockNumbers) { final Blockchain mockBlockchain = spy(blockchain); for (long blockNumber : blockNumbers) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java index 54864acbc62..2bfc84876a4 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java @@ -33,6 +33,7 @@ import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer.Responder; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; @@ -104,6 +105,7 @@ private PivotBlockRetriever createPivotBlockRetriever( protocolSchedule, ethProtocolManager.ethContext(), metricsSystem, + SynchronizerConfiguration.builder().build(), PIVOT_BLOCK_NUMBER, peersToQuery, pivotBlockDelta, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskParameterizedTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskParameterizedTest.java index ba0d0fb1da5..138106c5b6e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskParameterizedTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskParameterizedTest.java @@ -36,7 +36,12 @@ import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestBuilder; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -45,7 +50,9 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import java.io.IOException; +import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -57,6 +64,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; public class DetermineCommonAncestorTaskParameterizedTest { private final ProtocolSchedule protocolSchedule = ProtocolScheduleFixture.MAINNET; @@ -68,6 +76,7 @@ public class DetermineCommonAncestorTaskParameterizedTest { private static final int chainHeight = 50; private MutableBlockchain remoteBlockchain; + private PeerTaskExecutor peerTaskExecutor; @BeforeAll public static void setupClass() { @@ -89,6 +98,7 @@ public static void setupClass() { @BeforeEach public void setup() { remoteBlockchain = createInMemoryBlockchain(genesisBlock); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); } public static Stream parameters() throws IOException { @@ -96,15 +106,19 @@ public static Stream parameters() throws IOException { final Stream.Builder builder = Stream.builder(); for (final int requestSize : requestSizes) { for (int i = 0; i <= chainHeight; i++) { - builder.add(Arguments.of(requestSize, i)); + builder.add(Arguments.of(requestSize, i, true)); + builder.add(Arguments.of(requestSize, i, false)); } } return builder.build(); } - @ParameterizedTest(name = "requestSize={0}, commonAncestor={1}") + @ParameterizedTest(name = "requestSize={0}, commonAncestor={1}, isPeerTaskSystemEnabled={2}") @MethodSource("parameters") - public void searchesAgainstNetwork(final int headerRequestSize, final int commonAncestorHeight) { + public void searchesAgainstNetwork( + final int headerRequestSize, + final int commonAncestorHeight, + final boolean isPeerTaskSystemEnabled) { BlockHeader commonHeader = genesisBlock.getHeader(); for (long i = 1; i <= commonAncestorHeight; i++) { commonHeader = localBlockchain.getBlockHeader(i).get(); @@ -142,6 +156,7 @@ public void searchesAgainstNetwork(final int headerRequestSize, final int common .setWorldStateArchive(worldStateArchive) .setTransactionPool(mock(TransactionPool.class)) .setEthereumWireProtocolConfiguration(EthProtocolConfiguration.defaultConfig()) + .setPeerTaskExecutor(peerTaskExecutor) .build(); final RespondingEthPeer.Responder responder = RespondingEthPeer.blockchainResponder(remoteBlockchain); @@ -167,8 +182,35 @@ public void searchesAgainstNetwork(final int headerRequestSize, final int common ethContext, respondingEthPeer.getEthPeer(), headerRequestSize, + SynchronizerConfiguration.builder() + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) + .build(), metricsSystem); + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingEthPeer.getEthPeer()))) + .thenAnswer( + (invocationOnMock) -> { + GetHeadersFromPeerTask getHeadersTask = + invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + long blockNumber = getHeadersTask.getBlockNumber(); + int maxHeaders = getHeadersTask.getMaxHeaders(); + int skip = getHeadersTask.getSkip(); + + List headers = new ArrayList<>(); + long lowerBound = Math.max(0, blockNumber - (maxHeaders - 1) * (skip + 1)); + for (long i = blockNumber; i > lowerBound; i -= skip + 1) { + headers.add(remoteBlockchain.getBlockHeader(i).get()); + } + + return new PeerTaskExecutorResult>( + Optional.of(headers), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingEthPeer.getEthPeer())); + }); + final CompletableFuture future = task.run(); respondingEthPeer.respondWhile(responder, () -> !future.isDone()); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskTest.java index c8fb58ed43c..0a1bce7a6c5 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskTest.java @@ -46,7 +46,12 @@ import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.exceptions.EthTaskException; import org.hyperledger.besu.ethereum.eth.manager.exceptions.EthTaskException.FailureReason; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -56,13 +61,19 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.util.ExceptionUtils; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; public class DetermineCommonAncestorTaskTest { @@ -75,12 +86,14 @@ public class DetermineCommonAncestorTaskTest { private EthProtocolManager ethProtocolManager; private EthContext ethContext; private ProtocolContext protocolContext; + private PeerTaskExecutor peerTaskExecutor; @BeforeEach public void setup() { localGenesisBlock = blockDataGenerator.genesisBlock(); localBlockchain = createInMemoryBlockchain(localGenesisBlock); final WorldStateArchive worldStateArchive = createInMemoryWorldStateArchive(); + peerTaskExecutor = Mockito.mock(PeerTaskExecutor.class); ethProtocolManager = EthProtocolManagerTestBuilder.builder() .setProtocolSchedule(protocolSchedule) @@ -88,6 +101,7 @@ public void setup() { .setWorldStateArchive(worldStateArchive) .setTransactionPool(mock(TransactionPool.class)) .setEthereumWireProtocolConfiguration(EthProtocolConfiguration.defaultConfig()) + .setPeerTaskExecutor(peerTaskExecutor) .build(); ethContext = ethProtocolManager.ethContext(); protocolContext = @@ -113,6 +127,7 @@ public void shouldFailIfPeerDisconnects() { ethContext, respondingEthPeer.getEthPeer(), defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), metricsSystem); final AtomicReference failure = new AtomicReference<>(); @@ -131,6 +146,51 @@ public void shouldFailIfPeerDisconnects() { assertThat(((EthTaskException) error).reason()).isEqualTo(FailureReason.NO_AVAILABLE_PEERS); } + @Test + public void shouldFailIfPeerDisconnectsUsingPeerTaskSystem() { + final Block block = blockDataGenerator.nextBlock(localBlockchain.getChainHeadBlock()); + localBlockchain.appendBlock(block, blockDataGenerator.receipts(block)); + + final RespondingEthPeer respondingEthPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + + final EthTask task = + DetermineCommonAncestorTask.create( + protocolSchedule, + protocolContext, + ethContext, + respondingEthPeer.getEthPeer(), + defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + metricsSystem); + + PeerTaskExecutorResult> taskResult = + new PeerTaskExecutorResult<>( + Optional.of(Collections.emptyList()), + PeerTaskExecutorResponseCode.PEER_DISCONNECTED, + Optional.of(respondingEthPeer.getEthPeer())); + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingEthPeer.getEthPeer()))) + .thenReturn(taskResult); + + final AtomicReference failure = new AtomicReference<>(); + final CompletableFuture future = task.run(); + future.whenComplete( + (response, error) -> { + failure.set(error); + }); + + // Disconnect the target peer + respondingEthPeer.disconnect(DisconnectReason.CLIENT_QUITTING); + + assertThat(failure.get()).isNotNull(); + final Throwable error = ExceptionUtils.rootCause(failure.get()); + assertThat(error).isInstanceOf(EthTaskException.class); + assertThat(((EthTaskException) error).reason()).isEqualTo(FailureReason.PEER_DISCONNECTED); + } + @Test public void shouldHandleEmptyResponses() { final Blockchain remoteBlockchain = setupLocalAndRemoteChains(11, 11, 5); @@ -148,6 +208,7 @@ public void shouldHandleEmptyResponses() { ethContext, respondingEthPeer.getEthPeer(), defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), metricsSystem); // Empty response should be handled without any error @@ -196,6 +257,7 @@ public void shouldIssueConsistentNumberOfRequestsToPeer() { ethContext, respondingEthPeer.getEthPeer(), defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), metricsSystem); final DetermineCommonAncestorTask spy = spy(task); @@ -215,6 +277,44 @@ public void shouldIssueConsistentNumberOfRequestsToPeer() { verify(spy, times(3)).requestHeaders(); } + @Test + public void shouldIssueConsistentNumberOfRequestsToPeerUsingPeerTaskSystem() { + final Blockchain remoteBlockchain = setupLocalAndRemoteChains(101, 101, 1); + + final RespondingEthPeer respondingEthPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + + final EthTask task = + DetermineCommonAncestorTask.create( + protocolSchedule, + protocolContext, + ethContext, + respondingEthPeer.getEthPeer(), + defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + metricsSystem); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingEthPeer.getEthPeer()))) + .thenAnswer(peerTaskExecutorResultAnswer(remoteBlockchain)); + + final AtomicReference result = new AtomicReference<>(); + final CompletableFuture future = task.run(); + future.whenComplete( + (response, error) -> { + result.set(response); + }); + + Assertions.assertThat(result.get().getHash()) + .isEqualTo(MainnetBlockHeaderFunctions.createHash(localGenesisBlock.getHeader())); + + Mockito.verify(peerTaskExecutor, Mockito.times(2)) + .executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingEthPeer.getEthPeer())); + } + @Test public void shouldShortCircuitOnHeaderInInitialRequest() { final Blockchain remoteBlockchain = setupLocalAndRemoteChains(100, 100, 96); @@ -232,6 +332,7 @@ public void shouldShortCircuitOnHeaderInInitialRequest() { ethContext, respondingEthPeer.getEthPeer(), 10, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), metricsSystem); final DetermineCommonAncestorTask spy = spy(task); @@ -251,6 +352,51 @@ public void shouldShortCircuitOnHeaderInInitialRequest() { verify(spy, times(1)).requestHeaders(); } + @Test + public void shouldShortCircuitOnHeaderInInitialRequestUsingPeerTaskSystem() { + final Blockchain remoteBlockchain = setupLocalAndRemoteChains(100, 100, 96); + final BlockHeader commonHeader = localBlockchain.getBlockHeader(95).get(); + + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(remoteBlockchain); + final RespondingEthPeer respondingEthPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + + final DetermineCommonAncestorTask task = + DetermineCommonAncestorTask.create( + protocolSchedule, + protocolContext, + ethContext, + respondingEthPeer.getEthPeer(), + 10, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + metricsSystem); + final DetermineCommonAncestorTask spy = spy(task); + + Mockito.when( + peerTaskExecutor.executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), + Mockito.eq(respondingEthPeer.getEthPeer()))) + .thenAnswer(peerTaskExecutorResultAnswer(remoteBlockchain)); + + // Execute task + final CompletableFuture future = spy.run(); + respondingEthPeer.respondWhile(responder, () -> !future.isDone()); + + final AtomicReference result = new AtomicReference<>(); + future.whenComplete( + (response, error) -> { + result.set(response); + }); + + Assertions.assertThat(result.get().getHash()) + .isEqualTo(MainnetBlockHeaderFunctions.createHash(commonHeader)); + + Mockito.verify(peerTaskExecutor) + .executeAgainstPeer( + Mockito.any(GetHeadersFromPeerTask.class), Mockito.eq(respondingEthPeer.getEthPeer())); + } + @Test public void returnsImmediatelyWhenThereIsNoWorkToDo() throws Exception { final RespondingEthPeer respondingEthPeer = @@ -264,6 +410,32 @@ public void returnsImmediatelyWhenThereIsNoWorkToDo() throws Exception { ethContext, peer, defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + metricsSystem); + + final CompletableFuture result = task.run(); + assertThat(result).isCompletedWithValue(localGenesisBlock.getHeader()); + + // Make sure we didn't ask for any headers + verify(peer, times(0)).getHeadersByHash(any(), anyInt(), anyInt(), anyBoolean()); + verify(peer, times(0)).getHeadersByNumber(anyLong(), anyInt(), anyInt(), anyBoolean()); + verify(peer, times(0)).send(any()); + } + + @Test + public void returnsImmediatelyWhenThereIsNoWorkToDoUsingPeerTaskSystem() throws Exception { + final RespondingEthPeer respondingEthPeer = + spy(EthProtocolManagerTestUtil.createPeer(ethProtocolManager)); + final EthPeer peer = spy(respondingEthPeer.getEthPeer()); + + final EthTask task = + DetermineCommonAncestorTask.create( + protocolSchedule, + protocolContext, + ethContext, + peer, + defaultHeaderRequestSize, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), metricsSystem); final CompletableFuture result = task.run(); @@ -336,4 +508,27 @@ private Blockchain setupLocalAndRemoteChains( return remoteChain; } + + private Answer>> peerTaskExecutorResultAnswer( + final Blockchain remoteBlockchain) { + return new Answer>>() { + @Override + public PeerTaskExecutorResult> answer( + final InvocationOnMock invocationOnMock) throws Throwable { + GetHeadersFromPeerTask getHeadersTask = + invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + long blockNumber = getHeadersTask.getBlockNumber(); + int maxHeaders = getHeadersTask.getMaxHeaders(); + int skip = getHeadersTask.getSkip(); + + List headers = new ArrayList<>(); + for (long i = blockNumber; i > blockNumber - (maxHeaders - 1) * (skip + 1); i -= skip + 1) { + headers.add(remoteBlockchain.getBlockHeader(i).get()); + } + + return new PeerTaskExecutorResult>( + Optional.of(headers), PeerTaskExecutorResponseCode.SUCCESS, Optional.empty()); + } + }; + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java index c168594d0c1..f594942afac 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java @@ -30,10 +30,14 @@ import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.RetryingMessageTaskTest; import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.BlockHeadersMessage; import org.hyperledger.besu.ethereum.eth.messages.EthPV62; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.ValidationPolicy; import org.hyperledger.besu.ethereum.eth.sync.tasks.exceptions.InvalidBlockException; import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator; @@ -51,6 +55,7 @@ import java.util.stream.Collectors; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; public class DownloadHeaderSequenceTaskTest extends RetryingMessageTaskTest> { @@ -75,6 +80,7 @@ protected EthTask> createTask(final List requeste protocolSchedule, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, requestedData.size(), maxRetries, @@ -94,6 +100,7 @@ public void failsWhenPeerReturnsOnlyReferenceHeader() { protocolSchedule, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, 10, maxRetries, @@ -113,6 +120,37 @@ public void failsWhenPeerReturnsOnlyReferenceHeader() { assertNoBadBlocks(); } + @Test + public void failsWhenPeerReturnsOnlyReferenceHeaderUsingPeerTaskSystem() { + RespondingEthPeer respondingEthPeer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + + // Execute task and wait for response + final BlockHeader referenceHeader = blockchain.getChainHeadHeader(); + Mockito.when(peerTaskExecutor.execute(Mockito.any(GetHeadersFromPeerTask.class))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of(List.of(referenceHeader)), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingEthPeer.getEthPeer()))); + final EthTask> task = + DownloadHeaderSequenceTask.endingAtHeader( + protocolSchedule, + protocolContext, + ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + referenceHeader, + 10, + maxRetries, + validationPolicy, + metricsSystem); + final CompletableFuture> future = task.run(); + + assertThat(future.isDone()).isTrue(); + assertThat(future.isCompletedExceptionally()).isTrue(); + assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class); + assertNoBadBlocks(); + } + @Test public void failsWhenPeerReturnsOnlySubsetOfHeaders() { final RespondingEthPeer respondingPeer = @@ -125,6 +163,7 @@ public void failsWhenPeerReturnsOnlySubsetOfHeaders() { protocolSchedule, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, 10, maxRetries, @@ -157,6 +196,42 @@ public void failsWhenPeerReturnsOnlySubsetOfHeaders() { assertNoBadBlocks(); } + @Test + public void failsWhenPeerReturnsOnlySubsetOfHeadersUsingPeerTaskSystem() { + final RespondingEthPeer respondingPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + + // Execute task and wait for response + final BlockHeader referenceHeader = blockchain.getChainHeadHeader(); + Mockito.when(peerTaskExecutor.execute(Mockito.any(GetHeadersFromPeerTask.class))) + .thenReturn( + new PeerTaskExecutorResult<>( + Optional.of( + List.of( + referenceHeader, + blockchain.getBlockHeader(referenceHeader.getNumber() - 1).get())), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeer.getEthPeer()))); + + final EthTask> task = + DownloadHeaderSequenceTask.endingAtHeader( + protocolSchedule, + protocolContext, + ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + referenceHeader, + 10, + maxRetries, + validationPolicy, + metricsSystem); + final CompletableFuture> future = task.run(); + + assertThat(future.isDone()).isTrue(); + assertThat(future.isCompletedExceptionally()).isTrue(); + assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class); + assertNoBadBlocks(); + } + @Test public void marksBadBlockWhenHeaderValidationFails() { final RespondingEthPeer respondingPeer = @@ -175,6 +250,7 @@ public void marksBadBlockWhenHeaderValidationFails() { protocolScheduleSpy, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, blockCount - 1, // The reference header is not included in this count maxRetries, @@ -194,6 +270,67 @@ public void marksBadBlockWhenHeaderValidationFails() { assertBadBlock(badBlock); } + @Test + public void marksBadBlockWhenHeaderValidationFailsUsingPeerTaskSystem() { + final RespondingEthPeer respondingPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + // Set up a chain with an invalid block + final int blockCount = 5; + final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount; + final List chain = getBlockSequence(startBlock, blockCount); + final Block badBlock = chain.get(2); + ProtocolSchedule protocolScheduleSpy = setupHeaderValidationToFail(badBlock.getHeader()); + + Mockito.when(peerTaskExecutor.execute(Mockito.any(GetHeadersFromPeerTask.class))) + .then( + (invocationOnMock) -> { + GetHeadersFromPeerTask task = + invocationOnMock.getArgument(0, GetHeadersFromPeerTask.class); + List headers = new ArrayList<>(); + for (long i = task.getBlockNumber(); + i > task.getBlockNumber() - task.getMaxHeaders() * (task.getSkip() + 1); + i -= task.getSkip() + 1) { + Optional header = blockchain.getBlockHeader(i); + if (header.isPresent()) { + headers.add(header.get()); + } else { + break; + } + } + return new PeerTaskExecutorResult>( + Optional.of(headers), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(respondingPeer.getEthPeer())); + }); + + // Execute the task + final BlockHeader referenceHeader = chain.get(blockCount - 1).getHeader(); + final EthTask> task = + DownloadHeaderSequenceTask.endingAtHeader( + protocolScheduleSpy, + protocolContext, + ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + referenceHeader, + blockCount - 1, // The reference header is not included in this count + maxRetries, + validationPolicy, + metricsSystem); + final CompletableFuture> future = task.run(); + + final RespondingEthPeer.Responder fullResponder = getFullResponder(); + respondingPeer.respondWhile(fullResponder, () -> !future.isDone()); + + // Check that the future completed exceptionally + assertThat(future.isCompletedExceptionally()).isTrue(); + assertThatThrownBy(future::get) + .hasCauseInstanceOf(InvalidBlockException.class) + .hasMessageContaining("Header failed validation"); + + // Check bad blocks + assertBadBlock(badBlock); + } + @Test public void processHeaders_markBadBlockWhenHeaderValidationFails() { final RespondingEthPeer respondingPeer = @@ -212,6 +349,7 @@ public void processHeaders_markBadBlockWhenHeaderValidationFails() { protocolScheduleSpy, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, blockCount - 1, // The reference header is not included in this count maxRetries, @@ -257,6 +395,7 @@ public void processHeaders_markBadBlockHashWhenHeaderValidationFailsAndBodyUnava protocolScheduleSpy, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, blockCount - 1, // The reference header is not included in this count maxRetries, @@ -301,6 +440,7 @@ public void processHeaders_doesNotMarkBadBlockForOutOfRangeResponse() { protocolSchedule, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, segmentLength, maxRetries, @@ -347,6 +487,7 @@ public void processHeaders_doesNotMarkBadBlockForMisorderedBlocks() { protocolSchedule, protocolContext, ethContext, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), referenceHeader, segmentLength, maxRetries, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index ce3443eaa9d..4d47b909946 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -170,7 +170,7 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod ethPeers.setChainHeadTracker(mockCHT); final EthScheduler scheduler = new EthScheduler(1, 1, 1, metricsSystem); - final EthContext ethContext = new EthContext(ethPeers, ethMessages, scheduler); + final EthContext ethContext = new EthContext(ethPeers, ethMessages, scheduler, null); transactionPool = TransactionPoolFactory.createTransactionPool(