Skip to content

Commit

Permalink
refactor(common): simplify writeContract/sendTransaction actions (#3043)
Browse files Browse the repository at this point in the history
  • Loading branch information
holic authored Aug 20, 2024
1 parent 9e21e42 commit 2daaab1
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 111 deletions.
5 changes: 5 additions & 0 deletions .changeset/three-flies-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@latticexyz/common": patch
---

Refactored `writeContract` and `sendTransaction` actions for simplicity and better error messages.
23 changes: 13 additions & 10 deletions packages/common/src/createNonceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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 {
Expand Down
12 changes: 7 additions & 5 deletions packages/common/src/getNonceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
41 changes: 10 additions & 31 deletions packages/common/src/sendTransaction.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
},
},
Expand Down
78 changes: 13 additions & 65 deletions packages/common/src/writeContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand All @@ -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;
},
},
Expand Down

0 comments on commit 2daaab1

Please sign in to comment.