Implement price bump mechanism for transaction pool replacement. (hyp…

…erledger#928) * Implement price bump mechanism for transaction pool replacement. Signed-off-by: Abdelhamid Bakhta <[email protected]> Signed-off-by: Danno Ferrin <[email protected]>
shemnon · May 23, 2020 · 8e1ab90 · 8e1ab90
1 parent 3b24694
commit 8e1ab90
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 16 deletions.
diff --git a/...ain/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java b/...ain/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java
@@ -21,6 +21,7 @@ public class TransactionPoolConfiguration {
   public static final int MAX_PENDING_TRANSACTIONS = 4096;
   public static final int MAX_PENDING_TRANSACTIONS_HASHES = 4096;
   public static final int DEFAULT_TX_RETENTION_HOURS = 13;
+  public static final int DEFAULT_PRICE_BUMP = 10;
 
   private final int txPoolMaxSize;
   private final int pooledTransactionHashesSize;

diff --git a/...ava/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java b/...ava/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java
@@ -19,6 +19,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.fees.EIP1559;
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionInfo;
+import org.hyperledger.besu.util.number.Percentage;
 
 import java.util.List;
 import java.util.Optional;
@@ -30,7 +31,11 @@ public class TransactionPoolReplacementHandler {
   private final Optional<EIP1559> eip1559;
 
   public TransactionPoolReplacementHandler(final Optional<EIP1559> eip1559) {
-    this(asList(new TransactionReplacementByPriceRule()), eip1559);
+    this(
+        asList(
+            new TransactionReplacementByPriceRule(
+                Percentage.fromInt(TransactionPoolConfiguration.DEFAULT_PRICE_BUMP))),
+        eip1559);
   }
 
   @VisibleForTesting

diff --git a/...ava/org/hyperledger/besu/ethereum/eth/transactions/TransactionReplacementByPriceRule.java b/...ava/org/hyperledger/besu/ethereum/eth/transactions/TransactionReplacementByPriceRule.java
@@ -18,6 +18,7 @@
 import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.core.fees.TransactionPriceCalculator;
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionInfo;
+import org.hyperledger.besu.util.number.Percentage;
 
 import java.util.Optional;
 
@@ -26,6 +27,11 @@ public class TransactionReplacementByPriceRule implements TransactionPoolReplace
       TransactionPriceCalculator.frontier();
   private static final TransactionPriceCalculator EIP1559_CALCULATOR =
       TransactionPriceCalculator.eip1559();
+  private final Percentage priceBump;
+
+  public TransactionReplacementByPriceRule(final Percentage priceBump) {
+    this.priceBump = priceBump;
+  }
 
   @Override
   public boolean shouldReplace(
@@ -34,8 +40,11 @@ public boolean shouldReplace(
       final Optional<Long> baseFee) {
     assert existingTransactionInfo.getTransaction() != null
         && newTransactionInfo.getTransaction() != null;
-    return priceOf(newTransactionInfo.getTransaction(), baseFee)
-            .compareTo(priceOf(existingTransactionInfo.getTransaction(), baseFee))
+    final Wei replacementThreshold =
+        priceOf(existingTransactionInfo.getTransaction(), baseFee)
+            .multiply(100 + priceBump.getValue())
+            .divide(100);
+    return priceOf(newTransactionInfo.getTransaction(), baseFee).compareTo(replacementThreshold)
         > 0;
   }
 

diff --git a/...src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java b/...src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java
@@ -384,7 +384,8 @@ public void shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacements(
     final List<Transaction> replacedTransactions = new ArrayList<>();
     int remoteDuplicateCount = 0;
     for (int i = 0; i < replacedTxCount; i++) {
-      final Transaction duplicateTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, i + 1);
+      final Transaction duplicateTx =
+          transactionWithNonceSenderAndGasPrice(1, KEYS1, (i * 110 / 100) + 1);
       replacedTransactions.add(duplicateTx);
       if (i % 2 == 0) {
         transactions.addRemoteTransaction(duplicateTx);

diff --git a/...org/hyperledger/besu/ethereum/eth/transactions/TransactionReplacementByPriceRuleTest.java b/...org/hyperledger/besu/ethereum/eth/transactions/TransactionReplacementByPriceRuleTest.java
@@ -24,6 +24,7 @@
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionInfo;
+import org.hyperledger.besu.util.number.Percentage;
 
 import java.util.Collection;
 import java.util.Optional;
@@ -41,33 +42,42 @@ public class TransactionReplacementByPriceRuleTest {
   public static Collection<Object[]> data() {
     return asList(
         new Object[][] {
-          {frontierTx(5L), frontierTx(6L), empty(), true},
-          {frontierTx(5L), frontierTx(5L), empty(), false},
-          {frontierTx(5L), frontierTx(4L), empty(), false},
-          {frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(1L), false},
-          {frontierTx(5L), eip1559Tx(3L, 5L), Optional.of(3L), false},
-          {frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(3L), true},
-          {eip1559Tx(3L, 6L), eip1559Tx(3L, 6L), Optional.of(3L), false},
-          {eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(3L), false},
-          {eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(4L), true},
-          {eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(4L), false},
-          {eip1559Tx(3L, 8L), frontierTx(8L), Optional.of(4L), true}
+          {frontierTx(5L), frontierTx(6L), empty(), 0, true},
+          {frontierTx(5L), frontierTx(5L), empty(), 0, false},
+          {frontierTx(5L), frontierTx(4L), empty(), 0, false},
+          {frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(1L), 0, false},
+          {frontierTx(5L), eip1559Tx(3L, 5L), Optional.of(3L), 0, false},
+          {frontierTx(5L), eip1559Tx(3L, 6L), Optional.of(3L), 0, true},
+          {eip1559Tx(3L, 6L), eip1559Tx(3L, 6L), Optional.of(3L), 0, false},
+          {eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(3L), 0, false},
+          {eip1559Tx(3L, 6L), eip1559Tx(3L, 7L), Optional.of(4L), 0, true},
+          {eip1559Tx(3L, 8L), frontierTx(7L), Optional.of(4L), 0, false},
+          {eip1559Tx(3L, 8L), frontierTx(8L), Optional.of(4L), 0, true},
+          {frontierTx(100L), frontierTx(105L), empty(), 10, false},
+          {frontierTx(100L), frontierTx(110L), empty(), 10, false},
+          {frontierTx(100L), frontierTx(111L), empty(), 10, true},
+          {eip1559Tx(10L, 200L), eip1559Tx(10L, 200L), Optional.of(90L), 10, false},
+          {eip1559Tx(10L, 200L), eip1559Tx(15L, 200L), Optional.of(90L), 10, false},
+          {eip1559Tx(10L, 200L), eip1559Tx(21L, 200L), Optional.of(90L), 10, true},
         });
   }
 
   private final TransactionInfo oldTx;
   private final TransactionInfo newTx;
   private final Optional<Long> baseFee;
+  private final int priceBump;
   private final boolean expected;
 
   public TransactionReplacementByPriceRuleTest(
       final TransactionInfo oldTx,
       final TransactionInfo newTx,
       final Optional<Long> baseFee,
+      final int priceBump,
       final boolean expected) {
     this.oldTx = oldTx;
     this.newTx = newTx;
     this.baseFee = baseFee;
+    this.priceBump = priceBump;
     this.expected = expected;
   }
 
@@ -83,7 +93,9 @@ public void resetEIP1559() {
 
   @Test
   public void shouldReplace() {
-    assertThat(new TransactionReplacementByPriceRule().shouldReplace(oldTx, newTx, baseFee))
+    assertThat(
+            new TransactionReplacementByPriceRule(Percentage.fromInt(priceBump))
+                .shouldReplace(oldTx, newTx, baseFee))
         .isEqualTo(expected);
   }