Local network: pass mgmt contract to l2 contract deploy script #1664
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe changes across the two files indicate an update to the management contract address used within a testnet launcher and a contract deployer tool. A new parameter for specifying the management contract address has been introduced to the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- testnet/launcher/docker.go (1 hunks)
- testnet/launcher/l2contractdeployer/cmd/main.go (1 hunks)
Additional comments: 1
testnet/launcher/l2contractdeployer/cmd/main.go (1)
- 16-22:
The addition of the management contract address to the L2 contract deployment configuration is correctly implemented.
Comment on lines
124
to
130
| l2cd.WithL2Host("sequencer-host"), | ||
| l2cd.WithL2WSPort(81), | ||
| l2cd.WithL1PrivateKey("f52e5418e349dccdda29b6ac8b0abe6576bb7713886aa85abea6181ba731f9bb"), | ||
| l2cd.WithManagementContractAddress("0xeDa66Cc53bd2f26896f6Ba6b736B1Ca325DE04eF"), | ||
| l2cd.WithMessageBusContractAddress("0xFD03804faCA2538F4633B3EBdfEfc38adafa259B"), | ||
| l2cd.WithL2PrivateKey("8dfb8083da6275ae3e4f41e3e8a8c19d028d32c9247e24530933782f2a05035b"), | ||
| l2cd.WithHocPKString("6e384a07a01263518a09a5424c7b6bbfc3604ba7d93f47e3a455cbdd7f9f0682"), |
There was a problem hiding this comment.
Consider using a configuration file or environment variable for the management contract address to avoid hardcoding it and to facilitate easier updates or configurations for different environments.
moraygrieve
approved these changes
Nov 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this change is needed
Didn't realise the L2 contract deployer was called from this other place, hadn't added the new mgmt contract address arg.
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks