From 0660561545910b03f8358e5ed7698f74e64f955b Mon Sep 17 00:00:00 2001 From: Kevin Ingersoll Date: Mon, 9 Oct 2023 15:57:01 +0100 Subject: [PATCH] feat(common): add sendTransaction, add mempool queue to nonce manager (#1717) --- .changeset/few-brooms-accept.md | 7 ++ packages/common/package.json | 1 + packages/common/src/createNonceManager.ts | 9 ++- packages/common/src/getContract.ts | 22 +++++- packages/common/src/getNonceManager.ts | 4 +- packages/common/src/getNonceManagerId.ts | 1 + packages/common/src/index.ts | 1 + packages/common/src/sendTransaction.ts | 89 +++++++++++++++++++++ packages/common/src/writeContract.ts | 96 ++++++++--------------- pnpm-lock.yaml | 20 +++++ 10 files changed, 180 insertions(+), 70 deletions(-) create mode 100644 .changeset/few-brooms-accept.md create mode 100644 packages/common/src/sendTransaction.ts diff --git a/.changeset/few-brooms-accept.md b/.changeset/few-brooms-accept.md new file mode 100644 index 0000000000..3084ca6a8e --- /dev/null +++ b/.changeset/few-brooms-accept.md @@ -0,0 +1,7 @@ +--- +"@latticexyz/common": minor +--- + +- Added a `sendTransaction` helper to mirror viem's `sendTransaction`, but with our nonce manager +- Added an internal mempool queue to `sendTransaction` and `writeContract` for better nonce handling +- Defaults block tag to `pending` for transaction simulation and transaction count (when initializing the nonce manager) diff --git a/packages/common/package.json b/packages/common/package.json index 3a6720720e..d714d9f573 100644 --- a/packages/common/package.json +++ b/packages/common/package.json @@ -58,6 +58,7 @@ "chalk": "^5.2.0", "debug": "^4.3.4", "execa": "^7.0.0", + "p-queue": "^7.4.1", "p-retry": "^5.1.2", "prettier": "^2.8.4", "prettier-plugin-solidity": "^1.1.2", diff --git a/packages/common/src/createNonceManager.ts b/packages/common/src/createNonceManager.ts index e134726143..d3dbb1dba2 100644 --- a/packages/common/src/createNonceManager.ts +++ b/packages/common/src/createNonceManager.ts @@ -2,6 +2,7 @@ import { BaseError, BlockTag, Client, Hex, NonceTooHighError, NonceTooLowError } import { debug as parentDebug } from "./debug"; import { getNonceManagerId } from "./getNonceManagerId"; import { getTransactionCount } from "viem/actions"; +import PQueue from "p-queue"; const debug = parentDebug.extend("createNonceManager"); @@ -17,12 +18,13 @@ export type CreateNonceManagerResult = { nextNonce: () => number; resetNonce: () => Promise; shouldResetNonce: (error: unknown) => boolean; + mempoolQueue: PQueue; }; export function createNonceManager({ client, - address, - blockTag = "latest", + address, // TODO: rename to account? + blockTag = "pending", broadcastChannelName, }: CreateNonceManagerOptions): CreateNonceManagerResult { const nonceRef = { current: -1 }; @@ -68,10 +70,13 @@ export function createNonceManager({ ); } + const mempoolQueue = new PQueue({ concurrency: 1 }); + return { hasNonce, nextNonce, resetNonce, shouldResetNonce, + mempoolQueue, }; } diff --git a/packages/common/src/getContract.ts b/packages/common/src/getContract.ts index 634da2a305..ac809e7139 100644 --- a/packages/common/src/getContract.ts +++ b/packages/common/src/getContract.ts @@ -5,6 +5,7 @@ import { Chain, GetContractParameters, GetContractReturnType, + Hex, PublicClient, Transport, WalletClient, @@ -12,7 +13,7 @@ import { getContract as viem_getContract, } from "viem"; import { UnionOmit } from "./type-utils/common"; -import { WriteContractOptions, writeContract } from "./writeContract"; +import { writeContract } from "./writeContract"; // copied from viem because this isn't exported // TODO: import from viem? @@ -26,6 +27,12 @@ function getFunctionParameters(values: [args?: readonly unknown[], options?: obj return { args, options }; } +export type ContractWrite = { + id: string; + request: WriteContractParameters; + result: Promise; +}; + export type GetContractOptions< TTransport extends Transport, TAddress extends Address, @@ -35,7 +42,7 @@ export type GetContractOptions< TPublicClient extends PublicClient, TWalletClient extends WalletClient > = Required> & { - onWrite?: WriteContractOptions["onWrite"]; + onWrite?: (write: ContractWrite) => void; }; // TODO: migrate away from this approach once we can hook into viem: https://github.com/wagmi-dev/viem/discussions/1230 @@ -72,6 +79,7 @@ export function getContract< if (contract.write) { // Replace write calls with our own. Implemented ~the same as viem, but adds better handling of nonces (via queue + retries). + let nextWriteId = 0; contract.write = new Proxy( {}, { @@ -83,14 +91,20 @@ export function getContract< ] ) => { const { args, options } = getFunctionParameters(parameters); - return writeContract(walletClient, { + const request = { abi, address, functionName, args, ...options, onWrite, - } as unknown as WriteContractOptions); + } as unknown as WriteContractParameters; + const result = writeContract(walletClient, request); + + const id = `${walletClient.chain.id}:${walletClient.account.address}:${nextWriteId++}`; + onWrite?.({ id, request: request as WriteContractParameters, result }); + + return result; }; }, } diff --git a/packages/common/src/getNonceManager.ts b/packages/common/src/getNonceManager.ts index 46b450910e..8fe22ed95c 100644 --- a/packages/common/src/getNonceManager.ts +++ b/packages/common/src/getNonceManager.ts @@ -5,8 +5,8 @@ const nonceManagers = new Map(); export async function getNonceManager({ client, - address, - blockTag = "latest", + address, // TODO: rename to account? + blockTag = "pending", }: CreateNonceManagerOptions): Promise { const id = await getNonceManagerId({ client, address, blockTag }); diff --git a/packages/common/src/getNonceManagerId.ts b/packages/common/src/getNonceManagerId.ts index 85220aae08..950e9fbcfe 100644 --- a/packages/common/src/getNonceManagerId.ts +++ b/packages/common/src/getNonceManagerId.ts @@ -10,6 +10,7 @@ export async function getNonceManagerId({ address: Hex; blockTag: BlockTag; }): Promise { + // TODO: improve this so we don't have to call getChainId every time const chainId = client.chain?.id ?? (await getChainId(client)); return `mud:createNonceManager:${chainId}:${getAddress(address)}:${blockTag}`; } diff --git a/packages/common/src/index.ts b/packages/common/src/index.ts index 569315b26c..9a68f8eb5b 100644 --- a/packages/common/src/index.ts +++ b/packages/common/src/index.ts @@ -9,6 +9,7 @@ export * from "./hexToResource"; export * from "./readHex"; export * from "./resourceToHex"; export * from "./resourceTypes"; +export * from "./sendTransaction"; export * from "./spliceHex"; export * from "./transportObserver"; export * from "./writeContract"; diff --git a/packages/common/src/sendTransaction.ts b/packages/common/src/sendTransaction.ts new file mode 100644 index 0000000000..6f89df20ce --- /dev/null +++ b/packages/common/src/sendTransaction.ts @@ -0,0 +1,89 @@ +import { + Account, + CallParameters, + Chain, + Client, + SendTransactionParameters, + Transport, + WriteContractReturnType, +} from "viem"; +import { call, sendTransaction as viem_sendTransaction } from "viem/actions"; +import pRetry from "p-retry"; +import { debug as parentDebug } from "./debug"; +import { getNonceManager } from "./getNonceManager"; +import { parseAccount } from "viem/accounts"; + +const debug = parentDebug.extend("sendTransaction"); + +// TODO: migrate away from this approach once we can hook into viem's nonce management: https://github.com/wagmi-dev/viem/discussions/1230 + +export async function sendTransaction< + TChain extends Chain | undefined, + TAccount extends Account | undefined, + TChainOverride extends Chain | undefined +>( + client: Client, + request: SendTransactionParameters +): Promise { + const rawAccount = request.account ?? client.account; + if (!rawAccount) { + // TODO: replace with viem AccountNotFoundError once its exported + throw new Error("No account provided"); + } + const account = parseAccount(rawAccount); + + const nonceManager = await getNonceManager({ + client, + address: account.address, + blockTag: "pending", + }); + + async function prepare(): Promise> { + if (request.gas) { + debug("gas provided, skipping simulate", request.to); + return request; + } + + debug("simulating tx to", request.to); + await call(client, { + ...request, + blockTag: "pending", + account, + } as CallParameters); + + // TODO: estimate gas + + return request; + } + + const preparedRequest = await prepare(); + + return await nonceManager.mempoolQueue.add( + () => + pRetry( + async () => { + if (!nonceManager.hasNonce()) { + await nonceManager.resetNonce(); + } + + const nonce = nonceManager.nextNonce(); + debug("sending tx with nonce", nonce, "to", preparedRequest.to); + return await viem_sendTransaction(client, { nonce, ...preparedRequest }); + }, + { + retries: 3, + onFailedAttempt: async (error) => { + // On nonce errors, reset the nonce and retry + 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; + }, + } + ), + { throwOnTimeout: true } + ); +} diff --git a/packages/common/src/writeContract.ts b/packages/common/src/writeContract.ts index 4ce2a9353f..07a0dd0964 100644 --- a/packages/common/src/writeContract.ts +++ b/packages/common/src/writeContract.ts @@ -3,7 +3,6 @@ import { Account, Chain, Client, - Hex, SimulateContractParameters, Transport, WriteContractParameters, @@ -16,29 +15,6 @@ import { getNonceManager } from "./getNonceManager"; import { parseAccount } from "viem/accounts"; const debug = parentDebug.extend("writeContract"); -let nextWriteId = 0; - -export type ContractWrite< - TAbi extends Abi | readonly unknown[] = Abi, - TFunctionName extends string = string, - TChain extends Chain | undefined = Chain, - TAccount extends Account | undefined = Account | undefined, - TChainOverride extends Chain | undefined = Chain | undefined -> = { - id: string; - request: WriteContractParameters; - result: Promise; -}; - -export type WriteContractOptions< - TAbi extends Abi | readonly unknown[] = Abi, - TFunctionName extends string = string, - TChain extends Chain | undefined = Chain, - TAccount extends Account | undefined = Account | undefined, - TChainOverride extends Chain | undefined = Chain | undefined -> = WriteContractParameters & { - onWrite?: (write: ContractWrite) => void; -}; // TODO: migrate away from this approach once we can hook into viem's nonce management: https://github.com/wagmi-dev/viem/discussions/1230 @@ -50,71 +26,67 @@ export async function writeContract< TChainOverride extends Chain | undefined >( client: Client, - { onWrite, ...request_ }: WriteContractOptions + request: WriteContractParameters ): Promise { - const request = request_ as WriteContractParameters; - - const account_ = request.account ?? client.account; - if (!account_) { + const rawAccount = request.account ?? client.account; + if (!rawAccount) { // TODO: replace with viem AccountNotFoundError once its exported throw new Error("No account provided"); } - const account = parseAccount(account_); + const account = parseAccount(rawAccount); const nonceManager = await getNonceManager({ client, address: account.address, + blockTag: "pending", }); async function prepareWrite(): Promise< WriteContractParameters > { if (request.gas) { - debug("gas provided, skipping simulate", request); + debug("gas provided, skipping simulate", request.functionName, request.address); return request; } - debug("simulating write", request); + debug("simulating", request.functionName, "at", request.address); const result = await simulateContract(client, { ...request, + blockTag: "pending", account, } as unknown as SimulateContractParameters); return result.request as unknown as WriteContractParameters; } - async function write(): Promise { - const preparedWrite = await prepareWrite(); + const preparedWrite = await prepareWrite(); - return await pRetry( - async () => { - if (!nonceManager.hasNonce()) { - await nonceManager.resetNonce(); - } - - const nonce = nonceManager.nextNonce(); - debug("calling write function with nonce", nonce, preparedWrite); - return await viem_writeContract(client, { nonce, ...preparedWrite } as typeof preparedWrite); - }, - { - retries: 3, - onFailedAttempt: async (error) => { - // On nonce errors, reset the nonce and retry - if (nonceManager.shouldResetNonce(error)) { - debug("got nonce error, retrying", error); + return nonceManager.mempoolQueue.add( + () => + pRetry( + async () => { + if (!nonceManager.hasNonce()) { await nonceManager.resetNonce(); - return; } - // TODO: prepareWrite again if there are gas errors? - throw error; - }, - } - ); - } - - const result = write(); - - onWrite?.({ id: `${nextWriteId++}`, request, result }); - return result; + const nonce = nonceManager.nextNonce(); + debug("calling", preparedWrite.functionName, "with nonce", nonce, "at", preparedWrite.address); + return await viem_writeContract(client, { nonce, ...preparedWrite } as typeof preparedWrite); + }, + { + retries: 3, + onFailedAttempt: async (error) => { + // On nonce errors, reset the nonce and retry + 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; + }, + } + ), + { throwOnTimeout: true } + ); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 18dcf94413..e57b291ce2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -257,6 +257,9 @@ importers: execa: specifier: ^7.0.0 version: 7.0.0 + p-queue: + specifier: ^7.4.1 + version: 7.4.1 p-retry: specifier: ^5.1.2 version: 5.1.2 @@ -5789,6 +5792,10 @@ packages: resolution: {integrity: sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==} dev: false + /eventemitter3@5.0.1: + resolution: {integrity: sha512-GWkBvjiSZK87ELrYOSESUYeVIc9mvLLf/nXalMOS5dYrgZq9o5OVkbZAVM06CVxYsCwH9BDZFPlQTlPA1j4ahA==} + dev: false + /events@3.3.0: resolution: {integrity: sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==} engines: {node: '>=0.8.x'} @@ -8732,6 +8739,14 @@ packages: aggregate-error: 3.1.0 dev: true + /p-queue@7.4.1: + resolution: {integrity: sha512-vRpMXmIkYF2/1hLBKisKeVYJZ8S2tZ0zEAmIJgdVKP2nq0nh4qCdf8bgw+ZgKrkh71AOCaqzwbJJk1WtdcF3VA==} + engines: {node: '>=12'} + dependencies: + eventemitter3: 5.0.1 + p-timeout: 5.1.0 + dev: false + /p-retry@5.1.2: resolution: {integrity: sha512-couX95waDu98NfNZV+i/iLt+fdVxmI7CbrrdC2uDWfPdUAApyxT4wmDlyOtR5KtTDmkDO0zDScDjDou9YHhd9g==} engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} @@ -8740,6 +8755,11 @@ packages: retry: 0.13.1 dev: false + /p-timeout@5.1.0: + resolution: {integrity: sha512-auFDyzzzGZZZdHz3BtET9VEz0SE/uMEAx7uWfGPucfzEwwe/xH0iVeZibQmANYE/hp9T2+UUZT5m+BKyrDp3Ew==} + engines: {node: '>=12'} + dev: false + /p-try@1.0.0: resolution: {integrity: sha512-U1etNYuMJoIz3ZXSrrySFjsXQTWOx2/jdi86L+2pRvph/qMKL6sbcCYdH23fqsbm8TH2Gn0OybpT4eSFlCVHww==} engines: {node: '>=4'}