-
Notifications
You must be signed in to change notification settings - Fork 2
Remove broadcast directory #8
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
base: main
Are you sure you want to change the base?
Conversation
squadgazzz
left a comment
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.
Makes sense. I think the .gitignore file needs to be updated
anxolin
left a comment
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.
Great! works with me. Can you make sure we use this as a reference in the Template (we could link this PR)
| | select(.transactionType == "CREATE2") | ||
| | select(.hash != null) | ||
| | {($chainId): {address: .contractAddress, transactionHash: .hash }} | ||
| ' <"./broadcast/DeployableVM.s.sol/${chain_id}/run-latest.json" |
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.
Given its complexity, do you think is worth it to move to a script?
| After successfully deploying the contracts, a deployment file is automatically generated in the [./broadcast/DeployableVM.s.sol](broadcast/DeployableVM.s.sol) directory under the relevant chain subdirectory. Make sure to commit this file to the repository. | ||
|
|
||
| ### 8. Deployment addresses | ||
| ### 7. Deployment addresses |
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.
All sections use the imperative form to command the reader to do something
I would suggest to use a verb here. i.e. Update deployment addresses
The files in
/broadcastcontain mostly information that can be recovered from on-chain data.Sometimes there's no easy way to programmatically get the information on the file, for example if the deployment script hasn't been used to deploy the contract on a specific chain. This means either that the broadcast file is absent (and so
networks.jsoncannot be automatically generated) or that it should be manually created (which takes time and so there is the risk of introducing errors that are hard to verify).As far as I know, there's no concrete use for the broadcast files except for getting the transaction hash and the addresses.
For all these reasons, this PR drops these (and related) files from this repository.
Instead, we just keep the
networks.jsonfile: the address is handy to make sure a specific deployment is official, while the transaction hash allows to recover most of the information from the broadcast file.How to test
Check out that the readme is updated with the new deployment documentation process.