Doubts with the creation of contracts in wizard.openzeppelin

Support
, , ,
1

When creating contracts automatically, I have had some doubts about the import of some contracts and some of the functions shown.

For example, we create an updatable contract that is minteable, enumerable and ownable:

para_mandar
para_mandar1713×873 125 KB

The first question I have is about the need to import ERC721EnumerableUpgradeable.sol together with ERC721Upgradeable.sol.

The Enumerable contract already imports internally the ERC721Upgradeable, so it seems to me an unnecessary import.
In fact, if you ONLY import ERC721EnumerableUpgradeablel, the contract works correctly:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract MyToken is Initializable, ERC721EnumerableUpgradeable, OwnableUpgradeable {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

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

    function safeMint(address to, uint256 tokenId) public onlyOwner {
        _safeMint(to, tokenId);
    }

    // The following functions are overrides required by Solidity.

    function _update(address to, uint256 tokenId, address auth)
        internal
        override( ERC721EnumerableUpgradeable)
        returns (address)
    {
        return super._update(to, tokenId, auth);
    }

    function _increaseBalance(address account, uint128 value)
        internal
        override( ERC721EnumerableUpgradeable)
    {
        super._increaseBalance(account, value);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override( ERC721EnumerableUpgradeable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

The second question is somewhat similar. What is the need to override the functions _update, _increaseBalance ?

Especially because there is a comment that warns about it and I don't understand its necessity:

para
para856×330 14.2 KB

If I remove them, it compiles perfectly:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract MyToken is Initializable, ERC721EnumerableUpgradeable, OwnableUpgradeable {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

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

    function safeMint(address to, uint256 tokenId) public onlyOwner {
        _safeMint(to, tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override( ERC721EnumerableUpgradeable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

Obviously that doesn't mean I'm doing it right. And if you put a comment it must be for a reason unknown to me.
Would you be so kind as to explain to me the reasons for this required by solidity?

2

Seems like you've answered your own question, so just to make it explicit - if your source code compiles successfully after you remove an import statement, then that statement is (was) redundant.

This indeed looks like something that you can safely remove without any impact on the runtime behavior of your contract (well, it would actually reduce gas cost, though very slightly); perhaps the wizard creates these functions in order to make it easier for you to extend them according to your product requirements.

3

Thanks @barakman for your reply.

So this requirement mentioned here in the commentary is dispensable? Why do it then?

para
para856×330 14.2 KB

4

I understand that by selecting the ERC721 contract and then selecting the Enumerable, being an automatism, it adds the necessary library without eliminating the previous import, the first choice of ERC721.
Thus, it is explained, that two contracts are imported that in principle, if the EnumerableERC721 could be selected first, would not appear.

5

Not sure how to answer this question better than I already have:

6

Hey @Cainuriel

Seems like you've answered your own question, so just to make it explicit - if your source code compiles successfully after you remove an import statement, then that statement is (was) redundant.

I agree with this answer. The code provided by the wizard requires the overrides if there's a conflicting definition of a function. This is indeed redundant because ERC721Enumerable already uses ERC721, so it shouldn't change anything (perhaps just a bit) from the bytecode.

What's true though is that the Wizard shouldn't recommend using ERC721 if ERC721Enumerable already inherits from it.

So this requirement mentioned here in the commentary is dispensable? Why do it then?

It's only dispensable if you inherit only ERC721Enumerable, but not when you inherit both. The wizard detects these inheritance conflicts and fills the contract with the required overrides. Fixing this would not require fixing the override functions but fixing that the wizard recommends both ERC721 and ERC721Enumerable.

7

Hi @Cainuriel, to provide more context, Wizard adds ERC721 along with any extensions (such as ERC721Enumerable) as a best practice for clarity and maintainability of your contract, even if the extensions already inherit ERC721.

See reasoning here: https://github.com/OpenZeppelin/contracts-wizard/issues/101#issuecomment-1066978279

8

Thank you very much, this clears up all my doubts.