-
Notifications
You must be signed in to change notification settings - Fork 60
Call from EVM #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Call from EVM #201
Changes from all commits
cc57caa
73379dd
5814704
231e856
6e40182
e223861
923f041
f64ec1d
6e921bf
2331f37
ded9f42
b6a8ca3
ce2bab4
87c868b
16391e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,45 @@ | ||
| # ZetaChain Contracts Template | ||
| # Hello Example | ||
|
|
||
| ## Getting Started | ||
| ``` | ||
| yarn deploy | ||
| ``` | ||
|
|
||
| ## EVM | ||
|
|
||
| Successful call: | ||
|
|
||
| ``` | ||
| npx hardhat echo-call --contract 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --receiver 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --network localhost --types '["string"]' hello | ||
| ``` | ||
|
|
||
| Failed call: | ||
|
|
||
| ``` | ||
| npx hardhat echo-call --contract 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --receiver 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --network localhost --types '["uint256"]' 42 | ||
| ``` | ||
|
|
||
| Install dependencies: | ||
| Failed call with handled revert: | ||
|
|
||
| ``` | ||
| yarn | ||
| npx hardhat echo-call --contract 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --receiver 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --network localhost --revert-address 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --revert-message 0x --call-on-revert --types '["uint256"]' 42 | ||
| ``` | ||
|
|
||
| ## Next Steps | ||
| ## ZetaChain | ||
|
|
||
| Ready to dive in? Follow our [**🚀 smart contract | ||
| tutorials**](https://www.zetachain.com/docs/developers/tutorials/intro/) to | ||
| start building universal app contracts. | ||
| Successful call: | ||
|
|
||
| ``` | ||
| npx hardhat hello-call --contract 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --receiver 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --zrc20 0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe --function "hello(string)" --network localhost --types '["string"]' hello | ||
| ``` | ||
|
|
||
| Failed call: | ||
|
|
||
| ``` | ||
| npx hardhat hello-call --contract 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --receiver 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --zrc20 0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe --function "hello(string)" --network localhost --types '["uint256"]' 42 | ||
| ``` | ||
|
|
||
| Failed call with handled revert: | ||
|
|
||
| ``` | ||
| npx hardhat hello-call --contract 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --receiver 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --zrc20 0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe --function "hello(string)" --network localhost --revert-address 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --revert-message 0x --call-on-revert --types '["uint256"]' 42 | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity 0.8.26; | ||
|
|
||
| import {RevertContext} from "@zetachain/protocol-contracts/contracts/Revert.sol"; | ||
| import "@zetachain/protocol-contracts/contracts/evm/GatewayEVM.sol"; | ||
|
|
||
| contract Echo { | ||
| GatewayEVM public immutable gateway; | ||
|
|
||
| event RevertEvent(string, RevertContext); | ||
| event HelloEvent(string, string); | ||
|
|
||
| constructor(address payable gatewayAddress) { | ||
| gateway = GatewayEVM(gatewayAddress); | ||
| } | ||
|
|
||
| function hello(string memory message) external payable { | ||
| emit HelloEvent("Hello on EVM", message); | ||
| } | ||
|
|
||
| function onRevert(RevertContext calldata revertContext) external { | ||
| emit RevertEvent("Revert on EVM", revertContext); | ||
| } | ||
|
|
||
| function call( | ||
| address receiver, | ||
| bytes calldata message, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| gateway.call(receiver, message, revertOptions); | ||
| } | ||
|
Comment on lines
+25
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The This function correctly forwards the call to the gateway. However, consider adding access control to prevent unauthorized usage. Consider implementing the import "@openzeppelin/contracts/access/Ownable.sol";
contract Echo is Ownable {
// ...
function call(
address receiver,
bytes calldata message,
RevertOptions memory revertOptions
) external onlyOwner {
gateway.call(receiver, message, revertOptions);
}
// ...
} |
||
|
|
||
| receive() external payable {} | ||
|
|
||
| fallback() external payable {} | ||
|
Comment on lines
+33
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement withdrawal mechanism for received Ether. The Consider implementing a withdrawal function: function withdraw() external onlyOwner {
uint256 balance = address(this).balance;
payable(owner()).transfer(balance);
}Also, document the intended use of received Ether in the contract. |
||
| } | ||
|
Comment on lines
+7
to
+36
Check warningCode scanning / Slither Contracts that lock Ether Medium
Contract locking ether found:
Contract Echo has payable functions: - Echo.hello(string) - Echo.receive() - Echo.fallback() But does not have a function to withdraw the ether |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,10 +7,11 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import "@zetachain/protocol-contracts/contracts/zevm/GatewayZEVM.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contract Hello is UniversalContract { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GatewayZEVM public gateway; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GatewayZEVM public immutable gateway; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event HelloEvent(string, string); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event RevertEvent(string, RevertContext); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error TransferFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(address payable gatewayAddress) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway = GatewayZEVM(gatewayAddress); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -30,15 +31,51 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emit RevertEvent("Revert on ZetaChain", revertContext); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function gatewayCall( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function call( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bytes memory receiver, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address zrc20, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bytes calldata message, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint256 gasLimit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RevertOptions memory revertOptions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!IZRC20(zrc20).transferFrom(msg.sender, address(this), gasFee)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revert TransferFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IZRC20(zrc20).approve(address(gateway), gasFee); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway.call(receiver, zrc20, message, gasLimit, revertOptions); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check warningCode scanning / Slither Unused return Medium
Hello.call(bytes,address,bytes,uint256,RevertOptions) ignores return value by IZRC20(zrc20).approve(address(gateway),gasFee)
Check warningCode scanning / Slither Unused return Medium |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Approval with suggestion: Improved The renaming of However, there's an opportunity for further improvement: The return value of the Apply this diff to address the issue: - IZRC20(zrc20).approve(address(gateway), gasFee);
+ if (!IZRC20(zrc20).approve(address(gateway), gasFee))
+ revert TransferFailed();This change ensures that the approval operation is successful before proceeding with the gateway call. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function withdrawAndCall( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bytes memory receiver, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint256 amount, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address zrc20, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bytes calldata message, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint256 gasLimit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RevertOptions memory revertOptions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (address gasZRC20, uint256 gasFee) = IZRC20(zrc20) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .withdrawGasFeeWithGasLimit(gasLimit); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint256 target = zrc20 == gasZRC20 ? amount + gasFee : amount; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!IZRC20(zrc20).transferFrom(msg.sender, address(this), target)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revert TransferFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IZRC20(zrc20).approve(address(gateway), target); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (zrc20 != gasZRC20) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !IZRC20(gasZRC20).transferFrom( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| msg.sender, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address(this), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gasFee | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) revert TransferFailed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IZRC20(gasZRC20).approve(address(gateway), gasFee); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway.withdrawAndCall( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| receiver, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| amount, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| zrc20, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gasLimit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revertOptions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Approval with suggestions: New The addition of the However, there are opportunities for further improvement:
Apply these diffs to address the issues: - IZRC20(zrc20).approve(address(gateway), target);
+ if (!IZRC20(zrc20).approve(address(gateway), target))
+ revert TransferFailed();
- IZRC20(gasZRC20).approve(address(gateway), gasFee);
+ if (!IZRC20(gasZRC20).approve(address(gateway), gasFee))
+ revert TransferFailed();These changes ensure that all approval operations are successful before proceeding with the gateway call.
function _transferAndApprove(address token, address from, uint256 amount) internal {
if (!IZRC20(token).transferFrom(from, address(this), amount))
revert TransferFailed();
if (!IZRC20(token).approve(address(gateway), amount))
revert TransferFailed();
}This refactored function can then be used in both 🧰 Tools🪛 GitHub Check: Slither
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { task, types } from "hardhat/config"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { HardhatRuntimeEnvironment } from "hardhat/types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const main = async (args: any, hre: HardhatRuntimeEnvironment) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Specify the type for Using the Apply this change to define a type for -interface EchoCallArgs {
+interface EchoCallArgs {
+ contract: string;
+ txOptionsGasPrice?: number;
+ txOptionsGasLimit?: number;
+ callOnRevert: boolean;
+ revertAddress: string;
+ revertMessage: string;
+ receiver: string;
+ onRevertGasLimit?: number;
+ name: string;
+ types: string;
+ values: string[];
+}
-const main = async (args: any, hre: HardhatRuntimeEnvironment) => {
+const main = async (args: EchoCallArgs, hre: HardhatRuntimeEnvironment) => {
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { ethers } = hre; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [signer] = await ethers.getSigners(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const txOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gasPrice: args.txOptionsGasPrice, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gasLimit: args.txOptionsGasLimit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const revertOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| abortAddress: "0x0000000000000000000000000000000000000000", // not used | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callOnRevert: args.callOnRevert, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onRevertGasLimit: args.onRevertGasLimit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revertAddress: args.revertAddress, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revertMessage: ethers.utils.hexlify( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ethers.utils.toUtf8Bytes(args.revertMessage) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const types = JSON.parse(args.types); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (types.length !== args.values.length) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `The number of types (${types.length}) does not match the number of values (${args.values.length}).` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential JSON parsing errors for Parsing Implement error handling as follows: -const types = JSON.parse(args.types);
+let types: string[];
+try {
+ types = JSON.parse(args.types);
+} catch (error) {
+ throw new Error(`Invalid JSON format for types: ${error.message}`);
+}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const valuesArray = args.values.map((value: any, index: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const type = types[index]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (type === "bool") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return JSON.parse(value.toLowerCase()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Invalid boolean value: ${value}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (type.startsWith("uint") || type.startsWith("int")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ethers.BigNumber.from(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return value; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve value parsing logic for parameter types. Using Refactor the value parsing logic: const valuesArray = args.values.map((value: string, index: number) => {
const type = types[index];
- if (type === "bool") {
- try {
- return JSON.parse(value.toLowerCase());
- } catch (e) {
- throw new Error(`Invalid boolean value: ${value}`);
- }
- } else if (type.startsWith("uint") || type.startsWith("int")) {
+ if (type === "bool") {
+ if (value.toLowerCase() === "true") {
+ return true;
+ } else if (value.toLowerCase() === "false") {
+ return false;
+ } else {
+ throw new Error(`Invalid boolean value: ${value}`);
+ }
+ } else if (/^u?int(\d+)?$/.test(type)) {
return ethers.BigNumber.from(value);
} else {
return value;
}
});📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const encodedParameters = ethers.utils.defaultAbiCoder.encode( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| valuesArray | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const factory = (await hre.ethers.getContractFactory(args.name)) as any; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const contract = factory.attach(args.contract).connect(signer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid casting to Casting to Specify the contract type: -const factory = (await hre.ethers.getContractFactory(args.name)) as any;
+const factory = await hre.ethers.getContractFactory(args.name);
+const contract = factory.attach(args.contract).connect(signer);
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tx = await contract.call( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args.receiver, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| encodedParameters, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revertOptions, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| txOptions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement error handling for the contract call. If the contract call fails, the error should be caught and handled appropriately to prevent unhandled promise rejections. Add a try-catch block around the contract call: -let tx = await contract.call(
- args.receiver,
- encodedParameters,
- revertOptions,
- txOptions
-);
+let tx;
+try {
+ tx = await contract.call(
+ args.receiver,
+ encodedParameters,
+ revertOptions,
+ txOptions
+ );
+} catch (error) {
+ throw new Error(`Contract call failed: ${error.message}`);
+}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`Transaction hash: ${tx.hash}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await tx.wait(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("gatewayCall executed successfully"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task("echo-call", "Calls the gateway on a contract on EVM", main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addParam("contract", "The address of the deployed contract") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addOptionalParam( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "txOptionsGasPrice", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "The gas price for the transaction", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 10000000000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types.int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addOptionalParam( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "txOptionsGasLimit", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "The gas limit for the transaction", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 7000000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types.int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addFlag("callOnRevert", "Whether to call on revert") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addOptionalParam( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "revertAddress", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Revert address", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "0x0000000000000000000000000000000000000000" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addOptionalParam("revertMessage", "Revert message", "0x") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addParam( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "receiver", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "The address of the receiver contract on a connected chain" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addOptionalParam( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "onRevertGasLimit", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "The gas limit for the revert transaction", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 7000000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types.int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addParam("name", "The name of the contract", "Echo") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addParam("types", `The types of the parameters (example: '["string"]')`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addVariadicPositionalParam("values", "The values of the parameters"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Task Parameters Lack Proper Validation The task parameters defined in
🔗 Analysis chainValidate task parameters and provide user feedback. Ensure all task parameters have appropriate validations and default values. This improves user experience by providing clear error messages for invalid inputs. Review the parameter definitions using the following script: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: List all task parameters and verify their configurations.
# Expected Output: Parameter definitions with their types and default values.
grep -E '\.add(Param|OptionalParam|Flag)' examples/hello/tasks/echoCall.ts -A 4
Length of output: 1042 Script: #!/bin/bash
# Description: Verify that each task parameter has appropriate validation in the main function.
# List of parameters to validate
PARAMETERS=(
"contract"
"txOptionsGasPrice"
"txOptionsGasLimit"
"callOnRevert"
"revertAddress"
"revertMessage"
"receiver"
"onRevertGasLimit"
"name"
"types"
"values"
)
# Iterate over each parameter and search for validation statements
for param in "${PARAMETERS[@]}"; do
echo "Validations for parameter: $param"
rg -n "$param" examples/hello/tasks/echoCall.ts | grep -i 'validate\|ensure\|check'
echo ""
done
Length of output: 2591 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the implications of the payable
hellofunction.The
hellofunction is marked aspayablebut doesn't use the sent Ether. This could lead to locked Ether in the contract.Consider one of the following options:
payablekeyword if Ether is not needed.