fix: move system contract to contracts folder

[skosito]

SystemContract was in test/utils folder, move it back to contracts folder to avoid confusion.

closes: #347

Summary by CodeRabbit

• Chores

  • Simplified import paths in the SystemContract.sol file for better readability.
  • Updated import paths in test files to reflect the new directory structure for SystemContract.sol.

• New Features

  • Enhanced access control in the GatewayZEVM contract by centralizing functionality under the new PROTOCOL_ADDRESS.
  • Updated error messages in the IGatewayZEVMErrors interface for better clarity regarding protocol interactions.

These changes improve code organization and security without impacting functionality.

[coderabbitai]

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

• Rewrite ZRC20 to allow setting Gateway address #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

• feat: deploy zrc 20 script #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.

---

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7243743 and d2db39d.

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.sol

Additional 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 files

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/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:

#!/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: 8640

v2/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.

---

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.39%. Comparing base (292d235) to head (d2db39d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #352      +/-   ##
==========================================
- Coverage   97.37%   91.39%   -5.99%     
==========================================
  Files           7        8       +1     
  Lines         305      337      +32     
  Branches       98      110      +12     
==========================================
+ Hits          297      308      +11     
- Misses          8       29      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.