fix: use callOptions in call/withdrawAndCall #195
Conversation
📝 WalkthroughWalkthroughThis pull request involves the deletion of several Solidity smart contracts and associated files, primarily related to gas price management and token interactions. The Changes
Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@fadeev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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
Documentation and Community
|
![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 (14)
packages/tasks/src/index.ts (1)
Line range hint
1-20: Consider documenting the task migration for downstream consumers.The removal of
depositTask,verifyTask, andwithdrawTaskin favor of new EVM-specific implementations represents a breaking change that may affect downstream consumers.Consider:
- Adding migration guides in the documentation
- Ensuring the new EVM-specific tasks provide equivalent functionality
- If possible, maintaining temporary backwards compatibility or providing clear upgrade paths
packages/tasks/src/zetachainCall.ts (1)
80-86: Consider improving gas limit documentationWhile the parameters are well-structured, having multiple gas limit parameters (
txOptionsGasLimit,callOptionsGasLimit,onRevertGasLimit) might be confusing for users. Consider adding documentation that clarifies the purpose and differences between these gas limits..addOptionalParam( "callOptionsGasLimit", - "The gas limit for the call", + "The gas limit for the contract execution on ZetaChain (distinct from transaction gas limit)", 7000000, types.int )packages/client/src/evmCall.ts (1)
Line range hint
66-84: Consider documenting the hardcoded abort addressThe abort address is hardcoded to zero address with a comment "not used". Consider adding more detailed documentation explaining why this parameter exists but isn't used.
{ - abortAddress: "0x0000000000000000000000000000000000000000", // not used + abortAddress: "0x0000000000000000000000000000000000000000", // Zero address as this parameter is reserved for future usepackages/tasks/src/zetachainWithdrawAndCall.ts (2)
11-14: Consider adding type definitions for the args parameter.The
argsparameter is typed asany, which loses type safety. Consider creating an interface that defines the expected shape of the arguments.interface WithdrawAndCallArgs { callOptionsGasLimit: number; callOptionsIsArbitraryCall: boolean; amount: string; // ... other properties }
Line range hint
18-35: Enhance error handling with specific error types.The current error handling only logs the error. Consider:
- Adding specific error types for different failure scenarios
- Providing more detailed error information to users
- Potentially retrying on certain types of failures
try { // ... existing code ... } catch (e) { if (e instanceof ContractTransactionError) { console.error("Contract interaction failed:", e.message); // Potential retry logic } else if (e instanceof NetworkError) { console.error("Network error:", e.message); } else { console.error("Unexpected error:", e); } throw e; // Re-throw to allow handling by caller }packages/client/src/evmDeposit.ts (2)
Line range hint
41-49: Consider cleaning up unused revertOptions fields.The revertOptions object contains fields marked as "not used". Consider removing these fields or documenting why they're retained.
const revertOptions = { - abortAddress: "0x0000000000000000000000000000000000000000", // not used callOnRevert: args.revertOptions.callOnRevert, onRevertGasLimit: args.revertOptions.onRevertGasLimit, revertAddress: args.revertOptions.revertAddress, - // not used revertMessage: utils.hexlify( utils.toUtf8Bytes(args.revertOptions.revertMessage) ), };
Line range hint
67-68: Add error handling for ERC20 approve.The ERC20 approve transaction could fail. Consider adding proper error handling and checking the approval result.
- await erc20Contract.connect(signer).approve(args.gatewayEvm, value); + const approveTx = await erc20Contract.connect(signer).approve(args.gatewayEvm, value); + await approveTx.wait(); // Wait for approval confirmation + // Verify allowance + const allowance = await erc20Contract.allowance(await signer.getAddress(), args.gatewayEvm); + if (allowance.lt(value)) { + throw new Error("Failed to approve ERC20 transfer"); + }packages/client/src/zetachainWithdraw.ts (1)
Line range hint
82-89: Consider improving type safety and maintainabilityA few suggestions to enhance the code:
- Consider defining the method signature as a constant to prevent typos
- Consider creating a dedicated type for the revert options tuple structure
+const WITHDRAW_METHOD = "withdraw(bytes,uint256,address,(address,bool,address,bytes,uint256))" as const; - const method = - "withdraw(bytes,uint256,address,(address,bool,address,bytes,uint256))"; - const tx = await gateway[method]( + const tx = await gateway[WITHDRAW_METHOD](packages/client/src/zetachainCall.ts (3)
20-21: Improve documentation for callOptions parameterThe
callOptionsparameter documentation should specify the available properties, particularlygasLimitwhich was moved into this object.- * @param {object} args.callOptions - Call options. + * @param {object} args.callOptions - Call options for the contract call. + * @param {number} args.callOptions.gasLimit - The gas limit for the transaction. + * @param {boolean} [args.callOptions.isArbitraryCall] - Optional flag for arbitrary calls.
91-91: Add validation for required callOptions propertiesThe code assumes
gasLimitexists in callOptions without validation. Consider adding a check.+ if (typeof args.callOptions?.gasLimit !== 'number') { + throw new Error('gasLimit is required in callOptions and must be a number'); + } const [gasZRC20, gasFee] = await zrc20.withdrawGasFeeWithGasLimit( args.callOptions.gasLimit );
101-106: Consider extracting the function signature to a constantThe complex function signature would be more maintainable if extracted to a named constant. Also, consider adding a comment explaining the tuple structure.
+ // Function signature for gateway.call with parameters: + // - bytes: receiver address + // - address: gas ZRC20 address + // - bytes: encoded message + // - tuple(uint256 gasLimit, bool isArbitraryCall): call options + // - tuple(address revertAddress, bool callOnRevert, ...): revert options + const GATEWAY_CALL_SIGNATURE = + "call(bytes,address,bytes,(uint256,bool),(address,bool,address,bytes,uint256))"; + const tx = await gateway[ - "call(bytes,address,bytes,(uint256,bool),(address,bool,address,bytes,uint256))" + GATEWAY_CALL_SIGNATURE ](packages/client/src/evmDepositAndCall.ts (2)
Line range hint
71-86: Add validation for types and values array length mismatchThe type conversion logic should validate that
args.typesandargs.valueshave the same length before processing.Consider adding this validation at the start of the mapping:
+ if (args.types.length !== args.values.length) { + throw new Error('Types and values arrays must have the same length'); + } const valuesArray = args.values.map((value, index) => {
Line range hint
103-111: Consider implementing safer ERC20 approval patternThe current implementation might be vulnerable to front-running attacks. Consider implementing the following improvements:
- Check current allowance before approval
- Reset allowance to 0 first if needed
- Add error handling for failed approvals
Example implementation:
if (args.erc20) { const erc20Contract = new ethers.Contract(args.erc20, ERC20_ABI.abi, signer); const decimals = await erc20Contract.decimals(); const value = utils.parseUnits(args.amount, decimals); // Check current allowance const currentAllowance = await erc20Contract.allowance(await signer.getAddress(), args.gatewayEvm); if (currentAllowance.gt(0)) { await erc20Contract.approve(args.gatewayEvm, 0); } // Set new allowance const approveTx = await erc20Contract.approve(args.gatewayEvm, value); await approveTx.wait(); // Wait for confirmation // Continue with deposit... }packages/client/src/zetachainWithdrawAndCall.ts (1)
Inconsistent handling of arbitrary calls detected
The implementation differs between the two files:
zetachainCall.tsalways prepends the function signature to the encoded parameterszetachainWithdrawAndCall.tsonly prepends it whencallOptions.isArbitraryCallis trueThis inconsistency could lead to unexpected behavior. The
zetachainCallfunction should also checkisArbitraryCallbefore prepending the function signature.
packages/client/src/zetachainCall.ts: Update to conditionally prepend function signature based oncallOptions.isArbitraryCall🔗 Analysis chain
Line range hint
1-141: Verify consistent implementation across related filesSince this change introduces a new pattern using
callOptions, we should ensure consistent implementation across related files.
Let me check the implementation details in the related files to ensure consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related files that might need similar updates echo "Searching for related files..." rg -l "withdrawAndCall|zetachainCall" --type ts # Check if they use consistent parameter structure echo "Checking parameter consistency..." rg "callOptions.*:.*any" --type tsLength of output: 571
Script:
#!/bin/bash # Check implementation in zetachainCall.ts echo "Content of zetachainCall.ts:" cat packages/client/src/zetachainCall.ts # Check if both files use similar parameter structure and handling echo -e "\nChecking parameter handling consistency:" rg "callOptions\.isArbitraryCall" --type ts -A 3Length of output: 4642
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (73)
typechain-types/@openzeppelin/contracts/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC1155Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC20Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC721Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/ERC20.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/IERC20.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/Revert.sol/Revertable.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/Revert.sol/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/evm/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaReceiver.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaTokenConsumer.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/evm/interfaces/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/IZRC20.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/IZRC20Metadata.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/ZRC20Events.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/UniversalContract.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/ZContract.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/index.tsis excluded by!typechain-types/**typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/index.tsis excluded by!typechain-types/**typechain-types/contracts/EthZetaMock.sol/ZetaEthMock.tsis excluded by!typechain-types/**typechain-types/contracts/TestSystemContract.tsis excluded by!typechain-types/**typechain-types/contracts/TestZRC20.tsis excluded by!typechain-types/**typechain-types/contracts/ZetaConnectorMock.sol/ZetaConnectorMockValue.tsis excluded by!typechain-types/**typechain-types/contracts/index.tsis excluded by!typechain-types/**typechain-types/contracts/shared/MockSystemContract.sol/MockSystemContract.tsis excluded by!typechain-types/**typechain-types/contracts/shared/MockSystemContract.sol/index.tsis excluded by!typechain-types/**typechain-types/contracts/shared/MockZRC20.tsis excluded by!typechain-types/**typechain-types/contracts/shared/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC1155Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC20Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC721Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/ERC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/IERC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/Revert.sol/Revertable__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/Revert.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaCommonErrors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaConnector__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaReceiver__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/ZetaTokenConsumer__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/evm/interfaces/ZetaInterfaces.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/index.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/SystemContract.sol/SystemContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/IZRC20Metadata__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/IZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/ZRC20Events__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/UniversalContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/ZContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/UniversalContract.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/@zetachain/protocol-contracts/contracts/zevm/interfaces/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/EthZetaMock.sol/ZetaEthMock__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/OnlySystem__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/TestSystemContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/TestZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/ZetaConnectorMock.sol/ZetaConnectorMockValue__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/ZetaConnectorMock.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockSystemContract.sol/MockSystemContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockSystemContract.sol/SystemContractErrors__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockSystemContract.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/index.tsis excluded by!typechain-types/**typechain-types/hardhat.d.tsis excluded by!typechain-types/**typechain-types/index.tsis excluded by!typechain-types/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (26)
contracts/TestSystemContract.sol(0 hunks)contracts/ZetaConnectorMock.sol(0 hunks)contracts/shared/MockSystemContract.sol(0 hunks)package.json(1 hunks)packages/client/src/client.ts(0 hunks)packages/client/src/deposit.ts(0 hunks)packages/client/src/evmCall.ts(1 hunks)packages/client/src/evmDeposit.ts(1 hunks)packages/client/src/evmDepositAndCall.ts(1 hunks)packages/client/src/getBalances.ts(1 hunks)packages/client/src/getFees.ts(1 hunks)packages/client/src/getPools.ts(1 hunks)packages/client/src/getQuote.ts(1 hunks)packages/client/src/index.ts(0 hunks)packages/client/src/sendZeta.ts(1 hunks)packages/client/src/withdraw.ts(0 hunks)packages/client/src/zetachainCall.ts(5 hunks)packages/client/src/zetachainWithdraw.ts(1 hunks)packages/client/src/zetachainWithdrawAndCall.ts(5 hunks)packages/tasks/src/deposit.ts(0 hunks)packages/tasks/src/index.ts(1 hunks)packages/tasks/src/withdraw.ts(0 hunks)packages/tasks/src/zetachainCall.ts(2 hunks)packages/tasks/src/zetachainWithdrawAndCall.ts(2 hunks)packages/test/src/helpers.ts(0 hunks)packages/test/src/index.ts(0 hunks)
💤 Files with no reviewable changes (11)
- contracts/TestSystemContract.sol
- contracts/ZetaConnectorMock.sol
- contracts/shared/MockSystemContract.sol
- packages/client/src/client.ts
- packages/client/src/deposit.ts
- packages/client/src/index.ts
- packages/client/src/withdraw.ts
- packages/tasks/src/deposit.ts
- packages/tasks/src/withdraw.ts
- packages/test/src/helpers.ts
- packages/test/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- packages/client/src/getBalances.ts
- packages/client/src/getFees.ts
- packages/client/src/getPools.ts
- packages/client/src/getQuote.ts
🔇 Additional comments (16)
packages/tasks/src/index.ts (1)
4-6: LGTM! The new export statements follow consistent naming patterns.
The addition of EVM-specific tasks (evmCall, evmDeposit, evmDepositAndCall) aligns with the existing naming conventions in the codebase (e.g., solanaDeposit, zetachainCall).
Let's verify the new task implementations exist:
✅ Verification successful
Let me verify the implementation details of these tasks to ensure they follow consistent patterns:
The new EVM-specific task implementations are properly structured and consistent
The implementation of all three tasks (evmCall, evmDeposit, evmDepositAndCall) follows a consistent pattern:
- Each task properly exports its function
- All tasks use the
ZetaChainClientfrom the client package - They handle similar parameters (gasLimit, gasPrice, revertOptions) consistently
- Error handling is implemented uniformly across all tasks
- Task registration includes appropriate parameter definitions with descriptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and basic structure of the new task files
# Check if the new task files exist and contain exports
for task in evmCall evmDeposit evmDepositAndCall; do
echo "Checking $task..."
if [ -f "packages/tasks/src/${task}.ts" ]; then
echo "File exists. Checking content..."
rg -l "export.*${task}" "packages/tasks/src/${task}.ts"
else
echo "Warning: File packages/tasks/src/${task}.ts not found"
fi
done
Length of output: 989
Script:
#!/bin/bash
# Check the implementation of each task file
for task in evmCall evmDeposit evmDepositAndCall; do
echo "=== Content of ${task}.ts ==="
cat "packages/tasks/src/${task}.ts"
echo -e "\n"
done
Length of output: 6712
packages/tasks/src/zetachainCall.ts (2)
10-13: LGTM: Clean consolidation of call options
The creation of a dedicated callOptions object improves code organization by grouping related parameters.
19-19: Verify client interface compatibility
Let's ensure the ZetaChainClient correctly expects this callOptions structure.
✅ Verification successful
Based on the search results, I can now generate the final response:
Client interface is compatible with callOptions
The callOptions parameter is correctly used in the implementation. The client's zetachainCall function in packages/client/src/zetachainCall.ts explicitly accepts callOptions as part of its arguments interface:
args: {
callOptions: any;
function: string;
gatewayZetaChain: string;
...
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the client interface definition and usage
ast-grep --pattern 'interface $_ {
$$$
zetachainCall($_: {
callOptions: $_,
$$$
}): $_
$$$
}'
Length of output: 111
Script:
#!/bin/bash
# Let's try a broader search for zetachainCall usage and definition
rg "zetachainCall" -A 5 -B 5
Length of output: 6134
packages/client/src/evmCall.ts (3)
Line range hint 39-54: LGTM! Robust type conversion implementation
The parameter processing logic is well-implemented with:
- Proper type conversion for different parameter types
- Explicit error handling for boolean values
- Safe numeric handling using BigNumber
Line range hint 8-24: Verify implementation of callOptions
The PR title mentions "use callOptions in call/withdrawAndCall", but I don't see callOptions being implemented in this function. Should this be part of the changes?
1-1: Verify @zetachain/protocol-contracts dependency
The import path has been changed to use an external package. Let's verify this dependency is properly configured.
✅ Verification successful
The import path change is valid and the dependency is properly configured
The verification shows that @zetachain/protocol-contracts version "11.0.0-rc3" is correctly listed as a dependency in the root package.json. While the ABI file is not found in node_modules (which is expected in a sandbox environment), we found the same ABI file at ./packages/tasks/src/abi/GatewayEVM.sol/GatewayEVM.json, indicating this is a monorepo setup where the contract artifacts are shared between packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the dependency exists in package.json
jq '.dependencies["@zetachain/protocol-contracts"] // .devDependencies["@zetachain/protocol-contracts"]' package.json
# Verify the ABI file exists in the package
fd -t f "GatewayEVM.json" node_modules/@zetachain/protocol-contracts
Length of output: 331
Script:
#!/bin/bash
# Check if the dependency exists in the client package
jq '.dependencies["@zetachain/protocol-contracts"] // .devDependencies["@zetachain/protocol-contracts"]' packages/client/package.json
# Check if the ABI file exists in the expected path
fd -t f "GatewayEVM.json" packages/client/node_modules/@zetachain/protocol-contracts
# Let's also verify the import path structure in protocol-contracts package
fd -t f "GatewayEVM.json" .
Length of output: 537
Script:
#!/bin/bash
# Let's check the root package.json for workspace configuration
cat package.json
# Check the content of the found ABI file to verify it's the correct one
head -n 5 ./packages/tasks/src/abi/GatewayEVM.sol/GatewayEVM.json
# List all package.json files to understand project structure
fd package.json
Length of output: 135545
packages/tasks/src/zetachainWithdrawAndCall.ts (1)
86-92: Verify the default gas limit value.
The default gas limit of 7,000,000 seems high. Please verify if this value is appropriate for the expected operations.
packages/client/src/sendZeta.ts (2)
Line range hint 1-124: Implementation looks good and maintains backward compatibility
The changes are limited to updating import paths while preserving the existing functionality. The code continues to handle both native and non-native chains correctly, and the contract interaction patterns remain consistent with the new interfaces.
2-4: Verify ABI compatibility with the new contract interfaces
The imports have been updated to use new contract naming conventions (Native/NonNative instead of ETH/ZEVM). While this change aligns with the package update to v11.0.0-rc3, we should verify the compatibility of these new interfaces.
Let's verify the ABI compatibility:
✅ Verification successful
Import changes align with protocol-contracts package update
The changes to the imports are consistent with the protocol-contracts v11.0.0-rc3 update, which introduces a more generic naming convention (Native/NonNative). The code usage shows these imports are correctly referenced in the contract instantiation:
ZetaConnectorZEVM.abi/ZetaConnectorEth.abifor connector selection based onfromZetaChainZetaToken.abifor token operations
Since these changes are part of an official package update (v11.0.0-rc3) and the imports are properly aligned with their usage in the code, the changes are safe. The ABI files are not directly in the repository as they come from the @zetachain/protocol-contracts package dependency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the ABIs of old and new contracts to ensure compatibility
# Check if the new ABIs contain the required methods used in the code
echo "Verifying send method in ZetaConnectorNative:"
ast-grep --pattern 'function send($$$) {$$$}' packages/protocol-contracts/abi/ZetaConnectorNative.sol/ZetaConnectorNative.json
echo "Verifying approve method in ZetaNonEth:"
ast-grep --pattern 'function approve($$$) {$$$}' packages/protocol-contracts/abi/Zeta.non-eth.sol/ZetaNonEth.json
Length of output: 674
Script:
#!/bin/bash
# Let's first check if these files exist in the repository and their actual locations
echo "Searching for ABI files:"
fd -e json -d 5 "ZetaNonEth.json|ZetaConnectorNative.json|ZetaConnectorNonNative.json"
# Let's also check the package.json to understand the dependency
echo -e "\nChecking package dependency:"
fd "package.json" --type f --exec grep -l "@zetachain/protocol-contracts"
Length of output: 307
Script:
#!/bin/bash
# Let's examine the package.json to understand the dependency version
echo "Checking protocol-contracts dependency version:"
cat package.json | grep -A 2 "@zetachain/protocol-contracts"
# Let's also check if these imports are used in the code
echo -e "\nChecking usage of imports:"
rg "ZetaToken|ZetaConnectorEth|ZetaConnectorZEVM" packages/client/src/sendZeta.ts -A 2
Length of output: 907
packages/client/src/zetachainWithdraw.ts (2)
1-2: LGTM: Import centralization improves maintainability
The transition from local ABI imports to @zetachain/protocol-contracts package imports is a positive change that centralizes contract definitions and improves maintainability.
Line range hint 1-100: Verify PR title alignment with changes
The PR title mentions "use callOptions in call/withdrawAndCall" but this file shows different changes (ABI import updates). Please verify if:
- Additional changes are needed in this file to implement callOptions
- The PR title needs updating to reflect the actual changes
✅ Verification successful
Let me check the content of the withdraw-related files to understand if they need callOptions implementation.
PR title accurately reflects the changes, no issues found
The PR title "use callOptions in call/withdrawAndCall" is accurate as:
- The changes in
zetachainWithdraw.tsare only import reordering callOptionsis already implemented inzetachainWithdrawAndCall.tsand is being used for gas limit and arbitrary call functionality- No additional changes are needed in
zetachainWithdraw.tsas it's a simpler withdraw function that doesn't require callOptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for files containing callOptions implementation
rg -l 'callOptions'
# Search for other withdraw-related files that might need similar updates
fd -e ts -e sol withdraw
Length of output: 390
Script:
#!/bin/bash
# Check content of withdrawAndCall files to see callOptions implementation
rg -A 5 -B 5 'callOptions' packages/client/src/zetachainWithdrawAndCall.ts
rg -A 5 -B 5 'callOptions' packages/tasks/src/zetachainWithdrawAndCall.ts
# Check if there are any recent changes to these files
git diff HEAD~1 packages/client/src/zetachainWithdraw.ts
git diff HEAD~1 packages/client/src/zetachainWithdrawAndCall.ts
Length of output: 4654
packages/client/src/zetachainCall.ts (1)
Line range hint 33-106: Verify consistent usage of callOptions across the codebase
Let's ensure this change is consistently applied in all related files.
✅ Verification successful
Changes are consistently implemented across the codebase
The verification shows that the withdrawGasFeeWithGasLimit usage with callOptions.gasLimit is consistent in both zetachainCall.ts and zetachainWithdrawAndCall.ts. Both implementations follow the same pattern of using the gas limit from callOptions, and the subsequent gateway interactions maintain consistency in parameter handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of withdrawGasFeeWithGasLimit to ensure consistent usage
rg -l "withdrawGasFeeWithGasLimit"
# Search for other gateway.call invocations to verify consistent parameter passing
ast-grep --pattern 'gateway[$$$].call($$$)'
Length of output: 1094
Script:
#!/bin/bash
# Let's check the implementation in zetachainWithdrawAndCall.ts to compare the usage
rg -A 20 "withdrawGasFeeWithGasLimit" packages/client/src/zetachainWithdrawAndCall.ts
# Let's also check for any other gateway call patterns in the codebase
rg -A 10 'gateway\[.*call.*\]' packages/client/
Length of output: 692
packages/client/src/evmDepositAndCall.ts (2)
Line range hint 8-24: Well-documented implementation with clear parameter descriptions
The function documentation is comprehensive and accurately describes all parameters and their purposes.
2-2: Verify @zetachain/protocol-contracts dependency
The import path has been updated to use the external package. Let's ensure the dependency is properly configured.
package.json (1)
99-99: Review the use of release candidate version in production.
The dependency @zetachain/protocol-contracts is being updated to a release candidate version (11.0.0-rc3). This represents a major version bump from 9.0.0 and could introduce breaking changes.
Consider the following:
- Release candidates are pre-release versions and may contain bugs
- Major version bumps (9.x.x → 11.x.x) typically include breaking changes
Let's verify the changes and compatibility:
packages/client/src/zetachainWithdrawAndCall.ts (1)
1-2: LGTM: Improved modularity with centralized ABI imports
The change to import ABIs from @zetachain/protocol-contracts is a good practice that promotes better maintainability and version control.
| .addParam("amount", "The amount of tokens to send") | ||
| .addParam("function", `Function to call (example: "hello(string)")`) | ||
| .addOptionalParam("function", `Function to call (example: "hello(string)")`) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for function parameter.
Since the function parameter is now optional, consider adding validation to ensure either:
- A function is provided, or
- callOptionsIsArbitraryCall is true
This will prevent invalid states where both are missing.
if (!args.function && !args.callOptionsIsArbitraryCall) {
throw new Error("Either function must be provided or callOptionsIsArbitraryCall must be true");
}| @@ -1,7 +1,7 @@ | |||
| import ERC20_ABI from "@openzeppelin/contracts/build/contracts/ERC20.json"; | |||
| import GatewayABI from "@zetachain/protocol-contracts/abi/GatewayEVM.sol/GatewayEVM.json"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Update required: More files need the same import change
The ABI structure is compatible, containing both required deposit method signatures. However, two other files still use the old local import path:
- packages/client/src/evmCall.ts
- packages/client/src/evmDepositAndCall.ts
These files should also be updated to use the centralized ABI from @zetachain/protocol-contracts package for consistency.
🔗 Analysis chain
LGTM! Centralized ABI import.
The change to import GatewayABI from @zetachain/protocol-contracts package is a good practice as it centralizes the ABI definitions.
Let's verify the ABI compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the GatewayABI structure remains compatible
# by checking for the required deposit methods and their signatures
# Check if the required deposit methods exist in the ABI
ast-grep --pattern 'deposit(address,uint256,address,(address,bool,address,bytes,uint256))' && \
ast-grep --pattern 'deposit(address,(address,bool,address,bytes,uint256))'
# Additionally, check if there are any other files still using the old import path
rg -l "GatewayEVM.sol/GatewayEVM.json" --type ts
Length of output: 324
Script:
#!/bin/bash
# Let's check the content of the ABI file to verify the method signatures
fd GatewayEVM.json -t f --exec cat {} | jq '.abi[] | select(.type == "function" and .name == "deposit") | {name: .name, inputs: .inputs}'
# Also check other files that might need the same import update
rg -l --type ts "./abi/GatewayEVM.sol/GatewayEVM.json"
Length of output: 2241
| @@ -30,8 +30,8 @@ import type { revertOptions, txOptions } from "./types"; | |||
| export const zetachainCall = async function ( | |||
| this: ZetaChainClient, | |||
| args: { | |||
| callOptions: any; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace 'any' type with proper interface definition
Using any type reduces type safety. Define an interface for the callOptions object.
- callOptions: any;
+ callOptions: {
+ gasLimit: number;
+ isArbitraryCall?: boolean;
+ };📝 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.
| callOptions: any; | |
| callOptions: { | |
| gasLimit: number; | |
| isArbitraryCall?: boolean; | |
| }; |
| @@ -18,7 +18,7 @@ import type { revertOptions, txOptions } from "./types"; | |||
| * @param {string} args.types - JSON string representing the types of the function parameters (e.g., ["uint256", "address"]). | |||
| * @param {Array} args.values - The values to be passed to the function (should match the types). | |||
| * @param {string} args.zrc20 - The address of the ZRC20 token contract used for the withdrawal and fee payment. | |||
| * @param {number} args.gasLimit - The gas limit for the transaction. | |||
| * @param {any} args.callOptions - Call options. | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define explicit type for callOptions parameter
Using any type reduces type safety and IDE support. Consider creating an interface that explicitly defines the structure of callOptions.
interface CallOptions {
isArbitraryCall: boolean;
gasLimit: number | string;
// Add other relevant fields
}Then update the parameter type:
- callOptions: any;
+ callOptions: CallOptions;Also applies to: 35-36
| const zrc20 = new ethers.Contract(args.zrc20, ZRC20ABI.abi, signer); | ||
| const decimals = await zrc20.decimals(); | ||
| const value = utils.parseUnits(args.amount, decimals); | ||
| const [gasZRC20, gasFee] = await zrc20.withdrawGasFeeWithGasLimit( | ||
| args.gasLimit | ||
| args.callOptions.gasLimit |
There was a problem hiding this comment.
Add validation for gasLimit parameter
The gasLimit is used in withdrawGasFeeWithGasLimit without validation. Consider adding checks to ensure it's a valid positive number.
+ if (!args.callOptions.gasLimit || args.callOptions.gasLimit <= 0) {
+ throw new Error("Invalid gasLimit: must be a positive number");
+ }
const [gasZRC20, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(
args.callOptions.gasLimit
);Also applies to: 129-137
| let message; | ||
|
|
||
| if (args.callOptions.isArbitraryCall && args.function) { | ||
| let functionSignature = ethers.utils.id(args.function).slice(0, 10); | ||
| message = ethers.utils.hexlify( | ||
| ethers.utils.concat([functionSignature, encodedParameters]) | ||
| ); | ||
| } else { | ||
| message = encodedParameters; | ||
| } |
There was a problem hiding this comment.
Add validation for function parameter in arbitrary calls
When isArbitraryCall is true but function is undefined, the code will fail silently. Add explicit validation to prevent runtime errors.
- if (args.callOptions.isArbitraryCall && args.function) {
+ if (args.callOptions.isArbitraryCall) {
+ if (!args.function) {
+ throw new Error("Function parameter is required for arbitrary calls");
+ }📝 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.
| let message; | |
| if (args.callOptions.isArbitraryCall && args.function) { | |
| let functionSignature = ethers.utils.id(args.function).slice(0, 10); | |
| message = ethers.utils.hexlify( | |
| ethers.utils.concat([functionSignature, encodedParameters]) | |
| ); | |
| } else { | |
| message = encodedParameters; | |
| } | |
| let message; | |
| if (args.callOptions.isArbitraryCall) { | |
| if (!args.function) { | |
| throw new Error("Function parameter is required for arbitrary calls"); | |
| } | |
| let functionSignature = ethers.utils.id(args.function).slice(0, 10); | |
| message = ethers.utils.hexlify( | |
| ethers.utils.concat([functionSignature, encodedParameters]) | |
| ); | |
| } else { | |
| message = encodedParameters; | |
| } |
callOptionsSummary by CodeRabbit
Release Notes
evmCall,evmDeposit, andevmDepositAndCall.@zetachain/protocol-contractsto11.0.0-rc3.