Merge fd60543 into dc56285

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
@@ -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/AppStorage.sol

@@ -8,7 +8,9 @@ import "../common/UnstructuredStorage.sol";
 import "../kernel/IKernel.sol";
 
 
-contract AppStorage is UnstructuredStorage {
+contract AppStorage {
+    using UnstructuredStorage for bytes32;
+
     // keccak256("aragonOS.appStorage.kernel")
     bytes32 internal constant KERNEL_POSITION = 0x4172f0f7d2289153072b0a6ca36959e0cbe2efc3afe50fc81636caa96338137b;
     // keccak256("aragonOS.appStorage.appId")
@@ -17,26 +19,18 @@ contract AppStorage is UnstructuredStorage {
     bytes32 internal constant PINNED_CODE_POSITION = 0xdee64df20d65e53d7f51cb6ab6d921a0a6a638a91e942e1d8d02df28e31c038e;
 
     function kernel() public view returns (IKernel) {
-        return IKernel(getStorageAddress(KERNEL_POSITION));
+        return IKernel(KERNEL_POSITION.getStorageAddress());
     }
 
     function appId() public view returns (bytes32) {
-        return getStorageBytes32(APP_ID_POSITION);
+        return APP_ID_POSITION.getStorageBytes32();
     }
 
     function setKernel(IKernel _kernel) internal {
-        setStorageAddress(KERNEL_POSITION, address(_kernel));
+        KERNEL_POSITION.setStorageAddress(address(_kernel));
     }
 
     function setAppId(bytes32 _appId) internal {
-        setStorageBytes32(APP_ID_POSITION, _appId);
-    }
-
-    function setPinnedCode(address _pinnedCode) internal {
-        setStorageAddress(PINNED_CODE_POSITION, _pinnedCode);
-    }
-
-    function pinnedCode() internal view returns (address) {
-        return getStorageAddress(PINNED_CODE_POSITION);
+        APP_ID_POSITION.setStorageBytes32(_appId);
     }
 }

contracts/common/Initializable.sol

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

contracts/common/UnstructuredStorage.sol

@@ -5,16 +5,16 @@
 pragma solidity ^0.4.18;
 
 
-contract UnstructuredStorage {
-    function getStorageAddress(bytes32 position) public view returns (address data) {
+library UnstructuredStorage {
+    function getStorageAddress(bytes32 position) internal view returns (address data) {
         assembly { data := sload(position) }
     }
 
-    function getStorageBytes32(bytes32 position) public view returns (bytes32 data) {
+    function getStorageBytes32(bytes32 position) internal view returns (bytes32 data) {
         assembly { data := sload(position) }
     }
 
-    function getStorageUint256(bytes32 position) public view returns (uint256 data) {
+    function getStorageUint256(bytes32 position) internal view returns (uint256 data) {
         assembly { data := sload(position) }
     }
 

migrations/2_apm.js

@@ -36,7 +36,7 @@ module.exports = function (deployer, network, accounts, arts = null) {
     console.log('Deployed APM at:', apmAddr)
 
     const apm = getContract('APMRegistry').at(apmAddr)
-    console.log('Kernel:', await apm.kernel.call())
+    console.log('Kernel:', await apm.kernel())
 
     const ensSub = getContract('ENSSubdomainRegistrar').at(await apm.registrar())
     console.log('ENS:', await ensSub.ens())

test/apm_registry.js

@@ -42,7 +42,7 @@ contract('APMRegistry', accounts => {
         const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
         registry = APMRegistry.at(apmAddr)
 
-        dao = Kernel.at(await registry.kernel.call())
+        dao = Kernel.at(await registry.kernel())
         acl = ACL.at(await dao.acl())
         const subdomainRegistrar = baseDeployed[2]
 

test/ens_subdomains.js

@@ -45,7 +45,7 @@ contract('ENSSubdomainRegistrar', accounts => {
         const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
         registry = APMRegistry.at(apmAddr)
 
-        dao = Kernel.at(await registry.kernel.call())
+        dao = Kernel.at(await registry.kernel())
         acl = ACL.at(await dao.acl())
 
         registrar = getContract('ENSSubdomainRegistrar').at(await registry.registrar())

test/keccak_constants.js

@@ -78,12 +78,17 @@ contract('Constants', accounts => {
     assert.equal(await repo.CREATE_VERSION_ROLE(), await keccakConstants.CREATE_VERSION_ROLE(), "create version role doesn't match")
   })
 
-  it('checks Unstructured Storage constants', async () => {
+  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.getPinnedCodePosition(), await keccakConstants.pinnedCodePosition(), "pinnedCodePosition 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

@@ -4,8 +4,6 @@ import "../../contracts/apps/AragonApp.sol";
 
 
 contract AppStubStorage is AragonApp {
-    bytes32 constant public ROLE = bytes32(1);
-
     function initialize() onlyInit public {
         initialized();
     }
@@ -18,27 +16,15 @@ contract AppStubStorage is AragonApp {
         setAppId(_appId);
     }
 
-    function setPinnedCodeExt(address _pinnedCode) public {
-        setPinnedCode(_pinnedCode);
-    }
-
     function getKernelPosition() public view returns (bytes32) {
-        return kernelPosition;
+        return KERNEL_POSITION;
     }
 
     function getAppIdPosition() public view returns (bytes32) {
-        return appIdPosition;
-    }
-
-    function getPinnedCodePosition() public view returns (bytes32) {
-        return pinnedCodePosition;
+        return APP_ID_POSITION;
     }
 
     function getInitializationBlockPosition() public view returns (bytes32) {
-        return initializationBlockPosition;
-    }
-
-    function pinnedCodeExt() public view returns (address) {
-        return pinnedCode();
+        return INITIALIZATION_BLOCK_POSITION;
     }
 }

test/unstructured_storage.js

@@ -1,11 +1,14 @@
-const getContract = name => artifacts.require(name)
+const AppStubStorage = artifacts.require('AppStubStorage')
+const AppStubPinnedStorage = artifacts.require('AppStubPinnedStorage')
+const Kernel = artifacts.require('Kernel')
 
 contract('Unstructured storage', accounts => {
   let app, kernel
 
   beforeEach(async () => {
-    app = await getContract('AppStubStorage').new()
-    kernel = await getContract('Kernel').new()
+    app = await AppStubStorage.new()
+    appPinned = await AppStubPinnedStorage.new()
+    kernel = await Kernel.new()
   })
 
   it('tests init block', async () => {
@@ -20,7 +23,7 @@ contract('Unstructured storage', accounts => {
     )
     assert.equal(
       parseInt(await web3.eth.getStorageAt(app.address, (await app.getInitializationBlockPosition())), 16),
-      (await app.getInitializationBlock.call()).toString(),
+      (await app.getInitializationBlock()).toString(),
       'Init block should match'
     )
   })
@@ -30,7 +33,7 @@ contract('Unstructured storage', accounts => {
     //checks
     assert.equal(
       await web3.eth.getStorageAt(app.address, (await app.getKernelPosition())),
-      (await app.kernel.call()).toString(),
+      (await app.kernel()).toString(),
       'Kernel should match'
     )
     assert.equal(
@@ -46,7 +49,7 @@ contract('Unstructured storage', accounts => {
     //checks
     assert.equal(
       await web3.eth.getStorageAt(app.address, (await app.getAppIdPosition())),
-      (await app.appId.call()).toString(),
+      (await app.appId()).toString(),
       'appId should match'
     )
     assert.equal(
@@ -58,15 +61,15 @@ contract('Unstructured storage', accounts => {
 
   it('tests pinnedCode storage', async () => {
     const pinnedCode = '0x1200000000000000000000000000000000005678'
-    await app.setPinnedCodeExt(pinnedCode)
+    await appPinned.setPinnedCodeExt(pinnedCode)
     //checks
     assert.equal(
-      await web3.eth.getStorageAt(app.address, (await app.getPinnedCodePosition())),
-      (await app.pinnedCodeExt.call()).toString(),
+      await web3.eth.getStorageAt(appPinned.address, (await appPinned.getPinnedCodePosition())),
+      (await appPinned.pinnedCodeExt()).toString(),
       'Pinned Code should match'
     )
     assert.equal(
-      await web3.eth.getStorageAt(app.address, (await app.getPinnedCodePosition())),
+      await web3.eth.getStorageAt(appPinned.address, (await appPinned.getPinnedCodePosition())),
       pinnedCode,
       'Pinned Code original value should match'
     )