Remove LowestInvalidNonceCache (hyperledger#6148)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Justin Florentine <justin+github@florentine.us>

CHANGELOG.md

@@ -30,6 +30,7 @@
 - Add `yParity` to GraphQL and JSON-RPC for relevant querise. [6119](https://github.com/hyperledger/besu/pull/6119)
 - Force tx replacement price bump to zero when zero base fee market is configured or `--min-gas-price` is set to 0. This allows for easier tx replacement in networks where there is not gas price. [#6079](https://github.com/hyperledger/besu/pull/6079)
 - Introduce the possibility to limit the time spent selecting pending transactions during block creation, using the new experimental option `Xblock-txs-selection-max-time` on PoS and PoW networks (by default set to 5000ms) or `Xpoa-block-txs-selection-max-time` on PoA networks (by default 75% of the min block time) [#6044](https://github.com/hyperledger/besu/pull/6044)
+- Remove LowestInvalidNonceCache from `legacy` transaction pool to make it more private networks friendly [#6148](https://github.com/hyperledger/besu/pull/6148)
 
 ### Bug fixes
 - Upgrade netty to address CVE-2023-44487, CVE-2023-34462 [#6100](https://github.com/hyperledger/besu/pull/6100)

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java

@@ -71,8 +71,6 @@ public static RpcErrorType convertTransactionInvalidReason(
         return RpcErrorType.ETH_SEND_TX_REPLACEMENT_UNDERPRICED;
       case NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER:
         return RpcErrorType.NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER;
-      case LOWER_NONCE_INVALID_TRANSACTION_EXISTS:
-        return RpcErrorType.LOWER_NONCE_INVALID_TRANSACTION_EXISTS;
       case TOTAL_BLOB_GAS_TOO_HIGH:
         return RpcErrorType.TOTAL_BLOB_GAS_TOO_HIGH;
       case TX_POOL_DISABLED:

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java

@@ -39,7 +39,6 @@ public enum TransactionInvalidReason {
   MAX_PRIORITY_FEE_PER_GAS_EXCEEDS_MAX_FEE_PER_GAS,
   INITCODE_TOO_LARGE,
   NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER,
-  LOWER_NONCE_INVALID_TRANSACTION_EXISTS,
   TOTAL_BLOB_GAS_TOO_HIGH,
   GAS_PRICE_TOO_LOW,
   GAS_PRICE_BELOW_CURRENT_BASE_FEE,

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java

@@ -72,17 +72,6 @@ void manageBlockAdded(
 
   String logStats();
 
-  default List<Transaction> signalInvalidAndGetDependentTransactions(
-      final Transaction transaction) {
-    // ToDo: remove when the legacy tx pool is removed
-    return List.of();
-  }
-
-  default void signalInvalidAndRemoveDependentTransactions(final Transaction transaction) {
-    // ToDo: remove when the legacy tx pool is removed
-    // no-op
-  }
-
   @FunctionalInterface
   interface TransactionSelector {
     TransactionSelectionResult evaluateTransaction(PendingTransaction pendingTransaction);

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionAddedResult.java

@@ -35,8 +35,6 @@ private enum Status {
       new TransactionAddedResult(TransactionInvalidReason.TRANSACTION_REPLACEMENT_UNDERPRICED);
   public static final TransactionAddedResult NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER =
       new TransactionAddedResult(TransactionInvalidReason.NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER);
-  public static final TransactionAddedResult LOWER_NONCE_INVALID_TRANSACTION_KNOWN =
-      new TransactionAddedResult(TransactionInvalidReason.LOWER_NONCE_INVALID_TRANSACTION_EXISTS);
 
   public static final TransactionAddedResult ADDED = new TransactionAddedResult(Status.ADDED);
   public static final TransactionAddedResult TRY_NEXT_LAYER =

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java

@@ -55,7 +55,6 @@
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.IntSummaryStatistics;
@@ -94,8 +93,6 @@
 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 static final List<TransactionInvalidReason> INVALID_TX_CACHE_IGNORED_ERRORS =
-      new ArrayList<>(Arrays.asList(TransactionInvalidReason.NONCE_TOO_LOW));
   private final Supplier<PendingTransactions> pendingTransactionsSupplier;
   private final PluginTransactionValidator pluginTransactionValidator;
   private volatile PendingTransactions pendingTransactions;
@@ -280,11 +277,6 @@ private ValidationResult<TransactionInvalidReason> addTransaction(
           .log();
       metrics.incrementRejected(
           isLocal, hasPriority, validationResult.result.getInvalidReason(), "txpool");
-      if (!isLocal
-          && !INVALID_TX_CACHE_IGNORED_ERRORS.contains(
-              validationResult.result.getInvalidReason())) {
-        pendingTransactions.signalInvalidAndRemoveDependentTransactions(transaction);
-      }
     }
 
     return validationResult.result;

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java

@@ -16,7 +16,6 @@
 
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.ADDED;
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.ALREADY_KNOWN;
-import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.LOWER_NONCE_INVALID_TRANSACTION_KNOWN;
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER;
 import static org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult.REJECTED_UNDERPRICED_REPLACEMENT;
 
@@ -69,7 +68,6 @@
  * <p>This class is safe for use across multiple threads.
  */
 public abstract class AbstractPendingTransactionsSorter implements PendingTransactions {
-  private static final int DEFAULT_LOWEST_INVALID_KNOWN_NONCE_CACHE = 10_000;
   private static final Logger LOG =
       LoggerFactory.getLogger(AbstractPendingTransactionsSorter.class);
 
@@ -82,8 +80,6 @@ public abstract class AbstractPendingTransactionsSorter implements PendingTransa
   protected final Map<Address, PendingTransactionsForSender> transactionsBySender =
       new ConcurrentHashMap<>();
 
-  protected final LowestInvalidNonceCache lowestInvalidKnownNonceCache =
-      new LowestInvalidNonceCache(DEFAULT_LOWEST_INVALID_KNOWN_NONCE_CACHE);
   protected final Subscribers<PendingTransactionAddedListener> pendingTransactionSubscribers =
       Subscribers.create();
 
@@ -136,7 +132,6 @@ public AbstractPendingTransactionsSorter(
   public void reset() {
     pendingTransactions.clear();
     transactionsBySender.clear();
-    lowestInvalidKnownNonceCache.reset();
   }
 
   @Override
@@ -179,21 +174,10 @@ public List<Transaction> getPriorityTransactions() {
   public TransactionAddedResult addTransaction(
       final PendingTransaction transaction, final Optional<Account> maybeSenderAccount) {
 
-    if (!transaction.isReceivedFromLocalSource()
-        && lowestInvalidKnownNonceCache.hasInvalidLowerNonce(transaction.getTransaction())) {
-      LOG.atDebug()
-          .setMessage(
-              "Dropping transaction {} since the sender has an invalid transaction with lower nonce")
-          .addArgument(transaction::toTraceLog)
-          .log();
-      return LOWER_NONCE_INVALID_TRANSACTION_KNOWN;
-    }
-
     final TransactionAddedResult transactionAddedStatus =
         internalAddTransaction(transaction, maybeSenderAccount);
     if (transactionAddedStatus.equals(ADDED)) {
       if (!transaction.isReceivedFromLocalSource()) {
-        lowestInvalidKnownNonceCache.registerValidTransaction(transaction.getTransaction());
         remoteTransactionAddedCounter.inc();
       } else {
         localTransactionAddedCounter.inc();
@@ -223,7 +207,6 @@ public void manageBlockAdded(
 
   public void transactionAddedToBlock(final Transaction transaction) {
     removeTransaction(transaction, true);
-    lowestInvalidKnownNonceCache.registerValidTransaction(transaction);
   }
 
   private void incrementTransactionRemovedCounter(
@@ -259,8 +242,6 @@ public void selectTransactions(final TransactionSelector selector) {
 
           if (result.discard()) {
             transactionsToRemove.add(transactionToProcess.getTransaction());
-            transactionsToRemove.addAll(
-                signalInvalidAndGetDependentTransactions(transactionToProcess.getTransaction()));
           }
 
           if (result.stop()) {
@@ -504,34 +485,4 @@ public String toTraceLog() {
       return sb.toString();
     }
   }
-
-  @Override
-  public List<Transaction> signalInvalidAndGetDependentTransactions(final Transaction transaction) {
-    final long invalidNonce = lowestInvalidKnownNonceCache.registerInvalidTransaction(transaction);
-
-    PendingTransactionsForSender txsForSender = transactionsBySender.get(transaction.getSender());
-    if (txsForSender != null) {
-      return txsForSender
-          .streamPendingTransactions()
-          .filter(pendingTx -> pendingTx.getTransaction().getNonce() > invalidNonce)
-          .peek(
-              pendingTx ->
-                  LOG.atTrace()
-                      .setMessage(
-                          "Transaction {} invalid since there is a lower invalid nonce {} for the sender")
-                      .addArgument(pendingTx::toTraceLog)
-                      .addArgument(invalidNonce)
-                      .log())
-          .map(PendingTransaction::getTransaction)
-          .collect(Collectors.toList());
-    }
-    return List.of();
-  }
-
-  @Override
-  public void signalInvalidAndRemoveDependentTransactions(final Transaction transaction) {
-    synchronized (lock) {
-      signalInvalidAndGetDependentTransactions(transaction).forEach(this::removeTransaction);
-    }
-  }
 }