feat: v2 evm contracts deploy scripts #293
Conversation
WalkthroughThe recent changes enhance the deployment process for Ethereum smart contracts by introducing a sample environment configuration file, refining existing deployment logic, and improving documentation. Key updates include deterministic deployment scripts for ERC20 tokens and Zeta connectors, and increased security through the retrieval of sensitive information from environment variables. These adjustments aim to streamline contract deployment and clarify the setup process for developers. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Env as Environment Variables
participant Script as Deployment Script
participant Contract as Smart Contract
Dev->>Env: Configure .env.sample
Dev->>Script: Run deployment script
Script->>Env: Retrieve TSS, Admin, Gateway addresses
Script->>Contract: Deploy contract using retrieved addresses
Contract-->>Script: Confirm deployment success
Script-->>Dev: Notify deployment complete
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 (
|
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13159201 | Triggered | Alchemy API Key | 72e1200 | v2/lib/forge-std/src/StdChains.sol | View secret |
| 13159201 | Triggered | Alchemy API Key | 8fafcbd | v2/lib/forge-std/src/StdChains.sol | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #293 +/- ##
==========================================
+ Coverage 68.44% 68.97% +0.53%
==========================================
Files 7 7
Lines 244 245 +1
Branches 61 61
==========================================
+ Hits 167 169 +2
Misses 76 76
+ Partials 1 0 -1 ☔ View full report in Codecov by Sentry. |
skosito
requested review from
andresaiello,
brewmaster012,
lumtis,
CharlieMc0,
fadeev and
fbac
as code owners
![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 (9)
v2/scripts/deploy/deterministic/DeployTestERC20.s.sol (1)
7-8: Add documentation for the contract.Provide a brief description of the contract's purpose and usage.
+ /** + * @title DeployTestERC20 + * @dev Contract for deploying test ERC20 tokens for testing deployments. + */ contract DeployTestERC20 is Script {v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol (3)
Line range hint
1-2:
Specify a more recent version of Solidity.Using a more recent version of Solidity (e.g.,
0.8.19) can provide additional features and security improvements.- pragma solidity 0.8.26; + pragma solidity 0.8.19;
Line range hint
7-8:
Add documentation for the contract.Provide a brief description of the contract's purpose and usage.
+ /** + * @title UpgradeGatewayEVM + * @dev Contract for upgrading the GatewayEVM contract. + */ contract UpgradeGatewayEVM is Script {
Line range hint
13-25:
Consider adding error handling and logging.Adding error handling and logging can help in debugging and monitoring the upgrade process.
function run() external { address proxy = vm.envAddress("GATEWAY_EVM_PROXY"); GatewayEVM prevImpl = GatewayEVM(proxy); vm.startBroadcast(deployer); try Upgrades.upgradeProxy(proxy, "GatewayEVMUpgradeTest.sol", "") { } catch { revert("Failed to upgrade GatewayEVM contract"); } GatewayEVM newImpl = GatewayEVM(proxy); require(prevImpl.tssAddress() == newImpl.tssAddress(), "tss addr changed"); require(prevImpl.zetaToken() == newImpl.zetaToken(), "zeta token addr changed"); vm.stopBroadcast(); }v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (1)
7-8: Add documentation for the contract.Provide a brief description of the contract's purpose and usage.
+ /** + * @title DeployERC20Custody + * @dev Contract for deploying the ERC20 custody contract. + */ contract DeployERC20Custody is Script {v2/scripts/deploy/deterministic/readme.md (4)
1-5: Improved clarity in documentation.The section title update and notes about the
.envfile setup improve clarity. However, consider the following grammatical improvements:- 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)
Line range hint
6-15:
Enhanced understanding of the deployment process.The description of the
DeployGatewayEVMscript is clarified, and the deployment process is detailed. However, consider the following improvements:- Remaining deployment script work in similar way as `GatewayEVM` but much simpler because there is no proxy. + Remaining deployment scripts work in a similar way to `GatewayEVM` but are much simpler because there is no proxy.Also, format the bare URL:
- Foundry (https://book.getfoundry.sh/tutorials/create2-tutorial) + [Foundry](https://book.getfoundry.sh/tutorials/create2-tutorial)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)
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...ccount private key.DeployGatewayEVMscript uses create2 with Foundry (https://book...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~7-~7: Possible missing comma found.
Context: ...ntracts. This ensures that on every EVM chainGatewayEVMcontract will be on same a...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ... chainGatewayEVMcontract will be on same address. Since UUPS proxy is used for ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~9-~9: You might be missing the article “the” here.
Context: ... address. Since UUPS proxy is used forGatewayEVMcontract, both implementatio...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~9-~9: You might be missing the article “the” here.
Context: ...n andERC1967Proxyare deployed using above technique: - calculate expected addres...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Markdownlint
6-6: null
Bare URL used(MD034, no-bare-urls)
16-17: Clear and useful addition.The description of the
UpgradeGatewayEVMscript is clear and useful. However, consider the following improvements:- OpenZeppelin's Foundry Upgrades plugin (https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades) + OpenZeppelin's Foundry Upgrades plugin [here](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades)Tools
LanguageTool
[uncategorized] ~17-~17: You might be missing the article “the” here.
Context: ...there is no proxy.UpgradeGatewayEVMscript uses OpenZeppelin's Foundry Upgrades pl...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Markdownlint
17-17: null
Bare URL used(MD034, no-bare-urls)
22-26: Clear and necessary instructions.The instructions for executing the deployment script are clear and necessary. However, consider the following improvements:
- To execute deployment script, following format is needed: + To execute the deployment script, the following format is needed:Also, specify the language for the code block:
- ``` + ```shTools
LanguageTool
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...use plugin to upgrade proxy To execute deployment script, following format is needed: ``...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...de proxy To execute deployment script, following format is needed: ``` forge script scr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Markdownlint
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- .gitignore (1 hunks)
- v2/.env.sample (1 hunks)
- v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/DeployTestERC20.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/readme.md (2 hunks)
Files skipped from review due to trivial changes (2)
- .gitignore
- v2/.env.sample
Additional context used
LanguageTool
v2/scripts/deploy/deterministic/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: ...ccount private key.DeployGatewayEVMscript uses create2 with Foundry (https://book...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~15-~15: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...s expected Remaining deployment script work in similar way asGatewayEVMbut much...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~15-~15: You might be missing the article “a” here.
Context: ...ed Remaining deployment script work in similar way asGatewayEVMbut much simpler be...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~17-~17: You might be missing the article “the” here.
Context: ...there is no proxy.UpgradeGatewayEVMscript uses OpenZeppelin's Foundry Upgrades pl...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...use plugin to upgrade proxy To execute deployment script, following format is needed: ``...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...de proxy To execute deployment script, following format is needed: ``` forge script scr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Markdownlint
v2/scripts/deploy/deterministic/readme.md
6-6: null
Bare URL used(MD034, no-bare-urls)
17-17: null
Bare URL used(MD034, no-bare-urls)
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (16)
v2/scripts/deploy/deterministic/DeployTestERC20.s.sol (2)
1-2: Specify a more recent version of Solidity.Using a more recent version of Solidity (e.g.,
0.8.19) can provide additional features and security improvements.- pragma solidity 0.8.26; + pragma solidity 0.8.19;Likely invalid or redundant comment.
4-5: Verify the necessity of imports.Ensure that all imported files are necessary and used within the contract.
Verification successful
Both imports are necessary and used within the
DeployTestERC20.s.solcontract.
forge-std/Script.solis used as the base contract forDeployTestERC20.test/utils/TestERC20.solis used to create instances of theTestERC20token.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of imported files in `DeployTestERC20.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg --type solidity -A 5 $'import "forge-std/Script.sol";' rg --type solidity -A 5 $'import "test/utils/TestERC20.sol";'Length of output: 187
Script:
#!/bin/bash # Description: Verify the usage of imported files in `DeployTestERC20.s.sol`. # Test: Search for the usage of imported files within `DeployTestERC20.s.sol`. rg 'forge-std/Script' v2/scripts/deploy/deterministic/DeployTestERC20.s.sol rg 'test/utils/TestERC20' v2/scripts/deploy/deterministic/DeployTestERC20.s.solLength of output: 221
Script:
#!/bin/bash # Description: Verify the usage of imported entities in `DeployTestERC20.s.sol`. # Test: Search for the usage of entities from `forge-std/Script.sol` within `DeployTestERC20.s.sol`. rg 'Script' v2/scripts/deploy/deterministic/DeployTestERC20.s.sol # Test: Search for the usage of entities from `test/utils/TestERC20.sol` within `DeployTestERC20.s.sol`. rg 'TestERC20' v2/scripts/deploy/deterministic/DeployTestERC20.s.solLength of output: 482
v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol (2)
13-13: Ensure the environment variable is set.Verify that the
GATEWAY_EVM_PROXYenvironment variable is set before running the script.
Line range hint
4-10:
Verify the necessity of imports.Ensure that all imported files are necessary and used within the contract.
Verification successful
All imports in
UpgradeGatewayEVM.s.solare necessary and used within the contract.The verification scripts confirmed that each imported file is utilized in the
UpgradeGatewayEVM.s.solscript.
forge-std/Script.solsrc/evm/GatewayEVM.soltest/utils/GatewayEVMUpgradeTest.soltest/utils/TestERC20.sol@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.solopenzeppelin-foundry-upgrades/Upgrades.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of imported files in `UpgradeGatewayEVM.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg --type solidity -A 5 $'import "forge-std/Script.sol";' rg --type solidity -A 5 $'import "src/evm/GatewayEVM.sol";' rg --type solidity -A 5 $'import "test/utils/GatewayEVMUpgradeTest.sol";' rg --type solidity -A 5 $'import "test/utils/TestERC20.sol";' rg --type solidity -A 5 $'import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";' rg --type solidity -A 5 $'import { Upgrades } from "openzeppelin-foundry-upgrades/Upgrades.sol";'Length of output: 943
Script:
#!/bin/bash # Description: Verify the usage of imported files in `UpgradeGatewayEVM.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg 'import "forge-std/Script.sol";' -A 5 --glob '*.sol' rg 'import "src/evm/GatewayEVM.sol";' -A 5 --glob '*.sol' rg 'import "test/utils/GatewayEVMUpgradeTest.sol";' -A 5 --glob '*.sol' rg 'import "test/utils/TestERC20.sol";' -A 5 --glob '*.sol' rg 'import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";' -A 5 --glob '*.sol' rg 'import { Upgrades } from "openzeppelin-foundry-upgrades/Upgrades.sol";' -A 5 --glob '*.sol'Length of output: 14732
Script:
#!/bin/bash # Description: Verify the usage of imported files in `UpgradeGatewayEVM.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg 'import "forge-std/Script.sol";' -A 5 --glob '*.sol' rg 'import "src/evm/GatewayEVM.sol";' -A 5 --glob '*.sol' rg 'import "test/utils/GatewayEVMUpgradeTest.sol";' -A 5 --glob '*.sol' rg 'import "test/utils/TestERC20.sol";' -A 5 --glob '*.sol' rg 'import \{ ERC1967Proxy \} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";' -A 5 --glob '*.sol' rg 'import \{ Upgrades \} from "openzeppelin-foundry-upgrades/Upgrades.sol";' -A 5 --glob '*.sol'Length of output: 21529
v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (3)
9-27: Ensure the environment variables are set.Verify that the
TSS_ADDRESS,ERC20_CUSTODY_ADMIN_ADDRESS_EVM, andGATEWAY_PROXY_EVMenvironment variables are set before running the script.
1-2: Specify a more recent version of Solidity.Using a more recent version of Solidity (e.g.,
0.8.19) can provide additional features and security improvements.- pragma solidity 0.8.26; + pragma solidity 0.8.19;Likely invalid or redundant comment.
4-5: Verify the necessity of imports.Ensure that all imported files are necessary and used within the contract.
Verification successful
Both imports are necessary and used within the contract.
- The
Scriptclass fromforge-std/Script.solis used as the base class for theDeployERC20Custodycontract.- The
ERC20Custodycontract fromsrc/evm/ERC20Custody.solis instantiated and used within therunfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of imported files in `DeployERC20Custody.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg --type solidity -A 5 $'import "forge-std/Script.sol";' rg --type solidity -A 5 $'import "src/evm/ERC20Custody.sol";'Length of output: 187
Script:
#!/bin/bash # Description: Verify the usage of imported files in `DeployERC20Custody.s.sol`. # Test: Search for the usage of imported files. Expect: All imported files are used in the contract. rg 'import "forge-std/Script.sol";' v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol -A 10 rg 'import "src/evm/ERC20Custody.sol";' v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol -A 10Length of output: 991
Script:
#!/bin/bash # Description: Verify the usage of imported files in `DeployERC20Custody.s.sol`. # Test: Search for the usage of classes or functions from imported files. # `Script.sol` is likely to be used for scripting functionalities. rg 'Script' v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol -A 10 # `ERC20Custody.sol` should contain the `ERC20Custody` contract. rg 'ERC20Custody' v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol -A 10Length of output: 1459
v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol (5)
1-3: SPDX license identifier and pragma directive are correct.These lines are necessary and correctly included.
4-6: Imports are correctly specified.The necessary dependencies for the script are correctly imported.
7-14: Contract definition and environment variable usage are correct.The contract
DeployZetaConnectorNonNativeand therunfunction are correctly defined. The usage of environment variables enhances security and flexibility. The salt is correctly computed.
16-27: Deployment process and address verification are correct.The deployment process is correctly implemented with deterministic deployment using
create2. The address verification ensures correctness.
28-29: Broadcasting is correctly stopped.The broadcasting is correctly stopped, ensuring the deployment is complete.
v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol (4)
Line range hint
1-5:
SPDX license identifier, pragma directive, and imports are correct.These lines are necessary and correctly included.
9-13: Contract definition and environment variable usage are correct.The contract
DeployGatewayEVMand therunfunction are correctly defined. The usage of environment variables enhances security and flexibility.
18-19: Salts for deterministic deployment are correctly computed.The salts are correctly computed using
keccak256.
Line range hint
21-39:
Deployment process and address verification are correct.The deployment process is correctly implemented with deterministic deployment using
create2. The address verification ensures correctness.
| function run() external { | ||
| bytes32 erc20Salt = keccak256("TestERC20"); | ||
|
|
||
| vm.startBroadcast(); | ||
|
|
||
| TestERC20 zeta = new TestERC20{salt: erc20Salt}("zeta", "ZETA"); | ||
|
|
||
| address expectedAddr = vm.computeCreate2Address( | ||
| erc20Salt, | ||
| hashInitCode(type(TestERC20).creationCode, abi.encode("zeta", "ZETA")) | ||
| ); | ||
|
|
||
| require(expectedAddr == address(zeta), "zeta address doesn't match expected address"); | ||
|
|
||
| vm.stopBroadcast(); | ||
| } |
There was a problem hiding this comment.
Consider adding error handling and logging.
Adding error handling and logging can help in debugging and monitoring the deployment process.
function run() external {
bytes32 erc20Salt = keccak256("TestERC20");
vm.startBroadcast();
TestERC20 zeta;
try new TestERC20{salt: erc20Salt}("zeta", "ZETA") returns (TestERC20 _zeta) {
zeta = _zeta;
} catch {
revert("Failed to deploy TestERC20 contract");
}
address expectedAddr = vm.computeCreate2Address(
erc20Salt,
hashInitCode(type(TestERC20).creationCode, abi.encode("zeta", "ZETA"))
);
require(expectedAddr == address(zeta), "zeta address doesn't match expected address");
vm.stopBroadcast();
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function run() external { | |
| bytes32 erc20Salt = keccak256("TestERC20"); | |
| vm.startBroadcast(); | |
| TestERC20 zeta = new TestERC20{salt: erc20Salt}("zeta", "ZETA"); | |
| address expectedAddr = vm.computeCreate2Address( | |
| erc20Salt, | |
| hashInitCode(type(TestERC20).creationCode, abi.encode("zeta", "ZETA")) | |
| ); | |
| require(expectedAddr == address(zeta), "zeta address doesn't match expected address"); | |
| vm.stopBroadcast(); | |
| } | |
| function run() external { | |
| bytes32 erc20Salt = keccak256("TestERC20"); | |
| vm.startBroadcast(); | |
| TestERC20 zeta; | |
| try new TestERC20{salt: erc20Salt}("zeta", "ZETA") returns (TestERC20 _zeta) { | |
| zeta = _zeta; | |
| } catch { | |
| revert("Failed to deploy TestERC20 contract"); | |
| } | |
| address expectedAddr = vm.computeCreate2Address( | |
| erc20Salt, | |
| hashInitCode(type(TestERC20).creationCode, abi.encode("zeta", "ZETA")) | |
| ); | |
| require(expectedAddr == address(zeta), "zeta address doesn't match expected address"); | |
| vm.stopBroadcast(); | |
| } |
| address payable tss = payable(vm.envAddress("TSS_ADDRESS")); | ||
| address admin = vm.envAddress("ERC20_CUSTODY_ADMIN_ADDRESS_EVM"); | ||
| address gateway = vm.envAddress("GATEWAY_PROXY_EVM"); | ||
|
|
||
| bytes32 salt = keccak256("ERC20Custody"); | ||
|
|
||
| vm.startBroadcast(); | ||
|
|
||
| ERC20Custody custody = new ERC20Custody{salt: salt}(gateway, tss, admin); | ||
|
|
||
| address expectedAddr = vm.computeCreate2Address( | ||
| salt, | ||
| hashInitCode(type(ERC20Custody).creationCode, abi.encode(gateway, tss, admin)) | ||
| ); | ||
|
|
||
| require(expectedAddr == address(custody), "erc20 custody address doesn't match expected address"); | ||
|
|
||
| vm.stopBroadcast(); | ||
| } |
There was a problem hiding this comment.
Consider adding error handling and logging.
Adding error handling and logging can help in debugging and monitoring the deployment process.
function run() external {
address payable tss = payable(vm.envAddress("TSS_ADDRESS"));
address admin = vm.envAddress("ERC20_CUSTODY_ADMIN_ADDRESS_EVM");
address gateway = vm.envAddress("GATEWAY_PROXY_EVM");
bytes32 salt = keccak256("ERC20Custody");
vm.startBroadcast();
ERC20Custody custody;
try new ERC20Custody{salt: salt}(gateway, tss, admin) returns (ERC20Custody _custody) {
custody = _custody;
} catch {
revert("Failed to deploy ERC20Custody contract");
}
address expectedAddr = vm.computeCreate2Address(
salt,
hashInitCode(type(ERC20Custody).creationCode, abi.encode(gateway, tss, admin))
);
require(expectedAddr == address(custody), "erc20 custody address doesn't match expected address");
vm.stopBroadcast();
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| address payable tss = payable(vm.envAddress("TSS_ADDRESS")); | |
| address admin = vm.envAddress("ERC20_CUSTODY_ADMIN_ADDRESS_EVM"); | |
| address gateway = vm.envAddress("GATEWAY_PROXY_EVM"); | |
| bytes32 salt = keccak256("ERC20Custody"); | |
| vm.startBroadcast(); | |
| ERC20Custody custody = new ERC20Custody{salt: salt}(gateway, tss, admin); | |
| address expectedAddr = vm.computeCreate2Address( | |
| salt, | |
| hashInitCode(type(ERC20Custody).creationCode, abi.encode(gateway, tss, admin)) | |
| ); | |
| require(expectedAddr == address(custody), "erc20 custody address doesn't match expected address"); | |
| vm.stopBroadcast(); | |
| } | |
| address payable tss = payable(vm.envAddress("TSS_ADDRESS")); | |
| address admin = vm.envAddress("ERC20_CUSTODY_ADMIN_ADDRESS_EVM"); | |
| address gateway = vm.envAddress("GATEWAY_PROXY_EVM"); | |
| bytes32 salt = keccak256("ERC20Custody"); | |
| vm.startBroadcast(); | |
| ERC20Custody custody; | |
| try new ERC20Custody{salt: salt}(gateway, tss, admin) returns (ERC20Custody _custody) { | |
| custody = _custody; | |
| } catch { | |
| revert("Failed to deploy ERC20Custody contract"); | |
| } | |
| address expectedAddr = vm.computeCreate2Address( | |
| salt, | |
| hashInitCode(type(ERC20Custody).creationCode, abi.encode(gateway, tss, admin)) | |
| ); | |
| require(expectedAddr == address(custody), "erc20 custody address doesn't match expected address"); | |
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol (2 hunks)
- v2/scripts/deploy/deterministic/DeployTestERC20.s.sol (1 hunks)
- v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol
- v2/scripts/deploy/deterministic/DeployTestERC20.s.sol
- v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol
Additional comments not posted (6)
v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol (6)
11-13: Improved security and flexibility by using environment variables.The TSS address, admin address, and ZETA token address are now retrieved from environment variables, enhancing security and flexibility.
18-19: Enhanced uniqueness and security of salt values.The
implSaltandproxySaltare now derived from the keccak256 hash of strings, enhancing their uniqueness and security.
21-21: Simplified transaction broadcasting.The
vm.startBroadcastmethod is now called without the deployer address parameter, potentially simplifying the deployment process.
29-30: Improved robustness with require statement.A require statement has been added to ensure that the
gatewayImpldeployment does not fail, improving the robustness of the deployment process.
48-49: Improved robustness with require statement.A require statement has been added to ensure that the
gatewayProxydeployment does not fail, improving the robustness of the deployment process.
9-9: Contract name change acknowledged.The contract name has been changed from
DeployGatewayEVMCreate2toDeployGatewayEVM. Ensure that all references to this contract in the codebase are updated accordingly.Verification successful
Contract name change verified.
All references to the old contract name
DeployGatewayEVMCreate2have been successfully updated toDeployGatewayEVM.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the old contract name are updated to the new contract name. # Test: Search for the old contract name. Expect: No occurrences of the old contract name. rg --type solidity 'DeployGatewayEVMCreate2'Length of output: 77
Script:
#!/bin/bash # Description: Verify all references to the old contract name are updated to the new contract name. # Test: Search for the old contract name in Solidity files. Expect: No occurrences of the old contract name. rg 'DeployGatewayEVMCreate2' --glob '*.sol'Length of output: 43
I think @fbac opened a general issues for the contract addresses |
introduces deploy scripts for all evm protocol contracts
uses env variables to set needed params for scripts
we should probably push broadcast files for real networks deployments which contains deployment info, just gitignore local broadcast files
open questions: do we need to store addresses json files in data folder same as v1, and where are those used? if we push broadcast files for live networks, maybe we should wait for first testnet deployment to generate addresses, both manually and ones that is fetched from blockchains via scripts, separate issue needed for that
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation