search

OpenZeppelin / openzeppelin-contracts-upgradeable

Upgradeable variant of OpenZeppelin Contracts, meant for use in upgradeable contracts.
https://openzeppelin.com/contracts
MIT License
1k stars 436 forks source link

Missing __gap in Initializable #182

Closed gnarvaja closed 2 years ago

gnarvaja commented 2 years ago

The contract Initializable doesn't have a storage gap (__gap variable) as other similar contracts have. This means that if new attributes are added to this contract in the future, it will shift the storage layout of the inherited contracts, breaking upgrades.

Also, when inheriting from this contract, the storage layout of inherited contracts is no longer aligned with the convention of 50 slots for each contract in the inheritance chain.

You should add at the end of the contract something like this:

    /**
     * @dev This empty reserved space is put in place to allow future versions to add new
     * variables without shifting down storage in the inheritance chain.
     * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
     */
    uint256[49] private __gap;

💻 Environment

@openzeppelin/contracts-upgradeable == 4.7.3

📝 Details

You can check the storage layout in this picture: Selection_425

(More similar contract in https://github.com/ensuro/ensuro/pull/106#issue-1419895527)

Amxx commented 2 years ago

Hello,

This was already discussed many times, #111 being one of them.

TLDR: for some reason that we don't need to dive into, this has been the case for quite a while. Changing that today would be a breaking change. It would cause more issues then it would fix.