diff --git a/.changeset/three-flies-fly.md b/.changeset/three-flies-fly.md new file mode 100644 index 0000000000..043634fae0 --- /dev/null +++ b/.changeset/three-flies-fly.md @@ -0,0 +1,5 @@ +--- +"@latticexyz/common": patch +--- + +Refactored `writeContract` and `sendTransaction` actions for simplicity and better error messages. diff --git a/packages/common/src/createNonceManager.ts b/packages/common/src/createNonceManager.ts index 87806ec578..3c48eb72eb 100644 --- a/packages/common/src/createNonceManager.ts +++ b/packages/common/src/createNonceManager.ts @@ -30,7 +30,7 @@ export function createNonceManager({ broadcastChannelName, queueConcurrency = 1, }: CreateNonceManagerOptions): CreateNonceManagerResult { - const nonceRef = { current: -1 }; + const ref = { nonce: -1, noncePromise: null as Promise | null }; let channel: BroadcastChannel | null = null; if (typeof BroadcastChannel !== "undefined") { @@ -43,32 +43,35 @@ export function createNonceManager({ channel.addEventListener("message", (event) => { const nonce = JSON.parse(event.data); debug("got nonce from broadcast channel", nonce); - nonceRef.current = nonce; + ref.nonce = nonce; }); }); } function hasNonce(): boolean { - return nonceRef.current >= 0; + return ref.nonce >= 0; } function getNonce(): number { if (!hasNonce()) throw new Error("call resetNonce before using getNonce"); - return nonceRef.current; + return ref.nonce; } function nextNonce(): number { if (!hasNonce()) throw new Error("call resetNonce before using nextNonce"); - const nonce = nonceRef.current++; - channel?.postMessage(JSON.stringify(nonceRef.current)); + const nonce = ref.nonce++; + channel?.postMessage(JSON.stringify(ref.nonce)); return nonce; } async function resetNonce(): Promise { - const nonce = await getTransactionCount(client, { address, blockTag }); - nonceRef.current = nonce; - channel?.postMessage(JSON.stringify(nonceRef.current)); - debug("reset nonce to", nonceRef.current); + ref.noncePromise ??= (async (): Promise => { + ref.nonce = await getTransactionCount(client, { address, blockTag }); + ref.noncePromise = null; + channel?.postMessage(JSON.stringify(ref.nonce)); + debug("reset nonce to", ref.nonce); + })(); + await ref.noncePromise; } function shouldResetNonce(error: unknown): boolean { diff --git a/packages/common/src/getNonceManager.ts b/packages/common/src/getNonceManager.ts index 1209894863..f006f53e57 100644 --- a/packages/common/src/getNonceManager.ts +++ b/packages/common/src/getNonceManager.ts @@ -11,12 +11,14 @@ export async function getNonceManager({ }: CreateNonceManagerOptions): Promise { const id = await getNonceManagerId({ client, address, blockTag }); - const existingNonceManager = nonceManagers.get(id); - if (existingNonceManager) { - return existingNonceManager; + const nonceManager = nonceManagers.get(id) ?? createNonceManager({ client, address, blockTag, ...opts }); + if (!nonceManagers.has(id)) { + nonceManagers.set(id, nonceManager); + } + + if (!nonceManager.hasNonce()) { + await nonceManager.resetNonce(); } - const nonceManager = createNonceManager({ client, address, blockTag, ...opts }); - nonceManagers.set(id, nonceManager); return nonceManager; } diff --git a/packages/common/src/sendTransaction.ts b/packages/common/src/sendTransaction.ts index 9d323015fe..349a68cd1a 100644 --- a/packages/common/src/sendTransaction.ts +++ b/packages/common/src/sendTransaction.ts @@ -1,6 +1,5 @@ import { Account, - CallParameters, Chain, Client, SendTransactionParameters, @@ -8,7 +7,7 @@ import { SendTransactionReturnType, PublicClient, } from "viem"; -import { call, sendTransaction as viem_sendTransaction } from "viem/actions"; +import { sendTransaction as viem_sendTransaction } from "viem/actions"; import pRetry from "p-retry"; import { debug as parentDebug } from "./debug"; import { getNonceManager } from "./getNonceManager"; @@ -65,51 +64,31 @@ export async function sendTransaction< args: { chain }, }); - async function prepare(): Promise> { - if (request.gas) { - debug("gas provided, skipping simulate", request.to); - return request; - } - - debug("simulating tx to", request.to); - await call(opts.publicClient ?? client, { - ...request, - blockTag: "pending", - account, - } as CallParameters); - - return request; - } - return await nonceManager.mempoolQueue.add( () => pRetry( async () => { - const preparedRequest = await prepare(); - - if (!nonceManager.hasNonce()) { - await nonceManager.resetNonce(); - } - const nonce = nonceManager.nextNonce(); - debug("sending tx with nonce", nonce, "to", preparedRequest.to); - const parameters: SendTransactionParameters = { - ...preparedRequest, + const params: SendTransactionParameters = { + ...request, nonce, ...feeRef.fees, }; - return await viem_sendTransaction(client, parameters); + debug("sending tx to", request.to, "with nonce", nonce); + return await viem_sendTransaction(client, params); }, { retries: 3, onFailedAttempt: async (error) => { - // On nonce errors, reset the nonce and retry + // in case this tx failed before hitting the mempool (i.e. gas estimation error), reset nonce so we don't skip past the unused nonce + debug("failed, resetting nonce"); + await nonceManager.resetNonce(); + // retry nonce errors + // TODO: upgrade p-retry and move this to shouldRetry if (nonceManager.shouldResetNonce(error)) { debug("got nonce error, retrying", error.message); - await nonceManager.resetNonce(); return; } - // TODO: prepare again if there are gas errors? throw error; }, }, diff --git a/packages/common/src/writeContract.ts b/packages/common/src/writeContract.ts index 676ee4ad09..4dd2e2d283 100644 --- a/packages/common/src/writeContract.ts +++ b/packages/common/src/writeContract.ts @@ -9,13 +9,8 @@ import { ContractFunctionName, ContractFunctionArgs, PublicClient, - encodeFunctionData, - EncodeFunctionDataParameters, } from "viem"; -import { - prepareTransactionRequest as viem_prepareTransactionRequest, - writeContract as viem_writeContract, -} from "viem/actions"; +import { writeContract as viem_writeContract } from "viem/actions"; import pRetry from "p-retry"; import { debug as parentDebug } from "./debug"; import { getNonceManager } from "./getNonceManager"; @@ -63,11 +58,6 @@ export async function writeContract< const account = parseAccount(rawAccount); const chain = client.chain; - const defaultParameters = { - chain, - ...(chain?.fees ? { type: "eip1559" } : {}), - } satisfies Omit; - const nonceManager = await getNonceManager({ client: opts.publicClient ?? client, address: account.address, @@ -81,73 +71,31 @@ export async function writeContract< args: { chain }, }); - async function prepare(): Promise> { - if (request.gas) { - debug("gas provided, skipping preparation", request.functionName, request.address); - return request; - } - - const { abi, address, args, dataSuffix, functionName } = request; - const data = encodeFunctionData({ - abi, - args, - functionName, - } as EncodeFunctionDataParameters); - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { nonce, maxFeePerGas, maxPriorityFeePerGas, ...preparedTransaction } = await getAction( - client, - viem_prepareTransactionRequest, - "prepareTransactionRequest", - )({ - // The fee values don't need to be accurate for gas estimation - // and we can save a couple rpc calls by providing stubs here. - // These are later overridden with accurate values from `feeRef`. - maxFeePerGas: 0n, - maxPriorityFeePerGas: 0n, - // Send the current nonce without increasing the stored value - nonce: nonceManager.getNonce(), - ...defaultParameters, - ...request, - blockTag: "pending", - account, - // From `viem/writeContract` - data: `${data}${dataSuffix ? dataSuffix.replace("0x", "") : ""}`, - to: address, - } as never); - - return preparedTransaction as never; - } - return nonceManager.mempoolQueue.add( () => pRetry( async () => { - if (!nonceManager.hasNonce()) { - await nonceManager.resetNonce(); - } - - // We estimate gas before increasing the local nonce to prevent nonce gaps. - // Invalid transactions fail the gas estimation step are never submitted - // to the network, so they should not increase the nonce. - const preparedRequest = await prepare(); - const nonce = nonceManager.nextNonce(); - - const fullRequest = { ...preparedRequest, nonce, ...feeRef.fees }; - debug("calling", fullRequest.functionName, "with nonce", nonce, "at", fullRequest.address); - return await viem_writeContract(client, fullRequest as never); + const params: WriteContractParameters = { + ...request, + nonce, + ...feeRef.fees, + }; + debug("calling", params.functionName, "at", params.address, "with nonce", nonce); + return await getAction(client, viem_writeContract, "writeContract")(params); }, { retries: 3, onFailedAttempt: async (error) => { - // On nonce errors, reset the nonce and retry + // in case this tx failed before hitting the mempool (i.e. gas estimation error), reset nonce so we don't skip past the unused nonce + debug("failed, resetting nonce"); + await nonceManager.resetNonce(); + // retry nonce errors + // TODO: upgrade p-retry and move this to shouldRetry if (nonceManager.shouldResetNonce(error)) { debug("got nonce error, retrying", error.message); - await nonceManager.resetNonce(); return; } - // TODO: prepareWrite again if there are gas errors? throw error; }, },