Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Not sure about this change since this will change the deployed bytecode, thus the deployed address by default. |
|
It would probably make more sense to add a mandatory step in the docs with checking the deployed BalancerV2Vault address and source code. |
fedgiac
left a comment
There was a problem hiding this comment.
Thanks, good idea now that we know that the vault could be compromised.
Let's merge this also to the new-chain-deployments branch.
Not sure about this change since this will change the deployed bytecode, thus the deployed address by default.
No worries, changing bytecode mainly applies to .sol files that aren't tests or scripts.
It would probably make more sense to add a mandatory step in the docs with checking the deployed BalancerV2Vault address and source code.
Yep, can you add a mention to the readme?
I already updated the internal guide for deploying on new chains (here if you want more context).
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
d082d54 to
d2cb037
Compare
kaze-cow
left a comment
There was a problem hiding this comment.
I would suggest we have the ability to somehow override the balancer address through a command line argument or similar (ex. set to address 0). Just in case balancer is not deploying to a network and we still want to get the contract deployed. Based on how we have the change set up right now, it appears it will throw and not allow the user to continue if there is no balancer deployment.
Description
The key that the balancer vault used to be deployed from is compromised, so we can be sure that the contract deployed under this address is legit or if the contract is not deployed at all we can't be sure an attacker won't deploy to that address later. This PR removes the default address and asks the user to get a balancer address from the balancer team.