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

Provide granular functions to allow more customizable deployments #580

Closed
ericglau opened this issue May 20, 2022 · 5 comments · Fixed by #608
Closed

Provide granular functions to allow more customizable deployments #580

ericglau opened this issue May 20, 2022 · 5 comments · Fixed by #608
Labels

Comments

@ericglau
Copy link
Member

ericglau commented May 20, 2022

Background

The current functions for Hardhat and Truffle such as deployProxy, upgradeProxy, and prepareUpgrade can be considered as all-in-one commands that do a number of things together.

In the case of deployProxy for example, it does the following:

  • Checks that the implementation is upgrade safe
  • Deploys the implementation
  • Deploys the admin if needed for transparent proxy
  • Deploys the proxy to use the implementation and admin
  • Saves the above deployments and the implementation's storage layout to the network file

For upgradeProxy or prepareUpgrade, these also verify that the new implementation is upgrade safe when compared to the old implementation, in addition to deploying the implementation contract (and then upgrading the proxy in the case of upgradeProxy).

Problem statement

The all-in-one nature of the existing commands make it hard for users to customize their deployments.

For example, users may want to customize gas parameters for specific contract deployments, verify their implementation without deploying it, use different signers, or deploy only the proxy.

Proposal

Provide granular functions representing the different stages of implementation or proxy deployments or upgrades. This would allow advanced users to do each step independently and can customize when or how each contract is deployed.

The following enhancements are proposed for the different categories related to this:

Implementation contract

Since one of the major benefits of using the Upgrades Plugins is validating the implementation for upgrade safety (for the first version and later versions of the implementation), this should at minimum include the following functions:

  1. validateImplementation(Contract) - Only validates the implementation for upgrade safety. Similar to prepareUpgrade but for a first version of a contract, without deploying it, and without updating the network file.
  2. validateUpgrade(proxyOrBeaconAddress, Contract) or validateUpgrade(Contract, Contract) - Validates the implementation for upgrade safety and compares its storage layout with the old version of the implementation for storage conflicts. Similar to prepareUpgrade but without deploying it, and without updating the network file.
  3. deployImplementation(Contract) - Validates the implementation for upgrade safety, deploys the implementation, and records it in the network file. Similar to prepareUpgrade but can be used on any version of an implementation without comparing storage layouts (storage layouts will still be compared during upgradeProxy or upgradeBeacon).

Import

  • Enhance the forceImport command to allow importing an implementation address directly. This is only meant to be used as a last resort if the user needs to import an implementation that was not deployed by the upgrades plugins.

Common options

  • Support a new common option useDeployedImplementation where if the implementation is not found in the network file, the function should fail instead of deploying the implementation. This should be used with prepareUpgrade, defender.proposeUpgrade, deployProxy, deployBeacon, upgradeProxy, and upgradeBeacon so that they will use the implementation that was predeployed by deployImplementation.

Proxy admin

  • Support a new function deployProxyAdmin that only deploys the ProxyAdmin contract for use with Transparent proxies (if one was not already deployed according to the network file)
@DsAtHuH
Copy link

DsAtHuH commented May 24, 2022

GitHub reported a mention from this issue to #554, but I wasn't able to find it here...

@ericglau
Copy link
Member Author

ericglau commented May 24, 2022

@DsAtHuH this proposal does not address #554 so I removed the mention. #554 would still be needed.

@farruhsydykov
Copy link

I have faced a problem with my client where he had limited amount of ETH to deploy his contracts. After hunting for the right gasPrice deployProxy() ended up only deploying the implementation contract.

I can see how helpful this proposal in some situations.

@silasdavis
Copy link

Deployments to Polygon generally do not work with estimated gas, via hardhat setting baseFeePerGas and priorityFeePerGas is a huge pain.

A proposed solution is to override the getFeeData on the provider but since hardhat provides a readonly proxy you can't simple assign ethers.provider.getFeeData = ....

You end up needing wrap both the provider and the signer and then pass that into the factory.

More generally I was hoping that I could extract the core logic in openzeppelin-upgrades to use in vanilla typescript but it seems to be very deeply integrated with hardhat that it would be hard to do.

It would be great to see a more layered approach where there is a core library of upgrade-related functions that are deployment infrastructure agnostic (other than Typescript).

@ericglau
Copy link
Member Author

@silasdavis One of the main benefits of using the upgrades plugins is validating upgrade safety and storage layout. The Upgrades Core API provides a way to do this that is deployment infrastructure agnostic (see this forum post).

Or if you do use Hardhat or Truffle, the validateImplementation and validateUpgrade functions proposed above would also be useful to perform the validation, and then you can perform your deployments separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants