Plugin Api - Add evaluateTransactionPostProcessing to TransactionSelector interface

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

@@ -46,7 +46,6 @@
 import java.util.Optional;
 import java.util.concurrent.CancellationException;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -119,8 +118,7 @@ public BlockTransactionSelector(
             transactionPool);
     transactionSelectors = createTransactionSelectors(blockSelectionContext);
     externalTransactionSelectors =
-        createExternalTransactionSelectors(
-            transactionSelectorFactory.map(List::of).orElseGet(List::of));
+        transactionSelectorFactory.map(TransactionSelectorFactory::create).orElse(List.of());
   }
 
   /**
@@ -304,8 +302,15 @@ private TransactionSelectionResult evaluateTransactionPostProcessing(
       }
     }
 
-    // TODO: External selectors are not used here because TransactionProcessingResult is not
-    //  exposed to the Plugin API yet.
+    // Process the transaction through external selectors
+    for (var selector : externalTransactionSelectors) {
+      TransactionSelectionResult result =
+          selector.evaluateTransactionPostProcessing(pendingTransaction, processingResult);
+      // If the transaction is not selected by any external selector, return the result
+      if (!result.equals(TransactionSelectionResult.SELECTED)) {
+        return result;
+      }
+    }
 
     // If the transaction is selected by all selectors, return SELECTED
     return TransactionSelectionResult.SELECTED;
@@ -319,11 +324,4 @@ private List<AbstractTransactionSelector> createTransactionSelectors(
         new BlobPriceTransactionSelector(context),
         new ProcessingResultTransactionSelector(context));
   }
-
-  private List<TransactionSelector> createExternalTransactionSelectors(
-      final List<TransactionSelectorFactory> transactionSelectorFactory) {
-    return transactionSelectorFactory.stream()
-        .map(TransactionSelectorFactory::create)
-        .collect(Collectors.toList());
-  }
 }

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

@@ -26,6 +26,7 @@
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.datatypes.PendingTransaction;
 import org.hyperledger.besu.datatypes.TransactionType;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.GasLimitCalculator;
@@ -66,6 +67,7 @@
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
 import org.hyperledger.besu.plugin.data.TransactionSelectionResult;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
+import org.hyperledger.besu.plugin.services.txselection.TransactionSelector;
 import org.hyperledger.besu.plugin.services.txselection.TransactionSelectorFactory;
 import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage;
 
@@ -538,7 +540,7 @@ public void shouldDiscardTransactionsThatFailValidation() {
   }
 
   @Test
-  public void transactionSelectionPluginShouldWork() {
+  public void transactionSelectionPluginShouldWork_PreProcessing() {
     final ProcessableBlockHeader blockHeader = createBlock(300_000);
 
     final Transaction selected = createTransaction(0, Wei.of(10), 21_000);
@@ -552,13 +554,26 @@ public void transactionSelectionPluginShouldWork() {
 
     final TransactionSelectorFactory transactionSelectorFactory =
         () ->
-            pendingTx -> {
-              if (pendingTx.getTransaction().equals(notSelectedTransient))
-                return TransactionSelectionResult.invalidTransient("transient");
-              if (pendingTx.getTransaction().equals(notSelectedInvalid))
-                return TransactionSelectionResult.invalid("invalid");
-              return TransactionSelectionResult.SELECTED;
-            };
+            List.of(
+                new TransactionSelector() {
+                  @Override
+                  public TransactionSelectionResult evaluateTransactionPreProcessing(
+                      final PendingTransaction pendingTransaction) {
+                    if (pendingTransaction.getTransaction().equals(notSelectedTransient))
+                      return TransactionSelectionResult.invalidTransient("transient");
+                    if (pendingTransaction.getTransaction().equals(notSelectedInvalid))
+                      return TransactionSelectionResult.invalid("invalid");
+                    return TransactionSelectionResult.SELECTED;
+                  }
+
+                  @Override
+                  public TransactionSelectionResult evaluateTransactionPostProcessing(
+                      final PendingTransaction pendingTransaction,
+                      final org.hyperledger.besu.plugin.data.TransactionProcessingResult
+                          processingResult) {
+                    return TransactionSelectionResult.SELECTED;
+                  }
+                });
 
     final Address miningBeneficiary = AddressHelpers.ofValue(1);
     final BlockTransactionSelector selector =
@@ -586,6 +601,67 @@ public void transactionSelectionPluginShouldWork() {
             entry(notSelectedInvalid, TransactionSelectionResult.invalid("invalid")));
   }
 
+  @Test
+  public void transactionSelectionPluginShouldWork_PostProcessing() {
+    final ProcessableBlockHeader blockHeader = createBlock(300_000);
+
+    long maxGasUsedByTransaction = 21_000;
+
+    final Transaction selected = createTransaction(0, Wei.of(10), 21_000);
+    ensureTransactionIsValid(selected, maxGasUsedByTransaction, 0);
+
+    // Add + 1 to gasUsedByTransaction so it will fail in the post processing selection
+    final Transaction notSelected = createTransaction(1, Wei.of(10), 30_000);
+    ensureTransactionIsValid(notSelected, maxGasUsedByTransaction + 1, 0);
+
+    final Transaction selected3 = createTransaction(3, Wei.of(10), 21_000);
+    ensureTransactionIsValid(selected3, maxGasUsedByTransaction, 0);
+
+    final TransactionSelectorFactory transactionSelectorFactory =
+        () ->
+            List.of(
+                new TransactionSelector() {
+                  @Override
+                  public TransactionSelectionResult evaluateTransactionPreProcessing(
+                      final PendingTransaction pendingTransaction) {
+                    return TransactionSelectionResult.SELECTED;
+                  }
+
+                  @Override
+                  public TransactionSelectionResult evaluateTransactionPostProcessing(
+                      final PendingTransaction pendingTransaction,
+                      final org.hyperledger.besu.plugin.data.TransactionProcessingResult
+                          processingResult) {
+                    // the transaction with max gas +1 should fail
+                    if (processingResult.getEstimateGasUsedByTransaction()
+                        > maxGasUsedByTransaction) {
+                      return TransactionSelectionResult.invalidTransient("Invalid");
+                    }
+                    return TransactionSelectionResult.SELECTED;
+                  }
+                });
+
+    final Address miningBeneficiary = AddressHelpers.ofValue(1);
+    final BlockTransactionSelector selector =
+        createBlockSelectorWithTxSelPlugin(
+            transactionProcessor,
+            blockHeader,
+            Wei.ZERO,
+            miningBeneficiary,
+            Wei.ZERO,
+            MIN_OCCUPANCY_80_PERCENT,
+            transactionSelectorFactory);
+
+    transactionPool.addRemoteTransactions(List.of(selected, notSelected, selected3));
+
+    final TransactionSelectionResults transactionSelectionResults =
+        selector.buildTransactionListForBlock();
+
+    assertThat(transactionSelectionResults.getSelectedTransactions()).contains(selected, selected3);
+    assertThat(transactionSelectionResults.getNotSelectedTransactions())
+        .containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid")));
+  }
+
   @Test
   public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() {
     final ProcessableBlockHeader blockHeader = createBlock(5_000_000);

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/processing/TransactionProcessingResult.java

@@ -24,7 +24,8 @@
 
 import org.apache.tuweni.bytes.Bytes;
 
-public class TransactionProcessingResult {
+public class TransactionProcessingResult
+    implements org.hyperledger.besu.plugin.data.TransactionProcessingResult {
 
   /** The status of the transaction after being processed. */
   public enum Status {
@@ -113,6 +114,7 @@ public TransactionProcessingResult(
    *
    * @return the logs produced by the transaction
    */
+  @Override
   public List<Log> getLogs() {
     return logs;
   }
@@ -124,6 +126,7 @@ public List<Log> getLogs() {
    *
    * @return the gas remaining after the transaction was processed
    */
+  @Override
   public long getGasRemaining() {
     return gasRemaining;
   }
@@ -134,6 +137,7 @@ public long getGasRemaining() {
    *
    * @return the estimate gas used
    */
+  @Override
   public long getEstimateGasUsedByTransaction() {
     return estimateGasUsedByTransaction;
   }
@@ -147,28 +151,41 @@ public Status getStatus() {
     return status;
   }
 
+  @Override
   public Bytes getOutput() {
     return output;
   }
 
   /**
-   * Returns whether or not the transaction was invalid.
+   * Returns whether the transaction was invalid.
    *
    * @return {@code true} if the transaction was invalid; otherwise {@code false}
    */
+  @Override
   public boolean isInvalid() {
     return getStatus() == Status.INVALID;
   }
 
   /**
-   * Returns whether or not the transaction was successfully processed.
+   * Returns whether the transaction was successfully processed.
    *
    * @return {@code true} if the transaction was successfully processed; otherwise {@code false}
    */
+  @Override
   public boolean isSuccessful() {
     return getStatus() == Status.SUCCESSFUL;
   }
 
+  /**
+   * Returns whether the transaction failed.
+   *
+   * @return {@code true} if the transaction failed; otherwise {@code false}
+   */
+  @Override
+  public boolean isFailed() {
+    return getStatus() == Status.FAILED;
+  }
+
   /**
    * Returns the transaction validation result.
    *
@@ -183,6 +200,7 @@ public ValidationResult<TransactionInvalidReason> getValidationResult() {
    *
    * @return the revert reason.
    */
+  @Override
   public Optional<Bytes> getRevertReason() {
     return revertReason;
   }

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 = 'gfZY0boUMYJoAHwou3eEhcz7A/xFvJKnjMUONZ6hY3I='
+  knownHash = 'YmuzR1UGwriMV80R3IrIkXgs91gyrrSwJ/uwynRZo18='
 }
 check.dependsOn('checkAPIChanges')
 

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

@@ -0,0 +1,86 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.plugin.data;
+
+import org.hyperledger.besu.evm.log.Log;
+
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.tuweni.bytes.Bytes;
+
+public interface TransactionProcessingResult {
+
+  /**
+   * Return the logs produced by the transaction.
+   *
+   * <p>This is only valid when {@code TransactionProcessor#isSuccessful} returns {@code true}.
+   *
+   * @return the logs produced by the transaction
+   */
+  List<Log> getLogs();
+
+  /**
+   * Returns the gas remaining after the transaction was processed.
+   *
+   * <p>This is only valid when {@code TransactionProcessor#isSuccessful} returns {@code true}.
+   *
+   * @return the gas remaining after the transaction was processed
+   */
+  long getGasRemaining();
+
+  /**
+   * Returns the estimate gas used by the transaction Difference between the gas limit and the
+   * remaining gas
+   *
+   * @return the estimate gas used
+   */
+  long getEstimateGasUsedByTransaction();
+
+  /**
+   * Returns the output.
+   *
+   * @return the output.
+   */
+  Bytes getOutput();
+
+  /**
+   * Returns whether the transaction was invalid.
+   *
+   * @return {@code true} if the transaction was invalid; otherwise {@code false}
+   */
+  boolean isInvalid();
+
+  /**
+   * Returns whether the transaction was successfully processed.
+   *
+   * @return {@code true} if the transaction was successfully processed; otherwise {@code false}
+   */
+  boolean isSuccessful();
+
+  /**
+   * Returns whether the transaction failed.
+   *
+   * @return {@code true} if the transaction failed; otherwise {@code false}
+   */
+  boolean isFailed();
+
+  /**
+   * Returns the reason why a transaction was reverted (if applicable).
+   *
+   * @return the revert reason.
+   */
+  Optional<Bytes> getRevertReason();
+}

plugin-api/src/main/java/org/hyperledger/besu/plugin/services/txselection/TransactionSelector.java

@@ -17,6 +17,7 @@
 
 import org.hyperledger.besu.datatypes.PendingTransaction;
 import org.hyperledger.besu.plugin.Unstable;
+import org.hyperledger.besu.plugin.data.TransactionProcessingResult;
 import org.hyperledger.besu.plugin.data.TransactionSelectionResult;
 
 /** Interface for the transaction selector */
@@ -31,4 +32,15 @@ public interface TransactionSelector {
    */
   TransactionSelectionResult evaluateTransactionPreProcessing(
       PendingTransaction pendingTransaction);
+
+  /**
+   * Method called to decide whether a processed transaction is added to a block. The result can
+   * also indicate that no further transactions can be added to the block.
+   *
+   * @param pendingTransaction candidate transaction
+   * @param processingResult the transaction processing result
+   * @return TransactionSelectionResult that indicates whether to include the transaction
+   */
+  TransactionSelectionResult evaluateTransactionPostProcessing(
+      PendingTransaction pendingTransaction, TransactionProcessingResult processingResult);
 }