Allow a plugin to specify custom transaction selection results

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/TransactionSelectionResults.java

@@ -111,7 +111,7 @@ public void logSelectionStats() {
           "Selection stats: Totals[Evaluated={}, Selected={}, NotSelected={}, Discarded={}]; Detailed[{}]",
           selectedTransactions.size() + notSelectedTxs.size(),
           selectedTransactions.size(),
-          notSelectedStats.size(),
+          notSelectedTxs.size(),
           notSelectedStats.entrySet().stream()
               .filter(e -> e.getKey().discard())
               .map(Map.Entry::getValue)

ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java

@@ -89,8 +89,9 @@ private boolean transactionCurrentPriceBelowMin(final PendingTransaction pending
             .setMessage(
                 "Current gas price of {} is {} and lower than the configured minimum {}, skipping")
             .addArgument(pendingTransaction::toTraceLog)
-            .addArgument(transactionGasPriceInBlock)
-            .addArgument(context.miningParameters()::getMinTransactionGasPrice)
+            .addArgument(transactionGasPriceInBlock::toHumanReadableString)
+            .addArgument(
+                context.miningParameters().getMinTransactionGasPrice()::toHumanReadableString)
             .log();
         return true;
       }

ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java

@@ -583,9 +583,9 @@ public void transactionSelectionPluginShouldWork_PreProcessing() {
               public TransactionSelectionResult evaluateTransactionPreProcessing(
                   final PendingTransaction pendingTransaction) {
                 if (pendingTransaction.getTransaction().equals(notSelectedTransient))
-                  return TransactionSelectionResult.invalidTransient("transient");
+                  return PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT;
                 if (pendingTransaction.getTransaction().equals(notSelectedInvalid))
-                  return TransactionSelectionResult.invalid("invalid");
+                  return PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID;
                 return SELECTED;
               }
 
@@ -618,8 +618,10 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
     assertThat(transactionSelectionResults.getSelectedTransactions()).containsOnly(selected);
     assertThat(transactionSelectionResults.getNotSelectedTransactions())
         .containsOnly(
-            entry(notSelectedTransient, TransactionSelectionResult.invalidTransient("transient")),
-            entry(notSelectedInvalid, TransactionSelectionResult.invalid("invalid")));
+            entry(
+                notSelectedTransient,
+                PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT),
+            entry(notSelectedInvalid, PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID));
   }
 
   @Test
@@ -654,7 +656,7 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
                       processingResult) {
                 // the transaction with max gas +1 should fail
                 if (processingResult.getEstimateGasUsedByTransaction() > maxGasUsedByTransaction) {
-                  return TransactionSelectionResult.invalidTransient("Invalid");
+                  return PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT;
                 }
                 return SELECTED;
               }
@@ -678,7 +680,8 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
 
     assertThat(transactionSelectionResults.getSelectedTransactions()).contains(selected, selected3);
     assertThat(transactionSelectionResults.getNotSelectedTransactions())
-        .containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid")));
+        .containsOnly(
+            entry(notSelected, PluginTrasactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT));
   }
 
   @Test
@@ -1189,4 +1192,48 @@ protected MiningParameters createMiningParameters(
                 .build())
         .build();
   }
+
+  private static class PluginTrasactionSelectionResult extends TransactionSelectionResult {
+    private enum PluginStatus implements Status {
+      PLUGIN_INVALID(false, true),
+      PLUGIN_INVALID_TRANSIENT(false, false);
+
+      private final boolean stop;
+      private final boolean discard;
+
+      PluginStatus(final boolean stop, final boolean discard) {
+        this.stop = stop;
+        this.discard = discard;
+      }
+
+      @Override
+      public boolean stop() {
+        return stop;
+      }
+
+      @Override
+      public boolean discard() {
+        return discard;
+      }
+    }
+
+    public static final TransactionSelectionResult GENERIC_PLUGIN_INVALID_TRANSIENT =
+        invalidTransient("GENERIC_PLUGIN_INVALID_TRANSIENT");
+
+    public static final TransactionSelectionResult GENERIC_PLUGIN_INVALID =
+        invalid("GENERIC_PLUGIN_INVALID");
+
+    private PluginTrasactionSelectionResult(final Status status, final String invalidReason) {
+      super(status, invalidReason);
+    }
+
+    public static TransactionSelectionResult invalidTransient(final String invalidReason) {
+      return new PluginTrasactionSelectionResult(
+          PluginStatus.PLUGIN_INVALID_TRANSIENT, invalidReason);
+    }
+
+    public static TransactionSelectionResult invalid(final String invalidReason) {
+      return new PluginTrasactionSelectionResult(PluginStatus.PLUGIN_INVALID, invalidReason);
+    }
+  }
 }

plugin-api/build.gradle

@@ -69,7 +69,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 = 'gKRXd2Ow7wYKSgeGrDMRj0+2LdCzjOhLx8UEno9btGw='
+  knownHash = 'nB1LhUpMWYFQpBdNJ/3Q79c8kLgUgPmEFzlRMlLUl1Y='
 }
 check.dependsOn('checkAPIChanges')
 

plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java

@@ -24,7 +24,34 @@
  */
 public class TransactionSelectionResult {
 
-  private enum Status {
+  /**
+   * Represent the status of a transaction selection result. Plugin can extend this to implement its
+   * own statuses.
+   */
+  protected interface Status {
+    /**
+     * Should the selection process be stopped?
+     *
+     * @return true if the selection process needs to be stopped
+     */
+    boolean stop();
+
+    /**
+     * Should the current transaction be removed from the txpool?
+     *
+     * @return yes if the transaction should be removed from the txpool
+     */
+    boolean discard();
+
+    /**
+     * Name of this status
+     *
+     * @return the name
+     */
+    String name();
+  }
+
+  private enum BaseStatus implements Status {
     SELECTED,
     BLOCK_FULL(true, false),
     BLOCK_OCCUPANCY_ABOVE_THRESHOLD(true, false),
@@ -36,12 +63,12 @@ private enum Status {
     private final boolean stop;
     private final boolean discard;
 
-    Status() {
+    BaseStatus() {
       this.stop = false;
       this.discard = false;
     }
 
-    Status(final boolean stop, final boolean discard) {
+    BaseStatus(final boolean stop, final boolean discard) {
       this.stop = stop;
       this.discard = discard;
     }
@@ -50,26 +77,36 @@ private enum Status {
     public String toString() {
       return name() + " (stop=" + stop + ", discard=" + discard + ")";
     }
+
+    @Override
+    public boolean stop() {
+      return stop;
+    }
+
+    @Override
+    public boolean discard() {
+      return discard;
+    }
   }
 
   /** The transaction has been selected to be included in the new block */
   public static final TransactionSelectionResult SELECTED =
-      new TransactionSelectionResult(Status.SELECTED);
+      new TransactionSelectionResult(BaseStatus.SELECTED);
   /** The transaction has not been selected since the block is full. */
   public static final TransactionSelectionResult BLOCK_FULL =
-      new TransactionSelectionResult(Status.BLOCK_FULL);
+      new TransactionSelectionResult(BaseStatus.BLOCK_FULL);
   /** There was no more time to add transaction to the block */
   public static final TransactionSelectionResult BLOCK_SELECTION_TIMEOUT =
-      new TransactionSelectionResult(Status.BLOCK_SELECTION_TIMEOUT);
+      new TransactionSelectionResult(BaseStatus.BLOCK_SELECTION_TIMEOUT);
   /** Transaction took too much to evaluate */
   public static final TransactionSelectionResult TX_EVALUATION_TOO_LONG =
-      new TransactionSelectionResult(Status.TX_EVALUATION_TOO_LONG);
+      new TransactionSelectionResult(BaseStatus.TX_EVALUATION_TOO_LONG);
   /**
    * The transaction has not been selected since too large and the occupancy of the block is enough
    * to stop the selection.
    */
   public static final TransactionSelectionResult BLOCK_OCCUPANCY_ABOVE_THRESHOLD =
-      new TransactionSelectionResult(Status.BLOCK_OCCUPANCY_ABOVE_THRESHOLD);
+      new TransactionSelectionResult(BaseStatus.BLOCK_OCCUPANCY_ABOVE_THRESHOLD);
   /**
    * The transaction has not been selected since its gas limit is greater than the block remaining
    * gas, but the selection should continue.
@@ -99,11 +136,22 @@ public String toString() {
   private final Status status;
   private final Optional<String> maybeInvalidReason;
 
-  private TransactionSelectionResult(final Status status) {
+  /**
+   * Create a new transaction selection result with the passed status
+   *
+   * @param status the selection result status
+   */
+  protected TransactionSelectionResult(final Status status) {
     this(status, null);
   }
 
-  private TransactionSelectionResult(final Status status, final String invalidReason) {
+  /**
+   * Create a new transaction selection result with the passed status and invalid reason
+   *
+   * @param status the selection result status
+   * @param invalidReason string with a custom invalid reason
+   */
+  protected TransactionSelectionResult(final Status status, final String invalidReason) {
     this.status = status;
     this.maybeInvalidReason = Optional.ofNullable(invalidReason);
   }
@@ -116,7 +164,7 @@ private TransactionSelectionResult(final Status status, final String invalidReas
    * @return the selection result
    */
   public static TransactionSelectionResult invalidTransient(final String invalidReason) {
-    return new TransactionSelectionResult(Status.INVALID_TRANSIENT, invalidReason);
+    return new TransactionSelectionResult(BaseStatus.INVALID_TRANSIENT, invalidReason);
   }
 
   /**
@@ -127,7 +175,7 @@ public static TransactionSelectionResult invalidTransient(final String invalidRe
    * @return the selection result
    */
   public static TransactionSelectionResult invalid(final String invalidReason) {
-    return new TransactionSelectionResult(Status.INVALID, invalidReason);
+    return new TransactionSelectionResult(BaseStatus.INVALID, invalidReason);
   }
 
   /**
@@ -136,7 +184,7 @@ public static TransactionSelectionResult invalid(final String invalidReason) {
    * @return true if the selection process should stop, false otherwise
    */
   public boolean stop() {
-    return status.stop;
+    return status.stop();
   }
 
   /**
@@ -146,7 +194,7 @@ public boolean stop() {
    *     otherwise
    */
   public boolean discard() {
-    return status.discard;
+    return status.discard();
   }
 
   /**
@@ -155,7 +203,7 @@ public boolean discard() {
    * @return true if the candidate transaction is included in the new block, false otherwise
    */
   public boolean selected() {
-    return Status.SELECTED.equals(status);
+    return BaseStatus.SELECTED.equals(status);
   }
 
   /**