Do not leak references to PendingTransactions (hyperledger#5693)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
liquichain · Jul 21, 2023 · 2460ce2 · 2460ce2
1 parent ef0d2ff
commit 2460ce2
Show file tree

Hide file tree

Showing 63 changed files with 973 additions and 701 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java
@@ -83,7 +83,7 @@ protected MiningCoordinator createMiningCoordinator(
         new CliqueMinerExecutor(
             protocolContext,
             protocolSchedule,
-            transactionPool.getPendingTransactions(),
+            transactionPool,
             nodeKey,
             miningParameters,
             new CliqueBlockScheduler(

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/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.getPendingTransactions(),
+            transactionPool,
             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.getPendingTransactions().size(),
+                transactionPool.count(),
                 block.getHeader().getGasUsed(),
                 (block.getHeader().getGasUsed() * 100.0) / block.getHeader().getGasLimit(),
                 block.getHash().toHexString()));

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java
@@ -49,7 +49,7 @@ protected MiningCoordinator createMiningCoordinator(
         new PoWMinerExecutor(
             protocolContext,
             protocolSchedule,
-            transactionPool.getPendingTransactions(),
+            transactionPool,
             miningParameters,
             new DefaultBlockScheduler(
                 MainnetBlockHeaderValidator.MINIMUM_SECONDS_SINCE_PARENT,

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/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.getPendingTransactions(),
+        transactionPool,
         miningParameters,
         backwardSyncContext,
         depositContractAddress);

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/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.getPendingTransactions(),
+            transactionPool,
             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.getPendingTransactions().size(),
+                transactionPool.count(),
                 block.getHeader().getGasUsed(),
                 (block.getHeader().getGasUsed() * 100.0) / block.getHeader().getGasLimit(),
                 block.getHash().toHexString()));

diff --git a/...src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java b/...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.PendingTransactions;
+import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 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 pendingTransactions the pending transactions
+   * @param transactionPool 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 PendingTransactions pendingTransactions,
+      final TransactionPool transactionPool,
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
       final NodeKey nodeKey,
@@ -78,7 +78,7 @@ public CliqueBlockCreator(
         __ -> Util.publicKeyToAddress(nodeKey.getPublicKey()),
         targetGasLimitSupplier,
         extraDataCalculator,
-        pendingTransactions,
+        transactionPool,
         protocolContext,
         protocolSchedule,
         minTransactionGasPrice,

diff --git a/...rc/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java b/...rc/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.PendingTransactions;
+import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 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 pendingTransactions the pending transactions
+   * @param transactionPool 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 PendingTransactions pendingTransactions,
+      final TransactionPool transactionPool,
       final NodeKey nodeKey,
       final MiningParameters miningParams,
       final AbstractBlockScheduler blockScheduler,
       final EpochManager epochManager) {
-    super(protocolContext, protocolSchedule, pendingTransactions, miningParams, blockScheduler);
+    super(protocolContext, protocolSchedule, transactionPool, 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,
-                pendingTransactions,
+                transactionPool,
                 protocolContext,
                 protocolSchedule,
                 nodeKey,

diff --git a/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java
@@ -18,6 +18,7 @@
 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;
 
@@ -46,8 +47,14 @@
 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;
@@ -132,11 +139,7 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                blockchain::getChainHeadHeader),
+            createTransactionPool(),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -165,11 +168,7 @@ public void insertsValidVoteIntoConstructedBlock() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                blockchain::getChainHeadHeader),
+            createTransactionPool(),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -203,11 +202,7 @@ public void insertsNoVoteWhenAtEpoch() {
             coinbase,
             () -> Optional.of(10_000_000L),
             parent -> extraData,
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(5).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                blockchain::getChainHeadHeader),
+            createTransactionPool(),
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
@@ -220,4 +215,28 @@ 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;
+  }
 }
diff --git a/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java
@@ -16,6 +16,7 @@
 
 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;
 
@@ -38,8 +39,13 @@
 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;
@@ -65,6 +71,8 @@ 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();
@@ -82,6 +90,10 @@ 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();
   }
 
@@ -92,13 +104,8 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            CliqueProtocolSchedule.create(
-                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                CliqueMinerExecutorTest::mockBlockHeader),
+            cliqueProtocolSchedule,
+            createTransactionPool(),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -134,13 +141,8 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            CliqueProtocolSchedule.create(
-                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                CliqueMinerExecutorTest::mockBlockHeader),
+            cliqueProtocolSchedule,
+            createTransactionPool(),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -176,13 +178,8 @@ public void shouldUseLatestVanityData() {
     final CliqueMinerExecutor executor =
         new CliqueMinerExecutor(
             cliqueProtocolContext,
-            CliqueProtocolSchedule.create(
-                GENESIS_CONFIG_OPTIONS, proposerNodeKey, false, EvmConfiguration.DEFAULT),
-            new GasPricePendingTransactionsSorter(
-                ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build(),
-                TestClock.system(ZoneId.systemDefault()),
-                metricsSystem,
-                CliqueMinerExecutorTest::mockBlockHeader),
+            cliqueProtocolSchedule,
+            createTransactionPool(),
             proposerNodeKey,
             new MiningParameters.Builder()
                 .coinbase(AddressHelpers.ofValue(1))
@@ -206,6 +203,31 @@ 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());

diff --git a/...rc/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java b/...rc/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.PendingTransactions;
+import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 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 pendingTransactions the pending transactions
+   * @param transactionPool 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 PendingTransactions pendingTransactions,
+      final TransactionPool transactionPool,
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
       final Wei minTransactionGasPrice,
@@ -71,7 +71,7 @@ public BftBlockCreator(
         miningBeneficiaryCalculator(localAddress, forksSchedule),
         targetGasLimitSupplier,
         extraDataCalculator,
-        pendingTransactions,
+        transactionPool,
         protocolContext,
         protocolSchedule,
         minTransactionGasPrice,