eth upgradable contract #133
Conversation
|
|
||
| ## Update the public key | ||
|
|
||
| To update the public key, you also need to update the contract. Add a `reinitialize` function to the contract that set the public key to new one, and upgrade the contract as above. |
There was a problem hiding this comment.
what do you mean by Add a `reinitialize` function to the contract that set the public key to new one?
There was a problem hiding this comment.
I'll add an example in unit test
| const deploymentPath = `deployments/${networkName}.json`; | ||
|
|
||
| // Check if deployment file exists | ||
| if (!fs.existsSync(deploymentPath)) { |
There was a problem hiding this comment.
This relies on the local deployment file? Does this mean we need to commit that file to git every time we do initialDeploy or updateContract?
There was a problem hiding this comment.
Good point. Local deployment is gitignored. For testnet it should be commited
| /// @custom:oz-upgrades-unsafe-allow constructor | ||
| constructor() initializer {} | ||
|
|
||
| function initialize(PublicKey memory _publicKey) public initializer { | ||
| publicKey = _publicKey; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we need to add storage gap or is it handled by the openzepplin already? https://blog.openzeppelin.com/validate-smart-contract-storage-gaps-with-openzeppelin-upgrades-plugins
There was a problem hiding this comment.
Thanks for point out. I didn't know this. After reading this I think we don't need it, we can append new variables as long as we keep existing variables.
| npx hardhat run scripts/upgradeContract.js --network <network> | ||
| ``` | ||
|
|
||
| It will upgrade the implementation contract to the new version and keep the proxy contract address the same. Make sure you always extends the storage and never override it. For more details, please refer to [Writing Upgradeable Contracts](https://docs.openzeppelin.com/upgrades-plugins/writing-upgradeable). |
There was a problem hiding this comment.
I remember seeing that the order of declaring variables also can't change when we modify the contract? Does openzepplin warn against it?
There was a problem hiding this comment.
ah, it does warn!
Upgrading implementation...
Error: New storage layout is incompatible
ChainSignatures: Deleted `threshold`
> Keep the variable even if unused
contracts/ChainSignatures.sol:43: Inserted `threshold`
> New variables should be placed after all existing inherited variables
contracts/ChainSignatures.sol:44: Inserted `hihi`
> New variables should be placed after all existing inherited variables
This version only allows contract owner to upgrade, we may figure out a better permission control such as voting to agree contract upgrade in future. I deployed this proxy contract to 0x6348104D80e1376b59789cA5b65f9a45708A16d6 on sepolia.