Gateway localnet examples #189
Conversation
|
Warning Rate limit exceeded@fadeev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 4 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. WalkthroughWalkthroughThe recent changes introduce new Solidity contracts aimed at enhancing cross-chain interactions with ZetaChain. These contracts include structured logging for successful calls and comprehensive error handling mechanisms. Additionally, new Hardhat tasks have been implemented to facilitate contract interactions, thereby improving the developer experience and ensuring robust error tracking across blockchain environments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Hardhat
participant HelloContract
User->>Hardhat: Execute call-from-zetachain task
Hardhat->>HelloContract: Call onCrossChainCall with parameters
HelloContract-->>Hardhat: Emit HelloEvent
Hardhat-->>User: Display success message
sequenceDiagram
participant User
participant Hardhat
participant RevertContract
User->>Hardhat: Execute interact task
Hardhat->>RevertContract: Call onRevert with context
RevertContract-->>Hardhat: Emit ContextDataRevert
Hardhat-->>User: Display revert context
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (5)
universal/hello/README.md (1)
7-9: Specify language for code block.Add a language identifier to the fenced code block for improved syntax highlighting and readability.
- ``` + ```shTools
Markdownlint
7-7: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
universal/hello/hardhat.config.ts (1)
14-14: Incompatible Solidity Versions DetectedThe Solidity version specified in
hardhat.config.tsis "0.8.26". However, several contracts in the project use different versions, notably "0.8.7" and "0.5.16". This discrepancy could lead to compatibility issues.
Files using Solidity 0.8.7:
messaging/message/contracts/CrossChainMessage.solmessaging/nft/contracts/CrossChainNFT.solomnichain/swap/contracts/Swap.solomnichain/swap/contracts/SwapToAnyToken.solomnichain/staking/contracts/Staking.sol- ... (and others)
Files using Solidity 0.5.16:
omnichain/swap/contracts/shared/TestUniswapCore.solomnichain/staking/contracts/shared/TestUniswapCore.solomnichain/multioutput/contracts/shared/TestUniswapCore.solomnichain/nft/contracts/shared/TestUniswapCore.solRecommended Action:
Review the contracts and consider updating their Solidity versions to align with "0.8.26" if feasible, or adjust the Hardhat configuration to accommodate the versions in use. Ensure thorough testing after making changes to prevent any potential issues.Analysis chain
Verify Solidity version compatibility.
The Solidity version is set to "0.8.26". Ensure this version is compatible with all contracts and dependencies in the project.
Let's re-verify the Solidity version compatibility by correctly identifying Solidity files and checking their pragma statements.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of Solidity version with contracts. # Test: Search for all Solidity files and check their pragma statements. rg --type solidity -A 1 $'pragma solidity' | grep -v '0.8.26'Length of output: 93
Script:
#!/bin/bash # Description: Find all Solidity files and check their pragma statements for compatibility with version 0.8.26. # Find all Solidity files and check their pragma statements. fd --extension sol --exec grep -H 'pragma solidity' {} | grep -v '0.8.26'Length of output: 1701
universal/hello/tasks/interact.ts (1)
40-47: Task registration is correct, but improve parameter descriptions.Ensure that parameter descriptions provide clear guidance on expected input values.
task("interact", "Calls a ZetaChain contract from an EVM account", main) .addParam("name", "Name to encode in the contract call") .addParam("contract", "Address of the universal app contract on ZetaChain") .addOptionalParam( "gatewayEVM", "Address of the gateway contract on EVM", "0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" );universal/hello/package.json (2)
6-10: Update the test script and ensure comprehensive linting.The test script is a placeholder and should be updated to include actual tests. Ensure the lint scripts cover all necessary file types.
"scripts": { "test": "mocha --require ts-node/register 'test/**/*.spec.ts'", "lint:fix": "npx eslint . --ext .js,.ts --fix", "lint": "npx eslint . --ext .js,.ts" }
2-13: Fill in metadata fields for better documentation.Consider providing a description, author information, and keywords to improve project documentation and clarity.
"description": "A template project for Ethereum smart contract development", "author": "Your Name", "keywords": ["Ethereum", "smart contracts", "ZetaChain"]
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
universal/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (13)
- omnichain/swap/tasks/deploy.ts (1 hunks)
- universal/hello/.eslintignore (1 hunks)
- universal/hello/.eslintrc.js (1 hunks)
- universal/hello/.gitignore (1 hunks)
- universal/hello/LICENSE (1 hunks)
- universal/hello/README.md (1 hunks)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/foundry.toml (1 hunks)
- universal/hello/hardhat.config.ts (1 hunks)
- universal/hello/package.json (1 hunks)
- universal/hello/tasks/deploy.ts (1 hunks)
- universal/hello/tasks/interact.ts (1 hunks)
- universal/hello/tsconfig.json (1 hunks)
Files skipped from review due to trivial changes (4)
- universal/hello/.eslintignore
- universal/hello/.gitignore
- universal/hello/LICENSE
- universal/hello/foundry.toml
Additional context used
Markdownlint
universal/hello/README.md
7-7: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (17)
universal/hello/tsconfig.json (6)
3-4: Appropriate module settings for Node.js ES modules.The
moduleandmoduleResolutionoptions set tonodenextare suitable for projects using ES modules in Node.js environments.
5-5: EnablingesModuleInteropfor module compatibility.The
esModuleInteropoption is correctly enabled to facilitate interoperability between CommonJS and ES modules.
6-6: Consistent casing in file names enforced.The
forceConsistentCasingInFileNamesoption is appropriately enabled to avoid issues on case-sensitive file systems.
7-7: Strict type-checking enabled.The
strictoption is correctly enabled, ensuring comprehensive type-checking for improved code quality.
8-8: Skipping library type checks for performance.The
skipLibCheckoption is enabled, which can improve build performance by skipping type checks of declaration files. Ensure this aligns with your project's quality requirements.
9-9: JSON module resolution enabled.The
resolveJsonModuleoption is enabled, allowing JSON files to be imported as modules, which is useful for configuration and data handling.universal/hello/README.md (1)
11-15: Clear guidance for next steps.The "Next Steps" section effectively guides users to further resources and tutorials.
universal/hello/hardhat.config.ts (2)
1-5: Relevant task and module imports.The imports for custom tasks and third-party modules are appropriate for extending Hardhat's functionality in this project.
11-13: Dynamic network configuration.Using
getHardhatConfigNetworks()allows for flexible and dynamic network configurations, which is beneficial for different environments.universal/hello/.eslintrc.js (1)
1-47: ESLint Configuration Looks Good.The ESLint configuration is comprehensive, with TypeScript support and useful plugins for sorting and formatting. It aligns well with best practices.
universal/hello/contracts/Hello.sol (1)
1-46: Solidity Contract Implementation Looks Good.The
Hellocontract is well-implemented, with appropriate event emissions and function overrides. Ensure the events are used correctly in the broader system.Consider verifying that the emitted events are appropriately handled and logged in the system.
universal/hello/tasks/deploy.ts (1)
1-40: Deployment Script Looks Good.The deployment script is well-structured, with appropriate error handling and flexible output options. Ensure the commented-out code is intentional and documented.
Consider verifying whether the commented-out code is necessary or if it can be removed.
universal/hello/tasks/interact.ts (2)
1-3: Imports are appropriate and necessary.The imported modules and types are essential for defining the Hardhat task and interacting with the Ethereum contract.
14-17: Parameter validation forargs.name.Ensure that
args.nameis validated before encoding to prevent potential issues with unexpected input types.if (typeof args.name !== "string") { throw new Error("Invalid parameter: 'name' must be a string."); }omnichain/swap/tasks/deploy.ts (2)
8-12: Reconsider commenting out network validation.The removal of network validation allows deployments on any network, which may lead to unintended consequences. Consider re-enabling the validation or implementing a more flexible yet safe approach.
if (!/zeta_(testnet|mainnet)/.test(network)) { console.warn(`Deploying on an unverified network: ${network}. Proceed with caution.`); }
Line range hint
13-50: Deployment script is generally well-structured.The script correctly handles contract deployment and output formatting. Ensure signer retrieval is robust and consider handling scenarios where no signer is available.
universal/hello/package.json (1)
14-52: Dependencies are relevant and necessary.The listed dependencies are appropriate for Ethereum development and TypeScript support.
universal/hello/tasks/interact.ts
Outdated
| const main = async (args: any, hre: HardhatRuntimeEnvironment) => { | ||
| const [signer] = await hre.ethers.getSigners(); | ||
|
|
||
| const gateway = new hre.ethers.Contract( | ||
| args.gatewayEVM, | ||
| GatewayABI.abi, | ||
| signer | ||
| ); |
There was a problem hiding this comment.
Ensure signer retrieval handles errors gracefully.
While the current implementation assumes that getSigners() will return a valid signer, consider adding error handling to manage scenarios where no signer is available.
const signers = await hre.ethers.getSigners();
if (signers.length === 0) {
throw new Error("No signers available. Please configure a signer.");
}
const [signer] = signers;
universal/hello/tasks/interact.ts
Outdated
| try { | ||
| const callTx = await gateway.call( | ||
| args.contract, | ||
| message, | ||
| { | ||
| revertAddress: "0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82", | ||
| callOnRevert: true, | ||
| abortAddress: "0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82", | ||
| revertMessage: "0x", | ||
| }, | ||
| { | ||
| gasPrice: 10000000000, | ||
| gasLimit: 6721975, | ||
| } | ||
| ); | ||
| await callTx.wait(); | ||
| console.log("Contract on ZetaChain called from EVM"); | ||
| } catch (e) { | ||
| console.error("Error calling contract:", e); | ||
| } |
There was a problem hiding this comment.
Enhance error handling for contract interaction.
Consider providing more context in the error message or rethrowing the error for better debugging.
try {
const callTx = await gateway.call(
args.contract,
message,
{
revertAddress: "0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82",
callOnRevert: true,
abortAddress: "0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82",
revertMessage: "0x",
},
{
gasPrice: 10000000000,
gasLimit: 6721975,
}
);
await callTx.wait();
console.log("Contract on ZetaChain called from EVM");
} catch (e) {
console.error("Error calling contract:", e.message || e);
throw e; // Rethrow to propagate the error
}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
universal/hello/tasks/solana/interact.ts (1)
10-10: Add a description for the task.The task
"interact-solana"lacks a description. Adding a brief description would enhance clarity and usability.-task("interact-solana", "", main) +task("interact-solana", "Interacts with the Solana network to deposit SOL", main)universal/hello/tasks/solana/deposit.ts (1)
52-54: Enhance error logging for better debugging.Consider adding more context to the error logging to facilitate debugging.
- console.error("Transaction failed:", error); + console.error("Transaction failed during deposit:", error);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
universal/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (9)
- universal/hello/.gitignore (1 hunks)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/foundry.toml (1 hunks)
- universal/hello/hardhat.config.ts (1 hunks)
- universal/hello/package.json (1 hunks)
- universal/hello/tasks/interact.ts (1 hunks)
- universal/hello/tasks/solana/deposit.ts (1 hunks)
- universal/hello/tasks/solana/gateway.json (1 hunks)
- universal/hello/tasks/solana/interact.ts (1 hunks)
Files skipped from review due to trivial changes (4)
- universal/hello/.gitignore
- universal/hello/foundry.toml
- universal/hello/hardhat.config.ts
- universal/hello/package.json
Files skipped from review as they are similar to previous changes (2)
- universal/hello/contracts/Hello.sol
- universal/hello/tasks/interact.ts
Additional comments not posted (1)
universal/hello/tasks/solana/gateway.json (1)
1-428: Verify IDL alignment with program implementation.Ensure that the IDL accurately reflects the program's implementation, especially the instruction names, accounts, and error codes.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (5)
universal/hello/contracts/RevertContract.sol (2)
14-17: Consider implementing additional logic inonRevert.The
onRevertfunction currently only emits an event. Consider implementing additional logic to handle revert scenarios, such as logging more detailed information or triggering compensating actions.
16-16: Remove commented-out code or provide context.The commented-out line for emitting
RevertEventshould either be removed or accompanied by a comment explaining its intended use.- // emit RevertEvent("Event from RevertContract!!!"); + // Uncomment to emit a revert event with a custom message.universal/hello/contracts/Hello.sol (3)
14-22: Clarify the purpose of the forced revert inonCrossChainCall.The
onCrossChainCallfunction contains a forced revert, which might be intentional for testing purposes. Ensure that this behavior is documented, or consider removing it for production use.- revert("REVERTING!!!"); + // Revert is intentional for testing purposes. Remove for production use.
22-25: Remove or document commented-out code.The commented-out code for decoding a message should either be removed or documented to clarify its intended future use.
- // string memory decodedMessage; - // if (message.length > 0) { - // decodedMessage = abi.decode(message, (string)); - // } + // Uncomment and use this code to decode messages when needed.
28-30: Consider additional logic inonRevert.The
onRevertfunction currently only emits an event. Consider implementing additional logic to handle revert scenarios, such as logging more detailed information or triggering compensating actions.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/contracts/RevertContract.sol (1 hunks)
- universal/hello/tasks/interact.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- universal/hello/tasks/interact.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
universal/hello/tasks/callFromZetaChain.ts (1)
47-63: Improve parameter documentation for clarity.The task parameters are defined, but additional comments explaining each parameter's purpose and expected format would enhance readability and usability for other developers.
.addParam("message", "A message to be sent to the contract") .addParam("contract", "The address of the universal app on ZetaChain (must be a valid Ethereum address)") // ... other parameters
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/contracts/RevertContract.sol (1 hunks)
- universal/hello/hardhat.config.ts (1 hunks)
- universal/hello/package.json (1 hunks)
- universal/hello/tasks/callFromZetaChain.ts (1 hunks)
- universal/hello/tasks/interact.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- universal/hello/hardhat.config.ts
Files skipped from review as they are similar to previous changes (4)
- universal/hello/contracts/Hello.sol
- universal/hello/contracts/RevertContract.sol
- universal/hello/package.json
- universal/hello/tasks/interact.ts
| const main = async (args: any, hre: HardhatRuntimeEnvironment) => { | ||
| const [signer] = await hre.ethers.getSigners(); | ||
|
|
||
| const contractArtifact = await hre.artifacts.readArtifact("Hello"); | ||
| const contract = new hre.ethers.Contract( | ||
| args.contract, | ||
| contractArtifact.abi, | ||
| signer | ||
| ); |
There was a problem hiding this comment.
Consider validating input parameters.
While the function retrieves the signer and contract artifact correctly, it lacks validation for args.contract to ensure it is a valid Ethereum address. Consider adding validation to prevent runtime errors.
import { isAddress } from "ethers/lib/utils";
// Validate contract address
if (!isAddress(args.contract)) {
throw new Error("Invalid contract address");
}| try { | ||
| const tx = await contract.callFromZetaChain( | ||
| hre.ethers.utils.hexlify(args.receiver), | ||
| args.zrc20, | ||
| message, | ||
| args.gasLimit, | ||
| { | ||
| revertAddress: args.revertAddress, | ||
| callOnRevert: args.callOnRevert, | ||
| abortAddress: "0x0000000000000000000000000000000000000000", // not used | ||
| revertMessage: hre.ethers.utils.hexlify(revertMessageBytes), | ||
| }, | ||
| { | ||
| gasPrice: 10000000000, | ||
| gasLimit: 7000000, | ||
| } | ||
| ); | ||
|
|
||
| await tx.wait(); | ||
| console.log("Successfully called the contract on ZetaChain!"); | ||
| } catch (e) { | ||
| console.error("Error calling contract:", e); | ||
| } |
There was a problem hiding this comment.
Enhance error handling with more informative messages.
The current error handling logs a generic error message. Consider providing more context or rethrowing the error with additional information to aid debugging.
try {
const tx = await contract.callFromZetaChain(
// ... parameters
);
await tx.wait();
console.log("Successfully called the contract on ZetaChain!");
} catch (e) {
console.error("Error calling contract on ZetaChain:", e.message);
throw new Error(`Failed to call contract: ${e.message}`);
}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/tasks/callFromZetaChain.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- universal/hello/contracts/Hello.sol
Files skipped from review as they are similar to previous changes (1)
- universal/hello/tasks/callFromZetaChain.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- universal/hello/contracts/RevertContract.sol (1 hunks)
- universal/hello/tasks/callFromZetaChain.ts (1 hunks)
- universal/hello/tasks/interact.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- universal/hello/contracts/RevertContract.sol
- universal/hello/tasks/callFromZetaChain.ts
- universal/hello/tasks/interact.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- universal/hello/tasks/callFromZetaChain.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- universal/hello/tasks/callFromZetaChain.ts
universal/hello/contracts/Hello.sol
Outdated
| event HelloEvent(string, string); | ||
| event ContextDataRevert(RevertContext); | ||
|
|
||
| address constant gateway = 0x610178dA211FEF7D417bC0e6FeD39F05609AD788; |
There was a problem hiding this comment.
nit: let's call it gatewayAddress
omnichain/swap/tasks/deploy.ts
Outdated
| '🚨 Please use either "zeta_testnet" or "zeta_mainnet" network to deploy to ZetaChain.' | ||
| ); | ||
| } | ||
| // if (!/zeta_(testnet|mainnet)/.test(network)) { |
There was a problem hiding this comment.
if it's no longer valid just delete it, let's try to keep code clean
There was a problem hiding this comment.
Reverted, because it's still relevant: 4095527
universal/hello/tasks/deploy.ts
Outdated
| const main = async (args: any, hre: HardhatRuntimeEnvironment) => { | ||
| const network = hre.network.name as ParamChainName; | ||
|
|
||
| // if (!/zeta_(testnet|mainnet)/.test(network)) { |
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (3)
universal/hello/flat (3)
46-74: Review deprecation and transition strategies.The
zContractinterface is marked as deprecated, which is good for informing developers of upcoming changes. However, ensure that there is a clear transition strategy and timeline for developers to migrate to theUniversalContractto avoid disruptions.
83-117: Event documentation could be improved.The events in
IGatewayZEVMEventsare crucial for understanding cross-chain interactions. Consider adding more detailed comments explaining when each event should be expected and any important considerations regarding their parameters.
527-542: Enhance fallback and receive function documentation.The
ReceiverContractincludes fallback and receive functions but lacks documentation. Adding comments explaining their purpose and any specific behaviors expected (e.g., logging received funds) would improve code clarity and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
universal/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (8)
- universal/hello/contracts/Hello.sol (1 hunks)
- universal/hello/contracts/ReceiverContract.sol (1 hunks)
- universal/hello/flat (1 hunks)
- universal/hello/hardhat.config.ts (1 hunks)
- universal/hello/package.json (1 hunks)
- universal/hello/tasks/callFromZetaChain.ts (1 hunks)
- universal/hello/tasks/depositAndCall.ts (1 hunks)
- universal/hello/tasks/solana/gateway.json (1 hunks)
Files skipped from review due to trivial changes (1)
- universal/hello/package.json
Additional comments not posted (12)
universal/hello/hardhat.config.ts (1)
1-7: Imports and Configuration Settings are Correctly ImplementedThe imports are well-organized and relevant to the project's needs. The configuration settings are appropriately set up, ensuring that the project is ready for development and deployment on ZetaChain and other networks.
Also applies to: 9-10, 12-16
universal/hello/contracts/ReceiverContract.sol (1)
9-23: Contract Implementation is Correct and Well-DesignedThe
ReceiverContractis well-structured with appropriate event declarations and function implementations for handling messages and revert scenarios. The inclusion of receive and fallback functions is a good practice for handling Ether transactions.universal/hello/contracts/Hello.sol (1)
11-50: Contract Design and Implementation are RobustThe
Hellocontract is effectively designed for cross-chain interactions with appropriate event logging and error handling mechanisms. The implementation of functions for cross-chain calls and reverts is well-executed.universal/hello/tasks/depositAndCall.ts (4)
1-3: Imports are correctly structured.The import statements are appropriate for the task, ensuring that necessary modules and types are available.
5-12: Function setup is well-implemented.The async function setup is correct, and the use of
argsandhreto interact with the blockchain environment is appropriate. The retrieval of the signer and the instantiation of the contract usingGatewayABIare correctly implemented.
14-36: Encoding and transaction setup are secure and efficient.The use of
hre.ethers.utilsfor encoding and setting up transaction parameters is correctly implemented. The parameters for the transaction, includinggasPrice,gasLimit, andvalue, are appropriately set, ensuring efficient and secure transactions.
47-58: Task registration is complete and well-documented.The task is correctly registered with Hardhat, and all parameters are appropriately defined, including optional and required ones. The documentation for each parameter is clear, aiding developers in understanding their usage.
universal/hello/flat (3)
22-30: Ensure consistency in struct documentation.The
RevertContextstruct is well-documented with@paramtags. Ensure that all structs in the codebase follow a similar documentation standard to maintain consistency and clarity for developers.
32-38: Interface definition is clear and concise.The
Revertableinterface is clearly defined with a single functiononRevert. This is a good practice as it keeps interfaces focused and understandable.
119-157: Error handling is comprehensive, but ensure consistency.The error interfaces in
IGatewayZEVMErrorscover a wide range of potential issues. Ensure that these errors are consistently handled across all contracts that interact with the GatewayZEVM to prevent unhandled exceptions and enhance reliability.universal/hello/tasks/callFromZetaChain.ts (1)
4-57: Review of thecall-from-zetachaintask implementation
Input Validation: The existing comment about validating
args.contractis still valid and should be implemented to prevent runtime errors. The same validation should be applied toargs.zrc20since it is also an Ethereum address.Error Handling: The existing comment about enhancing error handling with more informative messages remains relevant. It's crucial to provide detailed error messages to aid in debugging, especially in a blockchain context where transactions are irreversible.
Gas Settings: The hardcoded gas price and limit (lines 31, 32, 48, 49) could be made configurable to adapt to different network conditions or user preferences. Consider adding parameters for these values or fetching them from a reliable source.
Use of Hardcoded Address: The
abortAddressis hardcoded as a zero address (line 44), which is typically used to indicate an unused parameter. If this is intentional, it should be clearly documented to avoid confusion.Logging: The success message in line 54 is clear, but consider also logging the transaction hash to provide a traceable reference for the operation.
Function Signature and Encoding: The way the function signature and parameters are encoded (lines 19-27) is correct but ensure that the
hello(string)function is implemented as expected on the smart contract side to match this encoding.Task Configuration: The task parameters are well-defined (lines 60-76). However, the use of
types.intforgasLimitis appropriate, but ensure that all user inputs are validated, especially for optional parameters likezrc20.Overall, the task script is well-structured and covers the essential functionalities required for interacting with the ZetaChain. Implementing the suggested improvements will enhance its robustness and usability.
The overall structure and logic of the task are sound, but the suggested improvements should be considered to enhance its functionality.
universal/hello/tasks/solana/gateway.json (1)
1-496: Review of thegateway.jsonconfiguration for Solana
Instruction Definitions: Each instruction (e.g.,
deposit,withdraw) is well-defined with appropriate discriminators and account specifications. Ensure that the discriminators are unique and correctly map to the intended instructions in the Solana program.Error Definitions: The error codes and messages (lines 416-460) are clearly defined. It's important that these errors are handled appropriately in the client and contract code to provide meaningful feedback to the user.
Account Specifications: The account configurations (e.g., lines 23-36 for
deposit) are detailed, specifying whether accounts are writable or signers. This is crucial for ensuring the correct permissions and behavior during transaction execution.Type Definitions: The type definitions (lines 462-496) are crucial for ensuring that the data structures used in the program are well-understood and correctly implemented. The use of complex types like arrays and structs needs careful handling in the Solana program to avoid issues during serialization and deserialization.
Documentation and Versioning: The metadata section (lines 3-8) provides basic information about the program. Consider including more detailed documentation, especially regarding the specific version compatibilities and any dependencies.
Security Considerations: Given the nature of blockchain applications, ensure that all security aspects are thoroughly reviewed, especially concerning account permissions and transaction validations.
Overall, the JSON configuration is comprehensive and appears to be correctly structured for use with a Solana program. Further integration testing and security audits are recommended to ensure that it functions as expected in a live environment.
The configuration is well-structured and detailed, providing a solid foundation for the Solana program. Further testing and audits are advised to ensure robustness and security.
| contract Hello is UniversalContract { | ||
| event HelloEvent(string, string); | ||
| event ContextDataRevert(RevertContext); | ||
|
|
||
| address constant gatewayAddress = | ||
| 0x610178dA211FEF7D417bC0e6FeD39F05609AD788; | ||
|
|
||
| function onCrossChainCall( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| bytes calldata message | ||
| ) external override { | ||
| string memory decodedMessage; | ||
| if (message.length > 0) { | ||
| decodedMessage = abi.decode(message, (string)); | ||
| } | ||
| emit HelloEvent("Hello from a universal app", decodedMessage); | ||
| } | ||
|
|
||
| function callFromZetaChain( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| IZRC20(zrc20).approve(gatewayAddress, 1_000_000_000); | ||
| IGatewayZEVM(gatewayAddress).call( | ||
| receiver, | ||
| zrc20, | ||
| message, | ||
| gasLimit, | ||
| revertOptions | ||
| ); | ||
| } | ||
|
|
||
| function onRevert(RevertContext calldata revertContext) external override { | ||
| emit ContextDataRevert(revertContext); | ||
| } |
There was a problem hiding this comment.
Consider Renaming gatewayAddress for Clarity
The contract is well-implemented with comprehensive logging and error handling. However, consider renaming the constant gatewayAddress to gatewayAddressZEVM for better clarity and alignment with its usage within ZetaChain environments, as suggested in a previous review.
| try { | ||
| const callTx = await gateway[ | ||
| "depositAndCall(address,bytes,(address,bool,address,bytes))" | ||
| ]( | ||
| args.contract, | ||
| message, | ||
| { | ||
| revertAddress: args.revertAddress, | ||
| callOnRevert: args.callOnRevert, | ||
| abortAddress: "0x0000000000000000000000000000000000000000", // not used | ||
| revertMessage: hre.ethers.utils.hexlify(revertMessageBytes), | ||
| }, | ||
| { | ||
| gasPrice: 10000000000, | ||
| gasLimit: 7000000, | ||
| value: hre.ethers.utils.parseEther(args.amount), | ||
| } | ||
| ); | ||
| await callTx.wait(); | ||
| console.log( | ||
| `Contract on ZetaChain called from EVM with ${args.amount} ETH` | ||
| ); | ||
| } catch (e) { | ||
| console.error("Error calling contract:", e); | ||
| } |
There was a problem hiding this comment.
Enhance error handling and logging for better debugging support.
While the transaction execution and waiting mechanism are correctly implemented, the error handling could be enhanced by providing more detailed error messages. This would aid in debugging and maintaining the system.
Consider adding more detailed error logging, such as:
catch (e) {
console.error(`Error calling contract with message: ${args.message} and amount: ${args.amount} ETH`, e);
}
universal/hello/flat
Outdated
| // File @zetachain/protocol-contracts/contracts/[email protected] | ||
|
|
||
| // Original license: SPDX_License_Identifier: MIT | ||
| pragma solidity 0.8.26; | ||
|
|
||
| /// @notice Struct containing revert options | ||
| /// @param revertAddress Address to receive revert. | ||
| /// @param callOnRevert Flag if onRevert hook should be called. | ||
| /// @param abortAddress Address to receive funds if aborted. | ||
| /// @param revertMessage Arbitrary data sent back in onRevert. | ||
| struct RevertOptions { | ||
| address revertAddress; | ||
| bool callOnRevert; | ||
| address abortAddress; | ||
| bytes revertMessage; | ||
| } |
There was a problem hiding this comment.
Consider using a library for common structures to reduce redundancy and enhance maintainability.
The RevertOptions struct is defined early in the file. If this struct or similar ones are used across multiple contracts within the project, consider defining them in a common library or contract and importing them where needed. This approach promotes code reusability and maintainability.
universal/hello/flat
Outdated
|
|
||
| /// @notice Call a smart contract on an external chain without asset transfer. | ||
| /// @param receiver The receiver address on the external chain. | ||
| /// @param zrc20 Address of zrc20 to pay fees. | ||
| /// @param message The calldata to pass to the contract call. | ||
| /// @param gasLimit Gas limit. | ||
| /// @param revertOptions Revert options. | ||
| function call( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions calldata revertOptions | ||
| ) | ||
| external; | ||
|
|
||
| /// @notice Deposit foreign coins into ZRC20. | ||
| /// @param zrc20 The address of the ZRC20 token. | ||
| /// @param amount The amount of tokens to deposit. | ||
| /// @param target The target address to receive the deposited tokens. | ||
| function deposit(address zrc20, uint256 amount, address target) external; | ||
|
|
||
| /// @notice Execute a user-specified contract on ZEVM. | ||
| /// @param context The context of the cross-chain call. | ||
| /// @param zrc20 The address of the ZRC20 token. | ||
| /// @param amount The amount of tokens to transfer. | ||
| /// @param target The target contract to call. | ||
| /// @param message The calldata to pass to the contract call. | ||
| function execute( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| address target, | ||
| bytes calldata message | ||
| ) | ||
| external; | ||
|
|
||
| /// @notice Deposit foreign coins into ZRC20 and call a user-specified contract on ZEVM. | ||
| /// @param context The context of the cross-chain call. | ||
| /// @param zrc20 The address of the ZRC20 token. | ||
| /// @param amount The amount of tokens to transfer. | ||
| /// @param target The target contract to call. | ||
| /// @param message The calldata to pass to the contract call. | ||
| function depositAndCall( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| address target, | ||
| bytes calldata message | ||
| ) | ||
| external; | ||
|
|
||
| /// @notice Deposit ZETA and call a user-specified contract on ZEVM. | ||
| /// @param context The context of the cross-chain call. | ||
| /// @param amount The amount of tokens to transfer. | ||
| /// @param target The target contract to call. | ||
| /// @param message The calldata to pass to the contract call. | ||
| function depositAndCall( | ||
| zContext calldata context, | ||
| uint256 amount, | ||
| address target, | ||
| bytes calldata message | ||
| ) | ||
| external; | ||
|
|
||
| /// @notice Revert a user-specified contract on ZEVM. | ||
| /// @param context The context of the revert call. | ||
| /// @param zrc20 The address of the ZRC20 token. | ||
| /// @param amount The amount of tokens to revert. | ||
| /// @param target The target contract to call. | ||
| /// @param message The calldata to pass to the contract call. | ||
| /// @param revertContext Revert context to pass to onRevert. | ||
| function executeRevert( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| address target, | ||
| bytes calldata message, | ||
| RevertContext calldata revertContext | ||
| ) | ||
| external; | ||
|
|
||
| /// @notice Deposit foreign coins into ZRC20 and revert a user-specified contract on ZEVM. | ||
| /// @param context The context of the revert call. | ||
| /// @param zrc20 The address of the ZRC20 token. | ||
| /// @param amount The amount of tokens to revert. | ||
| /// @param target The target contract to call. | ||
| /// @param message The calldata to pass to the contract call. | ||
| /// @param revertContext Revert context to pass to onRevert. | ||
| function depositAndRevert( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| address target, | ||
| bytes calldata message, | ||
| RevertContext calldata revertContext | ||
| ) | ||
| external; | ||
| } | ||
|
|
There was a problem hiding this comment.
Consider optimizing contract interfaces for gas efficiency.
The IGatewayZEVM interface defines multiple functions for withdrawing and calling contracts on external chains. Consider combining similar functions to reduce the contract size and potentially save on gas costs during deployment and interaction.
universal/hello/flat
Outdated
| contract Hello is UniversalContract { | ||
| event HelloEvent(string, string); | ||
| event ContextDataRevert(RevertContext); | ||
|
|
||
| address constant gateway = 0x610178dA211FEF7D417bC0e6FeD39F05609AD788; | ||
|
|
||
| function onCrossChainCall( | ||
| zContext calldata context, | ||
| address zrc20, | ||
| uint256 amount, | ||
| bytes calldata message | ||
| ) external override { | ||
| string memory decodedMessage; | ||
| if (message.length > 0) { | ||
| decodedMessage = abi.decode(message, (string)); | ||
| } | ||
| emit HelloEvent("Hello from a universal app", decodedMessage); | ||
| } | ||
|
|
||
| function callFromZetaChain( | ||
| bytes memory receiver, | ||
| address zrc20, | ||
| bytes calldata message, | ||
| uint256 gasLimit, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| IZRC20(zrc20).approve(gateway, 1_000_000_000); | ||
| IGatewayZEVM(gateway).call( | ||
| receiver, | ||
| zrc20, | ||
| message, | ||
| gasLimit, | ||
| revertOptions | ||
| ); | ||
| } | ||
|
|
||
| function onRevert(RevertContext calldata revertContext) external override { | ||
| emit ContextDataRevert(revertContext); | ||
| } | ||
| } |
There was a problem hiding this comment.
Review and optimize cross-chain call handling in the Hello contract.
The Hello contract implements the UniversalContract interface. The handling of cross-chain calls in onCrossChainCall should be reviewed to ensure that it correctly decodes and logs messages. Additionally, consider adding error handling for cases where message decoding fails.
|
@andresaiello please, review again. |
Summary by CodeRabbit
New Features
Hellocontract for enhanced cross-chain interaction and structured revert handling.RevertContractfor improved revert scenario management.ReceiverContractfor event logging and Ether reception.call-from-zetachainanddeposit-and-call, facilitating seamless interactions with ZetaChain contracts.Documentation
Chores