refactor(common): simplify writeContract/sendTransaction actions

.changeset/three-flies-fly.md

@@ -0,0 +1,5 @@
+---
+"@latticexyz/common": patch
+---
+
+Refactored `writeContract` and `sendTransaction` actions for simplicity and better error messages.

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<void> | 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<void> {
-    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<void> => {
+      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 {

packages/common/src/getNonceManager.ts

@@ -11,12 +11,14 @@ export async function getNonceManager({
 }: CreateNonceManagerOptions): Promise<CreateNonceManagerResult> {
   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;
 }

packages/common/src/sendTransaction.ts

@@ -1,14 +1,13 @@
 import {
   Account,
-  CallParameters,
   Chain,
   Client,
   SendTransactionParameters,
   Transport,
   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<SendTransactionParameters<chain, account, chainOverride>> {
-    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<chain>);
-
-    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<chain, account, chainOverride> = {
-            ...preparedRequest,
+          const params: SendTransactionParameters<chain, account, chainOverride> = {
+            ...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;
           },
         },

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<WriteContractParameters, "address" | "abi" | "account" | "functionName">;
-
   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<WriteContractParameters<abi, functionName, args, chain, account, chainOverride>> {
-    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<abi, functionName, args, chain, account, chainOverride> = {
+            ...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;
           },
         },