Revert "Do not leak references to PendingTransactions (hyperledger#5693…

…)"

This reverts commit 2460ce2.

besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java

@@ -83,7 +83,7 @@ protected MiningCoordinator createMiningCoordinator(
         new CliqueMinerExecutor(
             protocolContext,
             protocolSchedule,
-            transactionPool,
+            transactionPool.getPendingTransactions(),
             nodeKey,
             miningParameters,
             new CliqueBlockScheduler(

besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java

@@ -146,7 +146,7 @@ protected MiningCoordinator createMiningCoordinator(
     final BftProtocolSchedule bftProtocolSchedule = (BftProtocolSchedule) protocolSchedule;
     final BftBlockCreatorFactory<?> blockCreatorFactory =
         new BftBlockCreatorFactory<>(
-            transactionPool,
+            transactionPool.getPendingTransactions(),
             protocolContext,
             bftProtocolSchedule,
             forksSchedule,
@@ -310,7 +310,7 @@ private static MinedBlockObserver blockLogger(
                 block.getHeader().getCoinbase().equals(localAddress) ? "Produced" : "Imported",
                 block.getHeader().getNumber(),
                 block.getBody().getTransactions().size(),
-                transactionPool.count(),
+                transactionPool.getPendingTransactions().size(),
                 block.getHeader().getGasUsed(),
                 (block.getHeader().getGasUsed() * 100.0) / block.getHeader().getGasLimit(),
                 block.getHash().toHexString()));

besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java

@@ -49,7 +49,7 @@ protected MiningCoordinator createMiningCoordinator(
         new PoWMinerExecutor(
             protocolContext,
             protocolSchedule,
-            transactionPool,
+            transactionPool.getPendingTransactions(),
             miningParameters,
             new DefaultBlockScheduler(
                 MainnetBlockHeaderValidator.MINIMUM_SECONDS_SINCE_PARENT,

besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java

@@ -174,7 +174,7 @@ protected MiningCoordinator createTransitionMiningCoordinator(
           LOG.debug("Block builder executor status {}", blockBuilderExecutor);
           return CompletableFuture.runAsync(task, blockBuilderExecutor);
         },
-        transactionPool,
+        transactionPool.getPendingTransactions(),
         miningParameters,
         backwardSyncContext,
         depositContractAddress);

besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java

@@ -184,7 +184,7 @@ protected MiningCoordinator createMiningCoordinator(
     final BftProtocolSchedule bftProtocolSchedule = (BftProtocolSchedule) protocolSchedule;
     final BftBlockCreatorFactory<?> blockCreatorFactory =
         new QbftBlockCreatorFactory(
-            transactionPool,
+            transactionPool.getPendingTransactions(),
             protocolContext,
             bftProtocolSchedule,
             qbftForksSchedule,
@@ -381,7 +381,7 @@ private static MinedBlockObserver blockLogger(
                 block.getHeader().getCoinbase().equals(localAddress) ? "Produced" : "Imported",
                 block.getHeader().getNumber(),
                 block.getBody().getTransactions().size(),
-                transactionPool.count(),
+                transactionPool.getPendingTransactions().size(),
                 block.getHeader().getGasUsed(),
                 (block.getHeader().getGasUsed() * 100.0) / block.getHeader().getGasLimit(),
                 block.getHash().toHexString()));

consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java

@@ -33,7 +33,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.SealableBlockHeader;
 import org.hyperledger.besu.ethereum.core.Util;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
+import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions;
 
@@ -52,7 +52,7 @@ public class CliqueBlockCreator extends AbstractBlockCreator {
    * @param coinbase the coinbase
    * @param targetGasLimitSupplier the target gas limit supplier
    * @param extraDataCalculator the extra data calculator
-   * @param transactionPool the pending transactions
+   * @param pendingTransactions the pending transactions
    * @param protocolContext the protocol context
    * @param protocolSchedule the protocol schedule
    * @param nodeKey the node key
@@ -65,7 +65,7 @@ public CliqueBlockCreator(
       final Address coinbase,
       final Supplier<Optional<Long>> targetGasLimitSupplier,
       final ExtraDataCalculator extraDataCalculator,
-      final TransactionPool transactionPool,
+      final PendingTransactions pendingTransactions,
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
       final NodeKey nodeKey,
@@ -78,7 +78,7 @@ public CliqueBlockCreator(
         __ -> Util.publicKeyToAddress(nodeKey.getPublicKey()),
         targetGasLimitSupplier,
         extraDataCalculator,
-        transactionPool,
+        pendingTransactions,
         protocolContext,
         protocolSchedule,
         minTransactionGasPrice,

consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java

@@ -28,7 +28,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.Util;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
+import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.util.Subscribers;
 
@@ -54,7 +54,7 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor<CliqueBlockMiner>
    *
    * @param protocolContext the protocol context
    * @param protocolSchedule the protocol schedule
-   * @param transactionPool the pending transactions
+   * @param pendingTransactions the pending transactions
    * @param nodeKey the node key
    * @param miningParams the mining params
    * @param blockScheduler the block scheduler
@@ -63,12 +63,12 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor<CliqueBlockMiner>
   public CliqueMinerExecutor(
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
-      final TransactionPool transactionPool,
+      final PendingTransactions pendingTransactions,
       final NodeKey nodeKey,
       final MiningParameters miningParams,
       final AbstractBlockScheduler blockScheduler,
       final EpochManager epochManager) {
-    super(protocolContext, protocolSchedule, transactionPool, miningParams, blockScheduler);
+    super(protocolContext, protocolSchedule, pendingTransactions, miningParams, blockScheduler);
     this.nodeKey = nodeKey;
     this.localAddress = Util.publicKeyToAddress(nodeKey.getPublicKey());
     this.epochManager = epochManager;
@@ -85,7 +85,7 @@ public CliqueBlockMiner createMiner(
                 localAddress, // TOOD(tmm): This can be removed (used for voting not coinbase).
                 () -> targetGasLimit.map(AtomicLong::longValue),
                 this::calculateExtraData,
-                transactionPool,
+                pendingTransactions,
                 protocolContext,
                 protocolSchedule,
                 nodeKey,

consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java

@@ -18,7 +18,6 @@
 import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain;
 import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryWorldStateArchive;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -47,14 +46,8 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
-import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.Util;
-import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionBroadcaster;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolMetrics;
 import org.hyperledger.besu.ethereum.eth.transactions.sorter.GasPricePendingTransactionsSorter;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
@@ -139,7 +132,11 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            createTransactionPool(),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                blockchain::getChainHeadHeader),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -168,7 +165,11 @@ public void insertsValidVoteIntoConstructedBlock() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            createTransactionPool(),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                blockchain::getChainHeadHeader),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -202,7 +203,11 @@ public void insertsNoVoteWhenAtEpoch() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            createTransactionPool(),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                blockchain::getChainHeadHeader),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -215,28 +220,4 @@ public void insertsNoVoteWhenAtEpoch() {
     assertThat(createdBlock.getHeader().getNonce()).isEqualTo(CliqueBlockInterface.DROP_NONCE);
     assertThat(createdBlock.getHeader().getCoinbase()).isEqualTo(Address.fromHexString("0"));
   }
-
-  private TransactionPool createTransactionPool() {
-    final TransactionPoolConfiguration conf =
-        ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build();
-    final EthContext ethContext = mock(EthContext.class, RETURNS_DEEP_STUBS);
-    when(ethContext.getEthPeers().subscribeConnect(any())).thenReturn(1L);
-    final TransactionPool transactionPool =
-        new TransactionPool(
-            () ->
-                new GasPricePendingTransactionsSorter(
-                    conf,
-                    TestClock.system(ZoneId.systemDefault()),
-                    metricsSystem,
-                    blockchain::getChainHeadHeader),
-            protocolSchedule,
-            protocolContext,
-            mock(TransactionBroadcaster.class),
-            ethContext,
-            mock(MiningParameters.class),
-            new TransactionPoolMetrics(metricsSystem),
-            conf);
-    transactionPool.setEnabled();
-    return transactionPool;
-  }
 }

consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java

@@ -16,7 +16,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -39,13 +38,8 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.Util;
-import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionBroadcaster;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolMetrics;
 import org.hyperledger.besu.ethereum.eth.transactions.sorter.GasPricePendingTransactionsSorter;
-import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
@@ -71,8 +65,6 @@ public class CliqueMinerExecutorTest {
   private Address localAddress;
   private final List<Address> validatorList = Lists.newArrayList();
   private ProtocolContext cliqueProtocolContext;
-  private ProtocolSchedule cliqueProtocolSchedule;
-  private EthContext cliqueEthContext;
   private BlockHeaderTestFixture blockHeaderBuilder;
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
   private final CliqueBlockInterface blockInterface = new CliqueBlockInterface();
@@ -90,10 +82,6 @@ public void setup() {
 
     final CliqueContext cliqueContext = new CliqueContext(validatorProvider, null, blockInterface);
     cliqueProtocolContext = new ProtocolContext(null, null, cliqueContext, Optional.empty());
-    cliqueProtocolSchedule =
-        CliqueProtocolSchedule.create(
-            GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT);
-    cliqueEthContext = mock(EthContext.class, RETURNS_DEEP_STUBS);
     blockHeaderBuilder = new BlockHeaderTestFixture();
   }
 
@@ -104,8 +92,13 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            cliqueProtocolSchedule,
-            createTransactionPool(),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                CliqueMinerExecutorTest::mockBlockHeader),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -141,8 +134,13 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            cliqueProtocolSchedule,
-            createTransactionPool(),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                CliqueMinerExecutorTest::mockBlockHeader),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -178,8 +176,13 @@ public void shouldUseLatestVanityData() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            cliqueProtocolSchedule,
-            createTransactionPool(),
+            CliqueProtocolSchedule.create(
+                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
+            new GasPricePendingTransactionsSorter(
+                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
+                TestClock.system(ZoneId.systemDefault()),
+                metricsSystem,
+                CliqueMinerExecutorTest::mockBlockHeader),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -203,31 +206,6 @@ public void shouldUseLatestVanityData() {
     assertThat(cliqueExtraData.getVanityData()).isEqualTo(modifiedVanityData);
   }
 
-  private TransactionPool createTransactionPool() {
-    final var conf = ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build();
-
-    when(cliqueEthContext.getEthPeers().subscribeConnect(any())).thenReturn(1L);
-
-    final TransactionPool transactionPool =
-        new TransactionPool(
-            () ->
-                new GasPricePendingTransactionsSorter(
-                    conf,
-                    TestClock.system(ZoneId.systemDefault()),
-                    metricsSystem,
-                    CliqueMinerExecutorTest::mockBlockHeader),
-            cliqueProtocolSchedule,
-            cliqueProtocolContext,
-            mock(TransactionBroadcaster.class),
-            cliqueEthContext,
-            mock(MiningParameters.class),
-            new TransactionPoolMetrics(metricsSystem),
-            conf);
-
-    transactionPool.setEnabled();
-    return transactionPool;
-  }
-
   private static BlockHeader mockBlockHeader() {
     final BlockHeader blockHeader = mock(BlockHeader.class);
     when(blockHeader.getBaseFee()).thenReturn(Optional.empty());

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java

@@ -26,7 +26,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderBuilder;
 import org.hyperledger.besu.ethereum.core.SealableBlockHeader;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
+import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 
 import java.util.Optional;
@@ -46,7 +46,7 @@ public class BftBlockCreator extends AbstractBlockCreator {
    * @param localAddress the local address
    * @param targetGasLimitSupplier the target gas limit supplier
    * @param extraDataCalculator the extra data calculator
-   * @param transactionPool the pending transactions
+   * @param pendingTransactions the pending transactions
    * @param protocolContext the protocol context
    * @param protocolSchedule the protocol schedule
    * @param minTransactionGasPrice the min transaction gas price
@@ -59,7 +59,7 @@ public BftBlockCreator(
       final Address localAddress,
       final Supplier<Optional<Long>> targetGasLimitSupplier,
       final ExtraDataCalculator extraDataCalculator,
-      final TransactionPool transactionPool,
+      final PendingTransactions pendingTransactions,
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
       final Wei minTransactionGasPrice,
@@ -71,7 +71,7 @@ public BftBlockCreator(
         miningBeneficiaryCalculator(localAddress, forksSchedule),
         targetGasLimitSupplier,
         extraDataCalculator,
-        transactionPool,
+        pendingTransactions,
         protocolContext,
         protocolSchedule,
         minTransactionGasPrice,