feat: deploy zrc 20 script #338
Conversation
skosito
requested review from
andresaiello,
brewmaster012,
lumtis,
CharlieMc0,
fadeev and
fbac
as code owners
WalkthroughWalkthroughThe changes introduce a new deployment script for ZRC20 tokens, enhancing the project's configurability by adding environment variables for token parameters. The constructor's access control in the ZRC20 contract has been relaxed, allowing broader initialization access. Documentation updates include hyperlink modifications to ensure users reference the latest source code, improving navigability and accuracy for developers interacting with the system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeployZRC20 Script
participant ZRC20 Contract
participant Environment
User->>DeployZRC20 Script: Execute run()
DeployZRC20 Script->>Environment: Retrieve token parameters
DeployZRC20 Script->>ZRC20 Contract: Create instance with parameters
ZRC20 Contract-->>DeployZRC20 Script: Confirm deployment
DeployZRC20 Script-->>User: Return success
Assessment against linked 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? 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
+ Coverage 82.35% 82.62% +0.27%
==========================================
Files 7 7
Lines 306 305 -1
Branches 99 98 -1
==========================================
Hits 252 252
+ Misses 54 53 -1 ☔ View full report in Codecov by Sentry. |
![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, codebase verification and nitpick comments (4)
v2/scripts/deploy/readme.md (3)
3-3: Grammar Correction: Improve sentence structure.Consider rephrasing for clarity: "Check
.env.samplefor an example of how it should look."- check `.env.sample` for example on how it should look like. + check `.env.sample` for an example of how it should look.Tools
LanguageTool
[grammar] ~3-~3: ‘Like’ cannot be used with the question word ‘how’ in this context.
Context: ...pts, check.env.samplefor example on how it should look like. Currently,.env.sampleis set with t...(HOW_IT_SHOULD_BE)
6-6: Grammar Correction: Add missing articles.Consider adding "the" for clarity: "To execute the deployment script, the following format is needed:"
- To execute deployment script, following format is needed: + To execute the deployment script, the following format is needed:Tools
LanguageTool
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ... first account private key. To execute deployment script, following format is needed: ``...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...ate key. To execute deployment script, following format is needed: ``` forge script scr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
8-10: Specify Language for Code Block.Add a language identifier to the code block for better syntax highlighting in Markdown.
- ``` + ```shellTools
Markdownlint
8-8: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
v2/scripts/deploy/DeployZRC20.s.sol (1)
1-45: Deployment Script Implementation Looks Good.The script effectively uses environment variables for configuration and includes necessary checks to verify deployment success.
Consider improving the error message for clarity:
- require(address(zrc20) != address(0), "deployment failed"); + require(address(zrc20) != address(0), "ZRC20 deployment failed");
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (7)
v2/pkg/beaconproxy.sol/beaconproxy.gois excluded by!v2/pkg/**v2/pkg/gatewayevmzevm.t.sol/gatewayevmzevmtest.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.t.sol/gatewayzevminboundtest.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.t.sol/gatewayzevmoutboundtest.gois excluded by!v2/pkg/**v2/pkg/zrc20.sol/zrc20.gois excluded by!v2/pkg/**v2/pkg/zrc20.t.sol/zrc20test.gois excluded by!v2/pkg/**v2/types/factories/ZRC20.sol/ZRC20__factory.tsis excluded by!v2/types/**
Files selected for processing (38)
- v2/.env.sample (1 hunks)
- v2/contracts/zevm/ZRC20.sol (1 hunks)
- v2/docs/src/README.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/interface.Revertable.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/struct.RevertContext.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/struct.RevertOptions.md (1 hunks)
- v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md (1 hunks)
- v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorNative.sol/contract.ZetaConnectorNative.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorNonNative.sol/contract.ZetaConnectorNonNative.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyErrors.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMErrors.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IZetaNonEthNew.sol/interface.IZetaNonEthNew.md (1 hunks)
- v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md (1 hunks)
- v2/docs/src/contracts/zevm/ZRC20.sol/contract.ZRC20.md (1 hunks)
- v2/docs/src/contracts/zevm/ZRC20.sol/interface.ZRC20Errors.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMErrors.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMEvents.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/ISystem.sol/interface.ISystem.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IWZETA.sol/interface.IWETH9.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20Metadata.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.ZRC20Events.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.UniversalContract.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md (1 hunks)
- v2/docs/src/index.md (1 hunks)
- v2/scripts/deploy/DeployZRC20.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/readme.md (1 hunks)
- v2/scripts/deploy/readme.md (1 hunks)
Files skipped from review due to trivial changes (34)
- v2/docs/src/README.md
- v2/docs/src/contracts/Revert.sol/interface.Revertable.md
- v2/docs/src/contracts/Revert.sol/struct.RevertContext.md
- v2/docs/src/contracts/Revert.sol/struct.RevertOptions.md
- v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md
- v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md
- v2/docs/src/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md
- v2/docs/src/contracts/evm/ZetaConnectorNative.sol/contract.ZetaConnectorNative.md
- v2/docs/src/contracts/evm/ZetaConnectorNonNative.sol/contract.ZetaConnectorNonNative.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyErrors.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyEvents.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMErrors.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMEvents.md
- v2/docs/src/contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md
- v2/docs/src/contracts/evm/interfaces/IZetaNonEthNew.sol/interface.IZetaNonEthNew.md
- v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md
- v2/docs/src/contracts/zevm/ZRC20.sol/contract.ZRC20.md
- v2/docs/src/contracts/zevm/ZRC20.sol/interface.ZRC20Errors.md
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMErrors.md
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMEvents.md
- v2/docs/src/contracts/zevm/interfaces/ISystem.sol/interface.ISystem.md
- v2/docs/src/contracts/zevm/interfaces/IWZETA.sol/interface.IWETH9.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20Metadata.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.ZRC20Events.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.UniversalContract.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md
- v2/docs/src/index.md
- v2/scripts/deploy/deterministic/readme.md
Additional context used
LanguageTool
v2/scripts/deploy/readme.md
[grammar] ~3-~3: ‘Like’ cannot be used with the question word ‘how’ in this context.
Context: ...pts, check.env.samplefor example on how it should look like. Currently,.env.sampleis set with t...(HOW_IT_SHOULD_BE)
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ... first account private key. To execute deployment script, following format is needed: ``...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...ate key. To execute deployment script, following format is needed: ``` forge script scr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Markdownlint
v2/scripts/deploy/readme.md
8-8: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (2)
v2/.env.sample (1)
9-20: Environment Variables for ZRC20 Deployment Look Good.The addition of these environment variables aligns with the PR's objective to enhance configurability for ZRC20 deployments.
v2/contracts/zevm/ZRC20.sol (1)
Line range hint
42-42: Potential Security Risk: Relaxed Access Control in Constructor.The removal of the sender check in the constructor broadens accessibility, which could lead to unauthorized deployments. Ensure that this change does not introduce security vulnerabilities.
Consider reviewing the implications of this change in the broader context of the system's security model.
| address gateway = vm.envAddress("GATEWAY_PROXY_ZEVM"); | ||
| address systemContract = vm.envAddress("SYSTEM_CONTRACT"); | ||
| string memory name = vm.envString("ZRC20_NAME"); | ||
| string memory symbol = vm.envString("ZRC20_SYMBOL"); | ||
| uint8 decimals = uint8(vm.envUint("ZRC20_DECIMALS")); | ||
| uint chainID = vm.envUint("ZRC20_CHAIN_ID"); | ||
| CoinType coinType = CoinType(vm.envUint("ZRC20_COIN_TYPE")); | ||
| uint gasLimit = vm.envUint("ZRC20_GAS_LIMIT"); |
There was a problem hiding this comment.
nit: we should by default check programmatically that every variable is correctly set and contains a valid value. One way of doing it would be by using try catch after every address, and revert the script execution if it's not valid.
adds script to deploy zrc20 with all args from constructor passed as env vars to script
removes fungible module sender check from constructor in zrc20
do we need more zevm scripts? probably gateway zevm, can do in different PR
what about system contract?
closes: #333
Summary by CodeRabbit
New Features
Documentation
Bug Fixes