How to inherit ERC2771ContextUpgradeable?

Support Upgrades
1

:1234: Code to reproduce

ERC2771ContextUpgradeable has an constructor, if I inherit it, I cannot use upgrades to deploy it.

contract MyNFT is
    ERC2771ContextUpgradeable,
    UUPSUpgradeable,
    ERC721URIStorageUpgradeable,
 {

    constructor(address forwarder) ERC2771ContextUpgradeable(forwarder) {
    }
}

then, when I try to deploy it, error occurs:

Error: Contract `MyNFT` is not upgrade safe
Contract `MyNFT` has a constructor

:computer: Environment

  "devDependencies": {
    "@nomicfoundation/hardhat-chai-matchers": "^1.0.2",
    "@nomicfoundation/hardhat-network-helpers": "^1.0.3",
    "@nomicfoundation/hardhat-toolbox": "^1.0.2",
    "@nomiclabs/hardhat-ethers": "^2.1.0",
    "@nomiclabs/hardhat-etherscan": "^3.1.0",
    "@openzeppelin/contracts": "^4.7.1",
    "@openzeppelin/contracts-upgradeable": "^4.7.1",
    "@openzeppelin/hardhat-upgrades": "^1.19.0",
    "@typechain/ethers-v5": "^10.1.0",
    "@typechain/hardhat": "^6.1.2",
    "@types/chai": "^4.3.1",
    "@types/mocha": "^9.1.1",
    "chai": "^4.3.6",
    "decimal.js": "^10.3.1",
    "dotenv": "^16.0.1",
    "ethers": "^5.6.9",
    "hardhat": "^2.10.1",
    "hardhat-gas-reporter": "^1.0.8",
    "mocha": "^10.0.0",
    "prettier": "^2.7.1",
    "prettier-plugin-solidity": "^1.0.0-dev.23",
    "solhint": "^3.3.7",
    "solhint-plugin-prettier": "^0.0.5",
    "solidity-coverage": "^0.7.21",
    "ts-generator": "^0.1.1",
    "ts-node": "^10.9.1",
    "typechain": "^8.1.0",
    "typescript": "^4.7.4"
  }

node: 18.6.0

2

solved.

just put ahead:

    /// @custom:oz-upgrades-unsafe-allow constructor
3

How does your deployment script look, like this?

const MyNFT = await ethers.getContractFactory("MyNFT");
const instance = await upgrades.deployProxy(MyNFT, [forwarderAddress]);
await instance.deployed();

If so, what if you have two arguments? Which one goes to the constructor, and which one goes to the initializer? How would you direct them?

4

See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/pull/155#issuecomment-1330816863

1 Like
5

Thanks for the update, that really helped!

But I'm getting some strange inheritance issue when I try to write the upgrade contract:

contract MyContractV2 is MyContract {
    function version() public pure virtual override returns (string memory) {
        return "2.0.0";
    }
}

TypeError: Contract "MyContractV2" should be marked as abstract.
   --> contracts/MyContract.sol:537:1:
    |
537 | contract MyContractV2 is MyContract {
    | ^ (Relevant source part starts here and spans across multiple lines).
Note: Missing implementation: 
   --> contracts/MyContract.sol:123:5:
    |
123 |     constructor(MinimalForwarderUpgradeable forwarder)
    |     ^ (Relevant source part starts here and spans across multiple lines).


Error HH600: Compilation failed

I tried to implement the constructor to rid this abstract error but then I get this:

contract MyContractV2 is MyContract {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor(MinimalForwarderUpgradeable forwarder)
        ERC2771ContextUpgradeable(address(forwarder))
    {}

    function version() public pure virtual override returns (string memory) {
        return "2.0.0";
    }
}


DeclarationError: Base constructor arguments given twice.
   --> contracts/MyContract.sol:540:9:
    |
540 |         ERC2771ContextUpgradeable(address(forwarder))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Note: Second constructor call is here:
   --> contracts/MyContract.sol:124:9:
    |
124 |         ERC2771ContextUpgradeable(address(forwarder))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Also, the abstract error didn't go away either.

Do you have any idea as to what's going on?

6

The child contract should call the constructor of its parent contract, e.g.:

contract MyContractV2 is MyContract {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor(MinimalForwarderUpgradeable forwarder)
        MyContract(address(forwarder))
    {}

    function version() public pure virtual override returns (string memory) {
        return "2.0.0";
    }
}

Also, a reminder that as a best practice, include _disableInitializers(); in your constructor (at least in MyContract, or MyNFT in the original post) as described in https://docs.openzeppelin.com/contracts/4.x/api/proxy#Initializable

7

Oh great, thank you, that worked out! I haven't worked with upgradeable contracts that have a constructor before so it was a tad confusing at first.

Also, wondering what the deal is with _disableInitializers() in the constructor...

Locks the contract, preventing any future reinitialization. This cannot be part of an initializer call. Calling this in the constructor of a contract will prevent that contract from being initialized or reinitialized to any version. It is recommended to use this to lock implementation contracts that are designed to be called through proxies.

I thought this was taken care of already by initializer:

Since proxied contracts do not make use of a constructor, it’s common to move constructor logic to an external initializer function, usually called initialize . It then becomes necessary to protect this initializer function so it can only be called once. The initializer modifier provided by this contract will have this effect.

It's a bit unintuitive for me at first because when I see _disableInitializers(); in the constructor, that makes me think that the constructor will be called first which would disable initialize(), thus preventing the initialization of contract dependencies and state variables. Maybe I'm missing something here...

In any case, I've included _disableInitializers(); in my parent constructor, should I also include it in the child constructor as well, like such:

contract MyContractV2 is MyContract {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor(MinimalForwarderUpgradeable forwarder)
        MyContract(address(forwarder))
    {
        _disableInitializers();
    }

    function version() public pure virtual override returns (string memory) {
        return "2.0.0";
    }
}

Do let me know if I should, thanks!

8

The constructor with _disableInitializers() runs on the implementation contract, whereas the initialize() function runs in the context of the proxy. See Is _disableInitializers necessary? - #2 by ericglau for reasoning.

In any case, I've included _disableInitializers(); in my parent constructor , should I also include it in the child constructor as well

If it's already in the parent, it's not necessary to include it in the child.