Move AragonApp to unstructured storage (#376)

contracts/acl/ACL.sol

@@ -60,7 +60,7 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     */
     function initialize(address _permissionsCreator) public onlyInit {
         initialized();
-        require(msg.sender == address(kernel));
+        require(msg.sender == address(kernel()));
 
         _createPermission(_permissionsCreator, this, CREATE_PERMISSIONS_ROLE, _permissionsCreator);
     }

contracts/apm/APMRegistry.sol

@@ -39,7 +39,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
         registrar.pointRootNode(this);
 
         // Check APM has all permissions it needss
-        ACL acl = ACL(kernel.acl());
+        ACL acl = ACL(kernel().acl());
         require(acl.hasPermission(this, registrar, registrar.CREATE_NAME_ROLE()));
         require(acl.hasPermission(this, acl, acl.CREATE_PERMISSIONS_ROLE()));
     }
@@ -73,7 +73,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
         repo.newVersion(_initialSemanticVersion, _contractAddress, _contentURI);
 
         // Give permissions to _dev
-        ACL acl = ACL(kernel.acl());
+        ACL acl = ACL(kernel().acl());
         acl.revokePermission(this, repo, repo.CREATE_VERSION_ROLE());
         acl.grantPermission(_dev, repo, repo.CREATE_VERSION_ROLE());
         acl.setPermissionManager(_dev, repo, repo.CREATE_VERSION_ROLE());
@@ -85,7 +85,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
 
         Repo repo = newClonedRepo();
 
-        ACL(kernel.acl()).createPermission(_dev, repo, repo.CREATE_VERSION_ROLE(), _dev);
+        ACL(kernel().acl()).createPermission(_dev, repo, repo.CREATE_VERSION_ROLE(), _dev);
 
         // Creates [name] subdomain in the rootNode and sets registry as resolver
         // This will fail if repo name already exists
@@ -97,7 +97,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
     }
 
     function newClonedRepo() internal returns (Repo) {
-        return Repo(newAppProxy(kernel, repoAppId()));
+        return Repo(newAppProxy(kernel(), repoAppId()));
     }
 
     function repoAppId() internal view returns (bytes32) {

contracts/apps/AppProxyBase.sol

@@ -3,6 +3,7 @@ pragma solidity 0.4.18;
 import "./AppStorage.sol";
 import "../common/DepositableDelegateProxy.sol";
 import "../kernel/KernelStorage.sol";
+import "../kernel/IKernel.sol";
 
 
 contract AppProxyBase is AppStorage, DepositableDelegateProxy, KernelConstants {
@@ -13,14 +14,14 @@ contract AppProxyBase is AppStorage, DepositableDelegateProxy, KernelConstants {
     * @param _initializePayload Payload for call to be made after setup to initialize
     */
     function AppProxyBase(IKernel _kernel, bytes32 _appId, bytes _initializePayload) public {
-        kernel = _kernel;
-        appId = _appId;
+        setKernel(_kernel);
+        setAppId(_appId);
 
         // Implicit check that kernel is actually a Kernel
         // The EVM doesn't actually provide a way for us to make sure, but we can force a revert to
         // occur if the kernel is set to 0x0 or a non-code address when we try to call a method on
         // it.
-        address appCode = getAppBase(appId);
+        address appCode = getAppBase(_appId);
 
         // If initialize payload is provided, it will be executed
         if (_initializePayload.length > 0) {
@@ -32,6 +33,6 @@ contract AppProxyBase is AppStorage, DepositableDelegateProxy, KernelConstants {
     }
 
     function getAppBase(bytes32 _appId) internal view returns (address) {
-        return kernel.getApp(keccak256(APP_BASES_NAMESPACE, _appId));
+        return kernel().getApp(keccak256(APP_BASES_NAMESPACE, _appId));
     }
 }

contracts/apps/AppProxyPinned.sol

@@ -1,9 +1,15 @@
 pragma solidity 0.4.18;
 
+import "../common/UnstructuredStorage.sol";
 import "./AppProxyBase.sol";
 
 
 contract AppProxyPinned is AppProxyBase {
+    using UnstructuredStorage for bytes32;
+
+    // keccak256("aragonOS.appStorage.pinnedCode"), used by Proxy Pinned
+    bytes32 internal constant PINNED_CODE_POSITION = 0xdee64df20d65e53d7f51cb6ab6d921a0a6a638a91e942e1d8d02df28e31c038e;
+
     /**
     * @dev Initialize AppProxyPinned (makes it an un-upgradeable Aragon app)
     * @param _kernel Reference to organization kernel for the app
@@ -14,15 +20,15 @@ contract AppProxyPinned is AppProxyBase {
         AppProxyBase(_kernel, _appId, _initializePayload)
         public // solium-disable-line visibility-first
     {
-        pinnedCode = getAppBase(appId);
-        require(pinnedCode != address(0));
+        setPinnedCode(getAppBase(_appId));
+        require(pinnedCode() != address(0));
     }
 
     /**
      * @dev ERC897, the address the proxy would delegate calls to
      */
     function implementation() public view returns (address) {
-        return pinnedCode;
+        return pinnedCode();
     }
 
     /**
@@ -31,4 +37,12 @@ contract AppProxyPinned is AppProxyBase {
     function proxyType() public pure returns (uint256 proxyTypeId) {
         return FORWARDING;
     }
+
+    function setPinnedCode(address _pinnedCode) internal {
+        PINNED_CODE_POSITION.setStorageAddress(_pinnedCode);
+    }
+
+    function pinnedCode() internal view returns (address) {
+        return PINNED_CODE_POSITION.getStorageAddress();
+    }
 }

contracts/apps/AppProxyUpgradeable.sol

@@ -21,7 +21,7 @@ contract AppProxyUpgradeable is AppProxyBase {
      * @dev ERC897, the address the proxy would delegate calls to
      */
     function implementation() public view returns (address) {
-        return getAppBase(appId);
+        return getAppBase(appId());
     }
 
     /**

contracts/apps/AppStorage.sol

@@ -4,14 +4,33 @@
 
 pragma solidity ^0.4.18;
 
+import "../common/UnstructuredStorage.sol";
 import "../kernel/IKernel.sol";
 
 
 contract AppStorage {
-    IKernel public kernel;
-    bytes32 public appId;
-    address internal pinnedCode; // used by Proxy Pinned
-    uint256 internal initializationBlock; // used by Initializable
-    uint256[95] private storageOffset; // forces App storage to start at after 100 slots
-    uint256 private offset;
+    using UnstructuredStorage for bytes32;
+
+    // keccak256("aragonOS.appStorage.kernel")
+    bytes32 internal constant KERNEL_POSITION = 0x4172f0f7d2289153072b0a6ca36959e0cbe2efc3afe50fc81636caa96338137b;
+    // keccak256("aragonOS.appStorage.appId")
+    bytes32 internal constant APP_ID_POSITION = 0xd625496217aa6a3453eecb9c3489dc5a53e6c67b444329ea2b2cbc9ff547639b;
+    // keccak256("aragonOS.appStorage.pinnedCode"), used by Proxy Pinned
+    bytes32 internal constant PINNED_CODE_POSITION = 0xdee64df20d65e53d7f51cb6ab6d921a0a6a638a91e942e1d8d02df28e31c038e;
+
+    function kernel() public view returns (IKernel) {
+        return IKernel(KERNEL_POSITION.getStorageAddress());
+    }
+
+    function appId() public view returns (bytes32) {
+        return APP_ID_POSITION.getStorageBytes32();
+    }
+
+    function setKernel(IKernel _kernel) internal {
+        KERNEL_POSITION.setStorageAddress(address(_kernel));
+    }
+
+    function setAppId(bytes32 _appId) internal {
+        APP_ID_POSITION.setStorageBytes32(_appId);
+    }
 }

contracts/apps/AragonApp.sol

@@ -40,7 +40,7 @@ contract AragonApp is AppStorage, Initializable, ACLSyntaxSugar, VaultRecoverabl
                 mstore(how, byteLength)
             }
         }
-        return address(kernel) == 0 || kernel.hasPermission(_sender, address(this), _role, how);
+        return address(kernel()) == address(0) || kernel().hasPermission(_sender, address(this), _role, how);
     }
 
     /**
@@ -49,7 +49,7 @@ contract AragonApp is AppStorage, Initializable, ACLSyntaxSugar, VaultRecoverabl
     */
     function getRecoveryVault() public view returns (address) {
         // Funds recovery via a vault is only available when used with a kernel
-        require(address(kernel) != 0);
-        return kernel.getRecoveryVault();
+        require(address(kernel()) != address(0));
+        return kernel().getRecoveryVault();
     }
 }

contracts/common/Initializable.sol

@@ -4,32 +4,38 @@
 
 pragma solidity ^0.4.18;
 
+import "./UnstructuredStorage.sol";
 import "../apps/AppStorage.sol";
 import "../common/TimeHelpers.sol";
 
 
-contract Initializable is AppStorage, TimeHelpers {
+contract Initializable is TimeHelpers {
+    using UnstructuredStorage for bytes32;
+
+    // keccak256("aragonOS.initializable.initializationBlock")
+    bytes32 internal constant INITIALIZATION_BLOCK_POSITION = 0xebb05b386a8d34882b8711d156f463690983dc47815980fb82aeeff1aa43579e;
+
     modifier onlyInit {
-        require(initializationBlock == 0);
+        require(getInitializationBlock() == 0);
         _;
     }
 
     modifier isInitialized {
-        require(initializationBlock > 0);
+        require(getInitializationBlock() > 0);
         _;
     }
 
     /**
     * @return Block number in which the contract was initialized
     */
     function getInitializationBlock() public view returns (uint256) {
-        return initializationBlock;
+        return INITIALIZATION_BLOCK_POSITION.getStorageUint256();
     }
 
     /**
     * @dev Function to be called by top level contract after initialization has finished.
     */
     function initialized() internal onlyInit {
-        initializationBlock = getBlockNumber();
+        INITIALIZATION_BLOCK_POSITION.setStorageUint256(getBlockNumber());
     }
 }

contracts/common/TimeHelpers.sol

@@ -34,7 +34,7 @@ contract TimeHelpers {
     *      tests.
     */
     function getTimestamp() internal view returns (uint256) {
-        return now;
+        return now; // solium-disable-line security/no-block-members
     }
 
     /**

contracts/common/UnstructuredStorage.sol

@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identitifer:    MIT
+ */
+
+pragma solidity ^0.4.18;
+
+
+library UnstructuredStorage {
+    function getStorageAddress(bytes32 position) internal view returns (address data) {
+        assembly { data := sload(position) }
+    }
+
+    function getStorageBytes32(bytes32 position) internal view returns (bytes32 data) {
+        assembly { data := sload(position) }
+    }
+
+    function getStorageUint256(bytes32 position) internal view returns (uint256 data) {
+        assembly { data := sload(position) }
+    }
+
+    function setStorageAddress(bytes32 position, address data) internal {
+        assembly { sstore(position, data) }
+    }
+
+    function setStorageBytes32(bytes32 position, bytes32 data) internal {
+        assembly { sstore(position, data) }
+    }
+
+    function setStorageUint256(bytes32 position, uint256 data) internal {
+        assembly { sstore(position, data) }
+    }
+}

contracts/evmscript/EVMScriptRunner.sol

@@ -36,7 +36,7 @@ contract EVMScriptRunner is AppStorage, EVMScriptRegistryConstants {
     }
 
     function getExecutorRegistry() internal view returns (IEVMScriptRegistry) {
-        address registryAddr = kernel.getApp(EVMSCRIPT_REGISTRY_APP);
+        address registryAddr = kernel().getApp(EVMSCRIPT_REGISTRY_APP);
         return IEVMScriptRegistry(registryAddr);
     }
 
@@ -58,10 +58,10 @@ contract EVMScriptRunner is AppStorage, EVMScriptRegistryConstants {
     }
 
     modifier protectState {
-        address preKernel = kernel;
-        bytes32 preAppId = appId;
+        address preKernel = address(kernel());
+        bytes32 preAppId = appId();
         _; // exec
-        require(kernel == preKernel);
-        require(appId == preAppId);
+        require(address(kernel()) == preKernel);
+        require(appId() == preAppId);
     }
 }

test/keccak_constants.js

@@ -83,4 +83,18 @@ contract('Constants', accounts => {
 
     assert.equal(await repo.CREATE_VERSION_ROLE(), await keccakConstants.CREATE_VERSION_ROLE(), "create version role doesn't match")
   })
+
+  it('checks AppStorage unstructured storage constants', async () => {
+    const app = await getContract('AppStubStorage').new()
+
+    assert.equal(await app.getKernelPosition(), await keccakConstants.kernelPosition(), "kernelPosition doesn't match")
+    assert.equal(await app.getAppIdPosition(), await keccakConstants.appIdPosition(), "appIdPosition doesn't match")
+    assert.equal(await app.getInitializationBlockPosition(), await keccakConstants.initializationBlockPosition(), "initializationBlockPosition doesn't match")
+  })
+
+  it('checks AppProxyPinned unstructured storage constants', async () => {
+    const app = await getContract('AppStubPinnedStorage').new()
+
+    assert.equal(await app.getPinnedCodePosition(), await keccakConstants.pinnedCodePosition(), "pinnedCodePosition doesn't match")
+  })
 })

test/mocks/AppStubPinnedStorage.sol

@@ -0,0 +1,33 @@
+pragma solidity 0.4.18;
+
+import "../../contracts/apps/AppProxyPinned.sol";
+import "../../contracts/kernel/IKernel.sol";
+
+
+contract AppStubPinnedStorage is AppProxyPinned {
+    bytes32 constant FAKE_APP_ID = keccak256('FAKE_APP_ID');
+    address constant FAKE_APP_ADDR = address(1);
+
+    // Allow the mock to be created without any arguments
+    function AppStubPinnedStorage()
+        AppProxyPinned(IKernel(0), FAKE_APP_ID, new bytes(0))
+        public // solium-disable-line visibility-first
+    {}
+
+    // Overload base to return our own fake address
+    function getAppBase(bytes32 _appId) internal view returns (address) {
+        return FAKE_APP_ADDR;
+    }
+
+    function setPinnedCodeExt(address _pinnedCode) public {
+        setPinnedCode(_pinnedCode);
+    }
+
+    function getPinnedCodePosition() public view returns (bytes32) {
+        return PINNED_CODE_POSITION;
+    }
+
+    function pinnedCodeExt() public view returns (address) {
+        return pinnedCode();
+    }
+}

test/mocks/AppStubStorage.sol

@@ -0,0 +1,30 @@
+pragma solidity 0.4.18;
+
+import "../../contracts/apps/AragonApp.sol";
+
+
+contract AppStubStorage is AragonApp {
+    function initialize() onlyInit public {
+        initialized();
+    }
+
+    function setKernelExt(IKernel _kernel) public {
+        setKernel(_kernel);
+    }
+
+    function setAppIdExt(bytes32 _appId) public {
+        setAppId(_appId);
+    }
+
+    function getKernelPosition() public view returns (bytes32) {
+        return KERNEL_POSITION;
+    }
+
+    function getAppIdPosition() public view returns (bytes32) {
+        return APP_ID_POSITION;
+    }
+
+    function getInitializationBlockPosition() public view returns (bytes32) {
+        return INITIALIZATION_BLOCK_POSITION;
+    }
+}