Added gateway call to examples #200
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a new function Changes
Possibly related PRs
Suggested reviewers
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| function gatewayCall( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| (, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); | ||
| IZRC20(zrc20).approve(address(gateway), gasFee); | ||
| gateway.call(receiver, zrc20, message, gasLimit, revertOptions); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
| function gatewayCall( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| (, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); | ||
| IZRC20(zrc20).approve(address(gateway), gasFee); | ||
| gateway.call(receiver, zrc20, message, gasLimit, revertOptions); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (2)
examples/hello/tasks/gatewayCall.ts (2)
31-37: Simplify and enhance boolean value parsingUsing
JSON.parsefor boolean conversion can be unreliable. A direct comparison offers clarity and reduces potential errors.Refactor the boolean parsing logic:
if (type === "bool") { - try { - return JSON.parse(value.toLowerCase()); - } catch (e) { - throw new Error(`Invalid boolean value: ${value}`); - } + const lowerValue = value.toLowerCase(); + if (lowerValue === "true") { + return true; + } else if (lowerValue === "false") { + return false; + } else { + throw new Error(`Invalid boolean value: ${value}`); + }
89-89: Correct typographical error in parameter descriptionThere's a duplication in the description of the
gasLimitparameter.Amend the description for clarity:
- "Gas limit for for a cross-chain call", + "Gas limit for a cross-chain call",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
examples/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/swap/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
- examples/hello/contracts/Hello.sol (1 hunks)
- examples/hello/hardhat.config.ts (1 hunks)
- examples/hello/package.json (2 hunks)
- examples/hello/tasks/gatewayCall.ts (1 hunks)
- examples/hello/tasks/solana/deposit.ts (0 hunks)
- examples/hello/tasks/solana/gateway.json (0 hunks)
- examples/hello/tasks/solana/interact.ts (0 hunks)
- examples/swap/package.json (1 hunks)
- examples/swap/tasks/solana/deposit.ts (0 hunks)
- examples/swap/tasks/solana/gateway.json (0 hunks)
- examples/swap/tasks/solana/interact.ts (0 hunks)
💤 Files with no reviewable changes (6)
- examples/hello/tasks/solana/deposit.ts
- examples/hello/tasks/solana/gateway.json
- examples/hello/tasks/solana/interact.ts
- examples/swap/tasks/solana/deposit.ts
- examples/swap/tasks/solana/gateway.json
- examples/swap/tasks/solana/interact.ts
✅ Files skipped from review due to trivial changes (1)
- examples/hello/hardhat.config.ts
🧰 Additional context used
🪛 GitHub Check: Slither
examples/hello/contracts/Hello.sol
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by (None,gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit) (contracts/Hello.sol#40)
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by IZRC20(zrc20).approve(address(gateway),gasFee) (contracts/Hello.sol#41)
🔇 Additional comments (2)
examples/swap/package.json (1)
31-31: Verify implications of @zetachain/localnet version updateThe @zetachain/localnet dependency has been updated from version "3.0.2" to "^3.0.4". This change allows for minor version updates, which may introduce new features or bug fixes.
To ensure this update aligns with the project's stability requirements:
- Review the changelog for @zetachain/localnet versions 3.0.3 and 3.0.4.
- Verify that no breaking changes are introduced.
- Consider the potential impact on the local development environment.
If the changes are deemed acceptable, this update is appropriate. Otherwise, consider pinning the version to "3.0.2" or updating to a specific newer version.
examples/hello/tasks/gatewayCall.ts (1)
67-67: Confirm the format of 'args.receiver'Using
ethers.utils.hexlifyonargs.receiverassumes it's in the correct format. Ifargs.receiveris already a hexadecimal address string, this may not be necessary.Ensure that
args.receiveris appropriately formatted. If it's a standard address, consider removingethers.utils.hexlify:- ethers.utils.hexlify(args.receiver), + args.receiver,If a byte array or specific encoding is required, verify that
hexlifyis the correct method.
| @@ -28,7 +28,7 @@ | |||
| "@types/node": ">=12.0.0", | |||
| "@typescript-eslint/eslint-plugin": "^5.59.9", | |||
| "@typescript-eslint/parser": "^5.59.9", | |||
| "@zetachain/localnet": "3.0.3", | |||
| "@zetachain/localnet": "^3.0.4", | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent @zetachain/localnet Dependency Versions Detected
The @zetachain/localnet dependency has differing versions across the project:
examples/hello/package.json: "^3.0.4"examples/swap/package.json: "^3.0.4"omnichain/swap/package.json: "^1.0.1"
Please update omnichain/swap/package.json to use @zetachain/localnet version "^3.0.4" to ensure consistency and compatibility throughout the project.
🔗 Analysis chain
Approved: @zetachain/localnet dependency update.
The update of @zetachain/localnet from "3.0.3" to "^3.0.4" is appropriate. The use of the caret (^) allows for compatible updates, which is a recommended practice for dependencies.
To ensure consistency across the project, please run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify @zetachain/localnet version consistency across the project
# Test: Search for @zetachain/localnet dependency in all package.json files
rg --type json '"@zetachain/localnet":\s*"[^"]+"' -g 'package.json'
Length of output: 261
| function gatewayCall( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| (, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); | ||
| IZRC20(zrc20).approve(address(gateway), gasFee); | ||
| gateway.call(receiver, zrc20, message, gasLimit, revertOptions); | ||
| } |
There was a problem hiding this comment.
Validate Input Parameters to Enhance Security and Reliability
The gatewayCall function does not perform validation on its input parameters. It's recommended to add checks to ensure:
receiveris a valid non-empty value.zrc20is a valid contract address implementing theIZRC20interface.gasLimitis within acceptable bounds to prevent excessive gas consumption.
Implementing input validation enhances the security and reliability of the contract.
Apply the following diff to add input validation:
function gatewayCall(
bytes memory receiver,
address zrc20,
bytes calldata message,
uint256 gasLimit,
RevertOptions memory revertOptions
) external [onlyOwner] {
+ require(receiver.length > 0, "Invalid receiver");
+ require(zrc20 != address(0), "Invalid ZRC20 address");
+ require(gasLimit > 0, "Gas limit must be greater than zero");
(, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit);
IZRC20(zrc20).approve(address(gateway), gasFee);
gateway.call(receiver, zrc20, message, gasLimit, revertOptions);
}Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 GitHub Check: Slither
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by (None,gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit) (contracts/Hello.sol#40)
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by IZRC20(zrc20).approve(address(gateway),gasFee) (contracts/Hello.sol#41)
Add Access Control to 'gatewayCall' Function to Prevent Unauthorized Access
The gatewayCall function is marked as external and can be invoked by any account. To enhance security and prevent unauthorized usage, consider adding access control mechanisms, such as an onlyOwner modifier, to restrict access to authorized users.
Apply the following diff to implement access control using OpenZeppelin's Ownable contract:
+import "@openzeppelin/contracts/access/Ownable.sol";
contract Hello is UniversalContract, Ownable {
GatewayZEVM public gateway;
constructor(address payable gatewayAddress) {
gateway = GatewayZEVM(gatewayAddress);
}
...
function gatewayCall(
bytes memory receiver,
address zrc20,
bytes calldata message,
uint256 gasLimit,
RevertOptions memory revertOptions
- ) external {
+ ) external onlyOwner {
(, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit);
IZRC20(zrc20).approve(address(gateway), gasFee);
gateway.call(receiver, zrc20, message, gasLimit, revertOptions);
}
}Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 GitHub Check: Slither
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by (None,gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit) (contracts/Hello.sol#40)
[warning] 33-43: Unused return
Hello.gatewayCall(bytes,address,bytes,uint256,RevertOptions) (contracts/Hello.sol#33-43) ignores return value by IZRC20(zrc20).approve(address(gateway),gasFee) (contracts/Hello.sol#41)
| const types = JSON.parse(args.types); | ||
|
|
There was a problem hiding this comment.
Add error handling for JSON parsing of 'args.types'
Currently, the code assumes that args.types is a valid JSON string. If it's malformed, JSON.parse will throw an exception, potentially causing the task to fail unexpectedly.
Implement error handling to manage parsing exceptions gracefully:
+let types;
+try {
+ types = JSON.parse(args.types);
+} catch (error) {
+ throw new Error(`Invalid JSON string for 'types' parameter: ${error.message}`);
+}📝 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 types = JSON.parse(args.types); | |
| let types; | |
| try { | |
| types = JSON.parse(args.types); | |
| } catch (error) { | |
| throw new Error(`Invalid JSON string for 'types' parameter: ${error.message}`); | |
| } | |
| } else if (type.startsWith("uint") || type.startsWith("int")) { | ||
| return ethers.BigNumber.from(value); |
There was a problem hiding this comment.
Implement error handling for numeric value conversion
Converting value to a BigNumber may throw an error if the input is invalid. It's prudent to handle such exceptions.
Add a try-catch block to manage conversion errors:
} else if (type.startsWith("uint") || type.startsWith("int")) {
- return ethers.BigNumber.from(value);
+ try {
+ return ethers.BigNumber.from(value);
+ } catch (error) {
+ throw new Error(`Invalid numeric value for type '${type}': ${value}`);
+ }📝 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.
| } else if (type.startsWith("uint") || type.startsWith("int")) { | |
| return ethers.BigNumber.from(value); | |
| } else if (type.startsWith("uint") || type.startsWith("int")) { | |
| try { | |
| return ethers.BigNumber.from(value); | |
| } catch (error) { | |
| throw new Error(`Invalid numeric value for type '${type}': ${value}`); | |
| } |
| const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | ||
| const zrc20TransferTx = await zrc20.transfer( | ||
| args.contract, | ||
| gasFee, | ||
| txOptions | ||
| ); | ||
|
|
||
| await zrc20TransferTx.wait(); |
There was a problem hiding this comment.
Add error handling for contract interactions with ZRC20
Operations like withdrawGasFeeWithGasLimit and transfer can fail due to various reasons. Ensuring these failures are caught will enhance the robustness of the task.
Wrap the contract calls in try-catch blocks:
+try {
const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit);
+} catch (error) {
+ throw new Error(`Failed to withdraw gas fee: ${error.message}`);
+}
+try {
const zrc20TransferTx = await zrc20.transfer(
args.contract,
gasFee,
txOptions
);
await zrc20TransferTx.wait();
+} catch (error) {
+ throw new Error(`Failed to transfer gas fee: ${error.message}`);
+}📝 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 [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | |
| const zrc20TransferTx = await zrc20.transfer( | |
| args.contract, | |
| gasFee, | |
| txOptions | |
| ); | |
| await zrc20TransferTx.wait(); | |
| try { | |
| const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | |
| } catch (error) { | |
| throw new Error(`Failed to withdraw gas fee: ${error.message}`); | |
| } | |
| try { | |
| const zrc20TransferTx = await zrc20.transfer( | |
| args.contract, | |
| gasFee, | |
| txOptions | |
| ); | |
| await zrc20TransferTx.wait(); | |
| } catch (error) { | |
| throw new Error(`Failed to transfer gas fee: ${error.message}`); | |
| } |
| const tx = await contract.gatewayCall( | ||
| ethers.utils.hexlify(args.receiver), | ||
| args.zrc20, | ||
| message, | ||
| gasLimit, | ||
| revertOptions, | ||
| txOptions | ||
| ); | ||
|
|
||
| console.log(`Transaction hash: ${tx.hash}`); | ||
| await tx.wait(); | ||
| console.log("gatewayCall executed successfully"); |
There was a problem hiding this comment.
Ensure proper error handling for the 'gatewayCall' transaction
The invocation of gatewayCall may encounter errors. Implementing error handling will prevent unhandled promise rejections and provide clearer error messaging.
Encapsulate the transaction in a try-catch block:
+try {
const tx = await contract.gatewayCall(
ethers.utils.hexlify(args.receiver),
args.zrc20,
message,
gasLimit,
revertOptions,
txOptions
);
console.log(`Transaction hash: ${tx.hash}`);
await tx.wait();
console.log("gatewayCall executed successfully");
+} catch (error) {
+ throw new Error(`Failed to execute 'gatewayCall': ${error.message}`);
+}📝 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 tx = await contract.gatewayCall( | |
| ethers.utils.hexlify(args.receiver), | |
| args.zrc20, | |
| message, | |
| gasLimit, | |
| revertOptions, | |
| txOptions | |
| ); | |
| console.log(`Transaction hash: ${tx.hash}`); | |
| await tx.wait(); | |
| console.log("gatewayCall executed successfully"); | |
| try { | |
| const tx = await contract.gatewayCall( | |
| ethers.utils.hexlify(args.receiver), | |
| args.zrc20, | |
| message, | |
| gasLimit, | |
| revertOptions, | |
| txOptions | |
| ); | |
| console.log(`Transaction hash: ${tx.hash}`); | |
| await tx.wait(); | |
| console.log("gatewayCall executed successfully"); | |
| } catch (error) { | |
| throw new Error(`Failed to execute 'gatewayCall': ${error.message}`); | |
| } |
|
|
||
| const types = JSON.parse(args.types); | ||
|
|
||
| const valuesArray = args.values.map((value: any, index: number) => { |
There was a problem hiding this comment.
Validate that 'types' and 'values' arrays have the same length
To prevent runtime errors, ensure that the types array and args.values have the same length before proceeding.
Add a length check after parsing types:
const types = JSON.parse(args.types);
+if (types.length !== args.values.length) {
+ throw new Error(
+ `Mismatch between number of types (${types.length}) and values (${args.values.length})`
+ );
+}Committable suggestion was skipped due to low confidence.
gatewayCallfunction to Hello to showcase how you can rungateway.callfrom a universal appgateway-calltaskSummary by CodeRabbit
Release Notes
New Features
gatewayCallfunction in theHellocontract for cross-chain interactions.gateway-callto facilitate calling thegatewayCallfunction.Bug Fixes
@zetachain/localnetto ensure compatibility and improvements.Removals