Storage gaps improperly defined
Description
Multiple contracts across the codebase are theoretically upgradable. All upgradable contracts require defining the gap array that allows allocating the storage safely in the future upgrades of the contracts, therefore eliminating the risk of storage collisions.
For example, the Competition contract defines the following storage persistent variables.
contract Modeler is IModeler, Initializable, PausableUpgradeable {
using SafeERC20 for IERC20;
uint256 private maxValidators;
IERC20 private validatorToken;
uint256 private validatorStakeAmount;
mapping(address => DataTypes.ValidatorData) public validators;
mapping(address => DataTypes.ModelerData) public modelers;
mapping(address => bool) public isValidatorRegistered;
mapping(address => DataTypes.ModelerChallenge) public modelerChallenges;
mapping(address => DataTypes.ModelerChallenge) public ZKMLChallenges;
mapping(address => mapping (address => uint256)) public futureRandSlots; // validator => modeler => randSlot
mapping (uint256 => DataTypes.DrandProof) public drands; // blockNumber => DrandProof
address[] public validatorAddresses;
address[] public modelerAddresses;
string public ipfsTestingDataset;
address private competitionContract;
Despite all of the variables occupying more than one storage slot, the gap
is set to 50, the default allocation as per the standard.
Instead, the storage gaps should be set up depending on how many actual slots are occupied. the Modeler contract uses 13 slots, so the gap would need to be defined as gap[50 - 13] = gap[37]
.
Impact
Using the storage gap in an incorrect way may lead to storage collisions down the line, which could severely hamper the security of the protocol as a whole.
Recommendations
We recommend properly defining the storage gaps to match the actual slots currently in use for each contract. The forge inspect
can be used to see the amount of storage that is currently in use. More can be read about using forge
here↗. Moreover, to test the validity of storage gaps in case of an upgrade, read this post↗.