Missing initializer in ERC721EnumerableUpgradeable?

Support Contracts
1

Hey Guys,

I am wondering....is there a missing initializer call in your contract ERC721EnumerableUpgradeable?
It inherits from ERC721Upgradeable but it does not call its initializer?

I only noticed that cause I was missing the possibility to pass on name and symbol.

image
image1132×400 44.3 KB

Shouldnt it rather look like this?

abstract contract ERC721EnumerableUpgradeable is Initializable, ERC721Upgradeable, IERC721EnumerableUpgradeable {
    function __ERC721Enumerable_init(string memory _name, string memory _symbol) internal initializer {
        __ERC721_init(_name, _symbol);
        __Context_init_unchained();
        __ERC165_init_unchained();
        __ERC721Enumerable_init_unchained();
    }

same applies to ERC721URIStorageUpgradeable as I just noticed. I am wondering if this is by design for the extensions contracts but would that mean that when you are using them, you always also have to inherit directly from ERC721Upgradeable? Or how is it to be explained that some parent contract initializers are called and some are not?

2

You'll have to call the initializer for ERC721Upgradeable inside the initializer of the inheriting contract you are building, like this:

    function initialize() initializer public {
        __ERC721_init("MyToken", "MTK");
        __ERC721Enumerable_init();
        __Ownable_init();
        __UUPSUpgradeable_init();
    }

Initializers work a little different that regular constructors do.

3

Thanks for your response but I think you may have missed the core of the problem I am describing.

Here is a contract (ERC721EnumerableUpgradeable) that inherits from another (ERC721Upgradeable) and doesnt call its parents' initializer, so it breaks the chain....and there is no note that it does not do that for a reason or that people must call the ERC721Upgradeable initializer from the implementing contract. So I am suspecting a bug. Or a missing comment.

Hopefully an OZ team member can respond to this topic as well. Curious to hear their opinion.

4

It's not missing the initializer. This is replicating how the vanilla (non-upgradeable) contract works (ERC721Enumerable). The constructor for the ERC721 parent is not invoked, and the developer has to invoke it in the "derived" contract where they combine both ERC721 and ERC721Enumerable.

5

Hi Frangio,

thanks for your response. I am missing the answer to the "why" though.
Why do you call SOME parent initializers but not all?

And wouldnt it make sense to add a comment in the contract saying that you need to invoke some initializers that this contract is not invoking, even though most of the other contracts do that?

I think it confuses me because you are saying that in the derived contract they would combine both ERC721 and ERC721Enumerable - but in my understanding this would not even be necessary. ERC721Enumerable inherits from ERC721Upgradeable....so basically it is already a ERC721. Why would I need to add another ERC721 import in the derived contract? Seems like double inheritance to me.

I hope you understand my way of thinking and can help me to clear this up in my brain.
Thank you.

6

Yes I understand that this is confusing. Initializers are tricky right now. We're working on some tooling to make it easier and less error prone.