wighawag / hardhat-deploy

hardhat deployment plugin
MIT License
1.19k stars 292 forks source link

Cannot Access Ownable Functions in Upgradeable Contract #373

Open JohnSmithyy opened 2 years ago

JohnSmithyy commented 2 years ago

Describe the bug Cannot run functions that have the onlyOwner modifier in upgradeable contracts.

To Reproduce Code to deploy:

    await deploy("MyContract", {
        from: deployer,
        log: true,
        proxy: network.live == false ? false : {
            owner: deployer,
            proxyContract: "OptimizedTransparentProxy",
        },
    });

Expected behavior I should be allowed to run onlyOwner functions as the owner.

versions

Additional context I have a contract and after deploying with hardhat-deploy, I receive 3 json files namely DefaultProxyAdmin.json, MyContract_Proxy.json and MyContract_Implementation.json. Inspecting the owner for these contracts, I am the owner for DefaultProxyAdmin and MyContract_Implementation BUT NOT MyContract_Proxy.

I believe this prevents me from being able to run owner only functions since it reverts with the following message: Fail with error 'Ownable: caller is not the owner'.

When using the MyContract_Implementation, I am able to run owner only functions but it does not contain the data that lies in MyContract_Proxy.

Any idea what I'm missing?

wighawag commented 2 years ago

Ownable in the proxy is implementation specific not sure what you used for it and how you initialise it

OptimizedTransparentProxy does not have an owner but it has an admin. that admin is immutable (that's what makeOptimizedTransparentProxy optimized), it is set to the DefaultProxyAdmin by default

The DefautlProxyAdmin implements Ownable and it is the one specified in the deploy command (or deployer if not specified)

Now if you use deploy command to make upgrade that's fine

But form what you write I suspect you are using ownable for other function, that's up to your implementation and what owner you specify.

Remember that in a proxy setup, you cannot set the owner in the constructor, you have to provide an initialization function and call it as part of the deployment