Why we add `onlyInitializer` to internal functions?

Smart Contracts
1

q
I'm trying to understand and I'm stuck, can u explain logic pls?

For example I have some contracts and want to inherit them in one when deploying.

contract A {
1    function __A_init() internal {...}
2    function __A_init() internal onlyInitializer {...};

contract B {
1    function __B_init() internal {...}
2    function __B_init() internal onlyInitializer {...}

contract C is A, B {
    constructor() initializer {}
    function initialize(...) initializer {
        __A_init();
        __B_init();
    } 
}

I see codebases where devs use onlyInitializer modifier for init() functions in A and B contracts. But in some codebases devs don't use this modifier. Should we use it considering that init() functions are internal? What's the logic of this choice? I read docs/forum, but can catch it.

Also in some codebases in constructor ppl use disableInitializer() instead of initializer. I assume that in this case we want to leave the opportunity reinitialize in future?

2

Hi @poslednaya,

Have you gone through our Writing Upgradeable Contracts documentation? I think most of these questions can be answered from there.

3

yp, I read
In examples there initialize functions are public so modifier is obvious
But I can not imagine case when we need to use onlyInitializer with internal functions
Thx for answer, anyway

4

Right, all of the upgradeable contracts have an internal initializer that replaces the constructor for upgradeable contracts. However, when a contract is abstract (the majority of contracts in the library are abstract), the constructor behaves like an "internal initializer", this is why you need to do something like:

abstract contract A {
  constructor(uint256 foo, bytes32 bar) {}
}

contract B is A {
  constructor(uint foo, bytes32 bar) A(foo, bar) {}
}

The upgradeable variant of the library exposes these internal initializers to replicate the behavior of abstract constructors.

5

I got it. Sorry for inability to formulate a question. I'm looking at the code base right now. They have 2 contracts (not abstract) and then inherits in third. And I can not catch why absence of onlyInitializing is not a mistake. They literally rely on just internal, but in all examples in OpenZeppelin docs I see that modifier also is needed.

contract A {
    function __A_init() internal {...} // here without modifier

contract B {
    function __B_init() internal {...} // here without modifier

contract C is A, B {
    function initialize(...) initializer {
        __A_init();
        __B_init();
    }

I think I got how modifier works but can not understand why here it is not a mistake

6

I see, that's weird. Unless those internal functions are not reachable within the external contract scope I guess it's fine, but I can't confirm it's safe to do so.

The recommendation is to always use an onlyInitializer even if there's the need to reinitialize a contract. In such case, the onlyInitializer already takes reinitializations into account.

1 Like
7

It would be better if this was a critical vulnerability and I shared the reward with you, but..)
Thx a lot for your time, appreciate it, now it will more clear to me

1 Like