-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(common): move createContract's internal write logic to writeContract
#1693
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
8dca0bd
move logic to writeContract
holic 52daba2
revert example change
holic b0d5b7c
rename to better align with viem
holic e31403d
Create rotten-beers-learn.md
holic 069809b
add TODO
holic 2013116
createContract -> getContract everywhere
holic 94c1d3c
type cast ugh
holic 66c4e4b
string replace too aggressive
holic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| "@latticexyz/common": minor | ||
| --- | ||
|
|
||
| - Moves contract write logic out of `createContract` into its own `writeContract` method so that it can be used outside of the contract instance, and for consistency with viem. | ||
| - Deprecates `createContract` in favor of `getContract` for consistency with viem. | ||
| - Reworks `createNonceManager`'s `BroadcastChannel` setup and moves out the notion of a "nonce manager ID" to `getNonceManagerId` so we can create an internal cache with `getNonceManager` for use in `writeContract`. | ||
|
|
||
| If you were using the `createNonceManager` before, you'll just need to rename `publicClient` argument to `client`: | ||
|
|
||
| ```diff | ||
| const publicClient = createPublicClient({ ... }); | ||
| - const nonceManager = createNonceManager({ publicClient, ... }); | ||
| + const nonceManager = createNonceManager({ client: publicClient, ... }); | ||
| ``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,173 +1,4 @@ | ||
| import { | ||
| Abi, | ||
| Account, | ||
| Address, | ||
| Chain, | ||
| GetContractParameters, | ||
| GetContractReturnType, | ||
| Hex, | ||
| PublicClient, | ||
| SimulateContractParameters, | ||
| Transport, | ||
| WalletClient, | ||
| WriteContractParameters, | ||
| getContract, | ||
| } from "viem"; | ||
| import pRetry from "p-retry"; | ||
| import { createNonceManager } from "./createNonceManager"; | ||
| import { debug as parentDebug } from "./debug"; | ||
| import { UnionOmit } from "./type-utils/common"; | ||
| import { getContract } from "./getContract"; | ||
|
|
||
| const debug = parentDebug.extend("createContract"); | ||
|
|
||
| // copied from viem because this isn't exported | ||
| // TODO: import from viem? | ||
| function getFunctionParameters(values: [args?: readonly unknown[], options?: object]): { | ||
| args: readonly unknown[]; | ||
| options: object; | ||
| } { | ||
| const hasArgs = values.length && Array.isArray(values[0]); | ||
| const args = hasArgs ? values[0]! : []; | ||
| const options = (hasArgs ? values[1] : values[0]) ?? {}; | ||
| return { args, options }; | ||
| } | ||
|
|
||
| export type ContractWrite = { | ||
| id: string; | ||
| request: WriteContractParameters; | ||
| result: Promise<Hex>; | ||
| }; | ||
|
|
||
| export type CreateContractOptions< | ||
| TTransport extends Transport, | ||
| TAddress extends Address, | ||
| TAbi extends Abi, | ||
| TChain extends Chain, | ||
| TAccount extends Account, | ||
| TPublicClient extends PublicClient<TTransport, TChain>, | ||
| TWalletClient extends WalletClient<TTransport, TChain, TAccount> | ||
| > = Required<GetContractParameters<TTransport, TChain, TAccount, TAbi, TPublicClient, TWalletClient, TAddress>> & { | ||
| onWrite?: (write: ContractWrite) => void; | ||
| }; | ||
|
|
||
| export function createContract< | ||
| TTransport extends Transport, | ||
| TAddress extends Address, | ||
| TAbi extends Abi, | ||
| TChain extends Chain, | ||
| TAccount extends Account, | ||
| TPublicClient extends PublicClient<TTransport, TChain>, | ||
| TWalletClient extends WalletClient<TTransport, TChain, TAccount> | ||
| >({ | ||
| abi, | ||
| address, | ||
| publicClient, | ||
| walletClient, | ||
| onWrite, | ||
| }: CreateContractOptions< | ||
| TTransport, | ||
| TAddress, | ||
| TAbi, | ||
| TChain, | ||
| TAccount, | ||
| TPublicClient, | ||
| TWalletClient | ||
| >): GetContractReturnType<TAbi, TPublicClient, TWalletClient, TAddress> { | ||
| const contract = getContract<TTransport, TAddress, TAbi, TChain, TAccount, TPublicClient, TWalletClient>({ | ||
| abi, | ||
| address, | ||
| publicClient, | ||
| walletClient, | ||
| }) as unknown as GetContractReturnType<Abi, PublicClient, WalletClient>; | ||
|
|
||
| if (contract.write) { | ||
| let nextWriteId = 0; | ||
| const nonceManager = createNonceManager({ | ||
| publicClient: publicClient as PublicClient, | ||
| address: walletClient.account.address, | ||
| }); | ||
|
|
||
| // Replace write calls with our own proxy. Implemented ~the same as viem, but adds better handling of nonces (via queue + retries). | ||
| contract.write = new Proxy( | ||
| {}, | ||
| { | ||
| get(_, functionName: string): GetContractReturnType<Abi, PublicClient, WalletClient>["write"][string] { | ||
| async function prepareWrite( | ||
| options: WriteContractParameters | ||
| ): Promise<WriteContractParameters<TAbi, typeof functionName, TChain, TAccount>> { | ||
| if (options.gas) { | ||
| debug("gas provided, skipping simulate", functionName, options); | ||
| return options as unknown as WriteContractParameters<TAbi, typeof functionName, TChain, TAccount>; | ||
| } | ||
|
|
||
| debug("simulating write", functionName, options); | ||
| const { request } = await publicClient.simulateContract({ | ||
| ...options, | ||
| account: options.account ?? walletClient.account, | ||
| } as unknown as SimulateContractParameters<TAbi, typeof functionName, TChain>); | ||
|
|
||
| return request as unknown as WriteContractParameters<TAbi, typeof functionName, TChain, TAccount>; | ||
| } | ||
|
|
||
| async function write(options: WriteContractParameters): Promise<Hex> { | ||
| const preparedWrite = await prepareWrite(options); | ||
|
|
||
| return await pRetry( | ||
| async () => { | ||
| if (!nonceManager.hasNonce()) { | ||
| await nonceManager.resetNonce(); | ||
| } | ||
|
|
||
| const nonce = nonceManager.nextNonce(); | ||
| debug("calling write function with nonce", nonce, preparedWrite); | ||
| return await walletClient.writeContract({ | ||
| nonce, | ||
| ...preparedWrite, | ||
| }); | ||
| }, | ||
| { | ||
| retries: 3, | ||
| onFailedAttempt: async (error) => { | ||
| // On nonce errors, reset the nonce and retry | ||
| if (nonceManager.shouldResetNonce(error)) { | ||
| debug("got nonce error, retrying", error); | ||
| await nonceManager.resetNonce(); | ||
| return; | ||
| } | ||
| // TODO: prepareWrite again if there are gas errors? | ||
| throw error; | ||
| }, | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| return (...parameters) => { | ||
| const id = `${walletClient.chain.id}:${walletClient.account.address}:${nextWriteId++}`; | ||
| const { args, options } = < | ||
| { | ||
| args: unknown[]; | ||
| options: UnionOmit<WriteContractParameters, "address" | "abi" | "functionName" | "args">; | ||
| } | ||
| >getFunctionParameters(parameters as any); | ||
|
|
||
| const request = { | ||
| address, | ||
| abi, | ||
| functionName, | ||
| args, | ||
| ...options, | ||
| }; | ||
|
|
||
| const result = write(request); | ||
|
|
||
| onWrite?.({ id, request, result }); | ||
|
|
||
| return result; | ||
| }; | ||
| }, | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| return contract as unknown as GetContractReturnType<TAbi, TPublicClient, TWalletClient, TAddress>; | ||
| } | ||
| /** @deprecated use `getContract` instead */ | ||
| export const createContract = getContract; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly different approach to what we've been doing in the past to avoid a breaking change. We can just remove this in a future version as a breaking change, potentially batching with other breaking changes.
I kinda like the deprecate-then-break pattern so playing with it here.