Temporarily add v2 contracts to the toolkit #183
Conversation
WalkthroughWalkthroughThe pull request introduces multiple changes across various Solidity files, primarily updating the Solidity version from Changes
Possibly related PRs
Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (10)
Files selected for processing (4)
Additional comments not posted (5)
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: 0
Outside diff range and nitpick comments (1)
contracts/SystemContract.sol (1)
1-200: Overall, the contract design and implementation look good!The contract is well-structured, follows best practices, and correctly implements the intended behavior. The access control and event emission are properly handled.
Just one minor note:
The
zContractinterface import on line 5 is commented out. Is this intentional or should it be uncommented?
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (39)
typechain-types/contracts/Revert.sol/Revertable.tsis excluded by!typechain-types/**typechain-types/contracts/Revert.sol/index.tsis excluded by!typechain-types/**typechain-types/contracts/SystemContract.sol/SystemContract.tsis excluded by!typechain-types/**typechain-types/contracts/SystemContract.sol/SystemContractErrors.tsis excluded by!typechain-types/**typechain-types/contracts/SystemContract.sol/index.tsis excluded by!typechain-types/**typechain-types/contracts/UniversalContract.sol/UniversalContract.tsis excluded by!typechain-types/**typechain-types/contracts/UniversalContract.sol/ZContract.tsis excluded by!typechain-types/**typechain-types/contracts/UniversalContract.sol/index.tsis excluded by!typechain-types/**typechain-types/contracts/index.tsis excluded by!typechain-types/**typechain-types/contracts/shared/interfaces/IZRC20.sol/IZRC20.tsis excluded by!typechain-types/**typechain-types/contracts/shared/interfaces/IZRC20.sol/IZRC20Metadata.tsis excluded by!typechain-types/**typechain-types/contracts/shared/interfaces/IZRC20.sol/ZRC20Events.tsis excluded by!typechain-types/**typechain-types/contracts/shared/interfaces/IZRC20.sol/index.tsis excluded by!typechain-types/**typechain-types/contracts/shared/interfaces/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/ERC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/BytesHelperLib__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/Revert.sol/Revertable__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/Revert.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/SwapHelperLib__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/SystemContract.sol/SystemContractErrors__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/SystemContract.sol/SystemContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/SystemContract.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/TestZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/UniversalContract.sol/UniversalContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/UniversalContract.sol/ZContract__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/UniversalContract.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/TestUniswapRouter__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/interfaces/IZRC20.sol/IZRC20Metadata__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/interfaces/IZRC20.sol/IZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/interfaces/IZRC20.sol/ZRC20Events__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/interfaces/IZRC20.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/interfaces/index.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/libraries/SafeMath.sol/Math__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/libraries/UniswapV2Library__factory.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 (11)
- contracts/BytesHelperLib.sol (1 hunks)
- contracts/Revert.sol (1 hunks)
- contracts/SwapHelperLib.sol (1 hunks)
- contracts/SystemContract.sol (1 hunks)
- contracts/TestZRC20.sol (1 hunks)
- contracts/UniversalContract.sol (1 hunks)
- contracts/shared/MockZRC20.sol (1 hunks)
- contracts/shared/TestUniswapRouter.sol (1 hunks)
- contracts/shared/interfaces/IZRC20.sol (1 hunks)
- contracts/shared/libraries/SafeMath.sol (1 hunks)
- hardhat.config.ts (1 hunks)
Files skipped from review due to trivial changes (5)
- contracts/BytesHelperLib.sol
- contracts/TestZRC20.sol
- contracts/shared/MockZRC20.sol
- contracts/shared/TestUniswapRouter.sol
- contracts/shared/libraries/SafeMath.sol
Additional comments not posted (21)
hardhat.config.ts (1)
17-17: LGTM!Adding Solidity compiler version
0.8.26to thecompilersarray is a good change. It allows leveraging the latest features, optimizations, and bug fixes in Solidity0.8.26for compiling smart contracts.The change is straightforward and does not introduce any issues to the existing Hardhat configuration.
contracts/UniversalContract.sol (3)
6-10: LGTM!The
zContextstruct is well-defined with appropriate field types.
23-32: Verify ifUniversalContractis intended to replacezContract.The
UniversalContractinterface is well-defined with appropriate function signatures. TheonCrossChainCallfunction is identical to the one in the deprecatedzContractinterface, which might indicate thatUniversalContractis intended to replacezContract. Please confirm if this is the case.Also, the
onRevertfunction is new and takes aRevertContextparameter. It would be helpful to have documentation explaining the purpose and usage of this function.Run the following script to verify if
UniversalContractis intended to replacezContract:Do you want me to generate documentation for the
UniversalContractinterface and its functions?
12-21: Address the deprecation comment.The
zContractinterface is well-defined with an appropriate function signature. However, please ensure to remove this interface once v2 SystemContract is not used anymore, as indicated by the deprecation comment.Run the following script to verify the usage of
zContractinterface:contracts/Revert.sol (3)
10-16: LGTM!The
RevertOptionsstruct is well-defined and follows the Solidity naming conventions. The comments provide clear and helpful information about each field, including the note about theonRevertGasLimitfield being unused on GatewayZEVM methods.
22-26: LGTM!The
RevertContextstruct is well-defined and follows the Solidity naming conventions. The comments provide clear and helpful information about each field, including the note about theassetfield being empty if it's a gas token.
30-34: LGTM!The
Revertableinterface is well-defined and follows the Solidity naming conventions. The comment provides clear and helpful information about theonRevertfunction. The function takes aRevertContextstruct as a parameter, which is a good design choice as it encapsulates all the necessary information for handling reverts.contracts/shared/interfaces/IZRC20.sol (3)
6-50: LGTM!The
IZRC20interface correctly defines the standard functions for a ZRC20 token contract, includingtotalSupply,balanceOf,transfer,allowance,approve, andtransferFrom. The function signatures match the expected inputs and outputs for a ZRC20 token.In addition to the standard ZRC20 functions, the interface also includes Zeta-specific functions such as:
deposit: Allows depositing tokens to a specified address.burn: Allows burning tokens.withdraw: Allows withdrawing tokens to a specified address.withdrawGasFee: Returns the address and amount of the gas fee for withdrawals.withdrawGasFeeWithGasLimit: Returns the address and amount of the gas fee for withdrawals with a specified gas limit.PROTOCOL_FLAT_FEE: Returns the protocol flat fee.GAS_LIMIT: Returns the gas limit.setName: Allows setting the token name.setSymbol: Allows setting the token symbol.These additional functions provide functionality specific to the Zeta blockchain and enhance the capabilities of the ZRC20 token contract.
54-60: LGTM!The
IZRC20Metadatainterface correctly extendsIZRC20and defines the metadata functions for a ZRC20 token contract, includingname,symbol, anddecimals. The function signatures match the expected outputs for token metadata.
64-83: LGTM!The
ZRC20Eventsinterface correctly defines the events for a ZRC20 token contract, including:
Transfer: Emitted when tokens are transferred from one address to another.Approval: Emitted when an owner approves a spender to transfer tokens on their behalf.Deposit: Emitted when tokens are deposited to an address.Withdrawal: Emitted when tokens are withdrawn from an address.UpdatedSystemContract: Emitted when the system contract address is updated.UpdatedGateway: Emitted when the gateway address is updated.UpdatedGasLimit: Emitted when the gas limit is updated.UpdatedProtocolFlatFee: Emitted when the protocol flat fee is updated.The event signatures match the expected inputs for each event, providing a comprehensive set of notifications for various actions related to the ZRC20 token contract.
contracts/SystemContract.sol (9)
53-64: LGTM!The constructor is correctly setting the initial state and has the necessary access control check.
74-88: LGTM!The
depositAndCallfunction has the necessary access control and target address checks, and correctly interacts with the ZRC20 and target contracts.
96-105: LGTM!The
sortTokensfunction is correctly sorting the token addresses and handling the edge cases. The function visibility and state mutability are appropriate.
114-134: LGTM!The
uniswapv2PairForfunction is correctly calculating the pair address using the expected formula. The function visibility and state mutability are appropriate.
141-146: LGTM!The
setGasPricefunction has the necessary access control check and is correctly updating the state variable and emitting the event.
153-158: LGTM!The
setGasCoinZRC20function has the necessary access control check and is correctly updating the state variable and emitting the event.
165-175: LGTM!The
setGasZetaPoolfunction has the necessary access control check, correctly calculates the pool address, and updates the state variable and emits the event.
181-187: LGTM!The
setWZETAContractAddressfunction has the necessary access control and zero address checks, and correctly updates the state variable and emits the event.
193-199: LGTM!The
setConnectorZEVMAddressfunction has the necessary access control and zero address checks, and correctly updates the state variable and emits the event.contracts/SwapHelperLib.sol (2)
2-2: Thoroughly test the contract and ensure project-wide compatibility with Solidity 0.8.26.Updating the Solidity version from 0.8.7 to 0.8.26 is a significant change. Solidity 0.8.26 introduces new features, enhancements, and bug fixes that could affect the contract's behavior and compatibility.
Please ensure the following:
- Thoroughly test the
SwapHelperLibcontract to verify its functionality and correctness with Solidity 0.8.26.- Check the compatibility of the entire project, including all contracts and libraries, with Solidity 0.8.26. Update them if necessary.
- Verify that the project's dependencies are compatible with Solidity 0.8.26. Update the dependencies to their compatible versions if required.
6-7: Verify the functionality of the imported contracts and review their changes if modified.The import statements for
IZRC20.solandSystemContract.solhave been updated to use local paths, suggesting a project restructure.Please ensure the following:
- Verify that the locally imported
IZRC20.solandSystemContract.solcontracts provide the same functionality as the previously imported ones from the ZetaChain protocol contracts. Test theSwapHelperLibcontract thoroughly to ensure it still works as expected with these local imports.- If the imported contracts have been modified, carefully review their changes to understand any impact on the
SwapHelperLibcontract.
|
@zeta-chain/fullstack @andresaiello please, review. |
"true"becomes an actualtruevalue)Summary by CodeRabbit
New Features
SystemContractfor cross-chain interactions and liquidity management.Bug Fixes
Documentation
zContractinterface, encouraging transition toUniversalContract.Chores