👷 Chore: Productionize contract package #1442
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: f3ff7b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 162 files out of 279 files are above the max files limit of 75. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThe pull request introduces substantial updates to the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @roninjin10 and the rest of your teammates on |
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
48e1a50 to
f186e4f
Compare
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_common_package
branch
from
fad1479 to
3d1242a
Compare
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
2 times, most recently
from
f8a97f3 to
44d7eb4
Compare
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
5870a88 to
d60b8f8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (5)
packages/contract/src/read/readFactory.js (1)
57-58: TODO comment: Optimize method overload handling.The TODO comment highlights an opportunity for future optimization regarding the handling of method overloads:
- While the current implementation handles method overloads, it may not be the most efficient approach.
- Addressing this TODO comment and optimizing the method overload handling can potentially improve the performance and efficiency of the
readFactoryfunction.Consider creating a GitHub issue to track this optimization task and ensure that it is addressed in a future iteration of the codebase.
packages/contract/src/CreateContractParams.ts (1)
5-15: Enhance documentation by specifying type parameter constraints.While the documentation provides descriptions for each type parameter, it does not specify their expected types or constraints. Including the actual type constraints (e.g.,
string | undefined | neverforTName) would improve clarity and help developers understand the intended usage.packages/contract/src/write/writeFactory.js (2)
62-62: Address the efficiency TODO commentThere's a TODO comment indicating that the method overload handling could be more efficient. Optimizing this part of the code can improve performance.
Would you like me to propose an optimized solution for the method overload handling, or open a GitHub issue to track this improvement?
68-68: Use professional language in commentsThe comment uses informal language ("barf"). Consider rephrasing it to maintain a professional tone.
You could update the comment as follows:
- // viem and wagmi barf if we pass in undefined or [] for args so do this to accommodate viem and wagmi + // Avoid passing undefined or empty arrays for args to ensure compatibility with viem and wagmipackages/contract/src/createContract.js (1)
129-131: Use@paramInstead of@typein JSDoc forwithCodeFunctionThe JSDoc comment for the
withCodefunction uses the@typetag, which is intended for type declarations of variables or constants. Since you're documenting the parameters of a function, consider using@paramto properly describe each parameter.Apply this diff to correct the JSDoc comment:
/** - * @type {import('./Contract.js').Contract<any, any>['withCode']} params + * @param {object} params - Parameters for withCode. + * @param {string} [params.code] - Optional runtime bytecode. + * @param {string} [params.deployedBytecode] - Optional deployed bytecode. + * @param {string} [params.bytecode] - Optional creation bytecode. */
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (23)
- .changeset/long-plums-march.md (1 hunks)
- .changeset/poor-colts-learn.md (1 hunks)
- .changeset/swift-ducks-lie.md (1 hunks)
- .changeset/twenty-apricots-divide.md (1 hunks)
- packages/contract/src/Contract.ts (2 hunks)
- packages/contract/src/CreateContractFn.ts (1 hunks)
- packages/contract/src/CreateContractParams.ts (1 hunks)
- packages/contract/src/CreateScript.ts (0 hunks)
- packages/contract/src/DeployArgs.ts (0 hunks)
- packages/contract/src/contract-lib/ERC20.ts (1 hunks)
- packages/contract/src/contract-lib/ERC721.ts (1 hunks)
- packages/contract/src/contract-lib/SimpleContract.s.sol.ts (1 hunks)
- packages/contract/src/createContract.js (2 hunks)
- packages/contract/src/createContract.spec.ts (2 hunks)
- packages/contract/src/event/EventActionCreator.ts (2 hunks)
- packages/contract/src/event/eventFactory.js (2 hunks)
- packages/contract/src/index.ts (0 hunks)
- packages/contract/src/read/ReadActionCreator.ts (1 hunks)
- packages/contract/src/read/readFactory.js (1 hunks)
- packages/contract/src/read/readFactory.spec.ts (2 hunks)
- packages/contract/src/script.spec.ts (0 hunks)
- packages/contract/src/write/WriteActionCreator.ts (1 hunks)
- packages/contract/src/write/writeFactory.js (1 hunks)
Files not reviewed due to no reviewable changes (4)
- packages/contract/src/CreateScript.ts
- packages/contract/src/DeployArgs.ts
- packages/contract/src/index.ts
- packages/contract/src/script.spec.ts
Additional context used
Learnings (1)
packages/contract/src/read/readFactory.js (1)
Learnt from: roninjin10 PR: evmts/tevm-monorepo#836 File: packages/contract/src/read/readFactory.js:11-11 Timestamp: 2024-01-19T19:58:45.760Z Learning: The `readFactory` function in `packages/contract/src/read/readFactory.js` uses generic `any` types intentionally for its internal JSDoc comments, as the public API will apply more specific types based on the passed parameters.
Additional comments not posted (39)
.changeset/long-plums-march.md (1)
1-5: LGTM!The changeset is well-formatted and clearly communicates the intent of the documentation update. Improving the JSDoc comments will greatly benefit the users of the
@tevm/contractpackage by providing clearer and more accurate documentation..changeset/poor-colts-learn.md (1)
1-5: LGTM!The changeset is well-formatted and the message clearly describes the bug fix. The AI-generated summary provides additional context about the impact of the change.
The fix addresses an important issue related to method classification and improves the correctness of the contract package. It is unlikely to introduce any new issues.
.changeset/swift-ducks-lie.md (1)
1-5: Looks good! The changes align with the provided description.The changeset file correctly indicates a minor version update for the
@tevm/commonpackage, which is appropriate for introducing backward-compatible improvements or new functionality.The description of the change provides valuable context, explaining that the removal of the experimental script property from the contract instance enables tree-shaking in the
encodeDeployDatafunction. This optimization should result in more efficient deployment data encoding.Based on the AI-generated summary, there are no apparent issues or concerns with the changes. The modifications seem to be focused on improving the contract package and are ready to be merged.
packages/contract/src/read/ReadActionCreator.ts (4)
11-15: LGTM!The JSDoc comment provides clear documentation for the
ValueOf<T>utility type, explaining its purpose and theTtemplate parameter. This improves the overall clarity of the code without introducing any functional changes.
18-33: LGTM!The JSDoc comment provides clear documentation for the
ReadActionCreatortype, explaining its functionality and the various template parameters. The example usage demonstrates how to use the action creator with a contract method, improving the overall clarity of the code without introducing any functional changes.
40-53: LGTM!The comments in this code segment have been updated to provide more context and clarity, and the capitalization has been made more consistent. These changes improve the overall readability of the code without introducing any functional changes.
Line range hint
55-68: LGTM!The comments in this code segment have been updated to provide more context and clarity, and the capitalization has been made more consistent. These changes improve the overall readability of the code without introducing any functional changes.
packages/contract/src/write/WriteActionCreator.ts (3)
11-15: LGTM!The added JSDoc documentation for the
ValueOf<T>utility type is clear and informative. It follows the proper format and helps improve the understanding of the type's purpose.
18-33: LGTM!The updated JSDoc documentation for the
WriteActionCreatortype is comprehensive and informative. The expanded description clarifies the purpose and usage of the type, and the updated example demonstrates the creation of a write action, which aligns with the type's intended use.
Line range hint
40-73: LGTM!The updated comments within the
WriteActionCreatortype improve the clarity and readability of the code. The consistent use of complete sentences and proper capitalization enhances the overall documentation quality, making it easier for developers to understand the purpose and usage of the type.packages/contract/src/read/readFactory.js (3)
4-40: Excellent improvements to thereadFactoryfunction!The changes made to the
readFactoryfunction significantly enhance its clarity, usability, and maintainability:
- The expanded JSDoc comments provide a clear and detailed description of the function's purpose, parameters, and return value, making it easier for developers to understand and utilize the function effectively.
- The example usage is a valuable addition, illustrating how to create read actions for contract methods using the
readFactoryfunction.- The refined filtering logic ensures that only methods with a state mutability of 'view' or 'pure' are included in the resulting action creators, aligning perfectly with the function's intended functionality of generating read actions.
- The improved internal documentation for the action creator function enhances clarity and maintainability, making the code more readable and self-explanatory.
Overall, these changes greatly improve the quality and usability of the
readFactoryfunction, facilitating its integration and usage within the codebase.
46-49: Improved filtering logic for method selection.The updated filtering logic, which checks for the
stateMutabilityof the methods, is a valuable addition to thereadFactoryfunction:
- By specifically checking for methods with a state mutability of 'view' or 'pure', the function ensures that only read-only methods are included in the resulting action creators.
- This change clarifies the intent of the function and aligns it with its purpose of generating read actions for contract methods.
- It prevents the inclusion of methods that can modify the state, ensuring that the resulting action creators are limited to read-only operations.
This improvement enhances the correctness and reliability of the
readFactoryfunction, making it more robust and predictable in its behavior.
52-54: Improved internal documentation for the action creator function.The enhanced internal documentation for the action creator function is a valuable addition:
- It clearly specifies that the function creates a read action for a specific contract method.
- It provides details about the parameters accepted by the function, improving clarity and understanding.
This improvement in documentation facilitates better maintainability and readability of the code, making it easier for developers to comprehend and work with the action creator function.
packages/contract/src/event/eventFactory.js (3)
4-13: Excellent documentation improvements!The enhanced documentation for the
eventsFactoryfunction provides a clear and comprehensive explanation of its purpose, parameters, and return value. The additional details about the optional parameters and the return type make it easier for developers to understand and use this function effectively.
15-42: Great example!The provided example is an excellent addition to the documentation. It clearly demonstrates how to use the
eventsFactoryfunction to create an event filter and how to utilize that filter with thetevmlibrary to fetch logs. The example is well-structured, easy to follow, and covers the essential steps, making it a valuable resource for developers working with this functionality.
52-58: Improved inner function documentation!The updated documentation for the inner function that creates event filters is a significant improvement. It now clearly details the parameters the function accepts, including their expected types, and clarifies the return value. This enhanced documentation makes it easier for developers to understand and use the inner function effectively, improving the overall clarity and usability of the code.
packages/contract/src/contract-lib/SimpleContract.s.sol.ts (1)
15-49: Excellent documentation!The documentation comment provides a clear and comprehensive overview of the
SimpleContractconstant. It effectively communicates the purpose and functionality of the contract, making it easy for developers to understand and use.The examples are well-structured and cover two common scenarios: deploying a new contract and interacting with an existing deployment. The step-by-step instructions, along with the relevant console logs, make the examples easy to follow and understand.
The use of TypeScript syntax and the assumption of the
tevmlibrary are appropriate for the target audience of this documentation.Overall, the documentation enhances the usability and clarity of the
SimpleContractconstant.packages/contract/src/event/EventActionCreator.ts (3)
16-18: LGTM!The updated comment block provides a clear and concise description of the
MaybeExtractEventArgsFromAbitype's purpose and the expected types for its template parameters. This improves the understanding of how to use the type.
32-35: LGTM!The new utility type
ValueOfis a useful addition for getting the value type of an object. The accompanying comment block provides a clear explanation of the type's purpose and the expected type for its template parameter.
39-57: LGTM!The updated comment block for the
EventActionCreatortype provides a clear and detailed description of its purpose and template parameters. It explains how the type can be used to create event filters in a typesafe way, improving the understanding of its usage.The modified example usage illustrates the creation of an event filter for a
Transferevent, enhancing clarity on how to use theEventActionCreatortype in practice.These changes improve the documentation and clarity of the
EventActionCreatortype without altering its underlying logic or functionality.packages/contract/src/CreateContractFn.ts (3)
6-15: Excellent documentation!The JSDoc comment block provides clear and concise descriptions for each type parameter of the
createContractfunction. This will greatly improve the usability and maintainability of the code.
17-29: Great example for human-readable ABI usage!The example clearly demonstrates how to use the
createContractfunction with a human-readable ABI for an ERC20 contract. It showcases the import statements and the structure of the human-readable ABI, making it easy for developers to understand and apply in their own code.
32-75: Comprehensive example for JSON ABI usage!The example provides a detailed demonstration of how to use the
createContractfunction with a JSON ABI for an ERC20 contract. It showcases the import statements, the formatting of the JSON ABI using theformatAbifunction, and the inclusion of additional parameters such asaddress,bytecode,deployedBytecode, andcode.This example will be extremely helpful for developers looking to instantiate contracts with various attributes and understand the structure of a JSON ABI.
packages/contract/src/read/readFactory.spec.ts (2)
1-1: LGTM!The import statement for the
encodeDeployDatafunction from the@tevm/utilspackage is correctly added.
159-164: LGTM!The test case for working with a script is correctly updated to use the
.withCode()method and theencodeDeployDatafunction for encoding the contract's deployment data.packages/contract/src/Contract.ts (5)
7-17: LGTM!The JSDoc comments provide a clear and comprehensive overview of the
Contracttype and its generic parameters. The expanded documentation will help developers better understand the purpose and usage of theContracttype.
19-45: Great example!The updated example code clearly demonstrates how to create and use a
Contractinstance, including reading from the contract, writing to the contract, and creating event filters. The improved readability will make it easier for developers to understand and follow the example.
47-60: Excellent addition!The new example code showcases the interoperability of the
Contracttype with other libraries, such asviem. This demonstrates the flexibility and versatility of theContracttype, making it easier for developers to integrate it into their existing projects.
171-173: Improved type safety!The updated
withAddressmethod signature, using the new generic parameterTNewAddress, clarifies that the method returns a new contract instance with the updated address type. This enhances the type safety of the method and makes it more explicit for developers using theContracttype.
176-195: Useful addition!The new
withCodemethod is a valuable addition to theContracttype, allowing developers to update the bytecode properties of the contract and receive a new contract instance with the updated code properties. The method's signature and JSDoc comments are clear and informative, making it easy for developers to understand and use the method effectively.packages/contract/src/createContract.spec.ts (3)
369-376: LGTM!The test case correctly verifies the existence and type of the
withCodemethod on the contract object.
378-391: LGTM!The test case correctly verifies that the
withCodemethod updates the code properties of the contract object with the provided values.
393-405: LGTM!The test case correctly verifies that the
withCodemethod can handle partial updates to the code properties of the contract object, leaving the other properties unchanged.packages/contract/src/contract-lib/ERC20.ts (1)
30-60: Excellent documentation comment!The documentation comment provides a clear description of the
ERC20constant and includes practical usage examples. The examples cover important scenarios such as deploying the contract, interacting with a deployed contract, reading contract state, and executing transactions.Some additional suggestions to consider:
- You could mention the specific version or release of the OpenZeppelin library that the contract bytecode and ABI are based on, if applicable.
- Consider adding a note about any specific configuration or setup required to use the
tevmlibrary, if necessary.Overall, the documentation comment is well-written and provides valuable information for developers using the
ERC20constant.packages/contract/src/contract-lib/ERC721.ts (3)
37-38: LGTM!The updated comment header improves the accuracy and clarity of the description.
41-68: Great examples!The added examples provide clear and concise guidance on how to use the
ERC721contract in various scenarios. They demonstrate deploying the contract, interacting with it, reading token names, and transferring tokens. These examples will be very helpful for users to understand and utilize the contract effectively.
37-69: Documentation structure looks great!The overall structure of the documentation has been significantly improved. The changes make it much easier for users to understand how to utilize the
ERC721contract effectively. The clear organization and helpful examples enhance the readability and usability of the documentation.packages/contract/src/CreateContractParams.ts (1)
Line range hint
58-87: LGTM!The type definitions correctly enforce mutual exclusivity between
humanReadableAbiandabi, and the conditional types ensure proper type inference based on the provided ABI. The use ofnevereffectively prevents both properties from being used simultaneously.packages/contract/src/createContract.js (1)
145-145: Great Addition ofwithCodeMethod to Enhance FlexibilityThe inclusion of the
withCodemethod allows for dynamic updating of code-related parameters, enhancing the flexibility of the contract instances.
| --- | ||
| "@tevm/contract": minor | ||
| --- | ||
|
|
||
| Added `withCode` method to contract to add code. This replaces the previous script property |
There was a problem hiding this comment.
Tip
Codebase Verification
Inconsistency found: 'script:' property still present in codebase
The changeset message states that the 'withCode' method replaces the previous 'script' property. However, our verification reveals that the 'script:' property is still present in multiple files across the codebase. While the 'withCode' method has been implemented and is being used in tests, the transition appears to be incomplete.
Action items:
- Review all occurrences of 'script:' in the codebase and update them to use 'withCode' where appropriate.
- Update or remove 'script:' usage in files such as TevmActionsApi.ts, JsonRpcSchemaTevm.ts, and TevmClient.ts.
- Revise the changeset message to accurately reflect the current state of the changes, possibly indicating that this is a gradual transition rather than a complete replacement.
Please ensure that all necessary updates are made before finalizing this change to maintain consistency and prevent potential issues.
Analysis chain
Changeset looks good! Verify the impact of the breaking change.
The changeset file follows the correct format and provides a clear description of the changes. The addition of the withCode method and removal of the script property seems like a good improvement to streamline the process of adding code to contracts.
However, please ensure that all current usages of the script property are updated to use withCode instead, as this is a breaking change.
Run the following script to verify the usage of the script property:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all usages of the `script` property are updated to use `withCode`.
# Test: Search for the `script` property usage. Expect: No occurrences.
rg --type typescript $'script:'
Length of output: 65
Script:
#!/bin/bash
# Description: Verify all usages of the `script` property are updated to use `withCode`.
# Search for 'script:' in TypeScript files
echo "Searching for 'script:' usage:"
fd '\.(ts|tsx)$' | xargs rg 'script:' -n --context 2
# Search for 'withCode' method in TypeScript files
echo -e "\nSearching for 'withCode' usage:"
fd '\.(ts|tsx)$' | xargs rg 'withCode' -n --context 2
Length of output: 8429
| * const params: CreateContractParams = { | ||
| * name: 'ERC20', | ||
| * humanReadableAbi: [ | ||
| * 'function balanceOf(address owner) view returns (uint256)', | ||
| * 'function transfer(address to, uint256 amount) returns (bool)', | ||
| * ], | ||
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | ||
| * } | ||
| * ``` |
There was a problem hiding this comment.
Let TypeScript infer the generic type parameters in the example.
In the example, CreateContractParams is a generic type, but no type arguments are provided, which may lead to type inference issues. It's better to allow TypeScript to infer the types by omitting the explicit type annotation.
Apply this diff to let TypeScript infer the types:
-const params: CreateContractParams = {
+const params = {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * const params: CreateContractParams = { | |
| * name: 'ERC20', | |
| * humanReadableAbi: [ | |
| * 'function balanceOf(address owner) view returns (uint256)', | |
| * 'function transfer(address to, uint256 amount) returns (bool)', | |
| * ], | |
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | |
| * } | |
| * ``` | |
| * const params = { | |
| * name: 'ERC20', | |
| * humanReadableAbi: [ | |
| * 'function balanceOf(address owner) view returns (uint256)', | |
| * 'function transfer(address to, uint256 amount) returns (bool)', | |
| * ], | |
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | |
| * } | |
| * ``` |
| * const params: CreateContractParams = { | ||
| * name: 'ERC20', | ||
| * abi: [ | ||
| * { | ||
| * "inputs": [{"name": "owner", "type": "address"}], | ||
| * "name": "balanceOf", | ||
| * "outputs": [{"type": "uint256"}], | ||
| * "stateMutability": "view", | ||
| * "type": "function" | ||
| * }, | ||
| * ], | ||
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | ||
| * } | ||
| * ``` |
There was a problem hiding this comment.
Let TypeScript infer the generic type parameters in the example.
Similarly, in this example, omitting the explicit type annotation allows TypeScript to infer the generic type parameters, preventing potential type mismatch errors.
Apply this diff:
-const params: CreateContractParams = {
+const params = {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * const params: CreateContractParams = { | |
| * name: 'ERC20', | |
| * abi: [ | |
| * { | |
| * "inputs": [{"name": "owner", "type": "address"}], | |
| * "name": "balanceOf", | |
| * "outputs": [{"type": "uint256"}], | |
| * "stateMutability": "view", | |
| * "type": "function" | |
| * }, | |
| * ], | |
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | |
| * } | |
| * ``` | |
| * const params = { | |
| * name: 'ERC20', | |
| * abi: [ | |
| * { | |
| * "inputs": [{"name": "owner", "type": "address"}], | |
| * "name": "balanceOf", | |
| * "outputs": [{"type": "uint256"}], | |
| * "stateMutability": "view", | |
| * "type": "function" | |
| * }, | |
| * ], | |
| * address: '0x6B175474E89094C44Da98b954EedeAC495271d0F', | |
| * } | |
| * ``` |
| * @param {import('@tevm/utils').Hex} [params.code] - The runtime bytecode of the contract (optional). | ||
| * @param {import('@tevm/utils').Address} [params.address] - The address of the deployed contract (optional). | ||
| * @returns {import('./WriteActionCreator.js').WriteActionCreator<any, any, any>} An object containing write action creators for each payable and nonpayable function in the ABI. | ||
| * |
There was a problem hiding this comment.
Consider specifying concrete generic types in the return annotation
The return type uses generic any types, which can reduce type safety. Specifying concrete types for the generic parameters in WriteActionCreator enhances type safety and code clarity.
You could update the return annotation as follows:
- * @returns {import('./WriteActionCreator.js').WriteActionCreator<any, any, any>} An object containing write action creators for each payable and nonpayable function in the ABI.
+ * @returns {import('./WriteActionCreator.js').WriteActionCreator<ArgsType, ReturnType, StateType>} An object containing write action creators for each payable and nonpayable function in the ABI.Replace ArgsType, ReturnType, and StateType with the appropriate types based on your use case.
Committable suggestion was skipped due to low confidence.
| * Creates a write action for a specific contract method. | ||
| * @param {...any} args - The arguments for the contract method. | ||
| * @returns {object} An object representing the write action, including ABI and method information. | ||
| */ | ||
| const creator = (...args) => { | ||
| // Handle case where there is an overload | ||
| // TODO: make this more efficient | ||
| const methodAbi = methods.filter( | ||
| (m) => | ||
| /**@type {import('@tevm/utils').AbiFunction}*/ (m).name === | ||
| /**@type {import('@tevm/utils').AbiFunction}*/ (method)?.name, | ||
| ) | ||
| // viem and wagmi barf if we pass in undefined or [] for args so do this to accommodate viem and wagmi |
There was a problem hiding this comment.
Optimize method overload handling by precomputing methodAbi
Currently, methodAbi is recalculated every time the creator function is invoked, which may affect performance. You can optimize this by precomputing and caching methodAbi outside of the creator function.
Apply the following changes to precompute methodAbi:
.map((method) => {
+ // Precompute methodAbi outside of creator function to improve efficiency
+ const methodAbi = methods.filter(
+ (m) =>
+ /**@type {import('@tevm/utils').AbiFunction}*/ (m).name ===
+ /**@type {import('@tevm/utils').AbiFunction}*/ (method)?.name,
+ )
/**
* Creates a write action for a specific contract method.
* @param {...any} args - The arguments for the contract method.
* @returns {object} An object representing the write action, including ABI and method information.
*/
const creator = (...args) => {
- // Handle case where there is an overload
- // TODO: make this more efficient
- const methodAbi = methods.filter(
- (m) =>
- /**@type {import('@tevm/utils').AbiFunction}*/ (m).name ===
- /**@type {import('@tevm/utils').AbiFunction}*/ (method)?.name,
- )
// Avoid passing undefined or empty arrays for args to ensure compatibility with viem and wagmi
const maybeArgs = args.length > 0 ? { args } : {}This adjustment moves the computation of methodAbi outside the creator function, ensuring it is calculated only once per method.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Creates a write action for a specific contract method. | |
| * @param {...any} args - The arguments for the contract method. | |
| * @returns {object} An object representing the write action, including ABI and method information. | |
| */ | |
| const creator = (...args) => { | |
| // Handle case where there is an overload | |
| // TODO: make this more efficient | |
| const methodAbi = methods.filter( | |
| (m) => | |
| /**@type {import('@tevm/utils').AbiFunction}*/ (m).name === | |
| /**@type {import('@tevm/utils').AbiFunction}*/ (method)?.name, | |
| ) | |
| // viem and wagmi barf if we pass in undefined or [] for args so do this to accommodate viem and wagmi | |
| * Creates a write action for a specific contract method. | |
| * @param {...any} args - The arguments for the contract method. | |
| * @returns {object} An object representing the write action, including ABI and method information. | |
| */ | |
| // Precompute methodAbi outside of creator function to improve efficiency | |
| const methodAbi = methods.filter( | |
| (m) => | |
| /**@type {import('@tevm/utils').AbiFunction}*/ (m).name === | |
| /**@type {import('@tevm/utils').AbiFunction}*/ (method)?.name, | |
| ) | |
| const creator = (...args) => { | |
| // Avoid passing undefined or empty arrays for args to ensure compatibility with viem and wagmi | |
| const maybeArgs = args.length > 0 ? { args } : {} |
| ...(code !== undefined ? { code } : {}), | ||
| ...maybeArgs, | ||
| } | ||
| } | ||
| } | ||
| creator.abi = [method] | ||
| creator.humanReadableAbi = formatAbi([method]) | ||
| creator.code = code | ||
| creator.address = address | ||
| creator.to = address | ||
| return [/**@type {import('@tevm/utils').AbiFunction}*/ (method).name, creator] | ||
| }), | ||
| creator.abi = [method] | ||
| creator.humanReadableAbi = formatAbi([method]) | ||
| creator.code = code | ||
| creator.address = address | ||
| creator.to = address | ||
| return [/**@type {import('@tevm/utils').AbiFunction}*/ (method).name, creator] |
There was a problem hiding this comment.
Review the duplication of address and to properties
Both address and to properties are set to the same value in the returned action object and the creator function. If both are required for external compatibility, that's fine; otherwise, consider consolidating them to reduce redundancy.
Ensure that having both properties is necessary. If not, you could streamline the code:
- ...(address !== undefined ? { address, to: address } : {}),
+ ...(address !== undefined ? { to: address } : {}),And similarly for the creator properties:
- creator.address = addressCommittable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/contract/src/Contract.ts (2 hunks)
- packages/contract/src/createContract.js (2 hunks)
- packages/contract/src/createContract.spec.ts (6 hunks)
- packages/contract/src/read/readFactory.spec.ts (3 hunks)
- packages/contract/src/write/writeFactory.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/contract/src/Contract.ts
Additional context used
Biome
packages/contract/src/createContract.spec.ts
[error] 46-46: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
Additional comments not posted (13)
packages/contract/src/write/writeFactory.spec.ts (1)
78-93: LGTM!The new test case is well-written and covers an important scenario of handling contracts without
deployedBytecode. The test follows the AAA pattern, uses meaningful variable names, and provides clear assertions. It improves the overall test coverage and ensures that the code can handle the specific scenario correctly.packages/contract/src/createContract.js (5)
8-71: Documentation looks great!The JSDoc comment block has been thoroughly updated to reflect the changes in the
createContractfunction:
- The description now mentions support for both human-readable and JSON ABI formats.
- The new
@throwstag documents the validation check for ABI presence.- The two
@exampleblocks provide clear usage demonstrations for both ABI formats, using an ERC20 token contract.- The
@seetags have been updated with links to relevant documentation.Overall, the documentation is now more comprehensive and informative.
Line range hint
76-95: The updated function signature and ABI handling logic look solid!
- The function now accepts both
humanReadableAbiandabiparameters, providing flexibility in ABI format.- The initial logic ensures that at least one ABI format is provided, throwing an
InvalidParamsErrorwith a helpful message and documentation link if neither is present.- The ABI parsing and formatting logic is correctly implemented, handling both cases of
_abiand_humanReadableAbibeing provided.Great job on enhancing the function's parameter handling and validation!
127-139: The newwithCodefunction is a great addition!
- It provides a convenient way to create a new contract instance with a specific bytecode.
- The function correctly spreads the existing
baseContractproperties and overrides thecodeproperty with the providedencodedBytecode.- The JSDoc comment accurately annotates the
encodedBytecodeparameter type.This function enhances the flexibility of the
createContractfunction by allowing bytecode customization.
142-143: ExposingwithCodein the returned contract object is a nice touch!By adding
withCodeto the returned contract object, you make it easily accessible to the users of thecreateContractfunction. This allows them to conveniently create new contract instances with custom bytecode when needed.
149-151: The bytecode availability check in thedeployfunction is a valuable addition!By adding a check to ensure that
bytecodeis available before generating deploy data, you prevent the generation of invalid deploy data whenbytecodeis missing. Throwing an error with a clear message helps users identify the issue and take appropriate action.This check enhances the robustness of the
deployfunction and provides a better user experience by catching potential issues early.packages/contract/src/read/readFactory.spec.ts (2)
1-1: LGTM!The new import statement for
encodeDeployDatafollows the existing import style and is valid. It's likely used in the test file based on the file name.
140-154: Great test case!This test case covers an important scenario of a contract without
deployedBytecode. It verifies the expected behavior and structure of the object returned by thereadmethod. The assertions are comprehensive and improve the overall test coverage.packages/contract/src/createContract.spec.ts (5)
17-32: LGTM!The new test case for
deployedBytecodeproperty looks good. It correctly asserts the behavior when the property is not provided and when it is provided.
42-47: LGTM!The new test case for the
deploymethod correctly asserts that an error is thrown whenbytecodeis not provided. The use oftoThrowErrorMatchingInlineSnapshotis a good way to ensure the error message doesn't change unexpectedly.Tools
Biome
[error] 46-46: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
Line range hint
50-196: LGTM!The changes to the
deploymethod test case look good:
- The snapshot for the first test scenario has been updated to match the current behavior.
- The new test scenario for a constructor with arguments has been added and the snapshot correctly includes the
argsproperty.
388-396: LGTM!The new test case correctly verifies that the
withCodemethod is defined on the contract object and is of type 'function'. This is a good sanity check for the new functionality.
398-405: LGTM!The new test case for the
withCodemethod correctly asserts that callingwithCodewith a new code value returns a new contract object with thecodeproperty updated to the provided value. This test covers the core functionality of thewithCodemethod.
| }).withCode( | ||
| encodeDeployData({ | ||
| abi: dummyAbi, | ||
| bytecode: '0x420', | ||
| }), | ||
| ) |
There was a problem hiding this comment.
Tip
Codebase Verification
The .script() method is still in use and has not been fully deprecated.
The .script() method is still being used in the following files:
packages/decorators/src/actions/TevmActionsApi.tstest/bench/src/script/script.test.tspackages/client-types/src/TevmClient.tspackages/actions/src/Script/ScriptHandlerType.tspackages/actions/src/Call/CallParams.ts
Please ensure that .script() is properly deprecated or replaced with .withCode() across the codebase for consistency.
Analysis chain
Verify the impact of the .script() to .withCode() change.
The change from .script() to .withCode() suggests a refactoring or renaming of the method. Please ensure that this change is consistently applied across the codebase and that the .script() method has been properly deprecated or removed.
Additionally, verify if the encodeDeployData function is now the standard way of encoding deployment data throughout the codebase for consistency.
Run the following script to verify the usage of .script() and .withCode() methods:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `.script()` and `.withCode()` methods in the codebase.
# Test 1: Search for the usage of `.script()` method. Expect: No occurrences.
rg --type typescript -g '!*.spec.ts' $'\.script\('
# Test 2: Search for the usage of `.withCode()` method. Expect: Occurrences in relevant files.
rg --type typescript -g '!*.spec.ts' $'\.withCode\('
# Test 3: Search for the usage of `encodeDeployData` function. Expect: Occurrences in relevant files.
rg --type typescript -g '!*.spec.ts' $'encodeDeployData\('
Length of output: 270
Script:
#!/bin/bash
# Description: Verify the usage of `.script()` and `.withCode()` methods in the codebase.
# Test 1: Search for the usage of `.script()` method. Expect: No occurrences.
rg --type ts -g '!*.spec.ts' '\.script\('
# Test 2: Search for the usage of `.withCode()` method. Expect: Occurrences in relevant files.
rg --type ts -g '!*.spec.ts' '\.withCode\('
# Test 3: Search for the usage of `encodeDeployData` function. Expect: Occurrences in relevant files.
rg --type ts -g '!*.spec.ts' 'encodeDeployData\('
Length of output: 2176
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
7bb2864 to
5fb90ff
Compare
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
2b45d1a to
a29bb45
Compare
…struction_worker_chore_productionize_contract_package
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
2534b11 to
2e8e47d
Compare
roninjin10
force-pushed
the
09-21-_construction_worker_chore_productionize_contract_package
branch
from
ed00a30 to
f0376e7
Compare
roninjin10
deleted the
09-21-_construction_worker_chore_productionize_contract_package
branch
Description
Concise description of proposed changes
Testing
Explain the quality checks that have been done on the code changes
Additional Information
Your ENS/address:
Summary by CodeRabbit
New Features
withCodemethod to the@tevm/contractpackage for enhanced contract code management.createContractfunction to support both human-readable and JSON ABI formats.Contracttype, including updated examples and expanded comments.Bug Fixes
@tevm/contractpackage.Documentation
Contract,createContract, and other related functions.@tevm/contractpackage.Chores
"sideEffects": falseproperty to multiplepackage.jsonfiles for better bundler optimization.