Add docstring for the gap variable

[come-maiz]

Contracts have an undocumented uint256[50] private ______gap;

This variable should be documented with a very clear explanation, because it's weird.

[nneverlander]

What is this variable used for?

[nventuro]

@nevverlander Since OpenZeppelin contracts are used by inheritance, user-defined variables will be placed by the compiler after OpenZeppelin's ones. If, in a newer version, new variables are added by the library, the storage layouts would be incompatible, and an upgrade would not be possible.

The gap is a workaround to that issue: by leaving a 50-slot gap, we're able to increase the contract's storage by that amount (provided we also remove the same slots from the gap) with no clashing issues.

[ppoliani]

May I ask what would happen is some of the base contracts modify the layout e.g. adding a new field. Should we still keep the gap of 50 slot or should it be 50 -

For example, imagine the following hierarchy]

abstract contract Base1 {
  uint256 val1;

  uint256[50] private ______gap;
}

abstract contract Base2 {
  uint256 val2;
  uint256[50] private ______gap;
}

contract Child is Base1, Base2 {

}

Then at some point we decide to update the Base1 by adding a new fields val3

abstract contract Base1V2 {
  uint256 val1;
  uint256 val3;

  uint256[50] private ______gap;
}

abstract contract Base2 {
  uint256 val2;
  uint256[50] private ______gap;
}

contract Child is Base1V2, Base2 {}

Should we still keep uint256[50] private ______gap; in Base1V2 or should it become uint256[49] private ______gap;? Basically, will the val3 override val2 from Base2 if we keep the gaps of 50 slots?

[abcoathup]

Hi @ppoliani,

If you add a new state variable to Base1V2, then you should reduce the reserved storage gap to prevent the "shifting down" of all of the state variables below in the inheritance chain.

Please see the documentation for how storage gaps are used in OpenZeppelin Contracts: https://docs.openzeppelin.com/contracts/3.x/upgradeable#storage_gaps

If you have further questions, you can ask in the community forum: https://forum.openzeppelin.com/

[alexon1234]

Hi,

I am also wondering why it has to be an array of 50? Why not 40 or 30?

Thanks

[ppoliani]

@alexon1234 I'm not sure either maybe @abcoathup can correct me, but since the number of slots does not cause any overhead then something safe as 50 might not be a bad figure to use

[frangio]

The number 50 seemed enough. There is a very small overhead in code size because slot numbers will be slightly larger.

[GhasemiReza21]

Hi @abcoathup
when I added a new state variable to Base contracts and reduce the reserved storage gap and try to upgrade I get this error :
Bad array resize from 47 to 46 ,
New variables should be placed after all existing inherited variables

[frangio]

Fixed in OpenZeppelin/openzeppelin-transpiler#73.

---

@GhasemiReza21 Reducing gaps is not yet supported in the Upgrades Plugins but we're adding this in the next few weeks. For now you need to resolve this issue manually, if you need help please post in the OpenZeppelin Forum.