Use tx effective gas price when comparing with min mineable gas price configuration (#6958)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactions.java

@@ -158,10 +158,10 @@ public class BaseFeePrioritizedTransactions extends AbstractPrioritizedTransacti
 
     // priority txs are promoted even if they pay less
     if (!pendingTransaction.hasPriority()) {
-      // check if max fee per gas is higher than the min gas price
+      // check if effective gas price is higher than the min gas price
       if (pendingTransaction
           .getTransaction()
-          .getMaxGasPrice()
+          .getEffectiveGasPrice(nextBlockBaseFee)
           .lessThan(miningParameters.getMinTransactionGasPrice())) {
         return false;
       }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionEstimatedMemorySizeTest.java

@@ -69,7 +69,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
   @Test
   public void toSize() {
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), 10, 0);
+        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), Wei.ZERO, 10, 0);
     Transaction txTo =
         preparedTx.to(Optional.of(Address.extract(Bytes32.random()))).createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
@@ -115,7 +115,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
   public void payloadSize() {
 
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), 10, 0);
+        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), Wei.ZERO, 10, 0);
     Transaction txPayload = preparedTx.createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
     txPayload.writeTo(rlpOut);
@@ -207,7 +207,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
       final long containerSize,
       final long itemSize) {
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.BLOB, 10, Wei.of(500), 10, 1);
+        prepareTransaction(TransactionType.BLOB, 10, Wei.of(500), Wei.of(50), 10, 1);
     Transaction txBlob = preparedTx.createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
     TransactionEncoder.encodeRLP(txBlob, rlpOut, EncodingContext.POOLED_TRANSACTION);
@@ -239,7 +239,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
   @Test
   public void blobsWithCommitmentsSize() {
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.BLOB, 10, Wei.of(500), 10, 1);
+        prepareTransaction(TransactionType.BLOB, 10, Wei.of(500), Wei.of(50), 10, 1);
     Transaction txBlob = preparedTx.createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
     TransactionEncoder.encodeRLP(txBlob, rlpOut, EncodingContext.POOLED_TRANSACTION);
@@ -268,7 +268,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
   public void pendingTransactionSize() {
 
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), 10, 0);
+        prepareTransaction(TransactionType.ACCESS_LIST, 10, Wei.of(500), Wei.ZERO, 10, 0);
     Transaction txPayload = preparedTx.createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
     txPayload.writeTo(rlpOut);
@@ -300,7 +300,7 @@ public class PendingTransactionEstimatedMemorySizeTest extends BaseTransactionPo
     final List<AccessListEntry> ales = List.of(ale1);
 
     TransactionTestFixture preparedTx =
-        prepareTransaction(TransactionType.ACCESS_LIST, 0, Wei.of(500), 0, 0);
+        prepareTransaction(TransactionType.ACCESS_LIST, 0, Wei.of(500), Wei.ZERO, 0, 0);
     Transaction txAccessList = preparedTx.accessList(ales).createTransaction(KEYS1);
     BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
     txAccessList.writeTo(rlpOut);

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactionsTest.java

@@ -37,7 +37,6 @@ import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 
 import java.util.Comparator;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Random;
 import java.util.function.BiFunction;
@@ -45,9 +44,12 @@ import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 
 public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTransactionsTestBase {
   private static final FeeMarket EIP1559_FEE_MARKET = FeeMarket.london(0L);
+  private static final Wei DEFAULT_BASE_FEE = DEFAULT_MIN_GAS_PRICE.subtract(2);
   private static final Random randomizeTxType = new Random();
 
   @Override
@@ -72,7 +74,7 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
 
   @Override
   protected BlockHeader mockBlockHeader() {
-    return mockBlockHeader(Wei.ONE);
+    return mockBlockHeader(DEFAULT_BASE_FEE);
   }
 
   private BlockHeader mockBlockHeader(final Wei baseFee) {
@@ -112,19 +114,9 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
         keys);
   }
 
-  @Test
-  public void shouldPrioritizePriorityFeeThenTimeAddedToPoolOnlyEIP1559Txs() {
-    shouldPrioritizePriorityFeeThenTimeAddedToPoolSameTypeTxs(EIP1559);
-  }
-
-  @Test
-  public void shouldPrioritizeGasPriceThenTimeAddedToPoolOnlyFrontierTxs() {
-    shouldPrioritizePriorityFeeThenTimeAddedToPoolSameTypeTxs(FRONTIER);
-  }
-
   @Test
   public void shouldPrioritizeEffectivePriorityFeeThenTimeAddedToPoolOnMixedTypes() {
-    final var nextBlockBaseFee = Optional.of(Wei.ONE);
+    final var nextBlockBaseFee = Optional.of(DEFAULT_MIN_GAS_PRICE.subtract(1));
 
     final PendingTransaction highGasPriceTransaction =
         createRemotePendingTransaction(
@@ -167,7 +159,6 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
 
   @Test
   public void txBelowCurrentMineableMinPriorityFeeIsNotPrioritized() {
-    setBaseFee(DEFAULT_MIN_GAS_PRICE.subtract(2));
     miningParameters.setMinPriorityFeePerGas(Wei.of(5));
     final PendingTransaction lowPriorityFeeTx =
         createRemotePendingTransaction(
@@ -179,7 +170,6 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
 
   @Test
   public void txWithPriorityBelowCurrentMineableMinPriorityFeeIsPrioritized() {
-    setBaseFee(DEFAULT_MIN_GAS_PRICE.subtract(2));
     miningParameters.setMinPriorityFeePerGas(Wei.of(5));
     final PendingTransaction lowGasPriceTx =
         createRemotePendingTransaction(
@@ -188,11 +178,29 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
     assertTransactionPrioritized(lowGasPriceTx);
   }
 
-  private void shouldPrioritizePriorityFeeThenTimeAddedToPoolSameTypeTxs(
+  @ParameterizedTest
+  @EnumSource(
+      value = TransactionType.class,
+      names = {"EIP1559", "BLOB"})
+  public void txWithEffectiveGasPriceBelowCurrentMineableMinGasPriceIsNotPrioritized(
+      final TransactionType type) {
+    final PendingTransaction lowGasPriceTx =
+        createRemotePendingTransaction(
+            createTransaction(type, 0, DEFAULT_MIN_GAS_PRICE, Wei.ONE, 0, 1, KEYS1));
+    assertThat(prioritizeTransaction(lowGasPriceTx)).isEqualTo(DROPPED);
+    assertEvicted(lowGasPriceTx);
+    assertTransactionNotPrioritized(lowGasPriceTx);
+  }
+
+  @ParameterizedTest
+  @EnumSource(
+      value = TransactionType.class,
+      names = {"EIP1559", "FRONTIER"})
+  public void shouldPrioritizePriorityFeeThenTimeAddedToPoolSameTypeTxs(
       final TransactionType transactionType) {
     final PendingTransaction highGasPriceTransaction =
         createRemotePendingTransaction(
-            createTransaction(0, DEFAULT_MIN_GAS_PRICE.multiply(20), KEYS1));
+            createTransaction(0, DEFAULT_MIN_GAS_PRICE.multiply(200), KEYS1));
 
     final var lowValueTxs =
         IntStream.range(0, MAX_TRANSACTIONS)
@@ -202,7 +210,7 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
                         createTransaction(
                             transactionType,
                             0,
-                            DEFAULT_MIN_GAS_PRICE.add(1),
+                            DEFAULT_MIN_GAS_PRICE.add(1).multiply(20),
                             0,
                             SIGNATURE_ALGORITHM.get().generateKeyPair())))
             .collect(Collectors.toUnmodifiableList());
@@ -210,8 +218,4 @@ public class BaseFeePrioritizedTransactionsTest extends AbstractPrioritizedTrans
     shouldPrioritizeValueThenTimeAddedToPool(
         lowValueTxs.iterator(), highGasPriceTransaction, lowValueTxs.get(0));
   }
-
-  private void setBaseFee(final Wei baseFee) {
-    transactions.blockAdded(EIP1559_FEE_MARKET, mockBlockHeader(baseFee), Map.of());
-  }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java

@@ -94,7 +94,13 @@ public class BaseTransactionPoolTest {
   protected Transaction createEIP4844Transaction(
       final long nonce, final KeyPair keys, final int gasFeeMultiplier, final int blobCount) {
     return createTransaction(
-        TransactionType.BLOB, nonce, Wei.of(5000L).multiply(gasFeeMultiplier), 0, blobCount, keys);
+        TransactionType.BLOB,
+        nonce,
+        Wei.of(5000L).multiply(gasFeeMultiplier),
+        Wei.of(5000L).multiply(gasFeeMultiplier).divide(10),
+        0,
+        blobCount,
+        keys);
   }
 
   protected Transaction createTransaction(
@@ -112,17 +118,20 @@ public class BaseTransactionPoolTest {
       final Wei maxGasPrice,
       final int payloadSize,
       final KeyPair keys) {
-    return createTransaction(type, nonce, maxGasPrice, payloadSize, 0, keys);
+    return createTransaction(
+        type, nonce, maxGasPrice, maxGasPrice.divide(10), payloadSize, 0, keys);
   }
 
   protected Transaction createTransaction(
       final TransactionType type,
       final long nonce,
       final Wei maxGasPrice,
+      final Wei maxPriorityFeePerGas,
       final int payloadSize,
       final int blobCount,
       final KeyPair keys) {
-    return prepareTransaction(type, nonce, maxGasPrice, payloadSize, blobCount)
+    return prepareTransaction(
+            type, nonce, maxGasPrice, maxPriorityFeePerGas, payloadSize, blobCount)
         .createTransaction(keys);
   }
 
@@ -130,6 +139,7 @@ public class BaseTransactionPoolTest {
       final TransactionType type,
       final long nonce,
       final Wei maxGasPrice,
+      final Wei maxPriorityFeePerGas,
       final int payloadSize,
       final int blobCount) {
 
@@ -145,7 +155,7 @@ public class BaseTransactionPoolTest {
     }
     if (type.supports1559FeeMarket()) {
       tx.maxFeePerGas(Optional.of(maxGasPrice))
-          .maxPriorityFeePerGas(Optional.of(maxGasPrice.divide(10)));
+          .maxPriorityFeePerGas(Optional.of(maxPriorityFeePerGas));
       if (type.supportsBlob() && blobCount > 0) {
         final var versionHashes =
             IntStream.range(0, blobCount)

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayersTest.java

@@ -59,6 +59,8 @@ import org.junit.jupiter.params.provider.MethodSource;
 public class LayersTest extends BaseTransactionPoolTest {
   private static final int MAX_PRIO_TRANSACTIONS = 3;
   private static final int MAX_FUTURE_FOR_SENDER = 10;
+  private static final Wei BASE_FEE = Wei.ONE;
+  private static final Wei MIN_GAS_PRICE = BASE_FEE;
 
   private final TransactionPoolConfiguration poolConfig =
       ImmutableTransactionPoolConfiguration.builder()
@@ -96,7 +98,7 @@ public class LayersTest extends BaseTransactionPoolTest {
           this::transactionReplacementTester,
           FeeMarket.london(0L),
           new BlobCache(),
-          MiningParameters.newDefault());
+          MiningParameters.newDefault().setMinTransactionGasPrice(MIN_GAS_PRICE));
 
   private final LayeredPendingTransactions pendingTransactions =
       new LayeredPendingTransactions(poolConfig, prioritizedTransactions);
@@ -1180,7 +1182,7 @@ public class LayersTest extends BaseTransactionPoolTest {
 
   private static BlockHeader mockBlockHeader() {
     final BlockHeader blockHeader = mock(BlockHeader.class);
-    when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ONE));
+    when(blockHeader.getBaseFee()).thenReturn(Optional.of(BASE_FEE));
     return blockHeader;
   }