Putting `_disableInitializers();` in a parent constructor

Support Upgrades
1

Is there any foreseeable problem with putting the _disableInitializers(); call in a constructor of a parent instead of the most derived contract?

I want to offer an abstract contract template for people to implement that should be deployed through a proxy (UUPS, minimal proxy) and I want to prevent them from forgetting to disable the initializers on the implementation contract.

2

Hi OZ,

I wrote a contract that I offer to lots of users which they can inherit from. Basically, my contract is abstract contract. When people inherit from it, the idea is that their contract then should be deployed with proxy(uups, transparent or minimal). I have decided to do something in my contract.

abstract contract myOwn {
   
    constructor() {
         // Putting theses here so when users inherit from this contract, they don't
         // have to think about this(they might forget it in their overriden contract and pretty big problem.
         // So I do it here and everything is magically done. I need it to be disabled for extra safety.
         _disableInitializers();
    }
    // some functionality which really don't matter for this question.
}

// This will be deployed with `uups`, `transparent` or `minimal proxy`.
contract userContract is myOwn {

   
}

The only problem I found with the above is if I include disableInitializers in myOwn and people inherit from it, it wouldn't be possible for them to do:

abstract contract myOwn is Initializable {
    
    constructor() {
        _disableInitializers();
    }
}

contract userContract is myOwn {

    constructor() {
        initialize();
    }

    function initialize() public initializer {

    }

}

but it's debatabe whether the above is something they would normally do. could there be any other hidden problem ?

Thank you.

3

Is there any foreseeable problem with putting the _disableInitializers(); call in a constructor of a parent instead of the most derived contract?

This sounds reasonable. Just note that it would be slightly more inefficient if _disableInitializers(); was invoked multiple times (in the parent contract and derived contracts).

The only problem I found with the above is if I include disableInitializers in myOwn and people inherit from it, it wouldn't be possible for them to do:
...
constructor() {
initialize();
}

I would not recommend calling initializers from constructors, for several reasons:

  1. That would initialize the implementation contract itself, which may not be intended to be used directly by users. It could confuse users who might accidentally interact with the implementation instead of the proxy, both of which could have completely different states.
  2. The implementation itself is not upgradeable, so I'd argue users should NOT use it directly.
  3. Doing this for test purposes to test with the implementation by itself also does not realistically mimic behaviour through a proxy. Solidity takes care of automatically invoking the constructors of all ancestors of a contract. When writing an initializer, you need to take special care to manually call the initializers of all parent contracts. Calling initializers through constructors (for test purposes) could hide a bug in the initializers if you forgot to call a parent initializer.
1 Like
4

@ericglau Thanks.

Well, the Question now is why doesn't OZ include disableInitializers in the constructor of upgradeable contracts then to make sure we're all safe ? whats the reason behind it ?

If you could answer this, then I believe, the same answer would apply to our situation, because in the end, we're doing the same. We got our abstract contracts that users inherit from. Why not including disableInitializers in our contructors behind the hood so they don't have to worry about it ?

5

It would be very opinionated but it's something we could consider.

It's a breaking change so we wouldn't do it until a 5.0 release.

2 Likes