From fc9b3f2517d5834ce744058d4bb4270fa9966a16 Mon Sep 17 00:00:00 2001 From: Matilda Clerke Date: Fri, 20 Sep 2024 14:26:10 +1000 Subject: [PATCH] 7311: Adjust to the removal of PeerTaskFeatureToggle and use SynchronizerConfiguration to get the toggle instead Signed-off-by: Matilda Clerke --- .../java/org/hyperledger/besu/RunnerTest.java | 48 +++++++------------ .../CheckpointDownloadBlockStep.java | 7 ++- ...CheckpointSyncDownloadPipelineFactory.java | 2 +- .../sync/fastsync/DownloadReceiptsStep.java | 7 ++- .../FastSyncDownloadPipelineFactory.java | 2 +- .../CheckPointSyncChainDownloaderTest.java | 13 +++-- .../fastsync/DownloadReceiptsStepTest.java | 20 +++++--- .../PeerTaskFeatureToggleTestHelper.java | 36 -------------- 8 files changed, 49 insertions(+), 86 deletions(-) delete mode 100644 testutil/src/main/java/org/hyperledger/ethereum/eth/manager/peertask/PeerTaskFeatureToggleTestHelper.java diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java index 566331196a..5fbd75f0b3 100644 --- a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java +++ b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java @@ -75,7 +75,6 @@ import org.hyperledger.besu.services.BesuPluginContextImpl; import org.hyperledger.besu.services.PermissioningServiceImpl; import org.hyperledger.besu.services.RpcEndpointServiceImpl; import org.hyperledger.besu.testutil.TestClock; -import org.hyperledger.ethereum.eth.manager.peertask.PeerTaskFeatureToggleTestHelper; import java.io.IOException; import java.math.BigInteger; @@ -133,17 +132,6 @@ public final class RunnerTest { @Test public void getFixedNodes() throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); - doGetFixedNodes(); - } - - @Test - public void getFixedNodesUsingPeerTaskSystem() throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); - doGetFixedNodes(); - } - - private void doGetFixedNodes() { final EnodeURL staticNode = EnodeURLImpl.fromString( "enode://8f4b88336cc40ef2516d8b27df812e007fb2384a61e93635f1899051311344f3dcdbb49a4fe49a79f66d2f589a9f282e8cc4f1d7381e8ef7e4fcc6b0db578c77@127.0.0.1:30301"); @@ -162,43 +150,40 @@ public final class RunnerTest { @Test public void fullSyncFromGenesis() throws Exception { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); - doFullSyncFromGenesis(); + // set merge flag to false, otherwise this test can fail if a merge test runs first + MergeConfigOptions.setMergeEnabled(false); + + syncFromGenesis(SyncMode.FULL, getFastSyncGenesis(), false); } @Test public void fullSyncFromGenesisUsingPeerTaskSystem() throws Exception { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); - doFullSyncFromGenesis(); - } - - private void doFullSyncFromGenesis() throws Exception { // set merge flag to false, otherwise this test can fail if a merge test runs first MergeConfigOptions.setMergeEnabled(false); - syncFromGenesis(SyncMode.FULL, getFastSyncGenesis()); + syncFromGenesis(SyncMode.FULL, getFastSyncGenesis(), true); } @Test public void fastSyncFromGenesis() throws Exception { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); - doFastSyncFromGenesis(); + // set merge flag to false, otherwise this test can fail if a merge test runs first + MergeConfigOptions.setMergeEnabled(false); + + syncFromGenesis(SyncMode.FAST, getFastSyncGenesis(), false); } @Test public void fastSyncFromGenesisUsingPeerTaskSystem() throws Exception { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); - doFastSyncFromGenesis(); - } - - private void doFastSyncFromGenesis() throws Exception { // set merge flag to false, otherwise this test can fail if a merge test runs first MergeConfigOptions.setMergeEnabled(false); - syncFromGenesis(SyncMode.FAST, getFastSyncGenesis()); + syncFromGenesis(SyncMode.FAST, getFastSyncGenesis(), true); } - private void syncFromGenesis(final SyncMode mode, final GenesisConfigFile genesisConfig) + private void syncFromGenesis( + final SyncMode mode, + final GenesisConfigFile genesisConfig, + final boolean isPeerTaskSystemEnabled) throws Exception { final Path dataDirAhead = Files.createTempDirectory(temp, "db-ahead"); final Path dbAhead = dataDirAhead.resolve("database"); @@ -206,7 +191,10 @@ public final class RunnerTest { final NodeKey aheadDbNodeKey = NodeKeyUtils.createFrom(KeyPairUtil.loadKeyPair(dataDirAhead)); final NodeKey behindDbNodeKey = NodeKeyUtils.generate(); final SynchronizerConfiguration syncConfigAhead = - SynchronizerConfiguration.builder().syncMode(SyncMode.FULL).build(); + SynchronizerConfiguration.builder() + .syncMode(SyncMode.FULL) + .isPeerTaskSystemEnabled(isPeerTaskSystemEnabled) + .build(); final ObservableMetricsSystem noOpMetricsSystem = new NoOpMetricsSystem(); final var miningParameters = MiningParameters.newDefault(); final var dataStorageConfiguration = DataStorageConfiguration.DEFAULT_FOREST_CONFIG; 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 f255ec69d9..9b742af9f7 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 @@ -23,10 +23,10 @@ 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.PeerTaskFeatureToggle; import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetReceiptsFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.GetBlockFromPeerTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.fastsync.checkpoint.Checkpoint; import org.hyperledger.besu.ethereum.mainnet.BodyValidator; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -43,6 +43,7 @@ public class CheckpointDownloadBlockStep { private final EthContext ethContext; private final PeerTaskExecutor peerTaskExecutor; private final Checkpoint checkpoint; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; public CheckpointDownloadBlockStep( @@ -50,11 +51,13 @@ public class CheckpointDownloadBlockStep { 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; } @@ -75,7 +78,7 @@ public class CheckpointDownloadBlockStep { private CompletableFuture> downloadReceipts( final PeerTaskResult peerTaskResult) { final Block block = peerTaskResult.getResult(); - if (PeerTaskFeatureToggle.usePeerTaskSystem()) { + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { CompletableFuture> futureReceipts = new CompletableFuture<>(); GetReceiptsFromPeerTask task = new GetReceiptsFromPeerTask(List.of(block.getHeader()), new BodyValidator()); 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 714dfcb4af..0be1086986 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 @@ -86,7 +86,7 @@ public class CheckpointSyncDownloadPipelineFactory extends FastSyncDownloadPipel final CheckpointDownloadBlockStep checkPointDownloadBlockStep = new CheckpointDownloadBlockStep( - protocolSchedule, ethContext, peerTaskExecutor, checkpoint, metricsSystem); + protocolSchedule, ethContext, peerTaskExecutor, 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 bd8b0a8a42..a9ef69a86d 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 @@ -25,8 +25,8 @@ 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.PeerTaskFeatureToggle; import org.hyperledger.besu.ethereum.eth.manager.peertask.task.GetReceiptsFromPeerTask; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.tasks.GetReceiptsForHeadersTask; import org.hyperledger.besu.ethereum.mainnet.BodyValidator; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -42,14 +42,17 @@ public class DownloadReceiptsStep implements Function, CompletableFuture>> { private final EthContext ethContext; private final PeerTaskExecutor peerTaskExecutor; + private final SynchronizerConfiguration synchronizerConfiguration; private final MetricsSystem metricsSystem; public DownloadReceiptsStep( final EthContext ethContext, final PeerTaskExecutor peerTaskExecutor, + final SynchronizerConfiguration synchronizerConfiguration, final MetricsSystem metricsSystem) { this.ethContext = ethContext; this.peerTaskExecutor = peerTaskExecutor; + this.synchronizerConfiguration = synchronizerConfiguration; this.metricsSystem = metricsSystem; } @@ -57,7 +60,7 @@ public class DownloadReceiptsStep public CompletableFuture> apply(final List blocks) { final List headers = blocks.stream().map(Block::getHeader).collect(toList()); final CompletableFuture>> getReceipts; - if (PeerTaskFeatureToggle.usePeerTaskSystem()) { + if (synchronizerConfiguration.isPeerTaskSystemEnabled()) { GetReceiptsFromPeerTask getReceiptsFromPeerTask = new GetReceiptsFromPeerTask(headers, new BodyValidator()); PeerTaskExecutorResult>> getReceiptsResult = 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 63df4ee0cb..dc56891cef 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 @@ -144,7 +144,7 @@ public class FastSyncDownloadPipelineFactory implements DownloadPipelineFactory final DownloadBodiesStep downloadBodiesStep = new DownloadBodiesStep(protocolSchedule, ethContext, metricsSystem); final DownloadReceiptsStep downloadReceiptsStep = - new DownloadReceiptsStep(ethContext, peerTaskExecutor, metricsSystem); + new DownloadReceiptsStep(ethContext, peerTaskExecutor, syncConfig, metricsSystem); final ImportBlocksStep importBlockStep = new ImportBlocksStep( protocolSchedule, 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 d1a63e50c2..68e1023fbc 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 @@ -49,7 +49,6 @@ import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.metrics.SyncDurationMetrics; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; -import org.hyperledger.ethereum.eth.manager.peertask.PeerTaskFeatureToggleTestHelper; import java.lang.reflect.Field; import java.util.Collection; @@ -221,7 +220,6 @@ public class CheckPointSyncChainDownloaderTest { @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) public void shouldSyncToPivotBlockInMultipleSegments(final DataStorageFormat storageFormat) throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); setup(storageFormat); final RespondingEthPeer peer = @@ -233,6 +231,7 @@ public class CheckPointSyncChainDownloaderTest { SynchronizerConfiguration.builder() .downloaderChainSegmentSize(5) .downloaderHeadersRequestSize(3) + .isPeerTaskSystemEnabled(false) .build(); final long pivotBlockNumber = 25; ethContext @@ -258,7 +257,6 @@ public class CheckPointSyncChainDownloaderTest { @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) public void shouldSyncToPivotBlockInSingleSegment(final DataStorageFormat storageFormat) throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); setup(storageFormat); final RespondingEthPeer peer = @@ -267,7 +265,8 @@ public class CheckPointSyncChainDownloaderTest { RespondingEthPeer.blockchainResponder(otherBlockchain); final long pivotBlockNumber = 10; - final SynchronizerConfiguration syncConfig = SynchronizerConfiguration.builder().build(); + final SynchronizerConfiguration syncConfig = + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(); ethContext .getEthPeers() .streamAvailablePeers() @@ -292,7 +291,6 @@ public class CheckPointSyncChainDownloaderTest { public void shouldSyncToPivotBlockInMultipleSegmentsWithPeerTaskSystem( final DataStorageFormat storageFormat) throws IllegalAccessException, ExecutionException, InterruptedException, TimeoutException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); setup(storageFormat); final RespondingEthPeer peer = @@ -304,6 +302,7 @@ public class CheckPointSyncChainDownloaderTest { SynchronizerConfiguration.builder() .downloaderChainSegmentSize(5) .downloaderHeadersRequestSize(3) + .isPeerTaskSystemEnabled(true) .build(); final long pivotBlockNumber = 25; ethContext @@ -329,7 +328,6 @@ public class CheckPointSyncChainDownloaderTest { @ArgumentsSource(CheckPointSyncChainDownloaderTestArguments.class) public void shouldSyncToPivotBlockInSingleSegmentWithPeerTaskSystem( final DataStorageFormat storageFormat) throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); setup(storageFormat); final RespondingEthPeer peer = @@ -338,7 +336,8 @@ public class CheckPointSyncChainDownloaderTest { RespondingEthPeer.blockchainResponder(otherBlockchain); final long pivotBlockNumber = 10; - final SynchronizerConfiguration syncConfig = SynchronizerConfiguration.builder().build(); + final SynchronizerConfiguration syncConfig = + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(); ethContext .getEthPeers() .streamAvailablePeers() 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 1051578415..c6f4015160 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 @@ -34,10 +34,10 @@ 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; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; -import org.hyperledger.ethereum.eth.manager.peertask.PeerTaskFeatureToggleTestHelper; import java.util.HashMap; import java.util.List; @@ -57,7 +57,6 @@ public class DownloadReceiptsStepTest { private PeerTaskExecutor peerTaskExecutor; private EthProtocolManager ethProtocolManager; - private DownloadReceiptsStep downloadReceiptsStep; @BeforeAll public static void setUpClass() { @@ -79,14 +78,16 @@ public class DownloadReceiptsStepTest { protocolContext.getWorldStateArchive(), transactionPool, EthProtocolConfiguration.defaultConfig()); - downloadReceiptsStep = - new DownloadReceiptsStep( - ethProtocolManager.ethContext(), peerTaskExecutor, new NoOpMetricsSystem()); } @Test public void shouldDownloadReceiptsForBlocks() throws IllegalAccessException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(false); + DownloadReceiptsStep downloadReceiptsStep = + new DownloadReceiptsStep( + ethProtocolManager.ethContext(), + peerTaskExecutor, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(false).build(), + new NoOpMetricsSystem()); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); final List blocks = asList(block(1), block(2), block(3), block(4)); @@ -106,7 +107,12 @@ public class DownloadReceiptsStepTest { @Test public void shouldDownloadReceiptsForBlocksUsingPeerTaskSystem() throws IllegalAccessException, ExecutionException, InterruptedException { - PeerTaskFeatureToggleTestHelper.setPeerTaskFeatureToggle(true); + DownloadReceiptsStep downloadReceiptsStep = + new DownloadReceiptsStep( + ethProtocolManager.ethContext(), + peerTaskExecutor, + SynchronizerConfiguration.builder().isPeerTaskSystemEnabled(true).build(), + new NoOpMetricsSystem()); final List blocks = asList(mockBlock(), mockBlock(), mockBlock(), mockBlock()); Map> receiptsMap = new HashMap<>(); diff --git a/testutil/src/main/java/org/hyperledger/ethereum/eth/manager/peertask/PeerTaskFeatureToggleTestHelper.java b/testutil/src/main/java/org/hyperledger/ethereum/eth/manager/peertask/PeerTaskFeatureToggleTestHelper.java deleted file mode 100644 index 44a4e37b2e..0000000000 --- a/testutil/src/main/java/org/hyperledger/ethereum/eth/manager/peertask/PeerTaskFeatureToggleTestHelper.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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.ethereum.eth.manager.peertask; - -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskFeatureToggle; - -import java.lang.reflect.Field; - -import org.junit.platform.commons.util.ReflectionUtils; - -public class PeerTaskFeatureToggleTestHelper { - - public static void setPeerTaskFeatureToggle(final boolean usePeerTaskSystem) - throws IllegalAccessException { - Field usePeerTaskSystemField = - ReflectionUtils.findFields( - PeerTaskFeatureToggle.class, - (f) -> f.getName().equals("USE_PEER_TASK_SYSTEM"), - ReflectionUtils.HierarchyTraversalMode.TOP_DOWN) - .getFirst(); - usePeerTaskSystemField.setAccessible(true); - usePeerTaskSystemField.set(null, usePeerTaskSystem); - } -}