Why keep "__Ownable_init" beside "__Ownable_init_unchained"?

Support Upgrades
1

Hi, I was looking at the __Ownable_init function defined in the OwnableUpgradeable contract, and I couldn't understand why it is needed at all.

As far as I can tell, only the __Ownable_init_unchained method is called in the other contracts that inherit from OwnableUpgradeable in this repository.

So why keep the other method in the contract? Backwards compatibility?

1 Like
2

Hi @PaulRBerg, the _init functions have linearized calls to parent initializers, while _init_unchained functions do not. There is some documentation about this here: https://docs.openzeppelin.com/contracts/4.x/upgradeable#multiple-inheritance

In the case of OwnableUpgradable, there just happens to be no parent initializers to call. But the expected convention is for developers to just use the _init functions in the libraries that they are using directly, and those will behave similar to constructors.

1 Like
3

Great explanation, thanks @ericglau.

Now, just to double-check that I got this right. If OwnableUpgradeable had a parent initializer to call, it would call the _init_unchained function of that hypothetical parent contract, wouldn't it?

1 Like
4

Yes, the _init functions internally call _init_unchained functions.

5

Hey @ericglau, I think I understand the paragraph in the link conceptually, but I don't find an example that _init does call parents' initializers. For example, for this line in the contract AccessControlUpgradeable, the __AccessControl_init function does not call any other _init functions. Am I missing anything? Thanks.

6

Hi @maxareo, here is an example where an _init function calls _init_unchained. In this example it calls it's parent's parent's _init_unchained and then its own.

7

Thanks @ericglau. It seems a bit arbitrary to me. Is there any rules with _init functions? Please take a look at the snippet below. Should the _init functions for all inherited contracts be included in the initialize() function?

contract UpgradeableTestToken is ContextUpgradeable, OwnableUpgradeable, UUPSUpgradeable, ERC20Upgradeable {
    function initialize() external initializer {
        __Ownable_init_unchained();
        __UUPSUpgradeable_init();
        __ERC20_init("Test Token2", "TT2");
    }

    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
}
8

Would also like to expand this discussion to the DiamondStorage equivalent (which will be used in 5.x I believe).

With this pattern, there is also no constructor, so initialize needs to be called manually. So if you wanted to make an upgradable Governor, would this be the appropriate thing to do:

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.9;

import {ERC20Upgradeable} from "@gnus.ai/contracts-upgradeable-diamond/token/ERC20/ERC20Upgradeable.sol";
import {ERC20PermitUpgradeable} from "@gnus.ai/contracts-upgradeable-diamond/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";
import {ERC20VotesUpgradeable} from "@gnus.ai/contracts-upgradeable-diamond/token/ERC20/extensions/ERC20VotesUpgradeable.sol";

/// @dev    EIP-20 token name: "FUNToken"
/// @dev    EIP-20 token symbol: "FUN"
/// @dev    EIP-20 token decimals: 18
contract FUNTokenFacet is ERC20Upgradeable, ERC20PermitUpgradeable, ERC20VotesUpgradeable {
    function initialize() public payable {
        __ERC20_init("FUNToken", "FUN");
        __ERC20Permit_init("FUNToken");
        __ERC20Votes_init_unchained(); // Do we need this line?
    }

    function _afterTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal override(ERC20Upgradeable, ERC20VotesUpgradeable) {
        super._afterTokenTransfer(from, to, amount);
    }

    function _mint(address to, uint256 amount)
        internal
        override(ERC20Upgradeable, ERC20VotesUpgradeable)
    {
        super._mint(to, amount);
    }

    function _burn(address account, uint256 amount)
        internal
        override(ERC20Upgradeable, ERC20VotesUpgradeable)
    {
        super._burn(account, amount);
    }
}

But it's also really not clear if __ERC20Votes_init_unchained() is needed here so would be great to get guidance there.

9

In general you should only need to call the _init functions, not _init_unchained. We recommend to call the _init functions of all direct parents. So in your example, use __Ownable_init()

1 Like
10

In this case that function is empty so it doesn't do anything.

If using openzeppelin-contracts-upgradeable, you could call __ERC20Votes_init() (which also doesn't do anything in this case either, but is provided for consistency)

1 Like
11

Thanks @ericglau. It would be great if OZ can make a blog post on this topic. Guidelines and advices on best practices given in an organized and systematic way can benefit the community greatly.

4 Likes
12

If he doesn't use it, what can happen?

13

@Mylifechangefast.eth The need for this is explained in the second comment. If you don't call _init functions, then parent contracts would not be initialized.

14

5 posts were split to a new topic: How to use Pausable in upgradeble contracts