Make the no-sdr-grouping flag experimental, remove code no longer nee…

…ded since hyperledger#6106 merged Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
matthew1001 · Nov 8, 2023 · 3ae0747 · 3ae0747
1 parent 4199ec6
commit 3ae0747
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 71 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java
@@ -48,7 +48,6 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
   private static final String TX_POOL_ENABLE_SAVE_RESTORE = "--tx-pool-enable-save-restore";
   private static final String TX_POOL_SAVE_FILE = "--tx-pool-save-file";
   private static final String TX_POOL_PRICE_BUMP = "--tx-pool-price-bump";
-  private static final String TX_POOL_DISABLE_SENDER_GROUPING = "--tx-pool-disable-sender-grouping";
   private static final String RPC_TX_FEECAP = "--rpc-tx-feecap";
   private static final String STRICT_TX_REPLAY_PROTECTION_ENABLED_FLAG =
       "--strict-tx-replay-protection-enabled";
@@ -96,15 +95,6 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
       arity = "1")
   private Percentage priceBump = TransactionPoolConfiguration.DEFAULT_PRICE_BUMP;
 
-  @CommandLine.Option(
-      names = {TX_POOL_DISABLE_SENDER_GROUPING},
-      paramLabel = "<Boolean>",
-      description =
-          "Disable sender grouping of transactions during selection. (default: ${DEFAULT-VALUE})",
-      arity = "0..1")
-  private Boolean disableSenderTXGrouping =
-      TransactionPoolConfiguration.DEFAULT_DISABLE_SENDER_TX_GROUPING;
-
   @CommandLine.Option(
       names = {RPC_TX_FEECAP},
       description =
@@ -228,7 +218,6 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
     options.saveRestoreEnabled = config.getEnableSaveRestore();
     options.noLocalPriority = config.getNoLocalPriority();
     options.priceBump = config.getPriceBump();
-    options.disableSenderTXGrouping = config.getDisableSenderTXGrouping();
     options.txFeeCap = config.getTxFeeCap();
     options.saveFile = config.getSaveFile();
     options.strictTxReplayProtectionEnabled = config.getStrictTransactionReplayProtectionEnabled();
@@ -272,7 +261,6 @@ public TransactionPoolConfiguration toDomainObject() {
         .enableSaveRestore(saveRestoreEnabled)
         .noLocalPriority(noLocalPriority)
         .priceBump(priceBump)
-        .disableSenderTXGrouping(disableSenderTXGrouping)
         .txFeeCap(txFeeCap)
         .saveFile(saveFile)
         .strictTransactionReplayProtectionEnabled(strictTxReplayProtectionEnabled)

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/TransactionPoolOptions.java
@@ -33,6 +33,9 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
   private static final String ETH65_TX_ANNOUNCED_BUFFERING_PERIOD_FLAG =
       "--Xeth65-tx-announced-buffering-period-milliseconds";
 
+  private static final String TX_POOL_DISABLE_SENDER_GROUPING =
+      "--Xtx-pool-disable-sender-grouping";
+
   @CommandLine.Option(
       names = {TX_MESSAGE_KEEP_ALIVE_SEC_FLAG},
       paramLabel = "<INTEGER>",
@@ -54,6 +57,16 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
   private Duration eth65TrxAnnouncedBufferingPeriod =
       TransactionPoolConfiguration.Unstable.ETH65_TRX_ANNOUNCED_BUFFERING_PERIOD;
 
+  @CommandLine.Option(
+      names = {TX_POOL_DISABLE_SENDER_GROUPING},
+      paramLabel = "<Boolean>",
+      hidden = true,
+      description =
+          "Disable sender grouping of transactions during selection. (default: ${DEFAULT-VALUE})",
+      arity = "0..1")
+  private Boolean disableSenderTXGrouping =
+      TransactionPoolConfiguration.DEFAULT_DISABLE_SENDER_TX_GROUPING;
+
   private TransactionPoolOptions() {}
 
   /**
@@ -76,6 +89,7 @@ public static TransactionPoolOptions fromConfig(
     final TransactionPoolOptions options = TransactionPoolOptions.create();
     options.txMessageKeepAliveSeconds = config.getTxMessageKeepAliveSeconds();
     options.eth65TrxAnnouncedBufferingPeriod = config.getEth65TrxAnnouncedBufferingPeriod();
+    options.disableSenderTXGrouping = config.getDisableSenderTXGrouping();
     return options;
   }
 
@@ -84,6 +98,7 @@ public TransactionPoolConfiguration.Unstable toDomainObject() {
     return ImmutableTransactionPoolConfiguration.Unstable.builder()
         .txMessageKeepAliveSeconds(txMessageKeepAliveSeconds)
         .eth65TrxAnnouncedBufferingPeriod(eth65TrxAnnouncedBufferingPeriod)
+        .disableSenderTXGrouping(disableSenderTXGrouping)
         .build();
   }
 

diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml
@@ -180,7 +180,6 @@ tx-pool-no-local-priority=false
 tx-pool-priority-senders=["0xABC0000000000000000000000000000000001234","0xDEF0000000000000000000000000000000001234"]
 tx-pool-enable-save-restore=true
 tx-pool-save-file="txpool.dump"
-tx-pool-disable-sender-grouping=false
 ## Layered
 tx-pool-layer-max-capacity=12345678
 tx-pool-max-prioritized=9876
@@ -189,6 +188,8 @@ tx-pool-max-future-by-sender=321
 tx-pool-retention-hours=999
 tx-pool-max-size=1234
 tx-pool-limit-by-account-percentage=0.017
+## Experimental
+Xtx-pool-disable-sender-grouping=false
 
 # Revert Reason
 revert-reason-enabled=false

diff --git a/...ain/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java b/...ain/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java
@@ -47,6 +47,11 @@ default Duration getEth65TrxAnnouncedBufferingPeriod() {
     default int getTxMessageKeepAliveSeconds() {
       return DEFAULT_TX_MSG_KEEP_ALIVE;
     }
+
+    @Value.Default
+    default Boolean getDisableSenderTXGrouping() {
+      return DEFAULT_DISABLE_SENDER_TX_GROUPING;
+    }
   }
 
   enum Implementation {

diff --git a/.../hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java b/.../hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java
@@ -256,7 +256,7 @@ public void selectTransactions(final TransactionSelector selector) {
         for (final PendingTransaction transactionToProcess :
             accountTransactionOrder.transactionsToProcess(
                 highestPriorityPendingTransaction,
-                poolConfig.getDisableSenderTXGrouping() ? 1 : Integer.MAX_VALUE)) {
+                poolConfig.getUnstable().getDisableSenderTXGrouping() ? 1 : Integer.MAX_VALUE)) {
           final TransactionSelectionResult result =
               selector.evaluateTransaction(transactionToProcess);
 
@@ -273,50 +273,6 @@ public void selectTransactions(final TransactionSelector selector) {
         }
       }
 
-      if (poolConfig.getDisableSenderTXGrouping()) {
-        // If sender grouping of transactions is disabled, there will potentially
-        // be transactions left for the sender accounts we have selected at least 1 TX for already.
-
-        // Iterate over the remaining accounts that have at least 1 deferred transaction until there
-        // aren't any left.
-        while (accountTransactions.size() > 0) {
-
-          // List the current accounts with deferred transactions
-          List<Map.Entry<Address, AccountTransactionOrder>> accounts =
-              accountTransactions.entrySet().stream().toList();
-
-          for (Map.Entry<Address, AccountTransactionOrder> nextEntry : accounts) {
-            // Get the address and account transaction order
-            Address addr = nextEntry.getKey();
-            AccountTransactionOrder nextAccount = nextEntry.getValue();
-
-            // Get the highest priority deferred transaction and evaluate it
-            Optional<PendingTransaction> nextTransactionForSender =
-                nextAccount.getHighestPriorityDeferredTransaction();
-
-            if (nextTransactionForSender.isPresent()) {
-              PendingTransaction transactionToEvaluate = nextTransactionForSender.get();
-              final TransactionSelectionResult result =
-                  selector.evaluateTransaction(transactionToEvaluate);
-
-              if (result.discard()) {
-                transactionsToRemove.add(transactionToEvaluate.getTransaction());
-                transactionsToRemove.addAll(
-                    signalInvalidAndGetDependentTransactions(
-                        transactionToEvaluate.getTransaction()));
-              }
-
-              if (result.stop()) {
-                transactionsToRemove.forEach(this::removeTransaction);
-                return;
-              }
-            } else {
-              // This account is empty - remove from the list
-              accountTransactions.remove(addr);
-            }
-          }
-        }
-      }
       transactionsToRemove.forEach(this::removeTransaction);
     }
   }

diff --git a/...n/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AccountTransactionOrder.java b/...n/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AccountTransactionOrder.java
@@ -20,7 +20,6 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.NavigableSet;
-import java.util.Optional;
 import java.util.TreeSet;
 import java.util.stream.Stream;
 
@@ -69,16 +68,4 @@ public Iterable<PendingTransaction> transactionsToProcess(
     }
     return transactionsToApply;
   }
-
-  public Optional<PendingTransaction> getHighestPriorityDeferredTransaction() {
-    Optional<PendingTransaction> transactionToApply = Optional.empty();
-    if (!deferredTransactions.isEmpty()
-        && !transactionsForSender.isEmpty()
-        && deferredTransactions.first().equals(transactionsForSender.first())) {
-      transactionToApply = Optional.of(deferredTransactions.first());
-      deferredTransactions.remove(transactionToApply.get());
-      transactionsForSender.remove(transactionToApply.get());
-    }
-    return transactionToApply;
-  }
 }