From c71ac28ea4b976124876ccd470b1afbd3f62a99f Mon Sep 17 00:00:00 2001 From: Phillip Ho Date: Wed, 10 Apr 2024 16:07:56 -0700 Subject: [PATCH 1/4] fix: Cancel tx if populating a tx to retry fails --- src/worker/tasks/retryTx.ts | 105 ++++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 41 deletions(-) diff --git a/src/worker/tasks/retryTx.ts b/src/worker/tasks/retryTx.ts index 7b681a462..a305863c1 100644 --- a/src/worker/tasks/retryTx.ts +++ b/src/worker/tasks/retryTx.ts @@ -4,6 +4,7 @@ import { prisma } from "../../db/client"; import { getTxToRetry } from "../../db/transactions/getTxToRetry"; import { updateTx } from "../../db/transactions/updateTx"; import { TransactionStatus } from "../../server/schemas/transaction"; +import { cancelTransactionAndUpdate } from "../../server/utils/transaction"; import { getConfig } from "../../utils/cache/getConfig"; import { getSdk } from "../../utils/cache/getSdk"; import { parseTxError } from "../../utils/errors"; @@ -32,7 +33,7 @@ export const retryTx = async () => { walletAddress: tx.fromAddress!, }); const provider = sdk.getProvider() as StaticJsonRpcBatchProvider; - const blockNumber = await sdk.getProvider().getBlockNumber(); + const blockNumber = await provider.getBlockNumber(); if ( blockNumber - tx.sentAtBlockNumber! <= @@ -58,8 +59,7 @@ export const retryTx = async () => { }); const gasOverrides = await getGasSettingsForRetry(tx, provider); - let res: ethers.providers.TransactionResponse; - const txRequest = { + const transactionRequestRaw = { to: tx.toAddress!, from: tx.fromAddress!, data: tx.data!, @@ -67,15 +67,20 @@ export const retryTx = async () => { value: tx.value!, ...gasOverrides, }; + + // Populate transaction. + let transactionRequest: ethers.providers.TransactionRequest; try { - res = await sdk.getSigner()!.sendTransaction(txRequest); - } catch (err: any) { - logger({ - service: "worker", - level: "error", + transactionRequest = await sdk + .getSigner()! + .populateTransaction(transactionRequestRaw); + } catch (err) { + // Error populating transaction. This transaction will revert onchain. + + // Consume the nonce. + await cancelTransactionAndUpdate({ queueId: tx.id, - message: `Failed to retry`, - error: err, + pgtx, }); await updateTx({ @@ -87,22 +92,40 @@ export const retryTx = async () => { }, }); - reportUsageForQueueIds.push({ - input: { - fromAddress: tx.fromAddress || undefined, - toAddress: tx.toAddress || undefined, - value: tx.value || undefined, - chainId: tx.chainId || undefined, - functionName: tx.functionName || undefined, - extension: tx.extension || undefined, - retryCount: tx.retryCount + 1 || 0, - provider: provider.connection.url || undefined, + reportUsage([ + { + input: { + fromAddress: tx.fromAddress || undefined, + toAddress: tx.toAddress || undefined, + value: tx.value || undefined, + chainId: tx.chainId, + functionName: tx.functionName || undefined, + extension: tx.extension || undefined, + retryCount: tx.retryCount + 1, + provider: provider.connection.url, + }, + action: UsageEventTxActionEnum.ErrorTx, }, - action: UsageEventTxActionEnum.ErrorTx, - }); + ]); - reportUsage(reportUsageForQueueIds); + return; + } + // Send transaction. + let transactionResponse: ethers.providers.TransactionResponse; + try { + transactionResponse = await sdk + .getSigner()! + .sendTransaction(transactionRequest); + } catch (err: any) { + // The RPC rejected this transaction. Re-attempt later. + logger({ + service: "worker", + level: "error", + queueId: tx.id, + message: "Failed to retry", + error: err, + }); return; } @@ -112,35 +135,35 @@ export const retryTx = async () => { data: { sentAt: new Date(), status: TransactionStatus.Sent, - res: txRequest, + res: transactionRequestRaw, sentAtBlockNumber: await sdk.getProvider().getBlockNumber(), retryCount: tx.retryCount + 1, - transactionHash: res.hash, + transactionHash: transactionResponse.hash, }, }); - reportUsageForQueueIds.push({ - input: { - fromAddress: tx.fromAddress || undefined, - toAddress: tx.toAddress || undefined, - value: tx.value || undefined, - chainId: tx.chainId || undefined, - functionName: tx.functionName || undefined, - extension: tx.extension || undefined, - retryCount: tx.retryCount + 1, - transactionHash: res.hash || undefined, - provider: provider.connection.url || undefined, + reportUsage([ + { + input: { + fromAddress: tx.fromAddress || undefined, + toAddress: tx.toAddress || undefined, + value: tx.value || undefined, + chainId: tx.chainId, + functionName: tx.functionName || undefined, + extension: tx.extension || undefined, + retryCount: tx.retryCount + 1, + transactionHash: transactionResponse.hash || undefined, + provider: provider.connection.url, + }, + action: UsageEventTxActionEnum.SendTx, }, - action: UsageEventTxActionEnum.SendTx, - }); - - reportUsage(reportUsageForQueueIds); + ]); logger({ service: "worker", level: "info", queueId: tx.id, - message: `Retried with hash ${res.hash} for nonce ${res.nonce}`, + message: `Retried with hash ${transactionResponse.hash} for nonce ${transactionResponse.nonce}`, }); }, { From 54238ef9da091a510726282b1edbed0b7a1bb27b Mon Sep 17 00:00:00 2001 From: Phillip Ho Date: Wed, 10 Apr 2024 16:23:59 -0700 Subject: [PATCH 2/4] ensure cancel uses high enough gas --- src/server/utils/transaction.ts | 52 ++++++++++++++----------------- src/worker/tasks/retryTx.ts | 7 +---- src/worker/tasks/updateMinedTx.ts | 3 +- 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/server/utils/transaction.ts b/src/server/utils/transaction.ts index 0407567cf..d37eec414 100644 --- a/src/server/utils/transaction.ts +++ b/src/server/utils/transaction.ts @@ -1,10 +1,11 @@ -import { getDefaultGasOverrides } from "@thirdweb-dev/sdk"; +import { StaticJsonRpcProvider } from "@ethersproject/providers"; +import { Transactions } from "@prisma/client"; 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 { getGasSettingsForRetry } from "../../utils/gas"; import { createCustomError } from "../middleware/error"; import { TransactionStatus } from "../schemas/transaction"; @@ -65,11 +66,7 @@ export const cancelTransactionAndUpdate = async ({ switch (txData.status) { case TransactionStatus.Errored: { if (txData.chainId && txData.fromAddress && txData.nonce) { - const { message, transactionHash } = await sendNullTransaction({ - chainId: parseInt(txData.chainId), - walletAddress: txData.fromAddress, - nonce: txData.nonce, - }); + const { message, transactionHash } = await cancelTransaction(txData); if (transactionHash) { await updateTx({ queueId, @@ -114,11 +111,7 @@ export const cancelTransactionAndUpdate = async ({ ); case TransactionStatus.Sent: { if (txData.chainId && txData.fromAddress && txData.nonce) { - const { message, transactionHash } = await sendNullTransaction({ - chainId: parseInt(txData.chainId), - walletAddress: txData.fromAddress, - nonce: txData.nonce, - }); + const { message, transactionHash } = await cancelTransaction(txData); if (transactionHash) { await updateTx({ queueId, @@ -138,37 +131,40 @@ export const cancelTransactionAndUpdate = async ({ throw new Error("Unhandled cancellation state."); }; -const sendNullTransaction = async (args: { - chainId: number; - walletAddress: string; - nonce: number; - transactionHash?: string; -}): Promise<{ +const cancelTransaction = async ( + tx: Transactions, +): Promise<{ message: string; transactionHash?: string; }> => { - const { chainId, walletAddress, nonce, transactionHash } = args; + if (!tx.fromAddress || !tx.nonce) { + return { message: `Invalid transaction state to cancel. (${tx.id})` }; + } - const sdk = await getSdk({ chainId, walletAddress }); - const provider = sdk.getProvider(); + const sdk = await getSdk({ + chainId: parseInt(tx.chainId), + walletAddress: tx.fromAddress, + }); + const provider = sdk.getProvider() as StaticJsonRpcProvider; // Skip if the transaction is already mined. - if (transactionHash) { - const receipt = await provider.getTransactionReceipt(transactionHash); + if (tx.transactionHash) { + const receipt = await provider.getTransactionReceipt(tx.transactionHash); if (receipt) { return { message: "Transaction already mined." }; } } try { - const gasOverrides = await getDefaultGasOverrides(provider); + const gasOptions = await getGasSettingsForRetry(tx, provider); + // Send 0 currency to self. const { hash } = await sdk.wallet.sendRawTransaction({ - to: walletAddress, - from: walletAddress, + to: tx.fromAddress, + from: tx.fromAddress, data: "0x", value: "0", - nonce, - ...multiplyGasOverrides(gasOverrides, 2), + nonce: tx.nonce, + ...gasOptions, }); return { diff --git a/src/worker/tasks/retryTx.ts b/src/worker/tasks/retryTx.ts index a305863c1..1a1e4367c 100644 --- a/src/worker/tasks/retryTx.ts +++ b/src/worker/tasks/retryTx.ts @@ -10,11 +10,7 @@ import { getSdk } from "../../utils/cache/getSdk"; import { parseTxError } from "../../utils/errors"; import { getGasSettingsForRetry } from "../../utils/gas"; import { logger } from "../../utils/logger"; -import { - ReportUsageParams, - UsageEventTxActionEnum, - reportUsage, -} from "../../utils/usage"; +import { UsageEventTxActionEnum, reportUsage } from "../../utils/usage"; export const retryTx = async () => { try { @@ -27,7 +23,6 @@ export const retryTx = async () => { } const config = await getConfig(); - const reportUsageForQueueIds: ReportUsageParams[] = []; const sdk = await getSdk({ chainId: parseInt(tx.chainId!), walletAddress: tx.fromAddress!, diff --git a/src/worker/tasks/updateMinedTx.ts b/src/worker/tasks/updateMinedTx.ts index e3cf58a0e..b9a0e472b 100644 --- a/src/worker/tasks/updateMinedTx.ts +++ b/src/worker/tasks/updateMinedTx.ts @@ -7,6 +7,7 @@ import { updateTx } from "../../db/transactions/updateTx"; import { TransactionStatus } from "../../server/schemas/transaction"; import { cancelTransactionAndUpdate } from "../../server/utils/transaction"; import { getSdk } from "../../utils/cache/getSdk"; +import { msSince } from "../../utils/date"; import { logger } from "../../utils/logger"; import { ReportUsageParams, @@ -65,7 +66,7 @@ export const updateMinedTx = async () => { chainId: tx.chainId || undefined, transactionHash: tx.transactionHash || undefined, provider: provider.connection.url || undefined, - msSinceSend: Date.now() - tx.sentAt!.getTime(), + msSinceSend: msSince(tx.sentAt!), }, action: UsageEventTxActionEnum.CancelTx, }); From 4338294c0d864e25398cc5b2854e651cfb2038ae Mon Sep 17 00:00:00 2001 From: Phillip Ho Date: Wed, 10 Apr 2024 16:31:21 -0700 Subject: [PATCH 3/4] fix build --- src/db/transactions/cleanTxs.ts | 10 +++---- src/server/schemas/transaction/index.ts | 38 ------------------------- src/server/utils/transaction.ts | 36 ++++++++++++++++------- 3 files changed, 29 insertions(+), 55 deletions(-) diff --git a/src/db/transactions/cleanTxs.ts b/src/db/transactions/cleanTxs.ts index 064f334e1..5875c2d1e 100644 --- a/src/db/transactions/cleanTxs.ts +++ b/src/db/transactions/cleanTxs.ts @@ -15,16 +15,14 @@ export const cleanTxs = ( sentAt: tx.sentAt?.toISOString() || null, minedAt: tx.minedAt?.toISOString() || null, cancelledAt: tx.cancelledAt?.toISOString() || null, - status: !!tx.errorMessage + status: tx.errorMessage ? "errored" - : !!tx.minedAt + : tx.minedAt ? "mined" - : !!tx.cancelledAt + : tx.cancelledAt ? "cancelled" - : !!tx.sentAt && tx.retryCount === 0 + : tx.sentAt ? "sent" - : !!tx.sentAt && tx.retryCount > 0 - ? "retried" : "queued", }; }); diff --git a/src/server/schemas/transaction/index.ts b/src/server/schemas/transaction/index.ts index 88f363891..11fc00f0f 100644 --- a/src/server/schemas/transaction/index.ts +++ b/src/server/schemas/transaction/index.ts @@ -198,41 +198,3 @@ export enum TransactionStatus { // Tx was cancelled and will not be re-attempted. Cancelled = "cancelled", } - -export interface TransactionSchema { - identifier?: string; - walletAddress?: string; - contractAddress?: string; - chainId?: string; - extension?: string; - rawFunctionName?: string; - rawFunctionArgs?: string; - txProcessed?: boolean; - txSubmitted?: boolean; - txErrored?: boolean; - txMined?: boolean; - encodedInputData?: string; - txType?: number; - gasPrice?: string; - gasLimit?: string; - maxPriorityFeePerGas?: string; - maxFeePerGas?: string; - txHash?: string; - status?: string; - createdTimestamp?: Date; - txSubmittedTimestamp?: Date; - txProcessedTimestamp?: Date; - submittedTxNonce?: number; - deployedContractAddress?: string; - contractType?: string; - txValue?: string; - errorMessage?: string; - txMinedTimestamp?: Date; - blockNumber?: number; - toAddress?: string; - txSubmittedAtBlockNumber?: number; - numberOfRetries?: number; - overrideGasValuesForTx?: boolean; - overrideMaxFeePerGas?: string; - overrideMaxPriorityFeePerGas?: string; -} diff --git a/src/server/utils/transaction.ts b/src/server/utils/transaction.ts index d37eec414..efd7d9f8d 100644 --- a/src/server/utils/transaction.ts +++ b/src/server/utils/transaction.ts @@ -1,7 +1,7 @@ import { StaticJsonRpcProvider } from "@ethersproject/providers"; import { Transactions } from "@prisma/client"; import { StatusCodes } from "http-status-codes"; -import { getTxById } from "../../db/transactions/getTxById"; +import { prisma } from "../../db/client"; import { updateTx } from "../../db/transactions/updateTx"; import { PrismaTransaction } from "../../schema/prisma"; import { getSdk } from "../../utils/cache/getSdk"; @@ -18,15 +18,29 @@ export const cancelTransactionAndUpdate = async ({ queueId, pgtx, }: CancelTransactionAndUpdateParams) => { - const txData = await getTxById({ queueId, pgtx }); - if (!txData) { + const tx = await prisma.transactions.findUnique({ + where: { + id: queueId, + }, + }); + if (!tx) { return { message: `Transaction ${queueId} not found.`, }; } - if (txData.signerAddress && txData.accountAddress) { - switch (txData.status) { + const status: TransactionStatus = tx.errorMessage + ? TransactionStatus.Errored + : tx.minedAt + ? TransactionStatus.Mined + : tx.cancelledAt + ? TransactionStatus.Cancelled + : tx.sentAt + ? TransactionStatus.Sent + : TransactionStatus.Queued; + + if (tx.signerAddress && tx.accountAddress) { + switch (status) { case TransactionStatus.Errored: throw createCustomError( `Cannot cancel user operation because it already errored`, @@ -63,10 +77,10 @@ export const cancelTransactionAndUpdate = async ({ }; } } else { - switch (txData.status) { + switch (status) { case TransactionStatus.Errored: { - if (txData.chainId && txData.fromAddress && txData.nonce) { - const { message, transactionHash } = await cancelTransaction(txData); + if (tx.chainId && tx.fromAddress && tx.nonce) { + const { message, transactionHash } = await cancelTransaction(tx); if (transactionHash) { await updateTx({ queueId, @@ -81,7 +95,7 @@ export const cancelTransactionAndUpdate = async ({ } throw createCustomError( - `Transaction has already errored: ${txData.errorMessage}`, + `Transaction has already errored: ${tx.errorMessage}`, StatusCodes.BAD_REQUEST, "TransactionErrored", ); @@ -110,8 +124,8 @@ export const cancelTransactionAndUpdate = async ({ "TransactionAlreadyMined", ); case TransactionStatus.Sent: { - if (txData.chainId && txData.fromAddress && txData.nonce) { - const { message, transactionHash } = await cancelTransaction(txData); + if (tx.chainId && tx.fromAddress && tx.nonce) { + const { message, transactionHash } = await cancelTransaction(tx); if (transactionHash) { await updateTx({ queueId, From 4173a18f3a200d7cc7a5c3935c03c30074e3069e Mon Sep 17 00:00:00 2001 From: Phillip Ho Date: Wed, 10 Apr 2024 17:14:16 -0700 Subject: [PATCH 4/4] combine prepare + send flows again --- src/worker/tasks/retryTx.ts | 44 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/worker/tasks/retryTx.ts b/src/worker/tasks/retryTx.ts index 1a1e4367c..95990cece 100644 --- a/src/worker/tasks/retryTx.ts +++ b/src/worker/tasks/retryTx.ts @@ -54,7 +54,7 @@ export const retryTx = async () => { }); const gasOverrides = await getGasSettingsForRetry(tx, provider); - const transactionRequestRaw = { + const transactionRequest = { to: tx.toAddress!, from: tx.fromAddress!, data: tx.data!, @@ -63,21 +63,27 @@ export const retryTx = async () => { ...gasOverrides, }; - // Populate transaction. - let transactionRequest: ethers.providers.TransactionRequest; + // Send transaction. + let transactionResponse: ethers.providers.TransactionResponse; try { - transactionRequest = await sdk + transactionResponse = await sdk .getSigner()! - .populateTransaction(transactionRequestRaw); - } catch (err) { - // Error populating transaction. This transaction will revert onchain. + .sendTransaction(transactionRequest); + } catch (err: any) { + // The RPC rejected this transaction. + logger({ + service: "worker", + level: "error", + queueId: tx.id, + message: "Failed to retry", + error: err, + }); // Consume the nonce. await cancelTransactionAndUpdate({ queueId: tx.id, pgtx, }); - await updateTx({ pgtx, queueId: tx.id, @@ -106,32 +112,14 @@ export const retryTx = async () => { return; } - // Send transaction. - let transactionResponse: ethers.providers.TransactionResponse; - try { - transactionResponse = await sdk - .getSigner()! - .sendTransaction(transactionRequest); - } catch (err: any) { - // The RPC rejected this transaction. Re-attempt later. - logger({ - service: "worker", - level: "error", - queueId: tx.id, - message: "Failed to retry", - error: err, - }); - return; - } - await updateTx({ pgtx, queueId: tx.id, data: { sentAt: new Date(), status: TransactionStatus.Sent, - res: transactionRequestRaw, - sentAtBlockNumber: await sdk.getProvider().getBlockNumber(), + res: transactionRequest, + sentAtBlockNumber: await provider.getBlockNumber(), retryCount: tx.retryCount + 1, transactionHash: transactionResponse.hash, },