diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b670f3f76..ed7d0aa12f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ - Fix Shanghai/QBFT block import bug when syncing new nodes [#6765](https://github.com/hyperledger/besu/pull/6765) - Fix to avoid broadcasting full blob txs, instead of only the tx announcement, to a subset of nodes [#6835](https://github.com/hyperledger/besu/pull/6835) - Snap client fixes discovered during snap server testing [#6847](https://github.com/hyperledger/besu/pull/6847) +- Correctly initialize the txpool as disabled on creation [#6890](https://github.com/hyperledger/besu/pull/6890) ### Download Links diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java index bb079756d57..b5171ac7d50 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java @@ -89,7 +89,7 @@ public class TransactionPool implements BlockAddedObserver { private static final Logger LOG = LoggerFactory.getLogger(TransactionPool.class); private static final Logger LOG_FOR_REPLAY = LoggerFactory.getLogger("LOG_FOR_REPLAY"); private final Supplier pendingTransactionsSupplier; - private volatile PendingTransactions pendingTransactions; + private volatile PendingTransactions pendingTransactions = new DisabledPendingTransactions(); private final ProtocolSchedule protocolSchedule; private final ProtocolContext protocolContext; private final EthContext ethContext; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index 5410cfc92d7..d6b1b65381f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED; +import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LEGACY; import static org.hyperledger.besu.ethereum.mainnet.MainnetProtocolSchedule.DEFAULT_CHAIN_ID; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; @@ -27,6 +28,7 @@ import static org.mockito.Mockito.when; import org.hyperledger.besu.config.StubGenesisConfigOptions; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.BadBlockManager; @@ -262,33 +264,38 @@ public void incomingTransactionMessageHandlersRegisteredIfNoInitialSync() { "transaction messages handler should be enabled")); } + @Test + public void txPoolStartsDisabledWhenInitialSyncPhaseIsRequired() { + // when using any of the initial syncs (SNAP, FAST, ...), txpool starts disabled + // and is enabled only after it is in sync + setupInitialSyncPhase(true); + assertThat(pool.isEnabled()).isFalse(); + } + + @Test + public void callingGetNextNonceForSenderOnDisabledTxPoolWorks() { + setupInitialSyncPhase(true); + + assertThat(pool.getNextNonceForSender(Address.fromHexString("0x123abc"))).isEmpty(); + } + + @Test + public void txPoolStartsEnabledWhenFullSyncConfigured() { + // when using FULL sync, txpool starts enabled, because it could be + // that it is the first node on a new network and so, it is in sync with genesis + // and must accept txs. Otherwise, asap it find a sync target that is ahead, the txpool + // will be disabled, until in sync. + setupInitialSyncPhase(false); + assertThat(pool.isEnabled()).isTrue(); + } + private void setupInitialSyncPhase(final boolean hasInitialSyncPhase) { syncState = new SyncState(blockchain, ethPeers, hasInitialSyncPhase, Optional.empty()); setupInitialSyncPhase(syncState); } private void setupInitialSyncPhase(final SyncState syncState) { - pool = - TransactionPoolFactory.createTransactionPool( - schedule, - context, - ethContext, - TestClock.fixed(), - new TransactionPoolMetrics(new NoOpMetricsSystem()), - syncState, - ImmutableTransactionPoolConfiguration.builder() - .txPoolMaxSize(1) - .pendingTxRetentionPeriod(1) - .unstable( - ImmutableTransactionPoolConfiguration.Unstable.builder() - .txMessageKeepAliveSeconds(1) - .build()) - .build(), - peerTransactionTracker, - transactionsMessageSender, - newPooledTransactionHashesMessageSender, - new BlobCache(), - MiningParameters.newDefault()); + pool = createTransactionPool(LAYERED, syncState); ethProtocolManager = new EthProtocolManager( @@ -312,8 +319,7 @@ private void setupInitialSyncPhase(final SyncState syncState) { createLegacyTransactionPool_shouldUseBaseFeePendingTransactionsSorter_whenLondonEnabled() { setupScheduleWith(new StubGenesisConfigOptions().londonBlock(0)); - final TransactionPool pool = - createTransactionPool(TransactionPoolConfiguration.Implementation.LEGACY); + final TransactionPool pool = createAndEnableTransactionPool(LEGACY, syncState); assertThat(pool.pendingTransactionsImplementation()) .isEqualTo(BaseFeePendingTransactionsSorter.class); @@ -324,8 +330,7 @@ private void setupInitialSyncPhase(final SyncState syncState) { createLegacyTransactionPool_shouldUseGasPricePendingTransactionsSorter_whenLondonNotEnabled() { setupScheduleWith(new StubGenesisConfigOptions().berlinBlock(0)); - final TransactionPool pool = - createTransactionPool(TransactionPoolConfiguration.Implementation.LEGACY); + final TransactionPool pool = createAndEnableTransactionPool(LEGACY, syncState); assertThat(pool.pendingTransactionsImplementation()) .isEqualTo(GasPricePendingTransactionsSorter.class); @@ -336,7 +341,7 @@ private void setupInitialSyncPhase(final SyncState syncState) { createLayeredTransactionPool_shouldUseBaseFeePendingTransactionsSorter_whenLondonEnabled() { setupScheduleWith(new StubGenesisConfigOptions().londonBlock(0)); - final TransactionPool pool = createTransactionPool(LAYERED); + final TransactionPool pool = createAndEnableTransactionPool(LAYERED, syncState); assertThat(pool.pendingTransactionsImplementation()) .isEqualTo(LayeredPendingTransactions.class); @@ -349,7 +354,7 @@ private void setupInitialSyncPhase(final SyncState syncState) { createLayeredTransactionPool_shouldUseGasPricePendingTransactionsSorter_whenLondonNotEnabled() { setupScheduleWith(new StubGenesisConfigOptions().berlinBlock(0)); - final TransactionPool pool = createTransactionPool(LAYERED); + final TransactionPool pool = createAndEnableTransactionPool(LAYERED, syncState); assertThat(pool.pendingTransactionsImplementation()) .isEqualTo(LayeredPendingTransactions.class); @@ -378,27 +383,33 @@ private void setupScheduleWith(final StubGenesisConfigOptions config) { } private TransactionPool createTransactionPool( - final TransactionPoolConfiguration.Implementation implementation) { - final TransactionPool txPool = - TransactionPoolFactory.createTransactionPool( - schedule, - protocolContext, - ethContext, - TestClock.fixed(), - new NoOpMetricsSystem(), - syncState, - ImmutableTransactionPoolConfiguration.builder() - .txPoolImplementation(implementation) - .txPoolMaxSize(1) - .pendingTxRetentionPeriod(1) - .unstable( - ImmutableTransactionPoolConfiguration.Unstable.builder() - .txMessageKeepAliveSeconds(1) - .build()) - .build(), - new BlobCache(), - MiningParameters.newDefault()); + final TransactionPoolConfiguration.Implementation implementation, final SyncState syncState) { + return TransactionPoolFactory.createTransactionPool( + schedule, + context, + ethContext, + TestClock.fixed(), + new TransactionPoolMetrics(new NoOpMetricsSystem()), + syncState, + ImmutableTransactionPoolConfiguration.builder() + .txPoolImplementation(implementation) + .txPoolMaxSize(1) + .pendingTxRetentionPeriod(1) + .unstable( + ImmutableTransactionPoolConfiguration.Unstable.builder() + .txMessageKeepAliveSeconds(1) + .build()) + .build(), + peerTransactionTracker, + transactionsMessageSender, + newPooledTransactionHashesMessageSender, + new BlobCache(), + MiningParameters.newDefault()); + } + private TransactionPool createAndEnableTransactionPool( + final TransactionPoolConfiguration.Implementation implementation, final SyncState syncState) { + final TransactionPool txPool = createTransactionPool(implementation, syncState); txPool.setEnabled(); return txPool; }