Remove check that minGasPrice need to decrease to retry unprofitable …

…tx (#17) Signed-off-by: Fabio Di Fabio <[email protected]>
Consensys · May 30, 2024 · e27c5ca · e27c5ca
1 parent 3d56714
commit e27c5ca
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 81 deletions.
diff --git a/.../main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectionResult.java b/.../main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectionResult.java
@@ -26,8 +26,7 @@ private enum LineaStatus implements TransactionSelectionResult.Status {
     TX_MODULE_LINE_COUNT_OVERFLOW_CACHED(false, true),
     TX_UNPROFITABLE(false, false),
     TX_UNPROFITABLE_UPFRONT(false, false),
-    TX_UNPROFITABLE_RETRY_LIMIT(false, false),
-    TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED(false, false);
+    TX_UNPROFITABLE_RETRY_LIMIT(false, false);
 
     private final boolean stop;
     private final boolean discard;
@@ -70,6 +69,4 @@ protected LineaTransactionSelectionResult(LineaStatus status) {
       new LineaTransactionSelectionResult(LineaStatus.TX_UNPROFITABLE_UPFRONT);
   public static final TransactionSelectionResult TX_UNPROFITABLE_RETRY_LIMIT =
       new LineaTransactionSelectionResult(LineaStatus.TX_UNPROFITABLE_RETRY_LIMIT);
-  public static final TransactionSelectionResult TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED =
-      new LineaTransactionSelectionResult(LineaStatus.TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED);
 }
diff --git a/...va/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelector.java b/...va/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelector.java
@@ -15,7 +15,6 @@
 package net.consensys.linea.sequencer.txselection.selectors;
 
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE;
-import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED;
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_RETRY_LIMIT;
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_UPFRONT;
 import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.SELECTED;
@@ -28,7 +27,6 @@
 import net.consensys.linea.bl.TransactionProfitabilityCalculator;
 import net.consensys.linea.config.LineaProfitabilityConfiguration;
 import net.consensys.linea.config.LineaTransactionSelectorConfiguration;
-import org.apache.commons.lang3.mutable.MutableBoolean;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.PendingTransaction;
 import org.hyperledger.besu.datatypes.Transaction;
@@ -41,14 +39,12 @@
 @Slf4j
 public class ProfitableTransactionSelector implements PluginTransactionSelector {
   @VisibleForTesting protected static Set<Hash> unprofitableCache = new LinkedHashSet<>();
-  @VisibleForTesting protected static Wei prevMinGasPrice = Wei.MAX_WEI;
 
   private final LineaTransactionSelectorConfiguration txSelectorConf;
   private final LineaProfitabilityConfiguration profitabilityConf;
   private final TransactionProfitabilityCalculator transactionProfitabilityCalculator;
 
   private int unprofitableRetries;
-  private MutableBoolean minGasPriceDecreased;
 
   public ProfitableTransactionSelector(
       final LineaTransactionSelectorConfiguration txSelectorConf,
@@ -65,12 +61,6 @@ public TransactionSelectionResult evaluateTransactionPreProcessing(
 
     final Wei minGasPrice = evaluationContext.getMinGasPrice();
 
-    // update prev min gas price only if it is a new block
-    if (minGasPriceDecreased == null) {
-      minGasPriceDecreased = new MutableBoolean(minGasPrice.lessThan(prevMinGasPrice));
-      prevMinGasPrice = minGasPrice;
-    }
-
     if (!evaluationContext.getPendingTransaction().hasPriority()) {
       final Transaction transaction = evaluationContext.getPendingTransaction().getTransaction();
       final long gasLimit = transaction.getGasLimit();
@@ -87,33 +77,20 @@ public TransactionSelectionResult evaluateTransactionPreProcessing(
       }
 
       if (unprofitableCache.contains(transaction.getHash())) {
-        // only retry unprofitable txs if the min gas price went down
-        if (minGasPriceDecreased.isTrue()) {
-
-          if (unprofitableRetries >= txSelectorConf.unprofitableRetryLimit()) {
-            log.atTrace()
-                .setMessage("Limit of unprofitable tx retries reached: {}/{}")
-                .addArgument(unprofitableRetries)
-                .addArgument(txSelectorConf.unprofitableRetryLimit());
-            return TX_UNPROFITABLE_RETRY_LIMIT;
-          }
-
+        if (unprofitableRetries >= txSelectorConf.unprofitableRetryLimit()) {
           log.atTrace()
-              .setMessage("Retrying unprofitable tx. Retry: {}/{}")
+              .setMessage("Limit of unprofitable tx retries reached: {}/{}")
               .addArgument(unprofitableRetries)
               .addArgument(txSelectorConf.unprofitableRetryLimit());
-          unprofitableCache.remove(transaction.getHash());
-          unprofitableRetries++;
-
-        } else {
-          log.atTrace()
-              .setMessage(
-                  "Current block minGasPrice {} is higher than previous block {}, skipping unprofitable txs retry")
-              .addArgument(minGasPrice::toHumanReadableString)
-              .addArgument(prevMinGasPrice::toHumanReadableString)
-              .log();
-          return TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED;
+          return TX_UNPROFITABLE_RETRY_LIMIT;
         }
+
+        log.atTrace()
+            .setMessage("Retrying unprofitable tx. Retry: {}/{}")
+            .addArgument(unprofitableRetries)
+            .addArgument(txSelectorConf.unprofitableRetryLimit());
+        unprofitableCache.remove(transaction.getHash());
+        unprofitableRetries++;
       }
     }
 

diff --git a/...et/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelectorTest.java b/...et/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelectorTest.java
@@ -15,7 +15,6 @@
 package net.consensys.linea.sequencer.txselection.selectors;
 
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE;
-import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED;
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_RETRY_LIMIT;
 import static net.consensys.linea.sequencer.txselection.LineaTransactionSelectionResult.TX_UNPROFITABLE_UPFRONT;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -258,45 +257,6 @@ public void shouldNotRetryUnprofitableTxWhenRetryLimitReached() {
         null);
   }
 
-  @Test
-  public void shouldNotRetryUnprofitableTxWhenMinGasPriceNotDecreased() {
-    var minGasPriceBlock1 = Wei.of(1_000_000);
-    var mockTransactionProcessingResult1 = mockTransactionProcessingResult(21000);
-    var mockEvaluationContext1 =
-        mockEvaluationContext(false, 10000, Wei.of(1_000_010), minGasPriceBlock1, 210000);
-    // first try of first tx
-    verifyTransactionSelection(
-        transactionSelector,
-        mockEvaluationContext1,
-        mockTransactionProcessingResult1,
-        SELECTED,
-        TX_UNPROFITABLE);
-
-    assertThat(
-            transactionSelector.isUnprofitableTxCached(
-                mockEvaluationContext1.getPendingTransaction().getTransaction().getHash()))
-        .isTrue();
-
-    // simulate another block
-    transactionSelector = newSelectorForNewBlock();
-    // we keep the min gas price the same to avoid retry
-    var minGasPriceBlock2 = minGasPriceBlock1;
-
-    // we should remember of the unprofitable txs for the new block
-    assertThat(
-            transactionSelector.isUnprofitableTxCached(
-                mockEvaluationContext1.getPendingTransaction().getTransaction().getHash()))
-        .isTrue();
-
-    // second try of the first tx
-    verifyTransactionSelection(
-        transactionSelector,
-        mockEvaluationContext1.setMinGasPrice(minGasPriceBlock2),
-        mockTransactionProcessingResult1,
-        TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED,
-        null);
-  }
-
   @Test
   public void profitableAndUnprofitableTxsMix() {
     var minGasPriceBlock1 = Wei.of(1_000_000);
@@ -333,7 +293,7 @@ public void profitableAndUnprofitableTxsMix() {
 
     // simulate another block
     transactionSelector = newSelectorForNewBlock();
-    // we keep the min gas price the same to avoid retry
+    // we keep the min gas price the same
     var minGasPriceBlock2 = minGasPriceBlock1;
 
     // we should remember of the unprofitable txs for the new block
@@ -347,8 +307,8 @@ public void profitableAndUnprofitableTxsMix() {
         transactionSelector,
         mockEvaluationContext1.setMinGasPrice(minGasPriceBlock2),
         mockTransactionProcessingResult1,
-        TX_UNPROFITABLE_MIN_GAS_PRICE_NOT_DECREASED,
-        null);
+        SELECTED,
+        TX_UNPROFITABLE);
 
     var mockTransactionProcessingResult3 = mockTransactionProcessingResult(21000);
     var mockEvaluationContext3 =
@@ -449,7 +409,6 @@ boolean isUnprofitableTxCached(final Hash txHash) {
     }
 
     void reset() {
-      prevMinGasPrice = Wei.MAX_WEI;
       unprofitableCache.clear();
     }
   }