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

Unable to deploy upgradeable contract. "number of arguments do not match implementation constructor" #294

Closed
aster2709 opened this issue Mar 27, 2022 · 16 comments

Comments

@aster2709
Copy link

great plugin. I'm still new to the ways of it. trying to deploy an upgradeable contract

the contract

// SPDX-License-Identifier: MIT
pragma solidity 0.8.13;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract BoxV1 is Initializable {
  uint256 public value;
  event Deploy();
  event Increment();

  function initialize(uint256 _value) public initializer {
    value = _value;
    emit Deploy();
  }

  function increment() public {
    value++;
    emit Increment();
  }
}

the deploy script

import { DeployFunction } from "hardhat-deploy/types"

const deploy: DeployFunction = async (hre) => {
  const { deployer } = await hre.getNamedAccounts()
  await hre.deployments.deploy("BoxV1", {
    from: deployer,
    log: true,
    proxy: "initialize",
    args: [42],
  })
}
deploy.tags = ["BoxV1"]

export default deploy

the error

image

@aster2709 aster2709 changed the title Unable to deploy a proxy. "number of arguments do not match implementation constructor" Unable to deploy upgradeable contract. "number of arguments do not match implementation constructor" Mar 27, 2022
@wighawag
Copy link
Owner

wighawag commented Mar 27, 2022

the issue is that when you use args with a proxy it expects both the constructor and the init function to share the same args

The reason for this is to allow you to use the same contract as an immutable contract (Without any proxy) as well as an upgradeable contract without any change.

I use that feature a lot when I develop my front end and then release it on the mainnet. during development, it gets upgraded automatically but when I deploy I can simply disable the proxy and it still works the same.

So if you do not expect to deploy without proxy, you ll have to remove the args and instead use execute or init

like this:

import { DeployFunction } from "hardhat-deploy/types"

const deploy: DeployFunction = async (hre) => {
  const { deployer } = await hre.getNamedAccounts()
  await hre.deployments.deploy("BoxV1", {
    from: deployer,
    log: true,
    proxy: {
      execute: {
        init: {
          methodName: 'initialize",
          args: [43]
        }
      }
    }
  })
}
deploy.tags = ["BoxV1"]

export default deploy

@jwickers
Copy link

I have the same or very similar issue in 0.11.2 (or 0.11.0), but the code worked on 0.10.6:

{
    from: deployer,
    proxy: {
      owner: deployer,
      proxyContract: "OptimizedTransparentProxy",
      execute: {
          methodName: 'initialize',
          args: [ deployer ]
      }
    },
  }

Now results in Error: expected 0 constructor arguments, got 1

The contract does not have a constructor, only an initialize function.

I also tried what you posted using init: { ... } inside execute but that has the same error.

Did something break in 0.11.x ?

@wighawag
Copy link
Owner

@jwickers oh yes, there is indeed an issue

I just fixed it in 0.11.4

@wighawag
Copy link
Owner

Thanks for reporting!

@aster2709
Copy link
Author

works @wighawag thanks for addressing. quick follow up. how can you verify the implementation using etherscan-verify?

  • running the following verifies the proxy instead of implementation
npx hardhat etherscan-verify --contract-name BoxV1 --network polygon_testnet
  • is there an option to specify the address you want to verify?

@wighawag
Copy link
Owner

for implementation you can use BoxV1_Implementation
you should see a deployment file for it in the deployments/<network> folder

@aster2709
Copy link
Author

works well.

I couldn't maybe understand the docs to upgrade the proxy. could you tell how BoxV1 can be upgraded. what changes in the proxy property?

@wighawag
Copy link
Owner

@aster2709 the design philosophy of hardhat-deploy script is to be declarative

So for proxy upgrade, you are not supposed to change the proxy config
You just change the code of the implementation and reexecute the deploy command

hardaht-deploy will detect the changes and perform the upgrade, or tell you own to perform it

@aster2709
Copy link
Author

didn't upgrade for me like that. I added the decrement function to BoxV1

pragma solidity 0.8.13;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract BoxV1 is Initializable {
  uint256 public value;
  event Deploy();
  event Increment();
  event Decrement();

  function initialize(uint256 _value) public initializer {
    value = _value;
    emit Deploy();
  }

  function increment() public {
    value++;
    emit Increment();
  }

  function decrement() public {
    value--;
    emit Decrement();
  }
}

the script is untouched

import { DeployFunction } from "hardhat-deploy/types"

const deploy: DeployFunction = async (hre) => {
  const { deployer } = await hre.getNamedAccounts()
  await hre.deployments.deploy("BoxV1", {
    from: deployer,
    log: true,
    proxy: {
      execute: {
        methodName: "initialize",
        args: [42],
      },
    },
  })
}
deploy.tags = ["BoxV1"]

export default deploy

console output
image

@wighawag
Copy link
Owner

hmm, that should not be the case, do you have a repo I could try ?

@aster2709
Copy link
Author

aster2709 commented Mar 31, 2022

prepared one. still errors out for me on upgrade. here it is, https://github.com/aster2709/hardhat-template

  • Box.sol is the basic contract. you can uncomment the decrement fn during upgrade 👍

@zfogg
Copy link

zfogg commented Apr 12, 2022

+1 I can't get 0.11.4 to execute initialize for openzeppelin upgradables

@wighawag
Copy link
Owner

What error do you get ?

@aster2709 I tried your repo and the issue is that your initialize function can only be called once

If you want that you can use

await hre.deployments.deploy("Box", {
    from: deployer,
    log: true,
    proxy: {
      execute: {
        init: { methodName: "initialize",
          args: [42],
        }
      },
    },
  });

@aster2709
Copy link
Author

that worked! there's an important observation here.

  • hardhat-deploy allows to upgrade even if you break the rules of upgradability
  • using @openzeppelin/hardhat-upgrades deploy script detects them and prevents the upgrade. example:
    image

@wighawag

@wighawag
Copy link
Owner

Yes, that's a know fact. hardhat-deploy do not implement any verification.
It is your responsibility to ensure you do not do anything wrong with storage layout.

Having said, happy to receive contribution. There is actually a PR implementing this: #162 but did not found the time yet to look closely

@aster2709
Copy link
Author

thanks for clarifications and patience. keep building ser 👍

fedgiac added a commit to cowprotocol/contracts that referenced this issue May 4, 2023
Updates remaining outdated packages that are tightly coupled together
and need to be updated in parallel.

Note that Ethers has been updated to the latest minor version for v5, an
upgrade to v6 requires many changes and will trigger a new major release
of this project.

Also note that I only updated Tenderly to v1.1 and froze it at that
minor version. Using the latest version would require a few code
changes, importing an extra package, increase the number of network
request on _any_ invocation of `hardhat` and would annoyingly increase
the amount of logs that are printed every time `hardhat` is called (the
extra logs might be reduced but not configured away as the logging level
is hardcoded in the plugin). Issue #32 is related.

The changes to the proxy code appear to be caused by a change in
behavior in `hardhat-deploy`, the new code should have the same effect
(this is confirmed by our tests). I found a related issue
[here](wighawag/hardhat-deploy#294) and the
new behavior appears to be intended behavior.

### Test Plan

Test a few commands and verify that everything works as usual, notably
`yarn deploy` and `yarn bench:trace`.
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

4 participants