Fix/same nonce replacement txs

src/chains/ethereum/ethereum/src/transaction-pool.ts

@@ -1,23 +1,86 @@
-import Emittery from "emittery";
+import Emittery, { Typed } from "emittery";
 import Blockchain from "./blockchain";
 import { Heap } from "@ganache/utils";
 import { Data, Quantity, JsonRpcErrorCode, ACCOUNT_ZERO } from "@ganache/utils";
 import {
   GAS_LIMIT,
   INTRINSIC_GAS_TOO_LOW,
   NONCE_TOO_LOW,
-  CodedError
+  CodedError,
+  UNDERPRICED,
+  REPLACED
 } from "@ganache/ethereum-utils";
 import { EthereumInternalOptions } from "@ganache/ethereum-options";
 import { Executables } from "./miner/executables";
 import { TypedTransaction } from "@ganache/ethereum-transaction";
 
+/**
+ * Checks if the `replacer` is eligible to replace the `replacee` transaction
+ * in the transaction pool queue. Replacement eligibility requires that
+ * the transactions have the same nonce and the `replacer` has a gas price
+ * that is `gasPrice * priceBump` better than our `replacee`.
+ * @param replacee
+ * @param replaceeNonce
+ * @param replacerNonce
+ * @param replacerGasPrice
+ * @param priceBump
+ */
+function shouldReplace(
+  replacee: TypedTransaction,
+  replacerNonce: bigint,
+  replacerGasPrice: bigint,
+  priceBump: bigint
+): boolean {
+  const replaceeNonce = replacee.nonce.toBigInt();
+  // if the nonces differ, our replacer is not eligible to replace
+  if (replaceeNonce !== replacerNonce) {
+    return false;
+  }
+
+  const gasPrice = replacee.effectiveGasPrice.toBigInt();
+  const thisPricePremium = gasPrice + (gasPrice * priceBump) / 100n;
+
+  // if our replacer's price is `gasPrice * priceBumpPercent` better than our
+  // replacee's price, we should do the replacement!.
+  if (!replacee.locked && replacerGasPrice > thisPricePremium) {
+    return true;
+  } else {
+    throw new CodedError(UNDERPRICED, JsonRpcErrorCode.TRANSACTION_REJECTED);
+  }
+}
+
 function byNonce(values: TypedTransaction[], a: number, b: number) {
   return (
     (values[b].nonce.toBigInt() || 0n) > (values[a].nonce.toBigInt() || 0n)
   );
 }
-
+/**
+ * Used to track a transaction's placement in the transaction pool based off
+ * of the its nonce.
+ */
+export enum TriageOption {
+  /**
+   * Default value. A tx will be added to the future queue if it is not yet
+   * executable based off of the transaction's nonce.
+   */
+  FutureQueue = 0,
+  /**
+   * The transaction is currently executable based off the transaction's nonce.
+   */
+  Executable = 1,
+  /**
+   * The transaction is currently executable, has the same nonce as a pending
+   * transaction of the same origin, and has a gas price that is high enough to
+   * replace the currently pending transaction.
+   */
+  ReplacesPendingExecutable = 2,
+  /**
+   * The transaction is not currently executable but has the same nonce as a
+   * future queued transaction of the same origin and has a gas price that is
+   * high enough to replace the future queued transaction.
+   */
+  ReplacesFutureTransaction = 3
+}
 export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
   #options: EthereumInternalOptions["miner"];
 
@@ -29,17 +92,19 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
   #blockchain: Blockchain;
   constructor(
     options: EthereumInternalOptions["miner"],
-    blockchain: Blockchain
+    blockchain: Blockchain,
+    origins: Map<string, Heap<TypedTransaction>> = new Map()
   ) {
     super();
     this.#blockchain = blockchain;
     this.#options = options;
+    this.#origins = origins;
   }
   public readonly executables: Executables = {
     inProgress: new Set(),
     pending: new Map()
   };
-  readonly #origins: Map<string, Heap<TypedTransaction>> = new Map();
+  readonly #origins: Map<string, Heap<TypedTransaction>>;
   readonly #accountPromises = new Map<string, Promise<Quantity>>();
 
   /**
@@ -62,12 +127,9 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
     }
 
     const from = transaction.from;
-    let transactionNonce: bigint;
+    let txNonce: bigint;
     if (!transaction.nonce.isNull()) {
-      transactionNonce = transaction.nonce.toBigInt();
-      if (transactionNonce < 0n) {
-        throw new CodedError(NONCE_TOO_LOW, JsonRpcErrorCode.INVALID_INPUT);
-      }
+      txNonce = transaction.nonce.toBigInt();
     }
 
     const origin = from.toString();
@@ -104,10 +166,12 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
     const origins = this.#origins;
     const queuedOriginTransactions = origins.get(origin);
 
-    let isExecutableTransaction = false;
+    let transactionPlacement = TriageOption.FutureQueue;
     const executables = this.executables.pending;
     let executableOriginTransactions = executables.get(origin);
 
+    const priceBump = this.#priceBump;
+    const newGasPrice = transaction.effectiveGasPrice.toBigInt();
     let length: number;
     if (
       executableOriginTransactions &&
@@ -117,55 +181,41 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
       // executables queue already. Replace the matching transaction or throw this
       // new transaction away as necessary.
       const pendingArray = executableOriginTransactions.array;
-      const priceBump = this.#priceBump;
-      const newGasPrice = transaction.effectiveGasPrice.toBigInt();
       // Notice: we're iterating over the raw heap array, which isn't
       // necessarily sorted
       for (let i = 0; i < length; i++) {
-        const currentPendingTx = pendingArray[i];
-        const thisNonce = currentPendingTx.nonce.toBigInt();
-        if (thisNonce === transactionNonce) {
-          const gasPrice = currentPendingTx.effectiveGasPrice.toBigInt();
-          const thisPricePremium = gasPrice + (gasPrice * priceBump) / 100n;
-
-          // if our new price is `gasPrice * priceBumpPercent` better than our
-          // oldPrice, throw out the old now.
-          if (!currentPendingTx.locked && newGasPrice > thisPricePremium) {
-            isExecutableTransaction = true;
-            // do an in-place replace without triggering a re-sort because we
-            // already know where this transaction should go in this "byNonce"
-            // heap.
-            pendingArray[i] = transaction;
-
-            currentPendingTx.finalize(
-              "rejected",
-              new CodedError(
-                "Transaction replaced by better transaction",
-                JsonRpcErrorCode.TRANSACTION_REJECTED
-              )
-            );
-          } else {
-            throw new CodedError(
-              "replacement transaction underpriced",
-              JsonRpcErrorCode.TRANSACTION_REJECTED
-            );
-          }
-        }
-        if (thisNonce > highestNonce) {
-          highestNonce = thisNonce;
+        const pendingTx = pendingArray[i];
+        if (shouldReplace(pendingTx, txNonce, newGasPrice, priceBump)) {
+          // do an in-place replace without triggering a re-sort because we
+          // already know where this transaction should go in this "byNonce"
+          // heap.
+          pendingArray[i] = transaction;
+          // we don't want to mark this transaction as "executable" and thus
+          // have it added to the pool again. so use this flag to skip
+          // a re-queue.
+          transactionPlacement = TriageOption.ReplacesPendingExecutable;
+          pendingTx.finalize(
+            "rejected",
+            new CodedError(REPLACED, JsonRpcErrorCode.TRANSACTION_REJECTED)
+          );
+          break;
         }
+        // track the highest nonce for all transactions pending from this
+        // origin. If this transaction can't be used as a replacement, it will
+        // use this next highest nonce.
+        const pendingTxNonce = pendingTx.nonce.toBigInt();
+        if (pendingTxNonce > highestNonce) highestNonce = pendingTxNonce;
       }
-      if (transactionNonce === void 0) {
+
+      if (txNonce === void 0) {
         // if we aren't signed and don't have a transactionNonce yet set it now
-        transactionNonce = highestNonce + 1n;
-        transaction.nonce = Quantity.from(transactionNonce);
-        isExecutableTransaction = true;
-        highestNonce = transactionNonce;
-      } else if (transactionNonce === highestNonce + 1n) {
+        txNonce = highestNonce + 1n;
+        transaction.nonce = Quantity.from(txNonce);
+        transactionPlacement = TriageOption.Executable;
+      } else if (txNonce === highestNonce + 1n) {
         // if our transaction's nonce is 1 higher than the last transaction in the
         // origin's heap we are executable.
-        isExecutableTransaction = true;
-        highestNonce = transactionNonce;
+        transactionPlacement = TriageOption.Executable;
       }
     } else {
       // since we don't have any executable transactions at the moment, we need
@@ -180,20 +230,56 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
       const transactor = await transactorNoncePromise;
 
       const transactorNonce = transactor ? transactor.toBigInt() : 0n;
-      if (transactionNonce === void 0) {
+      if (txNonce === void 0) {
         // if we don't have a transactionNonce, just use the account's next
         // nonce and mark as executable
-        transactionNonce = transactorNonce ? transactorNonce : 0n;
-        highestNonce = transactionNonce;
-        isExecutableTransaction = true;
-        transaction.nonce = Quantity.from(transactionNonce);
-      } else if (transactionNonce < transactorNonce) {
+        txNonce = transactorNonce ? transactorNonce : 0n;
+        transaction.nonce = Quantity.from(txNonce);
+        transactionPlacement = TriageOption.Executable;
+      } else if (txNonce < transactorNonce) {
         // it's an error if the transaction's nonce is <= the persisted nonce
         throw new Error(
-          `the tx doesn't have the correct nonce. account has nonce of: ${transactorNonce} tx has nonce of: ${transactionNonce}`
+          `the tx doesn't have the correct nonce. account has nonce of: ${transactorNonce} tx has nonce of: ${txNonce}`
         );
-      } else if (transactionNonce === transactorNonce) {
-        isExecutableTransaction = true;
+      } else if (txNonce === transactorNonce) {
+        transactionPlacement = TriageOption.Executable;
+      }
+    }
+
+    // we have future transactions for this origin, this transaction is not yet
+    // executable, and this transaction is not replacing a previously queued/
+    // executable transaction, then this is potentially eligible to replace a
+    // future transaction
+    if (
+      queuedOriginTransactions &&
+      transactionPlacement !== TriageOption.Executable &&
+      transactionPlacement !== TriageOption.ReplacesPendingExecutable &&
+      (length = queuedOriginTransactions.length)
+    ) {
+      // check if a transaction with the same nonce is in the origin's
+      // future queue already. Replace the matching transaction or throw this
+      // new transaction away as necessary.
+
+      const queuedArray = queuedOriginTransactions.array;
+      // Notice: we're iterating over the raw heap array, which isn't
+      // necessarily sorted
+      for (let i = 0; i < length; i++) {
+        const queuedTx = queuedArray[i];
+        if (shouldReplace(queuedTx, txNonce, newGasPrice, priceBump)) {
+          // do an in-place replace without triggering a re-sort because we
+          // already know where this transaction should go in this "byNonce"
+          // heap.
+          queuedArray[i] = transaction;
+          // we don't want to mark this transaction as "FutureQueue" and thus
+          // have it added to the pool again. so use this flag to skip
+          // a re-queue.
+          transactionPlacement = TriageOption.ReplacesFutureTransaction;
+          queuedTx.finalize(
+            "rejected",
+            new CodedError(REPLACED, JsonRpcErrorCode.TRANSACTION_REJECTED)
+          );
+          break;
+        }
       }
     }
 
@@ -221,52 +307,62 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
       transaction.signAndHash(fakePrivateKey);
     }
 
-    if (isExecutableTransaction) {
-      // if it is executable add it to the executables queue
-      if (executableOriginTransactions) {
-        executableOriginTransactions.push(transaction);
-      } else {
-        // if we don't yet have an executables queue for this origin make one now
-        executableOriginTransactions = Heap.from(transaction, byNonce);
-        executables.set(origin, executableOriginTransactions);
-      }
-
-      // Now we need to drain any queued transactions that were previously
-      // not executable due to nonce gaps into the origin's queue...
-      if (queuedOriginTransactions) {
-        let nextExpectedNonce = transactionNonce + 1n;
-        while (true) {
-          const nextTx = queuedOriginTransactions.peek();
-          const nextTxNonce = nextTx.nonce.toBigInt() || 0n;
-          if (nextTxNonce !== nextExpectedNonce) {
-            break;
-          }
-
-          // we've got a an executable nonce! Put it in the executables queue.
-          executableOriginTransactions.push(nextTx);
+    switch (transactionPlacement) {
+      case TriageOption.Executable:
+        // if it is executable add it to the executables queue
+        if (executableOriginTransactions) {
+          executableOriginTransactions.push(transaction);
+        } else {
+          // if we don't yet have an executables queue for this origin make one now
+          executableOriginTransactions = Heap.from(transaction, byNonce);
+          executables.set(origin, executableOriginTransactions);
+        }
 
-          // And then remove this transaction from its origin's queue
-          if (!queuedOriginTransactions.removeBest()) {
-            // removeBest() returns `false` when there are no more items after
-            // the removed item. Let's do some cleanup when that happens.
-            origins.delete(origin);
-            break;
+        // Now we need to drain any queued transactions that were previously
+        // not executable due to nonce gaps into the origin's queue...
+        if (queuedOriginTransactions) {
+          let nextExpectedNonce = txNonce + 1n;
+          while (true) {
+            const nextTx = queuedOriginTransactions.peek();
+            const nextTxNonce = nextTx.nonce.toBigInt() || 0n;
+            if (nextTxNonce !== nextExpectedNonce) {
+              break;
+            }
+
+            // we've got a an executable nonce! Put it in the executables queue.
+            executableOriginTransactions.push(nextTx);
+
+            // And then remove this transaction from its origin's queue
+            if (!queuedOriginTransactions.removeBest()) {
+              // removeBest() returns `false` when there are no more items after
+              // the removed item. Let's do some cleanup when that happens.
+              origins.delete(origin);
+              break;
+            }
+
+            nextExpectedNonce += 1n;
           }
-
-          nextExpectedNonce += 1n;
         }
-      }
+        return true;
+
+      case TriageOption.FutureQueue:
+        // otherwise, put it in the future queue
+        if (queuedOriginTransactions) {
+          queuedOriginTransactions.push(transaction);
+        } else {
+          origins.set(origin, Heap.from(transaction, byNonce));
+        }
+        return false;
 
-      return true;
-    } else {
-      // otherwise, put it in the future queue
-      if (queuedOriginTransactions) {
-        queuedOriginTransactions.push(transaction);
-      } else {
-        origins.set(origin, Heap.from(transaction, byNonce));
-      }
+      case TriageOption.ReplacesPendingExecutable:
+        // we've replaced the best transaction from this origin for this nonce,
+        // and it is executable
+        return true;
 
-      return false;
+      case TriageOption.ReplacesFutureTransaction:
+        // we've replaced the best transaction from this origin for a future
+        // nonce, so this one isn't executable
+        return false;
     }
   }