fix: update equality checks for replacement transactions to prevent erroneous underpriced errors

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

@@ -7,7 +7,8 @@ import {
   INTRINSIC_GAS_TOO_LOW,
   CodedError,
   UNDERPRICED,
-  REPLACED
+  REPLACED,
+  TRANSACTION_LOCKED
 } from "@ganache/ethereum-utils";
 import { EthereumInternalOptions } from "@ganache/ethereum-options";
 import { Executables } from "./miner/executables";
@@ -27,24 +28,45 @@ import { TypedTransaction } from "@ganache/ethereum-transaction";
 function shouldReplace(
   replacee: TypedTransaction,
   replacerNonce: bigint,
-  replacerGasPrice: bigint,
+  replacer: TypedTransaction,
   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;
+  const replacerTip =
+    "maxPriorityFeePerGas" in replacer
+      ? replacer.maxPriorityFeePerGas.toBigInt()
+      : replacer.effectiveGasPrice.toBigInt();
+  const replacerMaxFee =
+    "maxFeePerGas" in replacer
+      ? replacer.maxFeePerGas.toBigInt()
+      : replacer.effectiveGasPrice.toBigInt();
+  const replaceeTip =
+    "maxPriorityFeePerGas" in replacee
+      ? replacee.maxPriorityFeePerGas.toBigInt()
+      : replacee.effectiveGasPrice.toBigInt();
+  const replaceeMaxFee =
+    "maxFeePerGas" in replacee
+      ? replacee.maxFeePerGas.toBigInt()
+      : replacee.effectiveGasPrice.toBigInt();
+
+  const tipPremium = replaceeTip + (replaceeTip * priceBump) / 100n;
+  const maxFeePremium = replaceeMaxFee + (replaceeMaxFee * 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 {
+  if (replacee.locked) {
+    throw new CodedError(
+      TRANSACTION_LOCKED,
+      JsonRpcErrorCode.TRANSACTION_REJECTED
+    );
+  } else if (replacerTip < tipPremium || replacerMaxFee < maxFeePremium) {
     throw new CodedError(UNDERPRICED, JsonRpcErrorCode.TRANSACTION_REJECTED);
+  } else {
+    return true;
   }
 }
 
@@ -83,10 +105,7 @@ export enum TriageOption {
 export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
   #options: EthereumInternalOptions["miner"];
 
-  /**
-   * Minimum price bump percentage to replace an already existing transaction (nonce)
-   */
-  #priceBump: bigint = 10n;
+  #priceBump: bigint;
 
   #blockchain: Blockchain;
   constructor(
@@ -98,6 +117,7 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
     this.#blockchain = blockchain;
     this.#options = options;
     this.#origins = origins;
+    this.#priceBump = options.priceBump;
   }
   public readonly executables: Executables = {
     inProgress: new Set(),
@@ -183,7 +203,6 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
     let executableOriginTransactions = executables.get(origin);
 
     const priceBump = this.#priceBump;
-    const newGasPrice = transaction.effectiveGasPrice.toBigInt();
     let length: number;
     if (
       executableOriginTransactions &&
@@ -197,7 +216,7 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
       // necessarily sorted
       for (let i = 0; i < length; i++) {
         const pendingTx = pendingArray[i];
-        if (shouldReplace(pendingTx, txNonce, newGasPrice, priceBump)) {
+        if (shouldReplace(pendingTx, txNonce, transaction, priceBump)) {
           // do an in-place replace without triggering a re-sort because we
           // already know where this transaction should go in this "byNonce"
           // heap.
@@ -277,7 +296,7 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> {
       // necessarily sorted
       for (let i = 0; i < length; i++) {
         const queuedTx = queuedArray[i];
-        if (shouldReplace(queuedTx, txNonce, newGasPrice, priceBump)) {
+        if (shouldReplace(queuedTx, txNonce, transaction, priceBump)) {
           // do an in-place replace without triggering a re-sort because we
           // already know where this transaction should go in this "byNonce"
           // heap.

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

@@ -33,7 +33,11 @@ describe("transaction pool", async () => {
   let common: Common;
   let blockchain: any;
   let origins: Map<string, Heap<TypedTransaction>>;
-  const optionsJson = { wallet: { deterministic: true } };
+  const priceBump = 10n;
+  const optionsJson = {
+    wallet: { deterministic: true },
+    miner: { priceBump: priceBump }
+  };
   const options = EthereumOptionsConfig.normalize(optionsJson);
   let futureNonceRpc, executableRpc: TypedRpcTransaction;
   before(function () {
@@ -44,19 +48,22 @@ describe("transaction pool", async () => {
       from: from,
       type: "0x2",
       maxFeePerGas: "0xffffffff",
+      maxPriorityFeePerGas: "0xff",
       gasLimit: "0xffff"
     };
     executableRpc = {
       from: from,
       type: "0x2",
       maxFeePerGas: "0xffffffff",
+      maxPriorityFeePerGas: "0xff",
       gasLimit: "0xffff",
       nonce: "0x0"
     };
     futureNonceRpc = {
       from: from,
       type: "0x2",
       maxFeePerGas: "0xffffff",
+      maxPriorityFeePerGas: "0xff",
       gasLimit: "0xffff",
       nonce: "0x2"
     };
@@ -165,9 +172,47 @@ describe("transaction pool", async () => {
       found.serialized.toString(),
       executableTx.serialized.toString()
     );
-    // the second time around, the gas price won't be high enough to replace, so it'll throw
+
+    const replacementRpc = JSON.parse(JSON.stringify(executableRpc));
+    replacementRpc.maxPriorityFeePerGas = "0xffff";
+    const replacementTx1 = TransactionFactory.fromRpc(replacementRpc, common);
+    // even if the tip is high enough, the max fee isn't enough to replace, so it'll throw
     await assert.rejects(
-      txPool.prepareTransaction(executableTx, secretKey),
+      txPool.prepareTransaction(replacementTx1, secretKey),
+      {
+        code: -32003,
+        message: "transaction underpriced"
+      },
+      "replacement transaction with insufficient gas price to replace should have been rejected"
+    );
+
+    replacementRpc.maxPriorityFeePerGas = executableRpc.maxPriorityFeePerGas;
+    replacementRpc.maxFeePerGas = "0xffffffffff";
+    const replacementTx2 = TransactionFactory.fromRpc(replacementRpc, common);
+    // even if the maxFee is high enough, the tip isn't enough to replace, so it'll throw
+    await assert.rejects(
+      txPool.prepareTransaction(replacementTx2, secretKey),
+      {
+        code: -32003,
+        message: "transaction underpriced"
+      },
+      "replacement transaction with insufficient gas price to replace should have been rejected"
+    );
+
+    const legacyReplacementRpc: TypedRpcTransaction = {
+      from: from,
+      type: "0x0",
+      gasPrice: "0xffffffff",
+      gasLimit: "0xffff",
+      nonce: "0x0"
+    };
+    const replacementTx3 = TransactionFactory.fromRpc(
+      legacyReplacementRpc,
+      common
+    );
+    // the gasPrice is higher than the tip but lower than the maxFee, which isn't enough, so it'll throw
+    await assert.rejects(
+      txPool.prepareTransaction(replacementTx3, secretKey),
       {
         code: -32003,
         message: "transaction underpriced"
@@ -251,12 +296,19 @@ describe("transaction pool", async () => {
       executableTx.serialized.toString()
     );
 
+    // raise our replacement transaction's prices by exactly the price bump amount
+    const originalMaxFee = Quantity.from(executableRpc.maxFeePerGas).toBigInt();
+    const originalTip = Quantity.from(
+      executableRpc.maxPriorityFeePerGas
+    ).toBigInt();
+    const maxFeePremium = originalMaxFee + (originalMaxFee * priceBump) / 100n;
+    const tipPremium = originalTip + (originalTip * priceBump) / 100n;
     // our replacement transaction needs to have a sufficiently higher gasPrice
     const replacementRpc: TypedRpcTransaction = {
       from: from,
       type: "0x2",
-      maxFeePerGas: "0xffffffffff",
-      maxPriorityFeePerGas: "0xffffffff",
+      maxFeePerGas: Quantity.from(maxFeePremium).toString(),
+      maxPriorityFeePerGas: Quantity.from(tipPremium).toString(),
       gasLimit: "0xffff",
       nonce: "0x0"
     };
@@ -298,6 +350,7 @@ describe("transaction pool", async () => {
       from: from,
       type: "0x2",
       maxFeePerGas: "0xffffffffff",
+      maxPriorityFeePerGas: "0xffff",
       gasLimit: "0xffff",
       nonce: "0x2"
     };

src/chains/ethereum/options/src/miner-options.ts

@@ -158,6 +158,18 @@ export type MinerConfig = {
       type: Data;
       hasDefault: true;
     };
+
+    /**
+     * Minimum price bump percentage needed to replace a transaction that already exists in the transaction pool.
+     *
+     * @defaultValue ""
+     */
+    priceBump: {
+      type: bigint;
+      rawType: number | bigint;
+      hasDefault: true;
+      cliType: string;
+    };
   };
 };
 
@@ -262,5 +274,12 @@ export const MinerOptions: Definitions<MinerConfig> = {
     cliDescription: "Set the extraData block header field a miner can include.",
     default: () => DATA_EMPTY,
     cliType: "string"
+  },
+  priceBump: {
+    normalize: BigInt,
+    cliDescription:
+      "Minimum price bump percentage needed to replace a transaction that already exists in the transaction pool.",
+    default: () => 10n,
+    cliType: "string"
   }
 };

src/chains/ethereum/utils/src/errors/errors.ts

@@ -38,3 +38,9 @@ export const VM_EXCEPTION = "VM Exception while processing transaction: ";
  */
 export const VM_EXCEPTIONS =
   "Multiple VM Exceptions while processing transactions: : \n\n";
+
+/**
+ * Returned if a replacement transaction is sent while the potentially replaced transaction is being mined.
+ */
+export const TRANSACTION_LOCKED =
+  "transaction can't be replaced, mining has already started. (we don't really know how you got here, couldy you open an issue with reproduction steps? https://github.com/trufflesuite/ganache/issues/new)";