chore: scripts for deploying implementation contracts #423
Conversation
📝 WalkthroughWalkthroughThis pull request introduces three new Solidity smart contracts for deploying specific implementations using the CREATE2 opcode: Changes
Possibly related PRs
Suggested reviewers
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #423 +/- ##
==========================================
- Coverage 84.27% 84.06% -0.22%
==========================================
Files 8 8
Lines 388 389 +1
Branches 123 124 +1
==========================================
Hits 327 327
- Misses 61 62 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
lumtis
requested review from
andresaiello,
brewmaster012,
CharlieMc0,
fadeev,
skosito and
fbac
as code owners
lumtis
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (6)
v2/scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol (1)
7-8: Add documentation and deployment safety checks.Consider adding:
- NatSpec documentation explaining the deployment process and expected outcomes
- A check to prevent accidental re-deployment if the implementation already exists
Example improvement:
+/// @title GatewayEVM Implementation Deployer +/// @notice Deploys the GatewayEVM implementation contract with a deterministic address using CREATE2 +/// @dev This is a one-time deployment script. The implementation address is derived from the salt "GatewayEVM" contract DeployGatewayEVM is Script { + /// @notice Deploys the GatewayEVM implementation + /// @dev Reverts if deployment fails or if the deployed address doesn't match the expected address function run() external { + // Check if implementation is already deployed + bytes32 implSalt = keccak256("GatewayEVM"); + address expectedImplAddress = vm.computeCreate2Address( + implSalt, + hashInitCode(type(GatewayEVM).creationCode) + ); + require(expectedImplAddress.code.length == 0, "Implementation already deployed");v2/scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol (2)
7-7: Consider renaming the contract for consistency.The contract name
DeployGatewayZEVMcould be renamed toDeployGatewayZEVMImplementationto match the file name and better reflect its purpose of deploying the implementation contract.
22-23: Enhance error messages for better debugging.The require statements could provide more detailed error messages to aid in debugging deployment issues.
- require(address(gatewayImpl) != address(0), "gatewayImpl deployment failed"); - require(expectedImplAddress == address(gatewayImpl), "impl address doesn't match expected address"); + require(address(gatewayImpl) != address(0), "GatewayZEVM: deployment failed with zero address"); + require( + expectedImplAddress == address(gatewayImpl), + string.concat( + "GatewayZEVM: deployed address mismatch (expected: ", + vm.toString(expectedImplAddress), + ", actual: ", + vm.toString(address(gatewayImpl)), + ")" + ) + );v2/scripts/deploy/readme.md (3)
93-93: Convert bold text to proper markdown headingsThe implementation sections use bold text for headers, but they should be proper markdown headings for better document structure and navigation.
-**GatewayEVM** +### GatewayEVM -**ERC20Custody** +### ERC20Custody -**GatewayZEVM** +### GatewayZEVMAlso applies to: 107-107, 121-121
🧰 Tools
🪛 Markdownlint
93-93: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
97-105: Add language specifiers to code blocksCode blocks should specify their language for proper syntax highlighting.
-``` +```bash forge script scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol \ ...-
+bash
forge script scripts/deploy/deterministic/DeployERC20CustodyImplementation.s.sol
...-``` +```bash forge script scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol \ ...-
+solidity
upgradeToAndCall(0, <implementation_address>, "")Also applies to: 111-119, 125-134, 138-140
🧰 Tools
🪛 Markdownlint
97-97: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
89-90: Add reference to OpenZeppelin's ERC1967 documentationThe documentation mentions ERC1967 standard but doesn't provide a reference link for developers who want to learn more.
-Protocol contracts (Gateway and ERC20Custody) follow ERC1967 standard. The contracts can be upgraded by deploying a new implementation and upgrading to this new implementation. +Protocol contracts (Gateway and ERC20Custody) follow [ERC1967 standard](https://eips.ethereum.org/EIPS/eip-1967). The contracts can be upgraded by deploying a new implementation and upgrading to this new implementation. For more details on the upgrade process, see [OpenZeppelin's documentation on upgradeable contracts](https://docs.openzeppelin.com/contracts/4.x/upgradeable).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
v2/scripts/deploy/deterministic/DeployERC20CustodyImplementation.s.sol(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol(1 hunks)v2/scripts/deploy/readme.md(2 hunks)
🧰 Additional context used
🪛 Markdownlint
v2/scripts/deploy/readme.md
93-93: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
107-107: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
121-121: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
111-111: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
125-125: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (9)
v2/scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol (1)
1-6: LGTM! Well-structured file with appropriate imports.
The file follows best practices with explicit SPDX license, precise Solidity version, and minimal necessary imports.
v2/scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol (2)
1-6: LGTM! Well-structured file header with appropriate imports.
The file uses a specific Solidity version and minimal imports, which is ideal for deterministic deployments.
8-26: Consider adding deployment versioning mechanism.
The current implementation uses a fixed salt which could be problematic if you need to deploy multiple versions of the implementation. Consider adding a version parameter or nonce to the salt computation to allow for multiple deployments.
Additionally, the run() function is publicly accessible without any access control. While this might be acceptable for a deployment script, consider documenting this assumption.
Let's verify if there are any existing deployments using this salt:
v2/scripts/deploy/deterministic/DeployERC20CustodyImplementation.s.sol (4)
7-7: LGTM: Contract declaration follows best practices
The contract name matches the file name and correctly inherits from Forge's Script contract.
10-10: Verify uniqueness of the implementation salt
The salt is derived from a fixed string "ERC20Custody". Ensure this salt hasn't been used before to prevent deployment conflicts.
#!/bin/bash
# Description: Check for existing contracts at the computed CREATE2 address
# Expected: No existing contract should be found at the computed address
# Search for previous deployments or references to ERC20Custody implementation
echo "Checking for previous deployment references:"
rg "ERC20Custody.*implementation" --type solidity
# Search for the specific salt string usage
echo "Checking for salt string usage:"
rg "keccak256\(.*ERC20Custody.*\)" --type solidity12-25: Verify broadcast scope is properly contained
The broadcast scope is correctly managed with start/stop calls, but verify there are no external calls within this scope that could lead to unexpected transactions.
#!/bin/bash
# Description: Check for potential external calls within broadcast scope
# Expected: No unexpected external calls between broadcast markers
# Search for external calls pattern in deployment scripts
echo "Checking for external calls in deployment scripts:"
ast-grep --pattern 'vm.startBroadcast();
$$$
vm.stopBroadcast();'1-6: Verify Solidity version compatibility with dependencies
The script uses Solidity 0.8.26, which is a very recent version. Ensure this version is compatible with forge-std and the ERC20Custody contract.
v2/scripts/deploy/readme.md (2)
138-140: Enhance the upgrade function documentation
The upgradeToAndCall function documentation could be more detailed to prevent potential upgrade issues.
Consider adding:
- A note about the first parameter being 0 (its purpose)
- Warning about the importance of proper implementation address verification
- Example of how to verify the implementation address before upgrade
+### Upgrading the Contract
+
+Before upgrading:
+1. Verify the implementation address matches the newly deployed contract
+2. Ensure all necessary storage layout checks have been performed
+
```solidity
-upgradeToAndCall(0, <implementation_address>, "")
+// The first parameter (0) indicates no value is being sent with the call
+// The last parameter ("") indicates no initialization function needs to be called
+function upgradeToAndCall(
+ uint256 value, // Must be 0 for standard upgrades
+ address newImplementation,
+ bytes memory data // Empty string for standard upgrades
+) external;+>
+This operation cannot be undone.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 Markdownlint</summary>
138-138: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
---
`206-210`: **LGTM: Updated script list is comprehensive**
The added implementation deployment scripts are properly documented in the script list, maintaining consistency with the rest of the documentation.
</details>
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
| // Simulate the upgrade | ||
| GatewayEVM proxy = GatewayEVM(proxyAddress); | ||
| vm.prank(adminAddress); | ||
| proxy.upgradeToAndCall(gatewayImpl, ""); |
There was a problem hiding this comment.
why we dont use plugin for upgrade, this wont perform any validation, so if storage is broken we wont get any hints?
imo best upgrade is to have previous version of contract, in new contract to add directive referencing previous version, and upgrade with plugin, that way, plugin will load previous version and do storage checks
there is existing script where plugin is used as reference: UpgradeGatewayEVM
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (8)
v2/scripts/upgrade/SimulateGatewayZEVMUpgrade.s.sol (1)
1-27: Consider adding dry-run and staging deployment strategy.For a robust upgrade process, consider implementing:
- Dry-run functionality to simulate upgrades on a fork before production
- Multi-step deployment process with staging environment validation
- Automated verification of contract interfaces and storage layouts
- Rollback procedures in case of upgrade failures
Would you like assistance in implementing these safety measures?
v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol (1)
8-9: Add natspec documentation for the contract and function.Consider adding comprehensive natspec documentation to improve maintainability and clarity.
+/// @title UpgradeSimulation +/// @notice Script to simulate the upgrade of ERC20Custody contract +/// @dev Uses Forge's scripting capabilities to simulate the upgrade process contract UpgradeSimulation is Script { + /// @notice Simulates the upgrade process for ERC20Custody + /// @dev Deploys new implementation and performs upgrade simulation function run() external {v2/scripts/upgrade/SimulateGatewayEVMUpgrade.s.sol (1)
11-12: Add environment variable validation.Consider adding require statements to validate that the environment variables are not zero addresses.
- address proxyAddress = vm.envAddress("PROXY_ADDRESS"); - address adminAddress = vm.envAddress("ADMIN_ADDRESS"); + address proxyAddress = vm.envAddress("PROXY_ADDRESS"); + address adminAddress = vm.envAddress("ADMIN_ADDRESS"); + require(proxyAddress != address(0), "Invalid proxy address"); + require(adminAddress != address(0), "Invalid admin address");v2/scripts/deploy/readme.md (4)
87-140: Add implementation address verification stepsThe section should include steps to verify the deployed implementation address matches the expected address. This is crucial for security as the implementation address will be used in the upgrade process.
Consider adding:
After deployment, verify the implementation address: 1. Get the deployed implementation address from the deployment transaction 2. Compare it with the expected address computed by the deployment script 3. Verify the implementation contract is verified on the block explorer🧰 Tools
🪛 Markdownlint
93-93: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
107-107: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
121-121: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
97-97: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
111-111: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
125-125: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
138-138: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
157-161: Structure environment variables setupThe environment variables section could be more organized. Consider using a table format:
Required environment variables: | Variable | Description | Example | |----------|-------------|---------| | PROXY_ADDRESS | Address of the contract proxy to test | 0x123... | | ADMIN_ADDRESS | Address of the admin that can upgrade | 0x456... |🧰 Tools
🪛 Markdownlint
158-158: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
178-180: Enhance state verification processManual verification of state variables is error-prone and could miss critical changes. Consider enhancing this section with:
- A checklist of critical state variables to verify
- Expected values or value ranges
- Common issues to watch out for
Critical state variables to verify: - [ ] Admin addresses remain unchanged - [ ] Protocol configurations are preserved - [ ] Connected contracts (TSS, tokens) maintain correct addresses - [ ] Custom state variables specific to the contract Common issues: - Storage layout changes affecting variable values - Uninitialized variables after upgrade - Incorrect permission settings
189-193: Sanitize example outputThe example output contains addresses that could be confused with real deployment addresses. Consider using clearly marked placeholder addresses.
- custody address: 0xD80BE3710F08D280F51115e072e5d2a778946cd7 - tss address: 0x8531a5aB847ff5B22D855633C25ED1DA3255247e - zetaConnector address: 0x0000000000000000000000000000000000000000 - zetaToken address: 0x1432612E60cad487C857E7D38AFf57134916c902 + custody address: 0xEXAMPLE1... + tss address: 0xEXAMPLE2... + zetaConnector address: 0x000...000 + zetaToken address: 0xEXAMPLE3...🧰 Tools
🪛 Gitleaks
192-192: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
v2/broadcast/DeployERC20CustodyImplementation.s.sol/97/run-latest.json (1)
24-39: Consider adding deployment events for better traceability.While the deployment was successful, no events were emitted during the contract creation. Consider adding deployment events to make it easier to track and verify deployments on-chain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
v2/broadcast/DeployERC20CustodyImplementation.s.sol/11155111/run-1730496331.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/11155111/run-latest.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/80002/run-1730495993.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/80002/run-latest.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/84532/run-1730495509.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/84532/run-latest.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/97/run-1730496209.json(1 hunks)v2/broadcast/DeployERC20CustodyImplementation.s.sol/97/run-latest.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/11155111/run-1730496266.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/11155111/run-latest.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/80002/run-1730495654.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/80002/run-latest.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/84532/run-1730495252.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/84532/run-latest.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/97/run-1730496146.json(1 hunks)v2/broadcast/DeployGatewayEVMImplementation.s.sol/97/run-latest.json(1 hunks)v2/broadcast/DeployGatewayZEVMImplementation.s.sol/7001/run-1730496448.json(1 hunks)v2/broadcast/DeployGatewayZEVMImplementation.s.sol/7001/run-latest.json(1 hunks)v2/scripts/deploy/readme.md(2 hunks)v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol(1 hunks)v2/scripts/upgrade/SimulateGatewayEVMUpgrade.s.sol(1 hunks)v2/scripts/upgrade/SimulateGatewayZEVMUpgrade.s.sol(1 hunks)
✅ Files skipped from review due to trivial changes (15)
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/11155111/run-1730496331.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/11155111/run-latest.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/80002/run-1730495993.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/80002/run-latest.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/84532/run-1730495509.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/84532/run-latest.json
- v2/broadcast/DeployERC20CustodyImplementation.s.sol/97/run-1730496209.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/11155111/run-1730496266.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/11155111/run-latest.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/80002/run-1730495654.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/80002/run-latest.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/84532/run-1730495252.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/97/run-1730496146.json
- v2/broadcast/DeployGatewayEVMImplementation.s.sol/97/run-latest.json
- v2/broadcast/DeployGatewayZEVMImplementation.s.sol/7001/run-1730496448.json
🧰 Additional context used
🪛 LanguageTool
v2/scripts/deploy/readme.md
[style] ~144-~144: Style-wise, it’s not ideal to insert an adverb (‘locally’) in the middle of an infinitive construction (‘to simulate’). Try moving the adverb to avoid split infinitives.
Context: ...tract upgrade The scripts in upgrade allow to locally simulate the upgrade process with the protocol contract and verify t...
(SPLIT_INFINITIVE)
[uncategorized] ~145-~145: Possible missing comma found.
Context: ...and verify the state is not corrupted. First a forked localnet must be started in a ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~165-~165: Loose punctuation mark.
Context: ...hen the script can be run. GatewayEVM: ``` forge script scripts/upgrade/Simula...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~169-~169: Loose punctuation mark.
Context: ...http://localhost:8545 `ERC20Custody`: forge script scripts/upgrade/Simula...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~173-~173: Loose punctuation mark.
Context: ... http://localhost:8545 `GatewayZEVM`: forge script scripts/upgrade/Simula...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint
v2/scripts/deploy/readme.md
93-93: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
107-107: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
121-121: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
111-111: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
125-125: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
148-148: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
152-152: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
158-158: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
166-166: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
170-170: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
174-174: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
184-184: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Gitleaks
v2/scripts/deploy/readme.md
192-192: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (14)
v2/scripts/upgrade/SimulateGatewayZEVMUpgrade.s.sol (2)
1-7: LGTM! File setup follows best practices.
The file uses appropriate SPDX licensing, specific Solidity version, and well-organized imports.
8-9: LGTM! Contract structure follows Foundry conventions.
The contract properly inherits from Script and declares an external run function as required by Foundry.
v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol (1)
1-7: LGTM! Well-structured file header with appropriate imports.
The file uses a specific Solidity version and includes all necessary dependencies.
v2/scripts/upgrade/SimulateGatewayEVMUpgrade.s.sol (2)
1-7: LGTM! Well-structured file header with appropriate imports.
The file uses a specific Solidity version and includes all necessary imports in a logical order.
1-30: Verify consistency with other upgrade simulation scripts.
Let's ensure this script follows the same pattern as other upgrade simulation scripts in the codebase.
✅ Verification successful
Upgrade simulation script follows consistent patterns across the codebase
The script follows the same upgrade pattern used in other simulation scripts:
- Uses identical
upgradeToAndCallmechanism - Follows similar state verification approach post-upgrade
- Maintains consistent environment variable usage (
PROXY_ADDRESS,ADMIN_ADDRESS) - All scripts verify critical contract-specific state variables after upgrade
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find other upgrade simulation scripts
fd "Simulate.*Upgrade.s.sol" "v2/scripts/upgrade/"
# Check for similar upgrade patterns
rg -A 10 "upgradeToAndCall" "v2/scripts/upgrade/"
Length of output: 2616
v2/scripts/deploy/readme.md (1)
260-264: LGTM! Clear and well-organized script listing
The new implementation deployment scripts are well-documented and follow a consistent naming convention.
v2/broadcast/DeployERC20CustodyImplementation.s.sol/97/run-latest.json (2)
4-18: LGTM! Deployment configuration looks secure.
The deployment uses CREATE2 for deterministic addressing and includes proper contract bytecode. The transaction is properly configured for BSC Testnet (Chain ID: 97).
44-47: Verify contract deployment on BSC Testnet.
The deployment was executed on BSC Testnet (chain 97) which is appropriate for testing. However, ensure to:
- Verify the contract source code on BSCscan
- Test the implementation thoroughly before deploying to mainnet
✅ Verification successful
Contract successfully verified on BSC Testnet
The contract has been successfully verified on BSC Testnet with full source code and ABI available. The verification status is "1" (success) and the complete source code is accessible, which ensures transparency and auditability. The implementation is an upgradeable ERC20 custody contract with comprehensive access control and safety features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify contract source code is accessible on BSCScan
# Note: This helps ensure transparency and auditability
# Test: Check if contract is verified on BSCScan testnet
curl -s "https://api-testnet.bscscan.com/api?module=contract&action=getabi&address=0x7524282ea07f95f9ba3ee1ed53e2ccbf72cd82eb" | jq -r '.status'
Length of output: 146
Script:
#!/bin/bash
# Let's get more details about the contract verification status and ABI
curl -s "https://api-testnet.bscscan.com/api?module=contract&action=getabi&address=0x7524282ea07f95f9ba3ee1ed53e2ccbf72cd82eb" | jq '.'
# Also check the contract source code
curl -s "https://api-testnet.bscscan.com/api?module=contract&action=getsourcecode&address=0x7524282ea07f95f9ba3ee1ed53e2ccbf72cd82eb" | jq '.'
Length of output: 135903
v2/broadcast/DeployGatewayEVMImplementation.s.sol/84532/run-latest.json (3)
3-18: Transaction configuration looks good!
The deployment is properly configured using CREATE2 for deterministic addressing, with appropriate gas limits and targeting the standard CREATE2 factory contract.
24-58: Transaction executed successfully with optimal gas usage!
The deployment completed successfully:
- Status: Success (0x1)
- Gas efficiency: Used ~75% of allocated gas limit
- Contract deployed to expected address: 0x7e10457ab8f12a2c114b5e46c938b807cd9eddc1
28-40: Verify implementation address consistency.
The deployment emitted the expected initialization event. However, we should verify that this implementation address (0x7e10457ab8f12a2c114b5e46c938b807cd9eddc1) matches the expected address calculated in the deployment script.
v2/broadcast/DeployGatewayZEVMImplementation.s.sol/7001/run-latest.json (3)
4-18: Gas usage is within reasonable limits for contract deployment.
The transaction configuration shows:
- Gas limit: ~4.5M gas (0x460694)
- Using CREATE2 for deterministic address deployment
- Appropriate chain ID for ZetaChain testnet (0x1b59)
55-61: Deployment metadata is properly recorded.
The metadata includes:
- Empty libraries array (as expected for implementation contract)
- Correct chain ID (7001 for ZetaChain testnet)
- Commit hash for traceability (2390cce)
23-53: Deployment completed successfully with expected events.
The transaction receipt confirms:
- Status: Success (0x1)
- Gas used: ~3.3M gas (0x32b285), which is well within the gas limit
- Expected initialization event was emitted with topic 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2
✅ Verification successful
Let me gather more information about the initialization event by searching for its topic hash and checking the contract implementation.
Let me check the contract implementation to verify the initialization event.
Let me check the Initializable contract from OpenZeppelin to verify the event.
Let me check the OpenZeppelin Initializable contract directly from the repository since node_modules is not available.
Based on the verification results, I can now provide the final response:
Deployment successfully initialized with correct event emission
The emitted event with topic 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2 is the standard Initialized(uint64) event from OpenZeppelin's Initializable contract. This event is correctly emitted during the initialization of upgradeable contracts to indicate successful initialization with version 1, as verified in the contract implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the deployed contract address matches across the codebase
# Test: Search for references to the deployed contract address
rg -i "0x8795b946b449096fa83e60626c838602fa7856dd"
# Test: Search for the initialization event signature to verify it's defined in the contract
ast-grep --pattern 'event $name($$$) {
$$$
}'
Length of output: 960
Script:
#!/bin/bash
# Search for the event topic hash
rg -i "0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2"
# Find Gateway implementation contract
fd -e sol Gateway
# Search for initialization related events in solidity files
rg -t sol "event.*Initialize"
Length of output: 13844
Script:
#!/bin/bash
# Check the contract implementation for the Initialized event
cat v2/contracts/zevm/GatewayZEVM.sol
# Also check the base contract that might define the event
rg -l "Initializable" v2/contracts/zevm/
Length of output: 16889
Script:
#!/bin/bash
# Find and check the Initializable contract
fd -e sol -e json Initializable node_modules/@openzeppelin/contracts-upgradeable/proxy/utils/
# Alternative: Check if the event is defined in the interface
cat v2/contracts/zevm/interfaces/IGatewayZEVM.sol
Length of output: 10710
Script:
#!/bin/bash
# Clone specific OpenZeppelin contracts to check Initializable
git clone --depth 1 https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git
cat openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol
# Also check if there are any other references to this event in the codebase
rg -i "Initialized\(" -A 2
Length of output: 51261
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
v2/lib/types.ts (1)
1-1: Consider documenting the token symbol naming conventionThe ParamSymbol type includes various token naming patterns:
- Chain-specific: "USDC.BSC", "ETH.BASE"
- Test tokens: "tBTC", "tBNB"
- Simple tokens: "USDC"
Consider adding documentation to explain these naming patterns and when each should be used.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (99)
v2/data/addresses.mainnet.jsonis excluded by!v2/data/**v2/data/addresses.testnet.jsonis excluded by!v2/data/**v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.mdis excluded by!v2/docs/**v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.mdis excluded by!v2/docs/**v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.mdis excluded by!v2/docs/**v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.mdis excluded by!v2/docs/**v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.mdis excluded by!v2/docs/**v2/pkg/address.sol/address.gois excluded by!v2/pkg/**v2/pkg/beaconproxy.sol/beaconproxy.gois excluded by!v2/pkg/**v2/pkg/console.sol/console.gois excluded by!v2/pkg/**v2/pkg/core.sol/core.gois excluded by!v2/pkg/**v2/pkg/defender.sol/defender.gois excluded by!v2/pkg/**v2/pkg/defenderdeploy.sol/defenderdeploy.gois excluded by!v2/pkg/**v2/pkg/erc1967proxy.sol/erc1967proxy.gois excluded by!v2/pkg/**v2/pkg/erc1967utils.sol/erc1967utils.gois excluded by!v2/pkg/**v2/pkg/erc20custody.sol/erc20custody.gois excluded by!v2/pkg/**v2/pkg/erc20custody.t.sol/erc20custodytest.gois excluded by!v2/pkg/**v2/pkg/erc20custodyupgradetest.sol/erc20custodyupgradetest.gois excluded by!v2/pkg/**v2/pkg/gatewayevm.sol/gatewayevm.gois excluded by!v2/pkg/**v2/pkg/gatewayevm.t.sol/gatewayevminboundtest.gois excluded by!v2/pkg/**v2/pkg/gatewayevm.t.sol/gatewayevmtest.gois excluded by!v2/pkg/**v2/pkg/gatewayevmupgradetest.sol/gatewayevmupgradetest.gois excluded by!v2/pkg/**v2/pkg/gatewayevmzevm.t.sol/gatewayevmzevmtest.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.sol/gatewayzevm.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/gatewayzevmupgradetest.sol/gatewayzevmupgradetest.gois excluded by!v2/pkg/**v2/pkg/math.sol/math.gois excluded by!v2/pkg/**v2/pkg/mockerc20.sol/mockerc20.gois excluded by!v2/pkg/**v2/pkg/mockerc721.sol/mockerc721.gois excluded by!v2/pkg/**v2/pkg/proxyadmin.sol/proxyadmin.gois excluded by!v2/pkg/**v2/pkg/receiverevm.sol/receiverevm.gois excluded by!v2/pkg/**v2/pkg/safeconsole.sol/safeconsole.gois excluded by!v2/pkg/**v2/pkg/safeerc20.sol/safeerc20.gois excluded by!v2/pkg/**v2/pkg/senderzevm.sol/senderzevm.gois excluded by!v2/pkg/**v2/pkg/signedmath.sol/signedmath.gois excluded by!v2/pkg/**v2/pkg/src/strings.sol/strings.gois excluded by!v2/pkg/**v2/pkg/stderror.sol/stderror.gois excluded by!v2/pkg/**v2/pkg/stdjson.sol/stdjson.gois excluded by!v2/pkg/**v2/pkg/stdmath.sol/stdmath.gois excluded by!v2/pkg/**v2/pkg/stdstorage.sol/stdstorage.gois excluded by!v2/pkg/**v2/pkg/stdstorage.sol/stdstoragesafe.gois excluded by!v2/pkg/**v2/pkg/stdstyle.sol/stdstyle.gois excluded by!v2/pkg/**v2/pkg/stdtoml.sol/stdtoml.gois excluded by!v2/pkg/**v2/pkg/storageslot.sol/storageslot.gois excluded by!v2/pkg/**v2/pkg/strings.sol/strings.gois excluded by!v2/pkg/**v2/pkg/systemcontract.sol/systemcontract.gois excluded by!v2/pkg/**v2/pkg/systemcontractmock.sol/systemcontractmock.gois excluded by!v2/pkg/**v2/pkg/testerc20.sol/testerc20.gois excluded by!v2/pkg/**v2/pkg/testuniversalcontract.sol/testuniversalcontract.gois excluded by!v2/pkg/**v2/pkg/transparentupgradeableproxy.sol/transparentupgradeableproxy.gois excluded by!v2/pkg/**v2/pkg/upgradeablebeacon.sol/upgradeablebeacon.gois excluded by!v2/pkg/**v2/pkg/upgrades.sol/unsafeupgrades.gois excluded by!v2/pkg/**v2/pkg/upgrades.sol/upgrades.gois excluded by!v2/pkg/**v2/pkg/utils.sol/utils.gois excluded by!v2/pkg/**v2/pkg/versions.sol/versions.gois excluded by!v2/pkg/**v2/pkg/wzeta.sol/weth9.gois excluded by!v2/pkg/**v2/pkg/zeta.non-eth.sol/zetanoneth.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornative.sol/zetaconnectornative.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornative.t.sol/zetaconnectornativetest.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornativeupgradetest.sol/zetaconnectornativeupgradetest.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornonnative.sol/zetaconnectornonnative.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornonnative.t.sol/zetaconnectornonnativetest.gois excluded by!v2/pkg/**v2/pkg/zetaconnectornonnativeupgradetest.sol/zetaconnectornonnativeupgradetest.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/Address__factory.tsis excluded by!v2/types/**v2/types/factories/BeaconProxy__factory.tsis excluded by!v2/types/**v2/types/factories/ERC1967Proxy__factory.tsis excluded by!v2/types/**v2/types/factories/ERC1967Utils__factory.tsis excluded by!v2/types/**v2/types/factories/ERC20CustodyUpgradeTest__factory.tsis excluded by!v2/types/**v2/types/factories/ERC20Custody__factory.tsis excluded by!v2/types/**v2/types/factories/GatewayEVMUpgradeTest__factory.tsis excluded by!v2/types/**v2/types/factories/GatewayEVM__factory.tsis excluded by!v2/types/**v2/types/factories/GatewayZEVMUpgradeTest__factory.tsis excluded by!v2/types/**v2/types/factories/GatewayZEVM__factory.tsis excluded by!v2/types/**v2/types/factories/Math__factory.tsis excluded by!v2/types/**v2/types/factories/MockERC20__factory.tsis excluded by!v2/types/**v2/types/factories/MockERC721__factory.tsis excluded by!v2/types/**v2/types/factories/ProxyAdmin__factory.tsis excluded by!v2/types/**v2/types/factories/ReceiverEVM__factory.tsis excluded by!v2/types/**v2/types/factories/SafeERC20__factory.tsis excluded by!v2/types/**v2/types/factories/SenderZEVM__factory.tsis excluded by!v2/types/**v2/types/factories/StdError.sol/StdError__factory.tsis excluded by!v2/types/**v2/types/factories/StdStorage.sol/StdStorageSafe__factory.tsis excluded by!v2/types/**v2/types/factories/Strings__factory.tsis excluded by!v2/types/**v2/types/factories/SystemContract.sol/SystemContract__factory.tsis excluded by!v2/types/**v2/types/factories/SystemContractMock.sol/SystemContractMock__factory.tsis excluded by!v2/types/**v2/types/factories/TestERC20__factory.tsis excluded by!v2/types/**v2/types/factories/TestUniversalContract__factory.tsis excluded by!v2/types/**v2/types/factories/TransparentUpgradeableProxy.sol/TransparentUpgradeableProxy__factory.tsis excluded by!v2/types/**v2/types/factories/UpgradeableBeacon__factory.tsis excluded by!v2/types/**v2/types/factories/WZETA.sol/WETH9__factory.tsis excluded by!v2/types/**v2/types/factories/ZRC20.sol/ZRC20__factory.tsis excluded by!v2/types/**v2/types/factories/Zeta.non-eth.sol/ZetaNonEth__factory.tsis excluded by!v2/types/**v2/types/factories/ZetaConnectorNativeUpgradeTest__factory.tsis excluded by!v2/types/**v2/types/factories/ZetaConnectorNative__factory.tsis excluded by!v2/types/**v2/types/factories/ZetaConnectorNonNativeUpgradeTest__factory.tsis excluded by!v2/types/**v2/types/factories/ZetaConnectorNonNative__factory.tsis excluded by!v2/types/**
📒 Files selected for processing (1)
v2/lib/types.ts(1 hunks)
🔇 Additional comments (1)
v2/lib/types.ts (1)
1-1: Verify the removal of "tMATIC" token symbol
The removal of "tMATIC" from ParamSymbol while retaining other test tokens (tBTC, tBNB) could be a breaking change. Please ensure:
- All existing code using "tMATIC" has been updated
- The removal is intentional and aligned with the project's token support strategy
- The presence of "MATIC.AMOY" is sufficient for your testing needs
Add scripts allowing to only deploy the implementation of the contract for the upgrade process
Summary by CodeRabbit
New Features
ERC20Custody,GatewayEVM, andGatewayZEVMimplementations using the CREATE2 opcode.upgradeToAndCallmethod.ERC20Custody,GatewayEVM, andGatewayZEVMcontracts.Documentation