Prioritize High Gas Price Transactions

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Additions and Improvements
 * `--random-peer-priority-enabled` flag added. Allows for incoming connections to be prioritized randomly. This will prevent (typically small, stable) networks from forming impenetrable peer cliques. [#1440](https://github.com/hyperledger/besu/pull/1440)
 * Hide deprecated `--host-whitelist` option. [\#1444](https://github.com/hyperledger/besu/pull/1444)
+* Prioritize high gas prices during mining. Previously we ordered only by the order in which the transactions were received. This will increase expected profit when mining. [\#1449](https://github.com/hyperledger/besu/pull/1449)
 
 ## Deprecated and Scheduled for removal in _Next_ Release
 

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java

@@ -24,6 +24,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.core.fees.EIP1559;
 import org.hyperledger.besu.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
 import org.hyperledger.besu.metrics.BesuMetricCategory;
@@ -71,6 +72,7 @@ public class PendingTransactions {
   private final NavigableSet<TransactionInfo> prioritizedTransactions =
       new TreeSet<>(
           comparing(TransactionInfo::isReceivedFromLocalSource)
+              .thenComparing(TransactionInfo::getGasPrice)
               .thenComparing(TransactionInfo::getSequence)
               .reversed());
   private final Map<Address, TransactionsForSenderInfo> transactionsBySender =
@@ -402,6 +404,10 @@ public Transaction getTransaction() {
       return transaction;
     }
 
+    public Wei getGasPrice() {
+      return transaction.getGasPrice();
+    }
+
     public long getSequence() {
       return sequence;
     }

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

@@ -38,6 +38,8 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalLong;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import com.google.common.collect.Lists;
 import org.junit.Test;
@@ -156,6 +158,27 @@ public void shouldPrioritizeLocalTransaction() {
     assertTransactionPending(localTransaction);
   }
 
+  @Test
+  public void shouldPrioritizeGasPriceThenTimeAddedToPool() {
+    final List<Transaction> lowGasPriceTransactions =
+        IntStream.range(0, MAX_TRANSACTIONS)
+            .mapToObj(i -> transactionWithNonceSenderAndGasPrice(i + 1, KEYS1, 10))
+            .collect(Collectors.toUnmodifiableList());
+
+    // Fill the pool
+    lowGasPriceTransactions.forEach(transactions::addRemoteTransaction);
+
+    // This should kick the oldest tx with the low gas price out, namely the first one we added
+    final Transaction highGasPriceTransaction =
+        transactionWithNonceSenderAndGasPrice(MAX_TRANSACTIONS + 1, KEYS1, 100);
+    transactions.addRemoteTransaction(highGasPriceTransaction);
+    assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS);
+
+    assertTransactionPending(highGasPriceTransaction);
+    assertTransactionNotPending(lowGasPriceTransactions.get(0));
+    lowGasPriceTransactions.stream().skip(1).forEach(this::assertTransactionPending);
+  }
+
   @Test
   public void shouldStartDroppingLocalTransactionsWhenPoolIsFullOfLocalTransactions() {
     final Transaction firstLocalTransaction = createTransaction(0);