From e27c5cabeade261b2929fed328ff4e6aa1bdfbf5 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 30 May 2024 19:28:54 +0200 Subject: [PATCH] Remove check that minGasPrice need to decrease to retry unprofitable tx (#17) Signed-off-by: Fabio Di Fabio --- .../LineaTransactionSelectionResult.java | 5 +- .../ProfitableTransactionSelector.java | 43 ++++------------- .../ProfitableTransactionSelectorTest.java | 47 ++----------------- 3 files changed, 14 insertions(+), 81 deletions(-) diff --git a/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectionResult.java b/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectionResult.java index b55d057a..8605dd5c 100644 --- a/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectionResult.java +++ b/arithmetization/src/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/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelector.java b/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelector.java index 5c5c53bd..99d208d6 100644 --- a/arithmetization/src/main/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelector.java +++ b/arithmetization/src/main/java/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 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/arithmetization/src/test/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelectorTest.java b/arithmetization/src/test/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelectorTest.java index a4635a65..75932f6b 100644 --- a/arithmetization/src/test/java/net/consensys/linea/sequencer/txselection/selectors/ProfitableTransactionSelectorTest.java +++ b/arithmetization/src/test/java/net/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(); } }