Initializer/reinitializer advice

Support Upgrades
1

OZ recommends that for upgradeable contracts, one must specify initializer on the initialize function and for every upgrade, we add initializeV2, initializeV3, and so on. This means that if the user wants to install the very latest version, he must call these in order:

  • first call initialize
  • then call initializeV2
  • then call initializeV3.

This is not doable for me due to some problems it raises. So I thought what if I always have 2 functions only. initialize and initializeFrom(uint8 prevVersion, bytes memory data) and depending on what prevVersion is, I will decode data accordingly and call appropriate internal functions.

Going with this way, i can not use initializer anymore on initialize function, because if I do so, then initializeFrom that will have reinitializer(x) will also be callable. So I thought to always add reinitializer with the same number to both initialize and initializeFrom functions. The only downside is that one can even call initialize on the upgrade as well. So I thought to do:

 modifier InitializationOnlyOnce() {
        if(_getInitializedVersion() != 0) {
            revert AlreadyInitialized();
        }
        _;
    }

    function initialize() external InitializationOnlyOnce reinitializer(x) {}

    function initializeFrom(
        uint16 _fromBuild,
        bytes calldata _initData
    ) external reinitializer(x) {}

Note that x are the same always. I think this solves everything and was wondering if OZ sees any attack scenario here. I'd appreciate your inputs.

2

It looks like you are trying to prevent initializing from the wrong version (in the initialize case, from any version). However, the initializeFrom function would still have this issue, for example if you incorrectly called initializeFrom(0, 0) during an upgrade on a proxy which was already initialized.

Perhaps you could simplify this by always checking the current version from _getInitializedVersion() and call appropriate internal functions based on that. In this case, you may not even need the uint16 _fromBuild parameter if you can detect the current version internally. I haven't tested this out though, and you should consider and fully test whether this works for your use case.

3

This is actually an amazing feedback. Thanks so much.

One caveat is that your modifier reinitializer is not virtual and problem i have is i need to do something like:

uint8 fromVersion = _getInitializedVersion() - 1;

I need to know from which version it's upgrading from, but with the above, caveat is that what if in the first contract, reinitializer was set to (2) and in the next, it was set to 10. Then, fromVersion would be as 9 and my checks for sure wouldn't account for 9.

Just wondering how to get the actual/100% correct prevVersion it's upgrading from. But I believe, with upgradeable contracts, we follow so many rules, so one extra rule where reinitializer(x) (x values must always increase by 1) can also be followed/obeyed.

4

If you call _getInitializedVersion() (for example in a separate modifier) before the initializer or reinitializer modifier, then that should be the previous version it's upgrading from. But I don't see a way to get that value into the function body without using an extra storage variable.

1 Like