fix: add storage gaps to GraphTokenGateway and GraphTokenUpgradeable (C4 M-244)

[pcarranzav]

Resolves code-423n4/2022-10-thegraph-findings#244

[codecov]

Codecov Report

Base: 91.58% // Head: 91.58% // No change to project coverage 👍

Coverage data is based on head (fa52952) compared to base (2dfacb6).
Patch has no changes to coverable lines.

Additional details and impacted files

@@                    Coverage Diff                     @@
##           pcv/700-699-n01-gas-check     #739   +/-   ##
==========================================================
  Coverage                      91.58%   91.58%           
==========================================================
  Files                             42       42           
  Lines                           1996     1996           
  Branches                         356      356           
==========================================================
  Hits                            1828     1828           
  Misses                           168      168           

Flag	Coverage Δ
unittests	91.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
contracts/gateway/GraphTokenGateway.sol	100.00% <ø> (ø)
contracts/l2/token/GraphTokenUpgradeable.sol	100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[abarmat]

We could just keep the gap at 50 until an upgrade happens. But it is nothing critical

[pcarranzav]

yeah, I chose 47 so that for both contracts we use a standard number of total used slots, i.e. 50

[trust1995]

Consider futureproofing and lining up the Managed.sol to be a 50 slot contract as well.
Same goes for Pausable, Governed contracts.
Without adding a [50] __gap slot for each base contract, that component will not be upgradeable.

[pcarranzav]

Consider futureproofing and lining up the Managed.sol to be a 50 slot contract as well.
Same goes for Pausable, Governed contracts.
Without adding a [50] __gap slot for each base contract, that component will not be upgradeable.

It's a good call, but those other contracts are used by several protocol contracts that are already deployed, so adding a storage gap would break updates in all of those. Luckily the scope for Managed, Pausable and Governed is limited and the behavior is well-defined, so we don't expect to need to add any storage variables to those.