-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Added a guide for using upgradeable proxies with hardhat ignition #4691
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This is a really good first pass. Thinking about the structure, I think we should move away from a straight description of the code towards:
What do you think? |
Part of the structure of this is what do we think our advice would be best practice for upgrading open zeppelin contracts using Ignition? The pattern in the code is nested modules where upgradeV2 uses deployV1; there is an extent to which I miss being able to express passing parameters: const {proxy: proxyV1, proxyAdmin} = m.useModule(DeployV1, {})
const {proxy: proxyV2} = m.useModule(UpgradeToV2, {proxyAdmin})
...
const {proxy: proxyVX} = m.useMOdule(UpgradeToVX, {proxyAdmin})
return {proxy: proxyVX} Maybe we can achieve the same thing internally within one module with plain functions, so something like: const upgrade = (m: ModuleBuilder, proxyAdmin, proxyAdminOwner, contractName: string) => {
const instance = m.contract(contractName);
m.call(proxyAdmin, "upgradeAndCall", [proxy, demoV2, "0x"], {
from: proxyAdminOwner,
});
const updatedProxy = m.contractAt(contractName, proxy);
return updatedProxy;
}
cont module = buildModule((m) => {
const proxyAdminOwner = m.getAccount(1);
const {proxy: proxyV1, proxyAdmin} = m.useModule(DeployV1, {});
upgrade(m, poxyAdmin, proxyAdminOwner, "DemoV2");
// ...
const proxyVX = upgrade(m, poxyAdmin, proxyAdminOwner, "DemoVX");
return {proxy: proxyVX}
}) The key point being to make the "I need to upgrade the contract to the next version" problem as concise to express as possible. |
My opinion is that, long or even medium term, none of this will be our recommended best practice for upgradeable proxy patterns in Ignition. The reason for that is because I think we can do better, whether that comes in the form of an My understanding was that this guide and the accompanying sample project were only meant to show how a user could leverage Ignition's current API to support their upgradeable proxy needs (mostly demonstrating how to use Additionally, there are other upgradeable contract patterns out there, and individual implementations can vary widely. My goal was to demonstrate the specific parts of the API that allow for upgradeable proxies to work in Ignition today and to explain why they are used how they are to enable users to apply those tools and logic to their own upgradeable contract system, whatever that may look like. |
|
||
Lastly, we again return the `ProxyAdmin` and proxy contracts so that we can use them in our next module. | ||
|
||
### Part 3: Interacting with our proxy |
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.
Let's make this part 2
149d5be
to
c5d1c58
Compare
c5d1c58
to
fd39779
Compare
I removed the unneeded variables in the monorepo, so I am updating the usage here as well.
Remove unused variables.
0a308c4
to
b6cb710
Compare
b6cb710
to
7140c3a
Compare
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.
Pre-approving. My only remaining question is whether we prefer the title "Upgradeable Contracts" to "Upgradeable Proxies"
Co-authored-by: John Kane <john@kanej.me>
Follows along with the example added in here