openzeppelin-foundry-upgrades icon indicating copy to clipboard operation
openzeppelin-foundry-upgrades copied to clipboard

Unclear error: revert: Failed to deploy contract using constructor data ""

Open yvesbou opened this issue 1 year ago • 1 comments

I’m using Upgrades.upgradeProxy() to upgrade proxy contracts pointing to new implementations. A vanilla test case works. But I tried to test revoke ownership and then upgrade which should fail, but it fails with an error that I assume is not correct.

I expect OwnableUnauthorizedAccount and I get this error (instead of passing, I assume)

[FAIL. Reason: revert: Failed to deploy contract MyTokenV2.sol:MyTokenV2 using constructor data ""] testRevokingOwnership() (gas: 8237465)

if I don’t expect OwnableUnauthorizedAccount I get this error

[FAIL. Reason: OwnableUnauthorizedAccount(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf)] testRevokingOwnership() (gas: 8242052)

which is what I expect, if I don't expect the OwnableUnauthorizedAccount error

So why do I get revert: Failed to deploy contract MyTokenV2.sol:MyTokenV2 using constructor data ""] - this doesn't compute with my current understanding

Here are necessary parts of the contracts and tests 👇

contract

I just have one argument in the initializer

contract MyToken is
    Initializable,
    ERC20Upgradeable,
    OwnableUpgradeable,
    ERC20PermitUpgradeable,
    ERC20VotesUpgradeable,
    ERC20BurnableUpgradeable,
    UUPSUpgradeable
{

		...
		
    function initialize(address initialOwner) public initializer {
        __ERC20_init("MyToken", "MTK");
        __Ownable_init(initialOwner);
        __ERC20Permit_init("MyToken");
        __ERC20Burnable_init();
        __ERC20Votes_init();
        __UUPSUpgradeable_init();

        _mint(msg.sender, 1000000 * 10 ** decimals());
    }
    
    ...
   
}

V2 of token

it's reinitialisable, thus I want to call it with an initialiser argument, like the first init

contract MyTokenV2 is
    Initializable,
    ERC20Upgradeable,
    OwnableUpgradeable,
    ERC20PermitUpgradeable,
    ERC20VotesUpgradeable,
    ERC20BurnableUpgradeable,
    UUPSUpgradeable
{
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

    function initialize(address initialOwner) public reinitializer(2) {
        __ERC20_init("MyTokenV2", "MTKV2");
        __Ownable_init(initialOwner);
        __ERC20Permit_init("MyTokenV2");
        __ERC20Burnable_init();
        __ERC20Votes_init();
        __UUPSUpgradeable_init();

        _mint(msg.sender, 1000000 * 10 ** decimals());
    }
    
}

setup

function setUp() public {
        // Deploy the token implementation
        MyToken implementation = new MyToken();
        // Define the owner address
        owner = vm.addr(1);

        vm.startPrank(owner);
        // Deploy the proxy and initialize the contract through the proxy
        proxy = new ERC1967Proxy(address(implementation), abi.encodeCall(implementation.initialize, owner));
        vm.stopPrank();

        // remember: Call your contract's functions as normal, but remember to always use the proxy address:
        // Attach the MyToken interface to the deployed proxy
        myToken = MyToken(address(proxy));
        // Emit the owner address for debugging purposes
        emit log_address(owner);
    }

test case

// upgrade should revert, since no ownership
function testRevokingOwnership() public {
        // revoke ownership
        vm.startPrank(owner);
        myToken.renounceOwnership();

        bytes memory data = abi.encodeCall(MyTokenV2.initialize, owner);

        bytes memory expectedRevertData =
            abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, owner);

        vm.expectRevert(expectedRevertData); // reverts with another reason (unclear if goal achieved)
        Upgrades.upgradeProxy(address(proxy), "MyTokenV2.sol:MyTokenV2", data, owner);

        // no mint possible (as expected)
        vm.expectRevert(expectedRevertData);
        myToken.mint(owner, 100e18);

        vm.stopPrank();
    }

yvesbou avatar Sep 02 '24 13:09 yvesbou

Upgrades.upgradeProxy consists of two separate calls: one to deploy the new implementation, and another to upgrade the proxy to use that new implementation.

It looks like the error is occurring on the call to deploy the new implementation. Specifically, it might be related to the "Gotcha: Usage with low-level calls" section of https://book.getfoundry.sh/cheatcodes/expect-revert

Since what you want to test is the revert reason during the upgrade, you can break this down into two separate steps: use Upgrade.prepareUpgrade to validate and deploy the new implementation itself, then use the vm.expectRevert before calling the proxy's upgradeToAndCall function.

For example:

    Options memory opts;
    address v2implementation = Upgrades.prepareUpgrade("MyTokenV2.sol:MyTokenV2", opts);

    vm.expectRevert(expectedRevertData);
    proxy.upgradeToAndCall(v2implementation, data);

ericglau avatar Sep 05 '24 21:09 ericglau

Closing as described above. Feel free to reopen if needed.

ericglau avatar Jan 14 '25 16:01 ericglau