Merge remote-tracking branch 'upstream/main' into zkbesu

.gitignore

@@ -9,6 +9,7 @@
 .DS_Store
 .externalToolBuilders/
 .gradle/
+.vscode/
 .idea/
 .loadpath
 .metadata

CHANGELOG.md

@@ -10,6 +10,7 @@
 - Implement debug_traceCall [#5885](https://github.com/hyperledger/besu/pull/5885)
 - Transactions that takes too long to evaluate, during block creation, are dropped from the txpool [#6163](https://github.com/hyperledger/besu/pull/6163)
 - New option `tx-pool-min-gas-price` to set a lower bound when accepting txs to the pool [#6098](https://github.com/hyperledger/besu/pull/6098)
+-  Allow a transaction selection plugin to specify custom selection results [#6190](https://github.com/hyperledger/besu/pull/6190)
 
 ## 23.10.2
 

besu/build.gradle

@@ -90,7 +90,7 @@ dependencies {
   testImplementation 'com.squareup.okhttp3:okhttp'
   testImplementation 'commons-io:commons-io'
   testImplementation 'io.opentelemetry:opentelemetry-api'
-  testImplementation 'junit:junit'
+  testImplementation 'org.mockito:mockito-junit-jupiter'
   testImplementation 'org.apache.commons:commons-text'
   testImplementation 'io.tmio:tuweni-bytes'
   testImplementation 'io.tmio:tuweni-units'

besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java

@@ -41,23 +41,21 @@
 import java.util.concurrent.CompletionException;
 
 import org.apache.tuweni.units.bigints.UInt256;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 /** Tests for {@link RlpBlockImporter}. */
-@RunWith(MockitoJUnitRunner.class)
+@ExtendWith(MockitoExtension.class)
 public final class RlpBlockImporterTest {
 
-  @Rule public final TemporaryFolder folder = new TemporaryFolder();
+  @TempDir Path dataDir;
 
   private final RlpBlockImporter rlpBlockImporter = new RlpBlockImporter();
 
   @Test
   public void blockImport() throws IOException {
-    final Path dataDir = folder.newFolder().toPath();
     final Path source = dataDir.resolve("1000.blocks");
     BlockTestUtil.write1000Blocks(source);
     final BesuController targetController =
@@ -90,7 +88,6 @@ public void blockImportRejectsBadPow() throws IOException {
     // set merge flag to false, otherwise this test can fail if a merge test runs first
     MergeConfigOptions.setMergeEnabled(false);
 
-    final Path dataDir = folder.newFolder().toPath();
     final Path source = dataDir.resolve("badpow.blocks");
     BlockTestUtil.writeBadPowBlocks(source);
     final BesuController targetController =
@@ -120,7 +117,6 @@ public void blockImportRejectsBadPow() throws IOException {
 
   @Test
   public void blockImportCanSkipPow() throws IOException {
-    final Path dataDir = folder.newFolder().toPath();
     final Path source = dataDir.resolve("badpow.blocks");
     BlockTestUtil.writeBadPowBlocks(source);
     final BesuController targetController =

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

@@ -57,6 +57,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 
+import com.google.common.base.Stopwatch;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -227,11 +228,11 @@ private TransactionSelectionResult evaluateTransaction(
       final PendingTransaction pendingTransaction) {
     checkCancellation();
 
-    final long evaluationStartedAt = System.currentTimeMillis();
+    final Stopwatch evaluationTimer = Stopwatch.createStarted();
 
     TransactionSelectionResult selectionResult = evaluatePreProcessing(pendingTransaction);
     if (!selectionResult.selected()) {
-      return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationStartedAt);
+      return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationTimer);
     }
 
     final WorldUpdater txWorldStateUpdater = blockWorldStateUpdater.updater();
@@ -243,13 +244,10 @@ private TransactionSelectionResult evaluateTransaction(
 
     if (postProcessingSelectionResult.selected()) {
       return handleTransactionSelected(
-          pendingTransaction, processingResult, txWorldStateUpdater, evaluationStartedAt);
+          pendingTransaction, processingResult, txWorldStateUpdater, evaluationTimer);
     }
     return handleTransactionNotSelected(
-        pendingTransaction,
-        postProcessingSelectionResult,
-        txWorldStateUpdater,
-        evaluationStartedAt);
+        pendingTransaction, postProcessingSelectionResult, txWorldStateUpdater, evaluationTimer);
   }
 
   /**
@@ -333,14 +331,14 @@ private TransactionProcessingResult processTransaction(
    * @param pendingTransaction The pending transaction.
    * @param processingResult The result of the transaction processing.
    * @param txWorldStateUpdater The world state updater.
-   * @param evaluationStartedAt when the evaluation of this tx started
+   * @param evaluationTimer tracks the evaluation elapsed time
    * @return The result of the transaction selection process.
    */
   private TransactionSelectionResult handleTransactionSelected(
       final PendingTransaction pendingTransaction,
       final TransactionProcessingResult processingResult,
       final WorldUpdater txWorldStateUpdater,
-      final long evaluationStartedAt) {
+      final Stopwatch evaluationTimer) {
     final Transaction transaction = pendingTransaction.getTransaction();
 
     final long gasUsedByTransaction =
@@ -369,44 +367,43 @@ private TransactionSelectionResult handleTransactionSelected(
       }
     }
 
-    final long evaluationTime = System.currentTimeMillis() - evaluationStartedAt;
     if (tooLate) {
       // even if this tx passed all the checks, it is too late to include it in this block,
       // so we need to treat it as not selected
 
       // check if this tx took too much to evaluate, and in case remove it from the pool
       final TransactionSelectionResult timeoutSelectionResult;
-      if (evaluationTime > blockTxsSelectionMaxTime) {
+      if (evaluationTimer.elapsed(TimeUnit.MILLISECONDS) > blockTxsSelectionMaxTime) {
         LOG.atWarn()
             .setMessage(
-                "Transaction {} is too late for inclusion, evaluated in {}ms that is over the max limit of {}"
+                "Transaction {} is too late for inclusion, evaluated in {} that is over the max limit of {}ms"
                     + ", removing it from the pool")
             .addArgument(transaction::toTraceLog)
-            .addArgument(evaluationTime)
+            .addArgument(evaluationTimer)
             .addArgument(blockTxsSelectionMaxTime)
             .log();
         timeoutSelectionResult = TX_EVALUATION_TOO_LONG;
       } else {
         LOG.atTrace()
             .setMessage("Transaction {} is too late for inclusion")
             .addArgument(transaction::toTraceLog)
-            .addArgument(evaluationTime)
+            .addArgument(evaluationTimer)
             .log();
         timeoutSelectionResult = BLOCK_SELECTION_TIMEOUT;
       }
 
       // do not rely on the presence of this result, since by the time it is added, the code
       // reading it could have been already executed by another thread
       return handleTransactionNotSelected(
-          pendingTransaction, timeoutSelectionResult, txWorldStateUpdater, evaluationStartedAt);
+          pendingTransaction, timeoutSelectionResult, txWorldStateUpdater, evaluationTimer);
     }
 
     pluginTransactionSelector.onTransactionSelected(pendingTransaction, processingResult);
     blockWorldStateUpdater = worldState.updater();
     LOG.atTrace()
-        .setMessage("Selected {} for block creation, evaluated in {}ms")
+        .setMessage("Selected {} for block creation, evaluated in {}")
         .addArgument(transaction::toTraceLog)
-        .addArgument(evaluationTime)
+        .addArgument(evaluationTimer)
         .log();
     return SELECTED;
   }
@@ -418,22 +415,22 @@ private TransactionSelectionResult handleTransactionSelected(
    *
    * @param pendingTransaction The unselected pending transaction.
    * @param selectionResult The result of the transaction selection process.
-   * @param evaluationStartedAt when the evaluation of this tx started
+   * @param evaluationTimer tracks the evaluation elapsed time
    * @return The result of the transaction selection process.
    */
   private TransactionSelectionResult handleTransactionNotSelected(
       final PendingTransaction pendingTransaction,
       final TransactionSelectionResult selectionResult,
-      final long evaluationStartedAt) {
+      final Stopwatch evaluationTimer) {
 
     transactionSelectionResults.updateNotSelected(
         pendingTransaction.getTransaction(), selectionResult);
     pluginTransactionSelector.onTransactionNotSelected(pendingTransaction, selectionResult);
     LOG.atTrace()
-        .setMessage("Not selected {} for block creation with result {}, evaluated in {}ms")
+        .setMessage("Not selected {} for block creation with result {}, evaluated in {}")
         .addArgument(pendingTransaction::toTraceLog)
         .addArgument(selectionResult)
-        .addArgument(() -> System.currentTimeMillis() - evaluationStartedAt)
+        .addArgument(evaluationTimer)
         .log();
 
     return selectionResult;
@@ -443,9 +440,9 @@ private TransactionSelectionResult handleTransactionNotSelected(
       final PendingTransaction pendingTransaction,
       final TransactionSelectionResult selectionResult,
       final WorldUpdater txWorldStateUpdater,
-      final long evaluationStartedAt) {
+      final Stopwatch evaluationTimer) {
     txWorldStateUpdater.revert();
-    return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationStartedAt);
+    return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationTimer);
   }
 
   private void checkCancellation() {

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

@@ -587,9 +587,9 @@ public void transactionSelectionPluginShouldWork_PreProcessing() {
               public TransactionSelectionResult evaluateTransactionPreProcessing(
                   final PendingTransaction pendingTransaction) {
                 if (pendingTransaction.getTransaction().equals(notSelectedTransient))
-                  return TransactionSelectionResult.invalidTransient("transient");
+                  return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT;
                 if (pendingTransaction.getTransaction().equals(notSelectedInvalid))
-                  return TransactionSelectionResult.invalid("invalid");
+                  return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID;
                 return SELECTED;
               }
 
@@ -622,8 +622,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,
+                PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT),
+            entry(notSelectedInvalid, PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID));
   }
 
   @Test
@@ -658,7 +660,7 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
                       processingResult) {
                 // the transaction with max gas +1 should fail
                 if (processingResult.getEstimateGasUsedByTransaction() > maxGasUsedByTransaction) {
-                  return TransactionSelectionResult.invalidTransient("Invalid");
+                  return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT;
                 }
                 return SELECTED;
               }
@@ -682,7 +684,8 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
 
     assertThat(transactionSelectionResults.getSelectedTransactions()).contains(selected, selected3);
     assertThat(transactionSelectionResults.getNotSelectedTransactions())
-        .containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid")));
+        .containsOnly(
+            entry(notSelected, PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT));
   }
 
   @Test
@@ -1193,4 +1196,48 @@ protected MiningParameters createMiningParameters(
                 .build())
         .build();
   }
+
+  private static class PluginTransactionSelectionResult 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 PluginTransactionSelectionResult(final Status status, final String invalidReason) {
+      super(status, invalidReason);
+    }
+
+    public static TransactionSelectionResult invalidTransient(final String invalidReason) {
+      return new PluginTransactionSelectionResult(
+          PluginStatus.PLUGIN_INVALID_TRANSIENT, invalidReason);
+    }
+
+    public static TransactionSelectionResult invalid(final String invalidReason) {
+      return new PluginTransactionSelectionResult(PluginStatus.PLUGIN_INVALID, invalidReason);
+    }
+  }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java

@@ -197,7 +197,7 @@ private boolean registerDisconnect(
         disconnectCallbacks.forEach(callback -> callback.onDisconnect(peer));
         peer.handleDisconnect();
         abortPendingRequestsAssignedToDisconnectedPeers();
-        LOG.debug("Disconnected EthPeer {}", peer.getShortNodeId());
+        LOG.debug("Disconnected EthPeer {}...", peer.getShortNodeId());
         LOG.trace("Disconnected EthPeer {}", peer);
       }
     }
@@ -391,7 +391,7 @@ public void disconnectWorstUselessPeer() {
             peer -> {
               LOG.atDebug()
                   .setMessage(
-                      "disconnecting peer {}. Waiting for better peers. Current {} of max {}")
+                      "disconnecting peer {}... Waiting for better peers. Current {} of max {}")
                   .addArgument(peer::getShortNodeId)
                   .addArgument(this::peerCount)
                   .addArgument(this::getMaxPeers)

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java

@@ -398,7 +398,7 @@ public void handleDisconnect(
           "Disconnect - {} - {} - {}... - {} peers left\n{}",
           initiatedByPeer ? "Inbound" : "Outbound",
           reason,
-          connection.getPeer().getId().slice(0, 16),
+          connection.getPeer().getId().slice(0, 8),
           ethPeers.peerCount(),
           ethPeers);
     }