coderabbitai[bot]
commented
2 weeks ago Walkthrough
## Walkthrough The changes involve updates to import paths in several Solidity files, specifically adjusting the locations of `SystemContract.sol` and related interfaces to simplify directory structure navigation. Additionally, there are significant modifications to the `GatewayZEVM` contract, including renaming constants and access control logic to enhance clarity and security. The `IGatewayZEVMErrors` interface has also been updated to reflect these changes. The modifications do not alter the overall functionality or logic of the contracts or tests. ## Changes | Files | Change Summary | |---------------------------------------|---------------------------------------------------------------------------------------------------------| | `v2/contracts/zevm/SystemContract.sol` | Updated import paths for `IZRC20` and `UniversalContract` from `../../contracts/zevm/interfaces/` to `./interfaces/`. | | `v2/contracts/zevm/GatewayZEVM.sol` | Renamed `FUNGIBLE_MODULE_ADDRESS` to `PROTOCOL_ADDRESS`, updated `onlyFungible` modifier to `onlyProtocol`, and modified related function access controls. | | `v2/contracts/zevm/interfaces/IGatewayZEVM.sol` | Renamed error messages to reflect protocol changes, specifically `CallerIsNotFungibleModule()` to `CallerIsNotProtocol()` and `OnlyWZETAOrFungible()` to `OnlyWZETAOrProtocol()`. | | `v2/test/GatewayEVMZEVM.t.sol` | Renamed `fungibleModuleAddress` to `protocolAddress`. | | `v2/test/GatewayZEVM.t.sol` | Renamed `fungibleModule` to `protocolAddress`, updated its usage, and changed the import path for `SystemContract.sol`. | | `v2/test/ZRC20.t.sol` | Renamed `fungibleModule` to `protocolAddress`, updated related test functions to reflect protocol changes. | ## Assessment against linked issues | Objective | Addressed | Explanation | |--------------------------------------------------|-----------|-----------------------------------------------| | Add system contract to v2 contracts (#347) | ✅ | | ## Possibly related issues - **#279**: The changes in the PR regarding the `GatewayZEVM` contract and its access control align with the objectives of rewriting the `ZRC20` contract to include a gateway address, as both focus on enhancing the protocol's address handling. ## Possibly related PRs - **#338**: The changes in this PR involve the ZRC20 contract, specifically the removal of the fungible module sender check in the constructor, which aligns with the main PR's focus on simplifying import paths and enhancing the overall structure of the ZRC20 deployment process.[!TIP]
OpenAI O1 model for chat
- We have deployed OpenAI's latest O1 model for chat. - OpenAI claims that this model has superior reasoning capabilities than their GPT-4o model. - Please share any feedback with us in the [discussions post](https://discord.com/channels/1134356397673414807/1283929536186155099).
Recent review details
**Configuration used: .coderabbit.yaml** **Review profile: CHILL**Commits
Files that changed from the base of the PR and between 7243743d7dcf8299a66ab1f03d8186879272cd21 and d2db39d41f4b87436e4b2f802bd9f9cfa60c9bb6.Files ignored due to path filters (13)
* `v2/pkg/gatewayevmzevm.t.sol/gatewayevmzevmtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.sol/gatewayzevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.t.sol/gatewayzevminboundtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.t.sol/gatewayzevmoutboundtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/igatewayzevm.sol/igatewayzevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/igatewayzevm.sol/igatewayzevmerrors.go` is excluded by `!v2/pkg/**` * `v2/pkg/senderzevm.sol/senderzevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/zrc20.t.sol/zrc20test.go` is excluded by `!v2/pkg/**` * `v2/types/GatewayZEVM.ts` is excluded by `!v2/types/**` * `v2/types/factories/GatewayZEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/IGatewayZEVM.sol/IGatewayZEVMErrors__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/IGatewayZEVM.sol/IGatewayZEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/SenderZEVM__factory.ts` is excluded by `!v2/types/**`Files selected for processing (5)
* v2/contracts/zevm/GatewayZEVM.sol (12 hunks) * v2/contracts/zevm/interfaces/IGatewayZEVM.sol (1 hunks) * v2/test/GatewayEVMZEVM.t.sol (1 hunks) * v2/test/GatewayZEVM.t.sol (20 hunks) * v2/test/ZRC20.t.sol (10 hunks)Files skipped from review due to trivial changes (1)
* v2/test/GatewayEVMZEVM.t.solAdditional comments not posted (41)
v2/contracts/zevm/interfaces/IGatewayZEVM.sol (2)
`75-76`: **LGTM!** The renaming of the error message from `CallerIsNotFungibleModule()` to `CallerIsNotProtocol()` improves clarity and aligns with the overall shift in the contract's logic to refer to a more general protocol account. The error message accurately describes the condition under which it is triggered. --- `84-85`: **LGTM!** The renaming of the error message from `OnlyWZETAOrFungible()` to `OnlyWZETAOrProtocol()` broadens the context of the allowed callers and enhances clarity regarding the roles of different entities in the contract's logic. The error message accurately describes the condition under which it is triggered.v2/test/ZRC20.t.sol (16)
Line range hint `27-50`: **LGTM!** The renaming of `fungibleModule` to `protocolAddress` is consistent with the AI-generated summary. The setup logic has been updated to use `protocolAddress` for deploying contracts and dealing ETH. --- Line range hint `65-77`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `setName` and `setSymbol` functions, which is consistent with the renaming. --- Line range hint `79-85`: **LGTM!** The function name has been updated to reflect the renaming of `fungibleModule` to `protocolAddress`, which is consistent with the changes. --- Line range hint `203-219`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for updating the gas limit and protocol flat fee, which is consistent with the renaming. --- `221-227`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for setting the gas coin ZRC20 address to zero, which is consistent with the renaming. --- Line range hint `229-235`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for setting the gas price to zero, which is consistent with the renaming. --- Line range hint `237-264`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for updating the gas limit and protocol flat fee, which is consistent with the renaming. The addition of the balance check for `protocolAddress` is a good improvement to the test coverage. --- Line range hint `266-280`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for updating the gas limit and protocol flat fee, which is consistent with the renaming. --- Line range hint `282-297`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for updating the gas limit and protocol flat fee, which is consistent with the renaming. --- `303-307`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `updateSystemContractAddress` function, which is consistent with the renaming. --- `309-313`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `updateSystemContractAddress` function, which is consistent with the renaming. --- `315-318`: **LGTM!** The function name has been updated to reflect the renaming of `fungibleModule` to `protocolAddress`, which is consistent with the changes. --- `320-324`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `updateGatewayAddress` function, which is consistent with the renaming. --- `326-330`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `updateGatewayAddress` function, which is consistent with the renaming. --- `332-335`: **LGTM!** The function name has been updated to reflect the renaming of `fungibleModule` to `protocolAddress`, which is consistent with the changes. --- `337-341`: **LGTM!** The test has been updated to use `protocolAddress` instead of `fungibleModule` for calling the `updateGasLimit` function, which is consistent with the renaming.v2/contracts/zevm/GatewayZEVM.sol (12)
`39-40`: **LGTM!** The renaming of the `onlyFungible` modifier to `onlyProtocol` aligns with the updated constant name `PROTOCOL_ADDRESS` and improves clarity. --- `41-42`: **LGTM!** The modifier logic correctly restricts access to only the protocol address and reverts with an appropriate error message if the condition fails. --- `74-74`: **LGTM!** The `receive` function correctly restricts access to only the `zetaToken` or protocol address and reverts with an appropriate error message if the condition fails. --- `103-103`: **LGTM!** The `_withdrawZRC20WithGasLimit` function correctly transfers the gas fee to the protocol address and reverts with an appropriate error message if the transfer fails. --- `213-213`: **LGTM!** The `withdraw` function for ZETA tokens correctly transfers the tokens to the protocol address before emitting the `Withdrawn` event and is restricted to only be called when the contract is not paused. --- `237-237`: **LGTM!** The `withdrawAndCall` function for ZETA tokens correctly transfers the tokens to the protocol address before emitting the `Withdrawn` event and is restricted to only be called when the contract is not paused. --- `262-262`: **LGTM!** The `call` function correctly transfers the gas fee to the protocol address and reverts with an appropriate error message if the transfer fails. --- `273-277`: **LGTM!** The `deposit` function is correctly restricted to only be called by the protocol address and checks for invalid target addresses to prevent unintended behavior. The error message is appropriate for the condition. --- Line range hint `296-318`: **LGTM!** The `execute` and `depositAndCall` functions are correctly restricted to only be called by the protocol address and check for zero addresses and invalid target addresses to prevent unintended behavior. The error messages are appropriate for the conditions. Also applies to: 323-323 --- `341-346`: **LGTM!** The `depositAndCall` function for ZETA tokens is correctly restricted to only be called by the protocol address and checks for zero address, zero amount, and invalid target addresses to prevent unintended behavior. The error messages are appropriate for the conditions. --- `355-355`: **LGTM!** The `executeRevert` and `depositAndRevert` functions are correctly restricted to only be called by the protocol address and check for zero addresses, zero amount, and invalid target addresses to prevent unintended behavior. The error messages are appropriate for the conditions. Also applies to: 373-378 --- `31-32`: **Verify the protocol address change.** The renaming of `FUNGIBLE_MODULE_ADDRESS` to `PROTOCOL_ADDRESS` improves clarity. However, the address value change is a significant modification. Run the following script to verify the usage of the new protocol address:Verification successful
--- **Renaming approved, but address remains unchanged.** The constant has been correctly renamed from `FUNGIBLE_MODULE_ADDRESS` to `PROTOCOL_ADDRESS` in `GatewayZEVM.sol`, which improves clarity. However, the address value (0x735b14BB79463307AAcBED86DAf3322B1e6226aB) remains the same as before. Consider the following: - Ensure this address is indeed intended to remain unchanged. - Review other files in the codebase (especially in v1 and documentation) to maintain consistency in naming: - v2/contracts/zevm/ZRC20.sol - v2/contracts/zevm/SystemContract.sol - v1/contracts/zevm/ZetaConnectorZEVM.sol - v1/contracts/zevm/SystemContract.sol - v1/contracts/zevm/ZRC20.sol - Various documentation filesScripts executed
The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the usage of the new protocol address. # Test: Search for the old protocol address. Expect: No occurrences. rg --type solidity $'0x735b14BB79463307AAcBED86DAf3322B1e6226aB' # Test: Search for the new protocol address. Expect: Only occurrences in intended locations. rg --type solidity -A 5 $'0x735b14BB79463307AAcBED86DAf3322B1e6226aB' ``` Length of output: 198 --- Script: ```shell #!/bin/bash # Description: Verify the usage of the new protocol address in Solidity files. # Find Solidity files and search for the old protocol address echo "Searching for the old protocol address (expect no results):" fd -e sol | xargs rg '0x735b14BB79463307AAcBED86DAf3322B1e6226aB' echo -e "\nSearching for the new protocol address:" fd -e sol | xargs rg -n -A 5 '0x735b14BB79463307AAcBED86DAf3322B1e6226aB' ``` Length of output: 8640v2/test/GatewayZEVM.t.sol (11)
`7-7`: **LGTM!** The change in the import path for `SystemContract.sol` reflects a better organization of the codebase. This change does not affect the functionality of the tests. --- `28-28`: **LGTM!** The renaming of the `fungibleModule` variable to `protocolAddress` enhances the clarity of the code and aligns with the broader architectural update in the codebase. This change does not affect the functionality of the tests. --- `47-55`: **LGTM!** The updates to the usage of the `protocolAddress` variable throughout the `setUp` function ensure consistency with the renaming of `fungibleModule` to `protocolAddress`. These changes do not alter the logic or control flow of the function. --- `104-104`: **LGTM!** The update to the caller address in the `zrc20.updateGasLimit` function call ensures that the correct address is used after the renaming of `fungibleModule` to `protocolAddress`. This change maintains the intended behavior of the test. --- Line range hint `227-241`: **LGTM!** The usage of `protocolAddress` to check the balance before and after the `gateway.withdraw` function call ensures that the correct address is used to verify the balance transfer. These changes maintain the intended behavior of the `testWithdrawZETA` test case. --- Line range hint `248-265`: **LGTM!** The usage of `protocolAddress` to check the balance before and after the `gateway.withdraw` function call ensures that the correct address is used to verify that the balance remains unchanged when the function call fails due to insufficient allowance. These changes maintain the intended behavior of the `testWithdrawZETAFailsIfNoAllowance` test case. --- Line range hint `282-299`: **LGTM!** The usage of `protocolAddress` to check the balance before and after the `gateway.withdrawAndCall` function call ensures that the correct address is used to verify the balance transfer. These changes maintain the intended behavior of the `testWithdrawZETAWithMessage` test case. --- Line range hint `306-324`: **LGTM!** The usage of `protocolAddress` to check the balance before and after the `gateway.withdrawAndCall` function call ensures that the correct address is used to verify that the balance remains unchanged when the function call fails due to insufficient allowance. These changes maintain the intended behavior of the `testWithdrawZETAWithMessageFailsIfNoAllowance` test case. --- `351-351`: **LGTM!** The renaming of the `fungibleModule` variable to `protocolAddress` enhances the clarity of the code and aligns with the broader architectural update in the codebase. This change does not affect the functionality of the tests. --- `377-386`: **LGTM!** The updates to the usage of the `protocolAddress` variable throughout the `setUp` function ensure consistency with the renaming of `fungibleModule` to `protocolAddress`. These changes do not alter the logic or control flow of the function. --- Line range hint `402-789`: **LGTM!** The updates to the usage of the `protocolAddress` variable throughout the test cases in the `GatewayZEVMOutboundTest` contract ensure consistency with the renaming of `fungibleModule` to `protocolAddress`. These changes maintain the intended behavior of the tests and do not introduce any new issues.
codecov-commenter
SystemContract was in test/utils folder, move it back to contracts folder to avoid confusion.
closes: #347
Summary by CodeRabbit
Chores
SystemContract.solfile for better readability.SystemContract.sol.New Features
GatewayZEVMcontract by centralizing functionality under the newPROTOCOL_ADDRESS.IGatewayZEVMErrorsinterface for better clarity regarding protocol interactions.These changes improve code organization and security without impacting functionality.