fix: remove gas limit on retries (#442)

* wip * updates for cancelTx * helper to multiple gas overrides * fix: add tests * better typing * update server language --------- Co-authored-by: farhanW3 <[email protected]>
thirdweb-dev · Mar 6, 2024 · 196ef61 · 196ef61
1 parent 6eb255d
commit 196ef61
Show file tree

Hide file tree

Showing 8 changed files with 141 additions and 106 deletions.
diff --git a/src/db/configuration/getConfiguration.ts b/src/db/configuration/getConfiguration.ts
@@ -178,13 +178,13 @@ export const getConfiguration = async (): Promise<Config> => {
         maxTxsToProcess: 30,
         minedTxListenerCronSchedule: "*/5 * * * * *",
         maxTxsToUpdate: 50,
-        retryTxListenerCronSchedule: "*/30 * * * * *",
-        minEllapsedBlocksBeforeRetry: 15,
+        retryTxListenerCronSchedule: "*/15 * * * * *",
+        minEllapsedBlocksBeforeRetry: 12,
         maxFeePerGasForRetries: ethers.utils
-          .parseUnits("1000", "gwei")
+          .parseUnits("10000", "gwei")
           .toString(),
         maxPriorityFeePerGasForRetries: ethers.utils
-          .parseUnits("1000", "gwei")
+          .parseUnits("10000", "gwei")
           .toString(),
         maxRetriesPerTx: 3,
         authDomain: "thirdweb.com",

diff --git a/src/server/index.ts b/src/server/index.ts
@@ -92,9 +92,11 @@ export const initServer = async () => {
   logger({
     service: "server",
     level: "info",
-    message: `Listening on ${env.ENABLE_HTTPS ? "https://" : "http://"}${
-      env.HOST
-    }:${env.PORT}`,
+    message: `Listening on ${
+      env.ENABLE_HTTPS ? "https://" : "http://"
+    }localhost:${
+      env.PORT
+    }. Manage your Engine from https://thirdweb.com/dashboard/engine.`,
   });
 
   writeOpenApiToFile(server);

diff --git a/src/server/utils/transaction.ts b/src/server/utils/transaction.ts
@@ -1,21 +1,24 @@
 import { TransactionResponse } from "@ethersproject/abstract-provider";
 import { getDefaultGasOverrides } from "@thirdweb-dev/sdk";
-import { BigNumber } from "ethers";
 import { StatusCodes } from "http-status-codes";
 import { getTxById } from "../../db/transactions/getTxById";
 import { updateTx } from "../../db/transactions/updateTx";
+import { PrismaTransaction } from "../../schema/prisma";
 import { getSdk } from "../../utils/cache/getSdk";
+import { multiplyGasOverrides } from "../../utils/gas";
 import { createCustomError } from "../middleware/error";
 import { TransactionStatusEnum } from "../schemas/transaction";
 
 interface CancelTransactionAndUpdateParams {
   queueId: string;
+  pgtx?: PrismaTransaction;
 }
 
 export const cancelTransactionAndUpdate = async ({
   queueId,
+  pgtx,
 }: CancelTransactionAndUpdateParams) => {
-  const txData = await getTxById({ queueId });
+  const txData = await getTxById({ queueId, pgtx });
   if (!txData) {
     return {
       message: `Transaction ${queueId} not found.`,
@@ -67,30 +70,31 @@ export const cancelTransactionAndUpdate = async ({
     switch (txData.status) {
       case TransactionStatusEnum.Errored:
         error = createCustomError(
-          `Cannot cancel errored transaction with queueId ${queueId}. Error: ${txData.errorMessage}`,
+          `Transaction has already errored: ${txData.errorMessage}`,
           StatusCodes.BAD_REQUEST,
           "TransactionErrored",
         );
         break;
       case TransactionStatusEnum.Cancelled:
         error = createCustomError(
-          `Transaction already cancelled with queueId ${queueId}`,
+          "Transaction is already cancelled.",
           StatusCodes.BAD_REQUEST,
           "TransactionAlreadyCancelled",
         );
         break;
       case TransactionStatusEnum.Queued:
         await updateTx({
           queueId,
+          pgtx,
           data: {
             status: TransactionStatusEnum.Cancelled,
           },
         });
-        message = "Transaction cancelled on-database successfully.";
+        message = "Transaction cancelled successfully.";
         break;
       case TransactionStatusEnum.Mined:
         error = createCustomError(
-          `Transaction already mined with queueId ${queueId}`,
+          "Transaction already mined.",
           StatusCodes.BAD_REQUEST,
           "TransactionAlreadyMined",
         );
@@ -105,10 +109,8 @@ export const cancelTransactionAndUpdate = async ({
         const txReceipt = await sdk
           .getProvider()
           .getTransactionReceipt(txData.transactionHash!);
-
         if (txReceipt) {
-          message =
-            "Transaction already mined. Cannot cancel transaction on-chain.";
+          message = "Transaction already mined.";
           break;
         }
 
@@ -119,17 +121,14 @@ export const cancelTransactionAndUpdate = async ({
           data: "0x",
           value: "0x00",
           nonce: txData.nonce!,
-          ...gasOverrides,
-          maxFeePerGas: BigNumber.from(gasOverrides.maxFeePerGas).mul(2),
-          maxPriorityFeePerGas: BigNumber.from(
-            gasOverrides.maxPriorityFeePerGas,
-          ).mul(2),
+          ...multiplyGasOverrides(gasOverrides, 2),
         });
 
-        message = "Cancellation Transaction sent on chain successfully.";
+        message = "Transaction cancelled successfully.";
 
         await updateTx({
           queueId,
+          pgtx,
           data: {
             status: TransactionStatusEnum.Cancelled,
           },

diff --git a/src/tests/gas.test.ts b/src/tests/gas.test.ts
@@ -1,7 +1,7 @@
 import { Transactions } from ".prisma/client";
 import { getDefaultGasOverrides } from "@thirdweb-dev/sdk";
 import { BigNumber, ethers, providers } from "ethers";
-import { getGasSettingsForRetry } from "../utils/gas";
+import { getGasSettingsForRetry, multiplyGasOverrides } from "../utils/gas";
 
 jest.mock("@thirdweb-dev/sdk");
 const mockGetDefaultGasOverrides =
@@ -139,3 +139,36 @@ describe("getGasSettingsForRetry", () => {
     });
   });
 });
+
+describe("multiplyGasOverrides", () => {
+  const gasOverridesLegacy = {
+    gasPrice: BigNumber.from(100),
+  };
+  const gasOverridesEip1155 = {
+    maxFeePerGas: BigNumber.from(50),
+    maxPriorityFeePerGas: BigNumber.from(10),
+  };
+
+  it("should multiply gasPrice by given factor", () => {
+    const result = multiplyGasOverrides(gasOverridesLegacy, 2);
+    expect(result.gasPrice).toEqual(BigNumber.from(200));
+  });
+
+  it("should multiply maxFeePerGas and maxPriorityFeePerGas by given factor", () => {
+    const result = multiplyGasOverrides(gasOverridesEip1155, 3);
+    expect(result.maxFeePerGas).toEqual(BigNumber.from(150));
+    expect(result.maxPriorityFeePerGas).toEqual(BigNumber.from(30));
+  });
+
+  it("should handle non-integer multiplication factor", () => {
+    const result = multiplyGasOverrides(gasOverridesEip1155, 1.5);
+    expect(result.maxFeePerGas).toEqual(BigNumber.from(75));
+    expect(result.maxPriorityFeePerGas).toEqual(BigNumber.from(15));
+  });
+
+  it("should handle multiplication by zero", () => {
+    const result = multiplyGasOverrides(gasOverridesEip1155, 0);
+    expect(result.maxFeePerGas).toEqual(BigNumber.from(0));
+    expect(result.maxPriorityFeePerGas).toEqual(BigNumber.from(0));
+  });
+});
diff --git a/src/utils/gas.ts b/src/utils/gas.ts
@@ -3,6 +3,10 @@ import { getDefaultGasOverrides } from "@thirdweb-dev/sdk";
 import { BigNumber, providers } from "ethers";
 import { maxBN } from "./bigNumber";
 
+type gasOverridesReturnType = Awaited<
+  ReturnType<typeof getDefaultGasOverrides>
+>;
+
 /**
  *
  * @param tx
@@ -12,43 +16,70 @@ import { maxBN } from "./bigNumber";
 export const getGasSettingsForRetry = async (
   tx: Transactions,
   provider: providers.StaticJsonRpcProvider,
-): ReturnType<typeof getDefaultGasOverrides> => {
-  // Default: get gas settings from chain.
-  const { gasPrice, maxFeePerGas, maxPriorityFeePerGas } =
-    await getDefaultGasOverrides(provider);
+): Promise<gasOverridesReturnType> => {
+  // Default: Use 2x gas settings from chain.
+  const defaultGasOverrides = multiplyGasOverrides(
+    await getDefaultGasOverrides(provider),
+    2,
+  );
 
   // Handle legacy gas format.
-  if (gasPrice) {
-    const newGasPrice = gasPrice.mul(2);
+  if (defaultGasOverrides.gasPrice) {
     // Gas settings must be 10% higher than a previous attempt.
-    const minGasPrice = BigNumber.from(tx.gasPrice!).mul(110).div(100);
-
+    const minGasOverrides = multiplyGasOverrides(
+      {
+        gasPrice: BigNumber.from(tx.gasPrice),
+      },
+      1.1,
+    );
     return {
-      gasPrice: maxBN(newGasPrice, minGasPrice),
+      gasPrice: maxBN(defaultGasOverrides.gasPrice, minGasOverrides.gasPrice),
     };
   }
 
   // Handle EIP 1559 gas format.
-  let newMaxFeePerGas = maxFeePerGas.mul(2);
-  let newMaxPriorityFeePerGas = maxPriorityFeePerGas.mul(2);
-
   if (tx.retryGasValues) {
     // If this tx is manually retried, override with provided gas settings.
-    newMaxFeePerGas = BigNumber.from(tx.retryMaxFeePerGas!);
-    newMaxPriorityFeePerGas = BigNumber.from(tx.retryMaxPriorityFeePerGas!);
+    defaultGasOverrides.maxFeePerGas = BigNumber.from(tx.retryMaxFeePerGas!);
+    defaultGasOverrides.maxPriorityFeePerGas = BigNumber.from(
+      tx.retryMaxPriorityFeePerGas!,
+    );
   }
 
-  // Gas settings muset be 10% higher than a previous attempt.
-  const minMaxFeePerGas = BigNumber.from(tx.maxFeePerGas!).mul(110).div(100);
-  const minMaxPriorityFeePerGas = BigNumber.from(tx.maxPriorityFeePerGas!)
-    .mul(110)
-    .div(100);
-
+  // Gas settings must be 10% higher than a previous attempt.
+  const minGasOverrides = multiplyGasOverrides(
+    {
+      maxFeePerGas: BigNumber.from(tx.maxFeePerGas!),
+      maxPriorityFeePerGas: BigNumber.from(tx.maxPriorityFeePerGas!),
+    },
+    1.1,
+  );
   return {
-    maxFeePerGas: maxBN(newMaxFeePerGas, minMaxFeePerGas),
+    maxFeePerGas: maxBN(
+      defaultGasOverrides.maxFeePerGas,
+      minGasOverrides.maxFeePerGas,
+    ),
     maxPriorityFeePerGas: maxBN(
-      newMaxPriorityFeePerGas,
-      minMaxPriorityFeePerGas,
+      defaultGasOverrides.maxPriorityFeePerGas,
+      minGasOverrides.maxPriorityFeePerGas,
     ),
   };
 };
+
+export const multiplyGasOverrides = <T extends gasOverridesReturnType>(
+  gasOverrides: T,
+  mul: number,
+): T => {
+  if (gasOverrides.gasPrice) {
+    return {
+      gasPrice: multiplyBN(gasOverrides.gasPrice, mul),
+    } as T;
+  }
+  return {
+    maxFeePerGas: multiplyBN(gasOverrides.maxFeePerGas, mul),
+    maxPriorityFeePerGas: multiplyBN(gasOverrides.maxPriorityFeePerGas, mul),
+  } as T;
+};
+
+const multiplyBN = (n: BigNumber, mul: number): BigNumber =>
+  Number.isInteger(mul) ? n.mul(mul) : n.mul(mul * 10_000).div(10_000);
diff --git a/src/worker/tasks/processTx.ts b/src/worker/tasks/processTx.ts
@@ -153,7 +153,7 @@ export const processTx = async () => {
             await Promise.all([
               sdk.wallet.getNonce("pending"),
               getDefaultGasOverrides(provider),
-              await provider.getBlockNumber(),
+              provider.getBlockNumber(),
             ]);
 
           // - Take the larger of the nonces, and update database nonce to mempool value if mempool is greater

diff --git a/src/worker/tasks/retryTx.ts b/src/worker/tasks/retryTx.ts
@@ -21,6 +21,7 @@ export const retryTx = async () => {
       async (pgtx) => {
         const tx = await getTxToRetry({ pgtx });
         if (!tx) {
+          // Nothing to retry.
           return;
         }
 
@@ -49,30 +50,14 @@ export const retryTx = async () => {
           return;
         }
 
-        const gasOverrides = await getGasSettingsForRetry(tx, provider);
-        if (
-          gasOverrides.maxFeePerGas?.gt(config.maxFeePerGasForRetries) ||
-          gasOverrides.maxPriorityFeePerGas?.gt(
-            config.maxPriorityFeePerGasForRetries,
-          )
-        ) {
-          // Return if gas settings exceed configured limits. Try again later.
-          logger({
-            service: "worker",
-            level: "warn",
-            queueId: tx.id,
-            message: `${tx.chainId} chain gas price is higher than maximum threshold MaxFeePerGas: ${config.maxFeePerGasForRetries}, MaxPriorityFeePerGas: ${config.maxPriorityFeePerGasForRetries}`,
-          });
-          return;
-        }
-
         logger({
           service: "worker",
           level: "info",
           queueId: tx.id,
           message: `Retrying with nonce ${tx.nonce}`,
         });
 
+        const gasOverrides = await getGasSettingsForRetry(tx, provider);
         let res: ethers.providers.TransactionResponse;
         const txRequest = {
           to: tx.toAddress!,