Not all invalid transient not selected txs should be penalized (#8231)

Signed-off-by: Fabio Di Fabio <[email protected]>
hyperledger · Feb 4, 2025 · 90df5e5 · 90df5e5
1 parent e006bbf
commit 90df5e5
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 14 deletions.
diff --git a/...esu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java b/...esu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java
@@ -85,14 +85,15 @@ private TransactionSelectionResult transactionSelectionResultForInvalidResult(
       final ValidationResult<TransactionInvalidReason> invalidReasonValidationResult) {
 
     final TransactionInvalidReason invalidReason = invalidReasonValidationResult.getInvalidReason();
-    // If the invalid reason is transient, then leave the transaction in the pool and continue
+    // If the invalid reason is transient, then penalize but leave the transaction in the pool and
+    // continue
     if (isTransientValidationError(invalidReason)) {
       LOG.atTrace()
-          .setMessage("Transient validation error {} for transaction {} keeping it in the pool")
+          .setMessage("Transient validation error {} for transaction {}, penalize it in the pool")
           .addArgument(invalidReason)
           .addArgument(transaction::toTraceLog)
           .log();
-      return TransactionSelectionResult.invalidTransient(invalidReason.name());
+      return TransactionSelectionResult.invalidPenalized(invalidReason.name());
     }
     // If the transaction was invalid for any other reason, delete it, and continue.
     LOG.atTrace()

diff --git a/...ava/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java b/...ava/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java
@@ -852,7 +852,7 @@ public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() {
         .containsOnly(
             entry(
                 futureTransaction,
-                TransactionSelectionResult.invalidTransient(
+                TransactionSelectionResult.invalidPenalized(
                     TransactionInvalidReason.NONCE_TOO_HIGH.name())));
   }
 
@@ -1035,13 +1035,13 @@ private void internalBlockSelectionTimeoutSimulation(
                 try {
                   Thread.sleep(t);
                 } catch (final InterruptedException e) {
-                  return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name());
+                  return TransactionSelectionResult.invalidPenalized(EXECUTION_INTERRUPTED.name());
                 }
               } else {
                 try {
                   Thread.sleep(fastProcessingTxTime);
                 } catch (final InterruptedException e) {
-                  return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name());
+                  return TransactionSelectionResult.invalidPenalized(EXECUTION_INTERRUPTED.name());
                 }
               }
               return SELECTED;
@@ -1190,13 +1190,13 @@ private void internalBlockSelectionTimeoutSimulationInvalidTxs(
                 try {
                   Thread.sleep(t);
                 } catch (final InterruptedException e) {
-                  return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name());
+                  return TransactionSelectionResult.invalidPenalized(EXECUTION_INTERRUPTED.name());
                 }
               } else {
                 try {
                   Thread.sleep(fastProcessingTxTime);
                 } catch (final InterruptedException e) {
-                  return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name());
+                  return TransactionSelectionResult.invalidPenalized(EXECUTION_INTERRUPTED.name());
                 }
               }
               return invalidSelectionResult;

diff --git a/...rg/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java b/...rg/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java
@@ -537,7 +537,7 @@ public void temporarilyInvalidTransactionIsKeptInPendingTransactions() {
     pendingTransactions.selectTransactions(
         pendingTx -> {
           parsedTransactions.add(pendingTx);
-          return TransactionSelectionResult.invalidTransient(
+          return TransactionSelectionResult.invalidPenalized(
               GAS_PRICE_BELOW_CURRENT_BASE_FEE.name());
         });
 

diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle
@@ -71,7 +71,7 @@ Calculated : ${currentHash}
 tasks.register('checkAPIChanges', FileStateChecker) {
   description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
   files = sourceSets.main.allJava.files
-  knownHash = 'CrhgK6qq7Ym5AARLCQ0lQvZQpsejUgoFcnL1rU+ZGBs='
+  knownHash = 'FEieWer0x6AdCmvf02G7yGQxS5JRxsIYrRDpqsNgQ+0='
 }
 check.dependsOn('checkAPIChanges')
 

diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java
@@ -66,7 +66,8 @@ private enum BaseStatus implements Status {
     BLOCK_SELECTION_TIMEOUT_INVALID_TX(true, true, true),
     TX_EVALUATION_TOO_LONG(true, false, true),
     INVALID_TX_EVALUATION_TOO_LONG(true, true, true),
-    INVALID_TRANSIENT(false, false, true),
+    INVALID_TRANSIENT(false, false, false),
+    INVALID_PENALIZED(false, false, true),
     INVALID(false, true, false);
 
     private final boolean stop;
@@ -160,21 +161,21 @@ public boolean penalize() {
    * price, but the selection should continue.
    */
   public static final TransactionSelectionResult CURRENT_TX_PRICE_BELOW_MIN =
-      TransactionSelectionResult.invalidTransient("CURRENT_TX_PRICE_BELOW_MIN");
+      TransactionSelectionResult.invalidPenalized("CURRENT_TX_PRICE_BELOW_MIN");
 
   /**
    * The transaction has not been selected since its blob price is below the current network blob
    * price, but the selection should continue.
    */
   public static final TransactionSelectionResult BLOB_PRICE_BELOW_CURRENT_MIN =
-      TransactionSelectionResult.invalidTransient("BLOB_PRICE_BELOW_CURRENT_MIN");
+      TransactionSelectionResult.invalidPenalized("BLOB_PRICE_BELOW_CURRENT_MIN");
 
   /**
    * The transaction has not been selected since its priority fee is below the configured min
    * priority fee per gas, but the selection should continue.
    */
   public static final TransactionSelectionResult PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN =
-      TransactionSelectionResult.invalidTransient("PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN");
+      TransactionSelectionResult.invalidPenalized("PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN");
 
   /**
    * The transaction has not been selected since its sender already had a previous transaction not
@@ -217,6 +218,18 @@ public static TransactionSelectionResult invalidTransient(final String invalidRe
     return new TransactionSelectionResult(BaseStatus.INVALID_TRANSIENT, invalidReason);
   }
 
+  /**
+   * Return a selection result that identify the candidate transaction as temporarily invalid and
+   * that it should be penalized, this means that the transaction could become valid at a later
+   * time.
+   *
+   * @param invalidReason the reason why transaction is invalid
+   * @return the selection result
+   */
+  public static TransactionSelectionResult invalidPenalized(final String invalidReason) {
+    return new TransactionSelectionResult(BaseStatus.INVALID_PENALIZED, invalidReason);
+  }
+
   /**
    * Return a selection result that identify the candidate transaction as permanently invalid, this
    * means that it could be removed safely from the transaction pool.