-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add mainnet broadcast information #425
Conversation
📝 WalkthroughWalkthroughThis pull request introduces multiple JSON files that log the deployment and interaction of two Ethereum smart contracts: 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 #425 +/- ##
==========================================
- 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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (15)
v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-1730753291.json (1)
159-159
: Consider including transaction receiptsThe
receipts
array is empty. While not critical for a dry run, including receipts in the broadcast file would help verify gas usage and event emissions.v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-latest.json (1)
159-165
: Consider adding post-deployment verificationThe broadcast file shows this is a dry run (from the file path) on mainnet (chain: 1). Consider adding post-deployment verification steps to ensure:
- The proxy is correctly initialized
- The admin roles are properly set
- The implementation contract is accessible
v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-1730752569.json (1)
1-166
: Consider implementing deployment verification testsTo ensure the safety and correctness of the deployment process, consider adding post-deployment verification tests that:
- Verify all roles are correctly assigned
- Confirm the proxy is correctly initialized
- Test basic functionality through the proxy
Would you like me to help create a deployment verification test suite?
v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-latest.json (1)
101-157
: Consider keeping deployer access.The deployer is revoking its own access to all roles. While this is good for decentralization, consider:
- Whether you need to maintain emergency access
- If there are sufficient admins set up before revoking deployer access
v2/broadcast/DeployERC20Custody.s.sol/137/dry-run/run-latest.json (1)
1-166
: Consider adding deployment verification steps.While the deployment configuration looks correct, consider adding post-deployment verification steps to ensure:
- Proxy implementation is correctly set
- Access control roles are properly configured
- Contract can be upgraded by the admin
Would you like me to provide a verification script template?
v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-1730737934.json (1)
159-162
: Consider adding transaction verificationThe
receipts
,libraries
,pending
, andreturns
sections are empty. While this is expected for a dry-run, consider adding post-deployment verification steps in your deployment script to ensure:
- All transactions are successful
- Events are emitted correctly
- State changes are applied as expected
v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-latest.json (1)
158-166
: Consider adding transaction receipts for deployment verification.The
receipts
array is empty. While this is expected for a dry run, consider:
- Adding a post-deployment verification step that checks the receipts
- Documenting the expected gas usage from the dry run
v2/broadcast/DeployERC20Custody.s.sol/1/run-1730753818.json (1)
54-56
: Consider optimizing gas usage for role assignment transactionsThe role assignment transactions could be batched to reduce the total gas cost. Consider implementing a batch assignment function in the contract to reduce the number of separate transactions.
Also applies to: 73-75, 92-94, 111-113, 130-132, 149-151
v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730751538.json (2)
44-157
: Review the permission setup sequenceThe deployment script follows a secure pattern for setting up permissions:
- Grants admin roles via
2f2ff15d
(grantRole)- Revokes unnecessary permissions via
36568abe
(revokeRole)Consider documenting the role hierarchy and access control design in the repository for better maintainability.
563-569
: LGTM: Deployment metadata is properly recordedThe deployment metadata includes:
- Chain ID: 8453 (Base)
- Timestamp: 1730751538
- Commit hash: 1e769a8
Consider adding a description field in the metadata to document the purpose of this deployment.
v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730745732.json (1)
563-569
: Consider documenting deployment metadataThe deployment metadata includes:
- Chain ID: 8453 (Base)
- Timestamp: 1730745732
- Commit: 1e769a8
Consider adding this information to deployment documentation for future reference.
v2/broadcast/DeployGatewayEVM.s.sol/1/run-latest.json (3)
3-21
: Deployment pattern follows best practicesThe deployment uses CREATE2 for deterministic addresses and implements the proxy upgrade pattern correctly. The GatewayEVM implementation is deployed first, followed by its proxy, which is a secure approach.
Consider documenting the CREATE2 salt and initialization parameters for future reference and verification.
Also applies to: 22-43
44-119
: Access control setup follows security best practicesThe deployment includes a proper sequence of access control setup:
- Initial role grants
- Role transfers
- Role revocations
Consider implementing a time delay or multi-sig for critical role transfers in future upgrades.
121-398
: Transaction execution and gas usage are optimizedAll transactions completed successfully with reasonable gas usage:
- Deployment transactions: ~312k and ~50k gas
- Access control transactions: ~7-13k gas each
The event logging provides good transparency for tracking the deployment process. Consider adding deployment verification scripts to validate the final state.
v2/broadcast/DeployGatewayEVM.s.sol/137/run-1730752013.json (1)
1-495
: Consider documenting deployment addressesFor better maintainability and future reference, consider:
- Adding comments explaining the role of each initialized address
- Documenting the admin address and its significance
- Including verification instructions for the deployed contracts
Would you like me to help generate a documentation template?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (99)
v2/data/addresses.mainnet.json
is excluded by!v2/data/**
v2/data/addresses.testnet.json
is excluded by!v2/data/**
v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md
is excluded by!v2/docs/**
v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md
is excluded by!v2/docs/**
v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md
is excluded by!v2/docs/**
v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md
is excluded by!v2/docs/**
v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md
is excluded by!v2/docs/**
v2/pkg/address.sol/address.go
is excluded by!v2/pkg/**
v2/pkg/beaconproxy.sol/beaconproxy.go
is excluded by!v2/pkg/**
v2/pkg/console.sol/console.go
is excluded by!v2/pkg/**
v2/pkg/core.sol/core.go
is excluded by!v2/pkg/**
v2/pkg/defender.sol/defender.go
is excluded by!v2/pkg/**
v2/pkg/defenderdeploy.sol/defenderdeploy.go
is excluded by!v2/pkg/**
v2/pkg/erc1967proxy.sol/erc1967proxy.go
is excluded by!v2/pkg/**
v2/pkg/erc1967utils.sol/erc1967utils.go
is excluded by!v2/pkg/**
v2/pkg/erc20custody.sol/erc20custody.go
is excluded by!v2/pkg/**
v2/pkg/erc20custody.t.sol/erc20custodytest.go
is excluded by!v2/pkg/**
v2/pkg/erc20custodyupgradetest.sol/erc20custodyupgradetest.go
is excluded by!v2/pkg/**
v2/pkg/gatewayevm.sol/gatewayevm.go
is excluded by!v2/pkg/**
v2/pkg/gatewayevm.t.sol/gatewayevminboundtest.go
is excluded by!v2/pkg/**
v2/pkg/gatewayevm.t.sol/gatewayevmtest.go
is excluded by!v2/pkg/**
v2/pkg/gatewayevmupgradetest.sol/gatewayevmupgradetest.go
is excluded by!v2/pkg/**
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/gatewayzevmupgradetest.sol/gatewayzevmupgradetest.go
is excluded by!v2/pkg/**
v2/pkg/math.sol/math.go
is excluded by!v2/pkg/**
v2/pkg/mockerc20.sol/mockerc20.go
is excluded by!v2/pkg/**
v2/pkg/mockerc721.sol/mockerc721.go
is excluded by!v2/pkg/**
v2/pkg/proxyadmin.sol/proxyadmin.go
is excluded by!v2/pkg/**
v2/pkg/receiverevm.sol/receiverevm.go
is excluded by!v2/pkg/**
v2/pkg/safeconsole.sol/safeconsole.go
is excluded by!v2/pkg/**
v2/pkg/safeerc20.sol/safeerc20.go
is excluded by!v2/pkg/**
v2/pkg/senderzevm.sol/senderzevm.go
is excluded by!v2/pkg/**
v2/pkg/signedmath.sol/signedmath.go
is excluded by!v2/pkg/**
v2/pkg/src/strings.sol/strings.go
is excluded by!v2/pkg/**
v2/pkg/stderror.sol/stderror.go
is excluded by!v2/pkg/**
v2/pkg/stdjson.sol/stdjson.go
is excluded by!v2/pkg/**
v2/pkg/stdmath.sol/stdmath.go
is excluded by!v2/pkg/**
v2/pkg/stdstorage.sol/stdstorage.go
is excluded by!v2/pkg/**
v2/pkg/stdstorage.sol/stdstoragesafe.go
is excluded by!v2/pkg/**
v2/pkg/stdstyle.sol/stdstyle.go
is excluded by!v2/pkg/**
v2/pkg/stdtoml.sol/stdtoml.go
is excluded by!v2/pkg/**
v2/pkg/storageslot.sol/storageslot.go
is excluded by!v2/pkg/**
v2/pkg/strings.sol/strings.go
is excluded by!v2/pkg/**
v2/pkg/systemcontract.sol/systemcontract.go
is excluded by!v2/pkg/**
v2/pkg/systemcontractmock.sol/systemcontractmock.go
is excluded by!v2/pkg/**
v2/pkg/testerc20.sol/testerc20.go
is excluded by!v2/pkg/**
v2/pkg/testuniversalcontract.sol/testuniversalcontract.go
is excluded by!v2/pkg/**
v2/pkg/transparentupgradeableproxy.sol/transparentupgradeableproxy.go
is excluded by!v2/pkg/**
v2/pkg/upgradeablebeacon.sol/upgradeablebeacon.go
is excluded by!v2/pkg/**
v2/pkg/upgrades.sol/unsafeupgrades.go
is excluded by!v2/pkg/**
v2/pkg/upgrades.sol/upgrades.go
is excluded by!v2/pkg/**
v2/pkg/utils.sol/utils.go
is excluded by!v2/pkg/**
v2/pkg/versions.sol/versions.go
is excluded by!v2/pkg/**
v2/pkg/wzeta.sol/weth9.go
is excluded by!v2/pkg/**
v2/pkg/zeta.non-eth.sol/zetanoneth.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornative.sol/zetaconnectornative.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornative.t.sol/zetaconnectornativetest.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornativeupgradetest.sol/zetaconnectornativeupgradetest.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornonnative.sol/zetaconnectornonnative.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornonnative.t.sol/zetaconnectornonnativetest.go
is excluded by!v2/pkg/**
v2/pkg/zetaconnectornonnativeupgradetest.sol/zetaconnectornonnativeupgradetest.go
is excluded by!v2/pkg/**
v2/pkg/zrc20.sol/zrc20.go
is excluded by!v2/pkg/**
v2/pkg/zrc20.t.sol/zrc20test.go
is excluded by!v2/pkg/**
v2/types/factories/Address__factory.ts
is excluded by!v2/types/**
v2/types/factories/BeaconProxy__factory.ts
is excluded by!v2/types/**
v2/types/factories/ERC1967Proxy__factory.ts
is excluded by!v2/types/**
v2/types/factories/ERC1967Utils__factory.ts
is excluded by!v2/types/**
v2/types/factories/ERC20CustodyUpgradeTest__factory.ts
is excluded by!v2/types/**
v2/types/factories/ERC20Custody__factory.ts
is excluded by!v2/types/**
v2/types/factories/GatewayEVMUpgradeTest__factory.ts
is excluded by!v2/types/**
v2/types/factories/GatewayEVM__factory.ts
is excluded by!v2/types/**
v2/types/factories/GatewayZEVMUpgradeTest__factory.ts
is excluded by!v2/types/**
v2/types/factories/GatewayZEVM__factory.ts
is excluded by!v2/types/**
v2/types/factories/Math__factory.ts
is excluded by!v2/types/**
v2/types/factories/MockERC20__factory.ts
is excluded by!v2/types/**
v2/types/factories/MockERC721__factory.ts
is excluded by!v2/types/**
v2/types/factories/ProxyAdmin__factory.ts
is excluded by!v2/types/**
v2/types/factories/ReceiverEVM__factory.ts
is excluded by!v2/types/**
v2/types/factories/SafeERC20__factory.ts
is excluded by!v2/types/**
v2/types/factories/SenderZEVM__factory.ts
is excluded by!v2/types/**
v2/types/factories/StdError.sol/StdError__factory.ts
is excluded by!v2/types/**
v2/types/factories/StdStorage.sol/StdStorageSafe__factory.ts
is excluded by!v2/types/**
v2/types/factories/Strings__factory.ts
is excluded by!v2/types/**
v2/types/factories/SystemContract.sol/SystemContract__factory.ts
is excluded by!v2/types/**
v2/types/factories/SystemContractMock.sol/SystemContractMock__factory.ts
is excluded by!v2/types/**
v2/types/factories/TestERC20__factory.ts
is excluded by!v2/types/**
v2/types/factories/TestUniversalContract__factory.ts
is excluded by!v2/types/**
v2/types/factories/TransparentUpgradeableProxy.sol/TransparentUpgradeableProxy__factory.ts
is excluded by!v2/types/**
v2/types/factories/UpgradeableBeacon__factory.ts
is excluded by!v2/types/**
v2/types/factories/WZETA.sol/WETH9__factory.ts
is excluded by!v2/types/**
v2/types/factories/ZRC20.sol/ZRC20__factory.ts
is excluded by!v2/types/**
v2/types/factories/Zeta.non-eth.sol/ZetaNonEth__factory.ts
is excluded by!v2/types/**
v2/types/factories/ZetaConnectorNativeUpgradeTest__factory.ts
is excluded by!v2/types/**
v2/types/factories/ZetaConnectorNative__factory.ts
is excluded by!v2/types/**
v2/types/factories/ZetaConnectorNonNativeUpgradeTest__factory.ts
is excluded by!v2/types/**
v2/types/factories/ZetaConnectorNonNative__factory.ts
is excluded by!v2/types/**
📒 Files selected for processing (24)
v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-1730753291.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/1/run-1730753818.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/1/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/137/dry-run/run-1730751736.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/137/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/137/run-1730752202.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/137/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-1730752569.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/56/run-1730753057.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/56/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-1730737934.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730745732.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730751538.json
(1 hunks)v2/broadcast/DeployERC20Custody.s.sol/8453/run-latest.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/1/dry-run/run-1730753279.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/1/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/1/run-1730753500.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/1/run-latest.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/137/dry-run/run-1730751719.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/137/dry-run/run-latest.json
(1 hunks)v2/broadcast/DeployGatewayEVM.s.sol/137/run-1730752013.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- v2/broadcast/DeployGatewayEVM.s.sol/137/dry-run/run-latest.json
🔇 Additional comments (87)
v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-1730753291.json (3)
4-21
: LGTM: Deterministic deployment using CREATE2
The deployment of ERC20Custody uses CREATE2 which ensures the contract address will be the same across different networks when deployed with the same parameters. This is a good practice for cross-chain applications.
22-43
: LGTM: Proxy pattern implementation
The ERC1967Proxy is correctly deployed with the ERC20Custody implementation address and initialization data. The proxy pattern allows for future upgrades while maintaining the same address.
44-157
: Verify role configuration sequence
The transaction sequence shows a systematic approach to role management:
- Grant admin roles (transactions 3-5)
- Revoke admin roles (transactions 6-8)
Ensure this role configuration aligns with the intended access control structure.
v2/broadcast/DeployERC20Custody.s.sol/1/dry-run/run-latest.json (4)
4-18
: Verify the contract deployment parameters and addresses
The deployment transactions use CREATE2 for deterministic addresses, which is good for address consistency across networks. However, please verify:
- The implementation address (0x89224928198F08F015C17942A04E2F766AFBe868)
- The proxy admin address (0x48b9aacc350b20147001f88821d31731ba4c30ed)
- The system admin address (0x70e967acfcc17c3941e87562161406d41676fd83)
Also applies to: 23-40
44-81
: Verify the access control setup for admin roles
The first two CALL transactions grant roles using grantRole
(function selector 0x2f2ff15d) to address 0xaeb6ddb7708467814d557e340283248be8e43124 for two different roles:
- 0x65d7a28e3265b37a6474929f336521b332c1681b933f6cb9f3376673440d862a
- 0x8619cecd8b9e095ab43867f5b69d492180450fe862e6b50bfbfb24b75dd84c8a
Ensure these are the correct role identifiers for the intended administrative permissions.
101-157
: Verify the role renunciation sequence
The last three transactions use renounceRole
(function selector 0x36568abe) to remove roles from the deployer address (0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38). This is good practice for decentralization, but verify:
- The roles being renounced match those granted earlier
- The sequence ensures no administrative gaps
82-100
:
Verify the default admin role assignment
The transaction grants the DEFAULT_ADMIN_ROLE (0x0000000000000000000000000000000000000000000000000000000000000000) to address 0xaeb6ddb7708467814d557e340283248be8e43124. This is a critical security setting.
v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-1730752569.json (5)
4-18
: Verify the deterministic deployment addresses
The ERC20Custody contract is being deployed using CREATE2 opcode at a deterministic address. Ensure this address matches across all environments (testnet/mainnet) for consistency.
23-40
: Validate the proxy initialization parameters
The ERC1967Proxy is initialized with specific parameters:
- Implementation address:
0x89224928198F08F015C17942A04E2F766AFBe868
- Admin addresses in the initialization data
Please verify:
- The implementation address matches the deployed ERC20Custody contract
- The admin addresses are correct for the production environment
44-81
: Review role assignments for access control
The first two proxy calls are granting roles using 2f2ff15d
(grantRole) to address 0xaf28a257d292e7f0e531073f70a175b57e0261a8
. Ensure this address is the intended admin for:
- Role:
0x65d7a28e3265b37a6474929f336521b332c1681b933f6cb9f3376673440d862a
- Role:
0x8619cecd8b9e095ab43867f5b69d492180450fe862e6b50bfbfb24b75dd84c8a
163-165
: Verify deployment chain and configuration
Please confirm:
- Chain ID 56 is the intended deployment network (BSC Mainnet)
- The commit hash
1e769a8
matches the audited contract code
109-135
:
Validate self-granted permissions
The deployer address 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
is granting itself roles using 36568abe
(revokeRole). This is a critical security configuration - ensure these permissions are intentional and necessary for contract management.
v2/broadcast/DeployERC20Custody.s.sol/56/dry-run/run-latest.json (3)
1-166
: LGTM! The deployment sequence follows best practices.
The deployment sequence is well-structured:
- Deploys implementation contract using CREATE2 for address determinism
- Deploys proxy pointing to implementation
- Sets up proper access control
28-31
: Verify the initialization parameters.
The proxy is initialized with three addresses:
- 0x48b9aacc350b20147001f88821d31731ba4c30ed
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 (deployer)
Ensure these addresses are correct for the BSC mainnet deployment.
44-100
: Verify the role assignments.
Three different roles are being granted to address 0xaf28a257d292e7f0e531073f70a175b57e0261a8:
- 0x65d7a28e3265b37a6474929f336521b332c1681b933f6cb9f3376673440d862a
- 0x8619cecd8b9e095ab43867f5b69d492180450fe862e6b50bfbfb24b75dd84c8a
- 0x0000000000000000000000000000000000000000000000000000000000000000
Ensure this address is the intended admin and these are the correct role identifiers.
v2/broadcast/DeployERC20Custody.s.sol/137/dry-run/run-1730751736.json (4)
3-21
: LGTM: Secure deployment using CREATE2
The deployment of ERC20Custody uses CREATE2 with appropriate parameters, ensuring a deterministic contract address across different networks.
22-43
: LGTM: Proxy setup follows best practices
The ERC1967Proxy deployment correctly:
- Uses the deployed ERC20Custody as implementation
- Includes initialization data in the constructor
- Uses CREATE2 for deterministic addressing
44-157
: LGTM: Role management follows the principle of least privilege
The permission setup sequence follows security best practices:
- Grants admin roles to the system account
- Revokes deployer's temporary permissions
1-166
: Verify the deployment configuration
Please ensure:
- The chainId (137) matches the intended Polygon mainnet deployment
- The deployer account (0x1804...1f38) has sufficient MATIC for all transactions
- The gas limits are sufficient during actual deployment
v2/broadcast/DeployERC20Custody.s.sol/137/dry-run/run-latest.json (3)
1-43
: LGTM: Contract deployment configuration is secure and follows best practices.
The deployment configuration:
- Uses CREATE2 for deterministic addresses
- Properly initializes the proxy with implementation
- Sets reasonable gas limits for deployment operations
44-157
: Verify the access control setup sequence.
The access control setup follows a secure pattern:
- Grants admin and operator roles to 0xf43c...108e
- Revokes admin, operator and default admin roles from deployer
- Uses separate transactions for each role operation
However, please verify:
- The intended role hierarchy
- That 0xf43c...108e is the correct admin address
158-166
: LGTM: Deployment metadata is properly configured.
The deployment is correctly configured for:
- Chain ID: 137 (Polygon Mainnet)
- Timestamp and commit hash are recorded
- Empty receipts array indicates this is a dry run
v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-1730737934.json (4)
4-18
: Verify the deterministic deployment addresses
The CREATE2 deployments use deterministic addresses for both contracts:
- ERC20Custody: 0x61ed1f2d85f95182740fa1f6618d4542fedb7468
- ERC1967Proxy: 0xd0d3bf442c247d85878f875d94bd15343376121a
Ensure these addresses match the expected addresses in your deployment configuration and that they haven't been previously deployed to on Base (chain 8453).
Also applies to: 23-40
28-31
: Verify the proxy initialization parameters
The ERC1967Proxy is initialized with:
- Implementation: 0x61Ed1f2D85f95182740fa1f6618D4542FEdb7468 (ERC20Custody)
- Setup data calling
initialize()
with parameters:- admin: 0x9f21b726fcb84d8e92cdc678772590dce5347d0b
- operator: 0x70997970c51812dc3a010c7d01b50e0d17dc79c8
- deployer: 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
Ensure these addresses are correct for your deployment environment.
44-157
: Review the proxy configuration sequence
The deployment includes a sequence of 6 access control setup calls:
- Grant role (0x65d7a2...) to 0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266
- Grant role (0x8619ce...) to 0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266
- Grant role (0x000000...) to 0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266
- Revoke role (0x65d7a2...) from 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
- Revoke role (0x8619ce...) from 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
- Revoke role (0x000000...) from 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
This appears to be transferring admin roles from the deployer to a new admin address.
163-165
: Verify chain configuration
Deployment is targeted at:
- Chain ID: 8453 (Base)
- Timestamp: 1730737934 (Fri Mar 15 2024 12:45:34 GMT+0000)
Ensure this matches your intended deployment environment.
v2/broadcast/DeployERC20Custody.s.sol/8453/dry-run/run-latest.json (3)
3-43
: Contract deployment configuration looks correct.
The deployment transactions are properly configured:
- ERC20Custody implementation is deployed first
- ERC1967Proxy is deployed with correct initialization parameters referencing the implementation
44-157
: Verify the access control configuration sequence.
The sequence of access control setup calls appears to follow a secure pattern:
- Grant roles to admin account
- Revoke roles from deployer
- Transfer admin rights
However, please verify:
- The role hashes match your intended access control structure
- The target addresses are correct for your environment
1-166
: Verify chain configuration for mainnet deployment.
The deployment is configured for Base (chain ID: 8453). Please verify:
- This is the intended target network for mainnet deployment
- The gas parameters are appropriate for Base network conditions
v2/broadcast/DeployGatewayEVM.s.sol/1/dry-run/run-1730753279.json (4)
3-21
: GatewayEVM deployment configuration looks good.
The deployment uses CREATE2 for deterministic address generation with appropriate gas limits and proper transaction structure.
22-43
: Verify proxy initialization parameters.
The ERC1967Proxy is correctly configured to point to the GatewayEVM implementation, but please verify the initialization parameters passed to the proxy:
- Implementation: 0x8EFaEd7f0b47f0B3CD412D83877918A6626D93CC
- Admin addresses in initialization data:
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xf091867ec603a6628ed83d274e835539d82e9cc8
- 0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
44-119
: Role management sequence is properly structured.
The sequence of transactions correctly implements the role transfer pattern:
- Grants specific roles to the new admin
- Revokes the same roles from the deployer
This ensures clean handover of administrative privileges.
120-128
: Verify mainnet deployment configuration.
This is a mainnet deployment (chain: 1). Please ensure:
- All contract addresses are correct for mainnet
- Gas prices are appropriate for mainnet conditions
- Proper testing has been done on testnet
v2/broadcast/DeployGatewayEVM.s.sol/1/dry-run/run-latest.json (3)
1-43
: Verify the proxy initialization parameters.
The deployment configuration looks correct with:
- GatewayEVM implementation deployed first using CREATE2
- ERC1967Proxy deployed with correct implementation address
- Appropriate initialization data provided to the proxy
Please verify:
- The proxy initialization parameters in line 30 match the expected configuration
- The gas limits are sufficient for mainnet deployment
44-119
: LGTM: Access control setup follows security best practices.
The sequence of role management calls follows a secure pattern:
- Grant required roles first (lines 44-81)
- Then revoke unnecessary permissions (lines 82-119)
This ensures a clean and minimal permission set after deployment.
120-128
: Verify mainnet deployment readiness.
This appears to be a dry-run deployment targeting Ethereum mainnet (chain ID: 1). Before proceeding with the actual deployment:
- Ensure all contract parameters are correctly set for production
- Verify gas costs on mainnet
- Consider running the deployment on a testnet first
v2/broadcast/DeployGatewayEVM.s.sol/137/dry-run/run-1730751719.json (4)
3-21
: LGTM: GatewayEVM deployment configuration is appropriate.
The deployment uses CREATE2 for deterministic addressing and sets a reasonable gas limit for a complex contract deployment.
22-43
: Verify proxy initialization parameters.
The proxy is correctly configured to point to the GatewayEVM implementation at 0x8EFaEd7f0b47f0B3CD412D83877918A6626D93CC
. Please verify:
- The initialization parameters in the constructor arguments:
- Address 1:
0x70e967acfcc17c3941e87562161406d41676fd83
- Address 2:
0xf091867ec603a6628ed83d274e835539d82e9cc8
- Address 3:
0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38
- Address 1:
44-119
: LGTM: Secure role transfer sequence.
The proxy setup follows a secure pattern for role transfer:
- Grants admin roles to
0xf43cf8b3f3d22d4cc33f964c59076eab2f8a108e
- Revokes admin roles from the deployer
This ensures a clean transfer of control without any period where multiple addresses have admin access.
120-128
: Verify mainnet deployment parameters.
This is a dry run for Polygon mainnet (chain ID: 137). Before proceeding with the actual deployment:
- Ensure all contract addresses and parameters are correct for production
- Verify the gas estimates are within acceptable limits
- Confirm the deployer account has sufficient MATIC for all transactions
v2/broadcast/DeployERC20Custody.s.sol/56/run-1730753057.json (5)
4-18
: Verify the contract deployment parameters for ERC20Custody
The ERC20Custody contract is deployed using CREATE2 with:
- Deployment address: 0x89224928198f08f015c17942a04e2f766afbe868
- Gas limit: 0x33270c (3,354,380 gas)
- Chain ID: 0x38 (BSC mainnet)
The gas limit seems reasonable for a complex contract deployment.
23-40
: Verify the proxy initialization parameters
The ERC1967Proxy is initialized with:
- Implementation: 0x89224928198F08F015C17942A04E2F766AFBe868 (ERC20Custody)
- Constructor args contain addresses for system setup:
- 0x48b9aacc350b20147001f88821d31731ba4c30ed
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xc81656b7bc994958c089bd1aa2d3be524718055f
Ensure these addresses are correct for the mainnet deployment.
44-157
: Review the initialization sequence
The deployment includes a series of 6 CALL transactions that:
- Grant roles using
2f2ff15d
(lines 44-62) - Set up permissions using
36568abe
(lines 102-157)
The sequence follows a secure pattern:
- First granting roles
- Then setting up specific permissions
- Using fixed gas limits for predictable execution
This is a good practice for initializing access control.
159-513
: Verify transaction receipts and events
All transactions were successful (status: "0x1") and emitted appropriate events:
- Contract creation events (bc7cd75a)
- Role assignment events (2f8788117e7eff1d82e926ec794901d17c78024a)
- Permission updates (f6391f5c32d9c69d2a47ea670b442974b53935d1)
The gas usage is within expected ranges:
- ERC20Custody deployment: 0x22fc71 (2,293,873 gas)
- Proxy deployment: 0x5c79e (378,782 gas)
- Role/permission calls: ~0x7365-0xdc18 (29,541-56,344 gas)
514-521
: Verify deployment metadata
The deployment was executed on:
- Chain ID: 56 (BSC mainnet)
- Timestamp: 1730753057
- Commit: 1e769a8
Ensure this matches the expected deployment environment and code version.
v2/broadcast/DeployERC20Custody.s.sol/56/run-latest.json (5)
3-21
: LGTM: ERC20Custody deployment configuration looks correct
The deployment of the ERC20Custody implementation contract using CREATE2 is properly configured with:
- Correct deployment address: 0x89224928198f08f015c17942a04e2f766afbe868
- Appropriate gas limits
- Zero value transfer as expected
22-43
: LGTM: ERC1967Proxy deployment and initialization
The proxy deployment is correctly configured with:
- Implementation pointing to ERC20Custody: 0x89224928198f08f015c17942a04e2f766afbe868
- Initialization data for setup
- Appropriate gas limits
44-157
: Verify the permission setup sequence
The transaction sequence sets up the following permissions:
- Grants admin roles to 0xaf28a257d292e7f0e531073f70a175b57e0261a8
- Revokes initial permissions from deployer (0xc81656b7bc994958c089bd1aa2d3be524718055f)
This follows the principle of least privilege by removing deployer access after setup.
159-513
: LGTM: All transactions were successful
Transaction receipts show:
- All transactions completed successfully (status: 0x1)
- Expected events were emitted
- Gas usage is within reasonable limits
515-521
: Verify deployment environment
The deployment was executed on:
- BSC Mainnet (chain ID: 56)
- Commit: 1e769a8
- Timestamp: 1730753057
Please ensure this matches the intended deployment environment.
v2/broadcast/DeployERC20Custody.s.sol/1/run-1730753818.json (3)
1-43
: LGTM: Contract deployments are properly configured
The deployment sequence follows best practices:
- ERC20Custody implementation is deployed first
- ERC1967Proxy is deployed with correct initialization parameters pointing to the implementation
44-157
: Verify the role assignments for security
The deployment script assigns multiple administrative roles. Please verify:
- The necessity of all role assignments
- The principle of least privilege is followed
- The addresses receiving admin roles are correct and trusted
159-516
: LGTM: All transactions executed successfully
Transaction receipts show:
- All transactions completed with status 0x1 (success)
- Proper event logs emitted for role assignments
- Reasonable gas consumption
v2/broadcast/DeployERC20Custody.s.sol/1/run-latest.json (5)
4-21
: Verify the ERC20Custody deployment configuration
The ERC20Custody contract is deployed using CREATE2 with a significant gas allocation (0x33270c = 3,354,380 gas). Consider:
- The high gas limit suggests complex initialization logic
- The deployment is deterministic via CREATE2, ensuring address consistency across chains
44-138
: Verify access control setup sequence
The deployment includes a sequence of access control setup calls:
- Grant roles to 0xaeb6ddb7708467814d557e340283248be8e43124
- Configure admin permissions
- Setup role hierarchy
This follows a secure initialization pattern, but verify that:
- The role assignments match the intended access control design
- The admin account has sufficient permissions for future management
159-516
: Validate deployment event logs
All transactions were successful (status: "0x1"). Key events to verify:
- Implementation contract upgrade events
- Role assignment events
- Proxy initialization events
The logs confirm proper contract initialization and role assignment.
518-524
: LGTM: Deployment metadata is complete
The deployment metadata includes:
- No external library dependencies
- Mainnet deployment (chain: 1)
- Commit hash for version tracking
23-43
:
Review proxy initialization parameters
The ERC1967Proxy is initialized with:
- Implementation: 0x89224928198F08F015C17942A04E2F766AFBe868
- Admin addresses in the initialization data
Ensure these addresses have been properly verified and match the expected configuration for the mainnet deployment.
v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730751538.json (2)
1-43
: Verify the initialization parameters of the proxy contract
The ERC1967Proxy is initialized with specific addresses that should be carefully verified:
- Implementation: 0x89224928198f08f015c17942a04e2f766afbe868
- Constructor args contain addresses:
- 0x48b9aacc350b20147001f88821d31731ba4c30ed
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xc81656b7bc994958c089bd1aa2d3be524718055f
Please confirm these addresses are correct for your deployment environment and have been properly audited.
158-562
: Verify successful execution of all transactions
All transactions were executed successfully with status "0x1". Key observations:
- Gas usage is reasonable for each operation
- Events are properly emitted for role changes
- L1 fees are properly calculated and paid
Consider adding these deployment logs to version control to maintain a history of mainnet deployments.
v2/broadcast/DeployERC20Custody.s.sol/8453/run-latest.json (5)
3-21
: Verify the ERC20Custody contract deployment parameters
The contract is deployed using CREATE2 for address determinism. Key points to verify:
- Deployment address:
0x89224928198f08f015c17942a04e2f766afbe868
- Chain ID: 8453 (Base)
- Gas limit: ~3.3M gas (0x33270c)
22-43
: Verify the ERC1967Proxy initialization parameters
The proxy is initialized with:
- Implementation:
0x89224928198F08F015C17942A04E2F766AFBe868
(ERC20Custody) - Constructor args contain addresses:
0x48b9aacc350b20147001f88821d31731ba4c30ed
0x70e967acfcc17c3941e87562161406d41676fd83
0xc81656b7bc994958c089bd1aa2d3be524718055f
Please verify these addresses match the intended admin and operator roles.
44-157
: Review permission setup sequence
The deployment includes a proper sequence of permission setup:
- Grant roles using
2f2ff15d
(lines 44-100) - Revoke roles using
36568abe
(lines 101-157)
This follows security best practices by:
- Setting up initial permissions immediately after deployment
- Using a clear and auditable sequence of role assignments
159-561
: Verify successful transaction execution
All transactions were successful (status: "0x1") and emitted appropriate events:
- Contract creation events for deployments
- Role assignment events for permission setup
- No unexpected events or errors
The gas usage and fees appear reasonable for the operations performed.
562-569
: LGTM: Deployment metadata is complete
The deployment metadata includes all necessary information:
- Timestamp
- Chain ID
- Commit hash
- No pending transactions
v2/broadcast/DeployERC20Custody.s.sol/8453/run-1730745732.json (4)
3-21
: Verify the ERC20Custody deployment parameters
The ERC20Custody contract is deployed using CREATE2 with a specific implementation address. Ensure that:
- The implementation contract at
0x61ed1f2d85f95182740fa1f6618d4542fedb7468
has been audited - The gas limit of
0x33270c
(~3.3M) is appropriate for the deployment
22-43
: Review proxy initialization parameters
The ERC1967Proxy is initialized with specific parameters:
- Implementation:
0x61Ed1f2D85f95182740fa1f6618D4542FEdb7468
- Admin addresses in initialization data:
0x77e707c7c3ebea44ba632efd8d04885358e3d392
0x70e967acfcc17c3941e87562161406d41676fd83
0xc81656b7bc994958c089bd1aa2d3be524718055f
Verify these addresses have the correct permissions for their intended roles.
44-157
: Analyze permission setup sequence
The deployment includes a sequence of permission grants using grantRole
(0x2f2ff15d) and revokeRole
(0x36568abe) calls. This follows a secure pattern of:
- Granting admin roles
- Setting up operational permissions
- Revoking temporary deployment permissions
This is a good security practice for initializing admin roles.
159-562
: Verify transaction success and event emissions
All transactions were successful (status: "0x1") and emitted appropriate events:
- Implementation contract deployment
- Proxy deployment with
Upgraded
event - Role assignments with
RoleGranted
events - Role revocations with
RoleRevoked
events
The event logs confirm the correct execution sequence.
v2/broadcast/DeployGatewayEVM.s.sol/1/run-1730753500.json (4)
3-151
: GatewayEVM deployment looks good!
The contract is deployed using CREATE2 for address determinism, with proper initialization events and reasonable gas usage. The deployment transaction completed successfully with status 0x1.
152-263
: Verify admin address configuration
The ERC1967Proxy deployment is successful and properly configured with the GatewayEVM implementation. Multiple initialization events confirm the setup of admin roles.
Please verify that these admin addresses are correct for the production environment:
- Implementation: 0x8EFaEd7f0b47f0B3CD412D83877918A6626D93CC
- Admin addresses in constructor args:
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xf091867ec603a6628ed83d274e835539d82e9cc8
- 0xc81656b7bc994958c089bd1aa2d3be524718055f
264-397
: Role configuration sequence is correct
The four configuration transactions properly set up the necessary roles and permissions:
- Grant role 0x65d7a...2a to 0xaeb6...3124
- Grant role 0x0000...00 to 0xaeb6...3124
- Revoke role 0x65d7a...2a from deployer
- Revoke role 0x0000...00 from deployer
All transactions completed successfully with appropriate events emitted, providing a clear audit trail.
398-405
: Deployment metadata is complete and well-structured
The deployment log contains all necessary information:
- Chain ID: 1 (mainnet)
- Timestamp: 1730753500
- Commit: 1e769a8
- No pending transactions or external libraries
v2/broadcast/DeployGatewayEVM.s.sol/1/run-latest.json (1)
28-31
: Verify initialization parameters carefully
The proxy is initialized with multiple system addresses:
- Implementation: 0x8EFaEd7f0b47f0B3CD412D83877918A6626D93CC
- Additional parameters encoded in the constructor data
Ensure these addresses are correct for the mainnet deployment and properly verified.
v2/broadcast/DeployGatewayEVM.s.sol/137/run-1730752013.json (4)
3-21
: Verify the GatewayEVM implementation contract deployment parameters
The GatewayEVM contract is deployed using CREATE2 with:
- Deployer: 0xc81656b7bc994958c089bd1aa2d3be524718055f
- Factory: 0x4e59b44847b379578588920ca78fbf26c0b4956c
- Implementation address: 0x8efaed7f0b47f0b3cd412d83877918a6626d93cc
Ensure this matches your expected deployment configuration.
22-43
: Verify the ERC1967Proxy initialization parameters
The proxy is initialized with:
- Implementation: 0x8EFaEd7f0b47f0B3CD412D83877918A6626D93CC (GatewayEVM)
- Constructor args containing three addresses:
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xf091867ec603a6628ed83d274e835539d82e9cc8
- 0xc81656b7bc994958c089bd1aa2d3be524718055f
Ensure these addresses are correct for your deployment environment.
44-119
: Review the sequence of administrative setup calls
The deployment includes 4 administrative calls:
- grantRole(ADMIN_ROLE, 0x7828f92e7d79e141189f24c98acef71bc07bad3f)
- grantRole(DEFAULT_ADMIN_ROLE, 0x7828f92e7d79e141189f24c98acef71bc07bad3f)
- revokeRole(ADMIN_ROLE, deployer)
- revokeRole(DEFAULT_ADMIN_ROLE, deployer)
This follows the proper pattern of:
- Setting up the new admin
- Revoking deployer permissions
121-487
: Verify successful transaction execution
All transactions were successful (status: "0x1") and generated expected events:
- Implementation deployment emitted initialization events
- Proxy deployment emitted UpgradeToAndCall events
- Admin setup emitted RoleGranted and RoleRevoked events
The gas usage appears reasonable for these operations.
v2/broadcast/DeployERC20Custody.s.sol/137/run-1730752202.json (4)
3-21
: Verify the ERC20Custody deployment configuration.
The ERC20Custody contract is deployed using CREATE2 with a deterministic address (0x89224928198f08f015c17942a04e2f766afbe868). Ensure this matches the expected address in your deployment plan.
22-43
: Verify the ERC1967Proxy initialization parameters.
The proxy is initialized with:
- Implementation: 0x89224928198F08F015C17942A04E2F766AFBe868 (ERC20Custody)
- Constructor args containing admin addresses:
- 0x48b9aacc350b20147001f88821d31731ba4c30ed
- 0x70e967acfcc17c3941e87562161406d41676fd83
- 0xc81656b7bc994958c089bd1aa2d3be524718055f
Ensure these addresses match your intended admin configuration.
44-157
: Review the role assignment transactions.
The deployment script assigns roles using grantRole and revokeRole calls:
- Grants roles to 0x7828f92e7d79e141189f24c98acef71bc07bad3f
- Revokes roles from deployer (0xc81656b7bc994958c089bd1aa2d3be524718055f)
This follows the principle of least privilege by removing deployer access after setup.
644-649
: Verify deployment environment configuration.
The deployment was executed on:
- Chain ID: 137 (Polygon Mainnet)
- Timestamp: 1730752202
- Commit: 1e769a8
Ensure these match your intended deployment environment.
Consider documenting the commit hash and deployment parameters in your deployment documentation for future reference.
v2/broadcast/DeployERC20Custody.s.sol/137/run-latest.json (5)
3-21
: Verify the ERC20Custody deployment parameters
The ERC20Custody contract is deployed using CREATE2 with a deterministic address (0x89224928198f08f015c17942a04e2f766afbe868). Ensure this matches the expected address in your deployment plan.
22-43
: Verify the ERC1967Proxy initialization parameters
The proxy is initialized with:
- Implementation: 0x89224928198f08f015c17942a04e2f766afbe868 (ERC20Custody)
- Admin addresses in constructor args
Please verify these addresses match your deployment configuration:
- Implementation address points to the correct ERC20Custody version
- Admin addresses are correctly set for your production environment
44-157
: Review the access control setup sequence
The deployment script executes a series of access control setup calls:
- Grants roles to 0x7828f92e7d79e141189f24c98acef71bc07bad3f
- Transfers admin roles using 36568abe function selector
- Sets up a proper role hierarchy
This follows security best practices for role-based access control initialization.
159-642
: Verify transaction receipts and gas optimization
All transactions were successful with reasonable gas usage:
- ERC20Custody deployment: ~143k gas
- Proxy deployment: ~37k gas
- Role management calls: ~8-13k gas each
The gas usage patterns are within expected ranges for these operations.
647-649
: Confirm deployment environment
The deployment was executed on:
- Chain ID: 137 (Polygon Mainnet)
- Timestamp: 1730752202
- Commit: 1e769a8
Please verify these match your intended production deployment parameters.
Consider documenting these deployment parameters in your project documentation for future reference and auditing purposes.
Initialize the
broadcast
dir in the repo to contains information about main deployed contractsSummary by CodeRabbit
GatewayEVM
andERC1967Proxy
smart contracts on the Ethereum blockchain.