How do you implement storage gap for abstract inherited contracts?
Do I need to inherit upgradable OZ contracts?
Do I need storage gap for each abstract contract?
Do I need storage gap for contract?
Do I need to change the storage gap for contract to__gap[48]?
Since contractB inherit contractA gap, does this mean that contractB has 100 gap?
Since contracC inherit contractB gap, does this mean that contractC has 100 gap?
Since I'm using immutable for contractA, does this means that I do not need to update the storage gap?
Aside from storage gap, apparently, the contract is still upgradable, I tested it with admin.upgrade(proxy, address(contract)); How is this possible?
Code to reproduce
This is my implementation.
abstract contractA is
Initializable,
ContextUpgradeable,
{
using AddressUpgradeable for address;
/// @custom:oz-upgrades-unsafe-allow state-variable-immutable
address public immutable protocol;
/// @custom:oz-upgrades-unsafe-allow state-variable-immutable
address public immutable registry;
/// @custom:oz-upgrades-unsafe-allow constructor
constructor(address _protocolAddress, address _RegistryAddress) {
protocol= _protocolAddress;
registry= _RegistryAddress;
}
uint256[50] private __gap;
-- SNIP--
}
abstract contract contractB is contractA {
/// @custom:oz-upgrades-unsafe-allow constructor
constructor(address _protocolAddress, address _RegistryAddress)
contractA(_protocolAddress, _RegistryAddress)
{}
uint256[50] private __gap;
}
contract contractC is contractB {
constructor(address _protocolAddress, address _Registry)
contractB(_protocolAddress, _Registry)
{}
How do you implement storage gap for abstract inherited contracts?
For more info on storage gaps, see https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#storage-gaps
For real examples, see https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/tree/v4.9.6/contracts
Do I need to inherit upgradable OZ contracts?
That is recommended, but you still need to manage either gaps or namespaced storage in your own contracts if you want to be able to make changes to your contracts during upgrades if they are inherited by another contract.
Do I need storage gap for each abstract contract?
Do I need storage gap for contract?
Same as above.
Do I need to change the storage gap for contract to__gap[48]?
Keeping a consistent number of total slots for each contract makes it easier to track.
Since contractB inherit contractA gap, does this mean that contractB has 100 gap?
Yes. But if you add variables in contractB before the gap, then the gaps would not be contiguous.
Since contracC inherit contractB gap, does this mean that contractC has 100 gap?
Yes.
Since I'm using immutable for contractA, does this means that I do not need to update the storage gap?
Immutable variables do not consume storage slots.
Aside from storage gap, apparently, the contract is still upgradable, I tested it with admin.upgrade(proxy, address(contract)); How is this possible?
How are you performing the upgrade? If you are upgrading via the contract directly, then it will let you upgrade even if it's unsafe! You should use the Upgrades Plugins which checks for storage layout compatibility.
2 Likes
For each contract that implement __gap[50] it is only affects that particular contract right? If that contract were to add a new variable you have to reduce to 49 then upgrade it, am I right?
So if contractB was to inherit the new contract, does it need to reduce the gap too?
How did contractC have 100 gap when you didn't really increment it though?
Since contractA and contractB has implement a gap does contractC require to implement a gap? Or it will be known that since it inherit it will automatically have 100 gap
Lastly, why is the contract upgradeable when I didn't inherit a ownable upgradeble? or is it because of the TransparentUpgradeableProxy deployed? and the contractB is the implementation hence able to call .upgrade?
For each contract that implement __gap[50] it is only affects that particular contract right?
It affects that contract and all contracts which inherit it.
If that contract were to add a new variable you have to reduce to 49 then upgrade it, am I right?
Generally yes, but keep in mind that your variables may be packed tightly depending on their type. See https://docs.soliditylang.org/en/latest/internals/layout_in_storage.html
So if contractB was to inherit the new contract, does it need to reduce the gap too?
If contractB did not inherit a specific contract before, then inherits it for an upgrade, then yes it needs to reduce its gap corresponding to the amount of storage slots used by variables in the newly inherited contract.
How did contractC have 100 gap when you didn't really increment it though?
C inherits B inherits A. B and A each use 50 slots.
Since contractA and contractB has implement a gap does contractC require to implement a gap? Or it will be known that since it inherit it will automatically have 100 gap
If there might be another contract D that will inherit C, then it would be useful to also have a gap in C if you want the flexibility to add variables in C later.
Lastly, why is the contract upgradeable when I didn't inherit a ownable upgradeble? or is it because of the TransparentUpgradeableProxy deployed? and the contractB is the implementation hence able to call .upgrade?
If you are using TransparentUpgradeableProxy, then the owner of the ProxyAdmin can upgrade the proxy.
1 Like
Hi, I got another question,
Contract A has __gap[50]
Contract B is an abstract contract and has __gap[50], inherits A
Contract C does not implement __gap[50], inherits B
Contract D has __gap[50], inherits C
Will there be any problems for Contract C if
- Upgrade in the future with new variables for Contract A
- Upgrade in the future with new variables for Contract B
- Upgrade in the future with new variables for Contract D
Hence is it must for Contract C to implement __gap[50]?
From the previous conversation:
If Contract A upgrades and add a uint and __gap[50] will be reduced to __gap[49], does Contract B and D require the gap to reducted to __gap[49] too?
In each of these cases, if you add new variables and reduce the gaps by a corresponding amount, then it won't affect the contracts that inherit it.
Hence is it must for Contract C to implement __gap[50]?
I don't think this is a must. But doing so will allow you to add variables in contract C without affecting the layout of contract D.
From the previous conversation:
If Contract A upgrades and add a uint and __gap[50] will be reduced to __gap[49], does Contract B and D require the gap to reducted to __gap[49] too?
No. In this case, only reduce the gap in Contract A to __gap[49].
What if C has a state variable that is neither constant nor immutable. Doesn't upgrading in A or B corrupt state variable in C?
If you adjust the gap sizes in A or B, then the state variable in C would still be at the same slot. That is the purpose of using gaps.
1 Like
Can you check whether this is a problem? I'm confused.
What you linked is similar to the examples that you described above.
For parent contracts that have gaps, you can add variables in those contracts (e.g. Contract A or B above, or BaseStrategyVault or BalancerStrategyBase in the link) and reduce their gaps, without affecting the layouts of child contracts.
For parent contracts that don't have gaps (Contract C above, or Boosted3TokenPoolMixin , MetaStable2TokenVaultMixin , TwoTokenPoolMixin , PoolMixin , AuraStakingMixin and BalancerOracleMixin in the link), adding variables to those during upgrades is unsafe, because it would corrupt the layouts of child contracts.
1 Like
Oh it makes sense now, thanks so much for your time!
1 Like