wighawag / hardhat-deploy

hardhat deployment plugin
MIT License
1.17k stars 283 forks source link

Add support for custom upgrade functions #478

Closed zmalatrax closed 8 months ago

zmalatrax commented 10 months ago

Is your feature request related to a problem? Please describe.

Current versions of OpenZeppelin's (OZ) Proxies (ERC1967Utils.sol & UUPSUpgradeable.sol) only implements the upgradeToAndCall(address newImplementation, bytes memory data) function. The ProxyAdmin contract only implements upgradeAndCall(address proxy, address implementation, bytes memory data).

When trying to upgrade the implementation contract of a Transparent or UUPS proxy one without performing a post-upgrade call through hardhat-deploy (the equivalent of the old upgradeTo(address newImplementation) included in hardhat-deploy), the plugin defaults to calling upgrade or upgradeTo (depending if a proxy admin has been defined or not) which are not implemented, resulting in the error method not implemented....

Here is the deployment script used when trying to initialize then upgrade a UUPS proxy, with the implementation only having upgradeToAndCall(address newImplementation, bytes memory data) defined:

const upgradeableDeployment = await deploy("UUPSUpgradeableMock", {
  from: deployer,
  proxy: {
  proxyContract: "UUPSProxy", //Custom UUPS Proxy adapted from OpenZeppelin's one
  /* proxyArgs, checkProxyAdmin & checkABIConflict must be specified
  because of using a "custom" proxy, which is not in the included contracts such as "UUPS" */
  proxyArgs: ["{implementation}", "{data}"],
  checkProxyAdmin: false,
  checkABIConflict: false,
  execute: {
      init: {
        methodName: "initialize",
        args: [deployer],
      },
    },
  },
  log: true,
  args: [],
});

Fig.1: hardhat-deploy deployment script of a custom UUPS proxy implementing upgradeToAndCall only for upgrading

It nicely deploys and initializes the proxy but, when modifying the implementation and running the script again (which should perform the upgrade), it errors with Error: No method named "upgradeTo" on contract deployed as "UUPSUpgradeableMock"

uups_hardhat-deploy_error Fig.2: hardhat-deploy logs when trying to update the UUPSUpgradeableMock with no updateMethod defined

Note: In helper.ts, the variables updateMethod & updateArgs are referring to the post-upgrade function that is delegatecalled to the new implementation once the upgrade is done. However, their name is misleading (to my understanding, update ~= upgrade) and should be renamed for better semantics, such as postUpgradeMethod & postUpgradeArgs

Describe the solution you'd like

This way, it would allow the use of newer versions of OZ's proxies but also custom proxies/admin proxies to be supported by hardhat-deploy while having backward compatibility with the current workflow. It should solve #146.

For the issue I faced, it would be resolved with this script:

const upgradeableDeployment = await deploy("UUPSUpgradeableMock", {
  from: deployer,
  proxy: {
    proxyContract: "UUPSProxy", //Custom UUPS Proxy adapted from OpenZeppelin's one
    /* proxyArgs, checkProxyAdmin & checkABIConflict must be specified
    because of using a "custom" proxy, which is not in the included contracts such as "UUPS" */
    proxyArgs: ["{implementation}", "{data}"],
    checkProxyAdmin: false,
    checkABIConflict: false,
    upgradeFunction: {
      methodName: "upgradeToAndCall",
      upgradeArgs: ['{implementation}', '{data}']
    }
    execute: {
      init: {
        methodName: "initialize",
        args: [deployer],
      },
    },
  },
  log: true,
  args: [],
});

As no updateMethod is defined, data defaults to '0x' thus not performing a delegatecall after the successful upgrade.

Describe alternatives you've considered

const upgradeableDeployment = await deploy("UUPSUpgradeableMock", {
  from: deployer,
  proxy: {
  proxyContract: "UUPSProxy", //Custom UUPS Proxy adapted from OpenZeppelin's one
  /* proxyArgs, checkProxyAdmin & checkABIConflict must be specified
  because of using a "custom" proxy, which is not in the included contracts such as "UUPS" */
  proxyArgs: ["{implementation}", "{data}"],
  checkProxyAdmin: false,
  checkABIConflict: false,
  execute: {
      init: {
        methodName: "initialize",
        args: [deployer],
      },
      onUpdate: {
        methodName: "counter",
        args: []
      }
    },
  },
  log: true,
  args: [],
});

Additional Context

jaybuidl commented 8 months ago

Can be marked as closed :)