From 12d42924fd2cbb0d1994a8bac6966a75b6a29aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 10 Oct 2018 11:16:06 -0300 Subject: [PATCH] Added a storage gap at the end of Initializable's storage layout. (#215) * Added a storage gap at the end of Initializable's storage layout. * Fixed storage layout test. --- packages/lib/contracts/Initializable.sol | 9 ++++++--- .../upgradeability/AdminUpgradeabilityProxy.test.js | 8 ++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/lib/contracts/Initializable.sol b/packages/lib/contracts/Initializable.sol index 6fd2d786c..26f6d3d76 100644 --- a/packages/lib/contracts/Initializable.sol +++ b/packages/lib/contracts/Initializable.sol @@ -10,7 +10,7 @@ pragma solidity ^0.4.24; * invoked. This applies both to deploying an Initializable contract, as well * as extending an Initializable contract via inheritance. * WARNING: When used with inheritance, manual care must be taken to not invoke - * a parent initializer twice, or ensure that all initializers are idempotent, + * a parent initializer twice, or ensure that all initializers are idempotent, * because this is not dealt with automatically as with constructors. */ contract Initializable { @@ -42,7 +42,7 @@ contract Initializable { /// @dev Returns true if and only if the function is running in the constructor function isConstructor() private view returns (bool) { - // extcodesize checks the size of the code stored in an address, and + // extcodesize checks the size of the code stored in an address, and // address returns the current address. Since the code is still not // deployed when running a constructor, any checks on its code size will // yield zero, making it an effective way to detect if a contract is @@ -51,4 +51,7 @@ contract Initializable { assembly { cs := extcodesize(address) } return cs == 0; } -} \ No newline at end of file + + // Reserved storage space to allow for layout changes in the future. + uint256[50] private ______gap; +} diff --git a/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.test.js b/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.test.js index 5a50b657a..20105877f 100644 --- a/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.test.js +++ b/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.test.js @@ -130,8 +130,12 @@ contract('AdminUpgradeabilityProxy', ([_, admin, anotherAccount]) => { }) it('uses the storage of the proxy', async function () { - // fetch the x value of Migratable at position 0 of the storage - const storedValue = await Proxy.at(this.proxyAddress).getStorageAt(1); + // storage layout should look as follows: + // - 0: Initializable storage + // - 1-50: Initailizable reserved storage (50 slots) + // - 51: initializerRan + // - 52: x + const storedValue = await Proxy.at(this.proxyAddress).getStorageAt(52); storedValue.should.be.bignumber.eq(42); }) })