Refactor TransactionSelectionService (#6595)

Signed-off-by: Fabio Di Fabio <[email protected]>
hyperledger · Feb 27, 2024 · 2c1733c · 2c1733c
1 parent b8ba3ee
commit 2c1733c
Show file tree

Hide file tree

Showing 67 changed files with 250 additions and 353 deletions.
diff --git a/...sl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/...sl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java
@@ -50,7 +50,6 @@
 import org.hyperledger.besu.plugin.services.StorageService;
 import org.hyperledger.besu.plugin.services.TransactionSelectionService;
 import org.hyperledger.besu.plugin.services.storage.rocksdb.RocksDBPlugin;
-import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 import org.hyperledger.besu.plugin.services.txvalidator.PluginTransactionValidatorFactory;
 import org.hyperledger.besu.services.BesuConfigurationImpl;
 import org.hyperledger.besu.services.BesuEventsImpl;
@@ -94,6 +93,7 @@ private BesuPluginContextImpl buildPluginContext(
       final BesuNode node,
       final StorageServiceImpl storageService,
       final SecurityModuleServiceImpl securityModuleService,
+      final TransactionSelectionServiceImpl transactionSelectionServiceImpl,
       final BesuConfiguration commonPluginConfiguration) {
     final CommandLine commandLine = new CommandLine(CommandSpec.create());
     final BesuPluginContextImpl besuPluginContext = new BesuPluginContextImpl();
@@ -102,7 +102,7 @@ private BesuPluginContextImpl buildPluginContext(
     besuPluginContext.addService(PicoCLIOptions.class, new PicoCLIOptionsImpl(commandLine));
     besuPluginContext.addService(RpcEndpointService.class, new RpcEndpointServiceImpl());
     besuPluginContext.addService(
-        TransactionSelectionService.class, new TransactionSelectionServiceImpl());
+        TransactionSelectionService.class, transactionSelectionServiceImpl);
     besuPluginContext.addService(
         PluginTransactionValidatorService.class, new PluginTransactionValidatorServiceImpl());
     final Path pluginsPath;
@@ -144,6 +144,8 @@ public void startNode(final BesuNode node) {
 
     final StorageServiceImpl storageService = new StorageServiceImpl();
     final SecurityModuleServiceImpl securityModuleService = new SecurityModuleServiceImpl();
+    final TransactionSelectionServiceImpl transactionSelectionServiceImpl =
+        new TransactionSelectionServiceImpl();
     final Path dataDir = node.homeDirectory();
     final BesuConfigurationImpl commonPluginConfiguration = new BesuConfigurationImpl();
     commonPluginConfiguration.init(
@@ -156,7 +158,11 @@ public void startNode(final BesuNode node) {
             node,
             n ->
                 buildPluginContext(
-                    node, storageService, securityModuleService, commonPluginConfiguration));
+                    node,
+                    storageService,
+                    securityModuleService,
+                    transactionSelectionServiceImpl,
+                    commonPluginConfiguration));
 
     GlobalOpenTelemetry.resetForTest();
     final ObservableMetricsSystem metricsSystem =
@@ -193,8 +199,8 @@ public void startNode(final BesuNode node) {
 
     final int maxPeers = 25;
 
-    final Optional<PluginTransactionSelectorFactory> transactionSelectorFactory =
-        getTransactionSelectorFactory(besuPluginContext);
+    final TransactionSelectionService transactionSelectorService =
+        getTransactionSelectorService(besuPluginContext);
 
     final PluginTransactionValidatorFactory pluginTransactionValidatorFactory =
         getPluginTransactionValidatorFactory(besuPluginContext);
@@ -220,7 +226,7 @@ public void startNode(final BesuNode node) {
         .maxRemotelyInitiatedPeers(15)
         .networkConfiguration(node.getNetworkingConfiguration())
         .randomPeerPriority(false)
-        .transactionSelectorFactory(transactionSelectorFactory)
+        .transactionSelectorService(transactionSelectorService)
         .pluginTransactionValidatorFactory(pluginTransactionValidatorFactory);
 
     node.getGenesisConfig()
@@ -332,11 +338,9 @@ public String getConsoleContents() {
     throw new RuntimeException("Console contents can only be captured in process execution");
   }
 
-  private Optional<PluginTransactionSelectorFactory> getTransactionSelectorFactory(
+  private TransactionSelectionService getTransactionSelectorService(
       final BesuPluginContextImpl besuPluginContext) {
-    final Optional<TransactionSelectionService> txSelectionService =
-        besuPluginContext.getService(TransactionSelectionService.class);
-    return txSelectionService.isPresent() ? txSelectionService.get().get() : Optional.empty();
+    return besuPluginContext.getService(TransactionSelectionService.class).orElseThrow();
   }
 
   private PluginTransactionValidatorFactory getPluginTransactionValidatorFactory(

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -174,7 +174,6 @@
 import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
 import org.hyperledger.besu.plugin.services.storage.PrivacyKeyValueStorageFactory;
 import org.hyperledger.besu.plugin.services.storage.rocksdb.RocksDBPlugin;
-import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 import org.hyperledger.besu.plugin.services.txvalidator.PluginTransactionValidatorFactory;
 import org.hyperledger.besu.services.BesuConfigurationImpl;
 import org.hyperledger.besu.services.BesuEventsImpl;
@@ -224,7 +223,6 @@
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
-import javax.annotation.Nonnull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
@@ -1793,7 +1791,7 @@ public BesuControllerBuilder getControllerBuilder() {
         .synchronizerConfiguration(buildSyncConfig())
         .ethProtocolConfiguration(unstableEthProtocolOptions.toDomainObject())
         .networkConfiguration(unstableNetworkingOptions.toDomainObject())
-        .transactionSelectorFactory(getTransactionSelectorFactory())
+        .transactionSelectorService(getTransactionSelectorService())
         .pluginTransactionValidatorFactory(getPluginTransactionValidatorFactory())
         .dataDirectory(dataDir())
         .dataStorageConfiguration(getDataStorageConfiguration())
@@ -1825,11 +1823,8 @@ public BesuControllerBuilder getControllerBuilder() {
         .cacheLastBlocks(numberOfblocksToCache);
   }
 
-  @Nonnull
-  private Optional<PluginTransactionSelectorFactory> getTransactionSelectorFactory() {
-    final Optional<TransactionSelectionService> txSelectionService =
-        besuPluginContext.getService(TransactionSelectionService.class);
-    return txSelectionService.isPresent() ? txSelectionService.get().get() : Optional.empty();
+  private TransactionSelectionService getTransactionSelectorService() {
+    return besuPluginContext.getService(TransactionSelectionService.class).orElseThrow();
   }
 
   private PluginTransactionValidatorFactory getPluginTransactionValidatorFactory() {
@@ -2147,6 +2142,7 @@ private MiningParameters getMiningParameters() {
     if (miningParameters == null) {
       miningOptions.setGenesisBlockPeriodSeconds(
           getGenesisBlockPeriodSeconds(getActualGenesisConfigOptions()));
+      miningOptions.setTransactionSelectionService(transactionSelectionServiceImpl);
       miningParameters = miningOptions.toDomainObject();
       initMiningParametersMetrics(miningParameters);
     }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.cli.options;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME;
@@ -37,6 +38,7 @@
 import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
 import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.MutableInitValues;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
+import org.hyperledger.besu.plugin.services.TransactionSelectionService;
 import org.hyperledger.besu.util.number.PositiveNumber;
 
 import java.util.List;
@@ -190,6 +192,7 @@ static class Unstable {
   }
 
   private OptionalInt maybeGenesisBlockPeriodSeconds;
+  private TransactionSelectionService transactionSelectionService;
 
   private MiningOptions() {}
 
@@ -212,6 +215,16 @@ public void setGenesisBlockPeriodSeconds(final OptionalInt genesisBlockPeriodSec
     maybeGenesisBlockPeriodSeconds = genesisBlockPeriodSeconds;
   }
 
+  /**
+   * Set the transaction selection service
+   *
+   * @param transactionSelectionService the transaction selection service
+   */
+  public void setTransactionSelectionService(
+      final TransactionSelectionService transactionSelectionService) {
+    this.transactionSelectionService = transactionSelectionService;
+  }
+
   /**
    * Validate that there are no inconsistencies in the specified options. For example that the
    * options are valid for the selected implementation.
@@ -299,6 +312,7 @@ public void validate(
   static MiningOptions fromConfig(final MiningParameters miningParameters) {
     final MiningOptions miningOptions = MiningOptions.create();
     miningOptions.setGenesisBlockPeriodSeconds(miningParameters.getGenesisBlockPeriodSeconds());
+    miningOptions.setTransactionSelectionService(miningParameters.getTransactionSelectionService());
     miningOptions.isMiningEnabled = miningParameters.isMiningEnabled();
     miningOptions.iStratumMiningEnabled = miningParameters.isStratumMiningEnabled();
     miningOptions.stratumNetworkInterface = miningParameters.getStratumNetworkInterface();
@@ -333,10 +347,12 @@ static MiningOptions fromConfig(final MiningParameters miningParameters) {
 
   @Override
   public MiningParameters toDomainObject() {
-    if (maybeGenesisBlockPeriodSeconds == null) {
-      throw new IllegalStateException(
-          "genesisBlockPeriodSeconds must be set before using this object");
-    }
+    checkNotNull(
+        maybeGenesisBlockPeriodSeconds,
+        "genesisBlockPeriodSeconds must be set before using this object");
+    checkNotNull(
+        transactionSelectionService,
+        "transactionSelectionService must be set before using this object");
 
     final var updatableInitValuesBuilder =
         MutableInitValues.builder()
@@ -355,6 +371,7 @@ public MiningParameters toDomainObject() {
 
     return ImmutableMiningParameters.builder()
         .genesisBlockPeriodSeconds(maybeGenesisBlockPeriodSeconds)
+        .transactionSelectionService(transactionSelectionService)
         .mutableInitValues(updatableInitValuesBuilder.build())
         .isStratumMiningEnabled(iStratumMiningEnabled)
         .stratumNetworkInterface(stratumNetworkInterface)

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java
@@ -97,9 +97,9 @@
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 import org.hyperledger.besu.metrics.ObservableMetricsSystem;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
+import org.hyperledger.besu.plugin.services.TransactionSelectionService;
 import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider;
 import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
-import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 import org.hyperledger.besu.plugin.services.txvalidator.PluginTransactionValidatorFactory;
 
 import java.io.Closeable;
@@ -187,7 +187,6 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides
 
   private NetworkingConfiguration networkingConfiguration;
   private Boolean randomPeerPriority;
-  private Optional<PluginTransactionSelectorFactory> transactionSelectorFactory = Optional.empty();
   /** the Dagger configured context that can provide dependencies */
   protected Optional<BesuComponent> besuComponent = Optional.empty();
 
@@ -535,14 +534,13 @@ public BesuControllerBuilder randomPeerPriority(final Boolean randomPeerPriority
   }
 
   /**
-   * sets the transactionSelectorFactory in the builder
+   * sets the transactionSelectionService in the builder
    *
-   * @param transactionSelectorFactory the optional transaction selector factory
+   * @param transactionSelectionService the transaction selector service
    * @return the besu controller builder
    */
-  public BesuControllerBuilder transactionSelectorFactory(
-      final Optional<PluginTransactionSelectorFactory> transactionSelectorFactory) {
-    this.transactionSelectorFactory = transactionSelectorFactory;
+  public BesuControllerBuilder transactionSelectorService(
+      final TransactionSelectionService transactionSelectionService) {
     return this;
   }
 
@@ -617,11 +615,7 @@ public BesuController build() {
 
     final ProtocolContext protocolContext =
         createProtocolContext(
-            blockchain,
-            worldStateArchive,
-            protocolSchedule,
-            this::createConsensusContext,
-            transactionSelectorFactory);
+            blockchain, worldStateArchive, protocolSchedule, this::createConsensusContext);
     validateContext(protocolContext);
 
     if (chainPrunerConfiguration.getChainPruningEnabled()) {
@@ -1057,22 +1051,15 @@ protected EthProtocolManager createEthProtocolManager(
    * @param worldStateArchive the world state archive
    * @param protocolSchedule the protocol schedule
    * @param consensusContextFactory the consensus context factory
-   * @param transactionSelectorFactory optional transaction selector factory
    * @return the protocol context
    */
   protected ProtocolContext createProtocolContext(
       final MutableBlockchain blockchain,
       final WorldStateArchive worldStateArchive,
       final ProtocolSchedule protocolSchedule,
-      final ConsensusContextFactory consensusContextFactory,
-      final Optional<PluginTransactionSelectorFactory> transactionSelectorFactory) {
+      final ConsensusContextFactory consensusContextFactory) {
     return ProtocolContext.init(
-        blockchain,
-        worldStateArchive,
-        protocolSchedule,
-        consensusContextFactory,
-        transactionSelectorFactory,
-        badBlockManager);
+        blockchain, worldStateArchive, protocolSchedule, consensusContextFactory, badBlockManager);
   }
 
   private Optional<SnapProtocolManager> createSnapProtocolManager(

diff --git a/...src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java b/...src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java
@@ -62,7 +62,6 @@
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 import org.hyperledger.besu.metrics.ObservableMetricsSystem;
 import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider;
-import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 
 import java.math.BigInteger;
 import java.nio.file.Path;
@@ -175,15 +174,9 @@ protected ProtocolContext createProtocolContext(
       final MutableBlockchain blockchain,
       final WorldStateArchive worldStateArchive,
       final ProtocolSchedule protocolSchedule,
-      final ConsensusContextFactory consensusContextFactory,
-      final Optional<PluginTransactionSelectorFactory> transactionSelectorFactory) {
+      final ConsensusContextFactory consensusContextFactory) {
     return MigratingProtocolContext.init(
-        blockchain,
-        worldStateArchive,
-        protocolSchedule,
-        consensusContextFactory,
-        transactionSelectorFactory,
-        badBlockManager);
+        blockchain, worldStateArchive, protocolSchedule, consensusContextFactory, badBlockManager);
   }
 
   @Override

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java
@@ -60,7 +60,6 @@
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 import org.hyperledger.besu.metrics.ObservableMetricsSystem;
 import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider;
-import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 
 import java.math.BigInteger;
 import java.nio.file.Path;
@@ -189,15 +188,10 @@ protected ProtocolContext createProtocolContext(
       final MutableBlockchain blockchain,
       final WorldStateArchive worldStateArchive,
       final ProtocolSchedule protocolSchedule,
-      final ConsensusContextFactory consensusContextFactory,
-      final Optional<PluginTransactionSelectorFactory> transactionSelectorFactory) {
+      final ConsensusContextFactory consensusContextFactory) {
     final ProtocolContext protocolContext =
         super.createProtocolContext(
-            blockchain,
-            worldStateArchive,
-            protocolSchedule,
-            consensusContextFactory,
-            transactionSelectorFactory);
+            blockchain, worldStateArchive, protocolSchedule, consensusContextFactory);
     transitionProtocolSchedule.setProtocolContext(protocolContext);
     return protocolContext;
   }

diff --git a/besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java b/besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.services;
 
 import org.hyperledger.besu.plugin.services.TransactionSelectionService;
+import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelector;
 import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 
 import java.util.Optional;
@@ -25,13 +26,15 @@ public class TransactionSelectionServiceImpl implements TransactionSelectionServ
   private Optional<PluginTransactionSelectorFactory> factory = Optional.empty();
 
   @Override
-  public Optional<PluginTransactionSelectorFactory> get() {
-    return factory;
+  public PluginTransactionSelector createPluginTransactionSelector() {
+    return factory
+        .map(PluginTransactionSelectorFactory::create)
+        .orElse(PluginTransactionSelector.ACCEPT_ALL);
   }
 
   @Override
-  public void registerTransactionSelectorFactory(
-      final PluginTransactionSelectorFactory transactionSelectorFactory) {
-    factory = Optional.ofNullable(transactionSelectorFactory);
+  public void registerPluginTransactionSelectorFactory(
+      final PluginTransactionSelectorFactory pluginTransactionSelectorFactory) {
+    factory = Optional.ofNullable(pluginTransactionSelectorFactory);
   }
 }