Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zoeyTM
Copy link
Contributor

@zoeyTM zoeyTM commented Dec 18, 2023

Follows along with the example added in here

Copy link

changeset-bot bot commented Dec 18, 2023

⚠️ No Changeset found

Latest commit: 99c738b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Dec 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 17, 2024 8:06pm

@kanej
Copy link
Member

kanej commented Dec 18, 2023

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:

  • write initial module
  • hook up to test
  • modify deployment to upgrade the contract
  • look the test code keeps the same structure (but the version changed)!

What do you think?

@kanej
Copy link
Member

kanej commented Dec 18, 2023

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.

@kanej kanej added no changeset needed This PR doesn't require a changeset and removed status:triaging labels Dec 18, 2023
@zoeyTM
Copy link
Contributor Author

zoeyTM commented Dec 19, 2023

@kanej

Part of the structure of this is what do we think our advice would be best practice for upgrading open zeppelin contracts using Ignition?

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 UpgradeableProxyStrategy or some other abstraction, similar to or inspired by OpenZeppelin's hardhat-upgrades plugin.

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 m.contractAt(...), to be entirely honest) since a more complete solution is not in our immediate roadmap.

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.

When we deploy the proxy, it will automatically create a new `ProxyAdmin` contract within its constructor. We'll need to get the address of this contract so that we can interact with it later. To do this, we'll use the `m.readEventArgument(...)` method to read the `newAdmin` argument from the `AdminChanged` event that is emitted when the proxy is deployed.

Finally, we'll use the `m.contractAt(...)` method to get the `ProxyAdmin` contract at the address we retrieved from the event and return it along with the proxy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explain the cast of proxy to demo here, so that this example is self-contained. That means, for example, that users can stop here and start testing their upgradeable demo


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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changeset needed This PR doesn't require a changeset
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

None yet

3 participants