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

Deterministic deployments #272

Open
bh2smith opened this issue Jan 5, 2021 · 3 comments
Open

Deterministic deployments #272

bh2smith opened this issue Jan 5, 2021 · 3 comments

Comments

@bh2smith
Copy link

bh2smith commented Jan 5, 2021

When using upgrades.deployProxy the DeployOptions type interface provided is very different than the usual interface provided in the usual hardhat package. In particular, there are several commonly used deployment options that are no longer available. For example:

  const options: DeployOptions = {
    from: deployer,
    gasLimit: 2000000,
    args: [owner],
    deterministicDeployment: SALT,
    log: true,
  };

Is it possible that the Options interface defined in deploy-proxy could extend the usual hardhat Options to overcome this issue or are these parameters no longer possible when working with proxies?

@frangio
Copy link
Contributor

frangio commented Jan 5, 2021

This is similar to #85, generalizing the idea to all available deployment options.

It would be nice to do something like this, but the additional issue that we have is that a deployProxy call can potentially perform 3 different deployments (implementation, admin, proxy), and upgradeProxy will additionally send a transaction (the upgrade function call). So the question we need to figure out is: should these deployment options be the same for all of these transactions? Or should it be possible to specify options for each deployment separately?

#213 adds a from option that is shared by all deployments and transactions, which I think makes sense. But it is less clear what should happen for gasLimit, for example.

Let me know what you think!

@bh2smith
Copy link
Author

bh2smith commented Jan 6, 2021

It seems the motivation for wanting these is very particular to each use case. For ours, it would be the deterministic deployment that is of most importance (as this can be used to ensure the contracts will all have the same address on different networks - such as xDai). While it is clear that some of these parameters may need to be specified for each individual deployment within deployProxy there are surely some that could easily be carried over as "universal". From those options provided above, it seems to me that log and deterministicDeployment could all be easily considered as universalDeployOptions.

Perhaps its best to tackle this issue in two steps: First being the universal deployment options and the latter (if/when needed) the specifics.

As I am most interested in deterministic deployment, my opinion is rather bias. I would be happy to know if this suggestion might be considered for the project and could perhaps even attempt at making/helping with the changes.

#213 adds a from option that is shared by all deployments and transactions

Yes, this is a nice start, but it appears this PR is dedicated to the truffle plugin as opposed to hardhat. It might be worth mentioning to propagate these changes into the hardhat plugin as well.

@bh2smith
Copy link
Author

bh2smith commented Jan 6, 2021

I have re-evaluated the controversial notion "universal deployment options" and fallen back to something more generic that allows users to specify their own deployer in which they can use whichever DeployOptions they like. This is a non-invasive way of making the requested feature available for those who want it, but leaving things as is for those who are uninterested in such additional overhead.

@frangio frangio changed the title [hardhat-upgrades] DeployOptions missing deterministicDeployment Deterministic deployments Jan 18, 2021
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

No branches or pull requests

2 participants