reinitializers call `_upgrade` function which is marked as `onlyInitializing`

[JorgeAtPaladin]

gif-next/contracts/upgradeability/Versionable.sol

Line 34 in 6319a56

_upgrade(data);

Seems like this flow is validating that the initializing is occuring twice. Looking at current OZ's implementation that's not a big deal as reinitizaling modifier sets initializing to true again, but I was concerned that this would revert. It might make sense to have the safeguard on the modifiers/outer facing functions and have the inner one without a check. Otherwise leaving this unchanged is fine as well.

[rapidddenis]

The reason why _upgrade() have onlyInitializing() modifier is from OZ's Initializable.sol:

/**
 * @dev Modifier to protect an initialization function so that it can only be invoked by functions with the
 * {initializer} and {reinitializer} modifiers, directly or indirectly.
 */
modifier onlyInitializing() {
    _checkInitializing();
    _;
}

[rapidddenis]

1. initializeVersionable() is missing comment for initialization in same tx, right after deployment (the way ProxyManager uses versionables) and/or access restriction
   

   gif-next/contracts/upgradeability/Versionable.sol

   Line 23 in 6319a56

   initializer

2. upgradeVersionable() is missing access restriction (must be restricted to deployer)
   

   gif-next/contracts/upgradeability/Versionable.sol

   Line 32 in 6319a56

   reinitializer(VersionLib.toUint64(getVersion()))

3. _initialize() implementations are using initializer() instead of onlyInitializing(). It works because called _initialize() is called in constructor.
   

   gif-next/contracts/instance/InstanceService.sol

   Line 426 in b7eedf9

   initializer()