-
Notifications
You must be signed in to change notification settings - Fork 75
fix: zkSync SpokePool deployment script #340
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
Conversation
This fix uses zkSync-specific packages, which are necessary for contract deployment on zkSync. The zkEthBridge configurable is also removed, since it is no longer needed for SpokePool deployment on zkSync. Note also that there is explicit allowance for delegatecall in this script. It currently incorrectly resolves the upstream AddressUpgradeable contract, rather than the local AddressLibUpgradeable. See the following commit for more details: 02cff2c The relevant zkSync documentation can be found here: https://era.zksync.io/docs/tools/hardhat/hardhat-zksync-deploy.html https://era.zksync.io/docs/tools/hardhat/hardhat-zksync-upgradable.html Note that there are some outstanding issues, since deploying OZ upgradeable contracts seems to bypass the normal hardhat-deploy workflow, meaning that deployment metadata is not saved to deployments/<network>.
nicholaspai
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.
How can we overcome the missing deployment metadata issue?
These were recreated by fixing the deployment scripts and deploying a new contract to zkSync. The deployment artifacts are now saved automatically. It was necessary to update the deployed address before committing. As a sanity check, the deployed bytecode for the original deployment and the 2nd deployment are identical, and both point to implementation contract 0x21aa92c23BC2A0E2DaB0335F5f864757FE68c0A7. See the original and 2nd deployments respectively: explorer.zksync.io/address/0xE0B015E54d54fc84a6cB9B666099c46adE9335FF explorer.zksync.io/address/0x02D2B95F631E0CF6c203E77f827381B0885F7822
.upgradable/zkSync-era.json
Outdated
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.
@nicholaspai Not sure whether we actually need this, but I added it just to be on the safe side. I saw that you previously added .upgradable for the zksync-goerli deployments, but the fact that it's hidden in a . folder makes me unsure if it really needs to be part of the repo.
The Ethereum SpokePool deployment script was unable to correctly resolve the HubPool address because it assumed that a companionNetworks entry was defined in the HardhatUserConfig in hardhat.config.ts. Resolve this by adjusting the Ethereum (HubPool) SpokePool deployment script to avoid using companionNetworks. Additionally, the other SpokePool deployment scripts incorrectly logged the chain ID for the HubPool deployment. This change now factors out the task of resolving the chain IDs and returns them in an unambiguous form. This also simplifies most of the deployment scripts.
This fix uses zkSync-specific packages, which are necessary for contract deployment on zkSync. The zkEthBridge configurable is also removed, since it is no longer needed for SpokePool deployment on zkSync. Note also that there is explicit allowance for delegatecall in this script. It currently incorrectly resolves the upstream AddressUpgradeable contract, rather than the local AddressLibUpgradeable. See the following commit for more details: 02cff2c
The relevant zkSync documentation can be found here:
https://era.zksync.io/docs/tools/hardhat/hardhat-zksync-deploy.html
https://era.zksync.io/docs/tools/hardhat/hardhat-zksync-upgradable.html