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

Add an option to conditionally perform upgrade #215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

landakram
Copy link

Passing proxy.performUpgrade: false allows not running the changeImplementation/upgradeTo
transaction. This supports deployments where the owner is a multisig and changing implementation requires
approvals / multiple signatures out of band. We can still deploy the implementation and
and save the deployment in a way that hardhat-deploy expects / makes etherscan
verification work, but the actual upgrade can be performed separately.

Passing `proxy.performUpgrade: false` allows not running the changeImplementation/upgradeTo
transaction. This supports deployments where the owner is a multisig that requires
approvals / multiple signatures out of band. We can still deploy the implementation and
and save the deployment in a way that hardhat-deploy expects / makes etherscan
verification work, but the actual upgrade can be performed separately.
@aureliusbtc
Copy link
Contributor

You can do this right now using await catchUnknownSigner()

See

async function catchUnknownSigner(

@landakram
Copy link
Author

Using catchUnknownSigner would prevent the error but wouldn't write out the updated proxy deployment file since the error is thrown before the proxy deployment is updated. Ideally we would get the changes to the proxy deployment file too (implementation, upgradeIndex, etc), without having to re-run the deploy script, since we sometimes have other actions in our deploy scripts that aren't idempotent.

This PR would support that, but if there's another way to achieve this / another direction to go in, I'm open to it

@wighawag
Copy link
Owner

been thinking of an option where it would pause execution ask you to perform the upragde (or any tx that requires an external signer) and then allow you to continue after you perform the operation

Your solution can be achieved by checking if the contract exist and skip the call to deploy, no ?

@landakram
Copy link
Author

landakram commented Nov 17, 2021

Honestly, what would be great is to be able to optionally pass in an "upgrade handler" that controls / performs the upgrade i.e. the function signature is:

(proxyAddress: string, implementationAddress: string) => Promise<void>

and, if not passed in, defaults to the existing upgrade logic in hardhat-deploy that runs upgradeMethod/upgradeToAndCall and uses the available signer. If the promise succeeds, hardhat-deploy should treat the implementation as "upgraded". That way, the user has maximal flexibility in deciding how the upgrade should be performed (and hardhat-deploy could potentially provide other plug-and-play strategies). In our case, we batch all of the upgrade txs into a multisend and execute it at the end of the deployment to produce as few transactions as possible. We would use the upgrade handler to add the upgrade tx to a list for batching later.

If you think this would be a viable direction, I'd be happy to put up another PR

@wighawag
Copy link
Owner

wighawag commented Nov 18, 2021

I like that, one thing that this should do is replay the deployment, after the promise resolve (this way hardhat-deploy can automatically update the deployments file)

the promise could also resolve to a tx hash that hardhat-deploy can then wait for

or it could also resolve to a deployment object so hardhat-deploy can save it.

Need to think about it more and usual implementation work help to see what's best

@mattvv
Copy link

mattvv commented Nov 11, 2022

@wighawag was there any method like this added in?

I also have a similar use case, we use a safe for ownership on our proxies after initial deploy, my use case here is to deploy an implementation only (and update the hardhat-deploy info) and then run a defender propose upgrade.

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

Successfully merging this pull request may close these issues.

4 participants