Add ability to enable / disable depositable functionality (#401)

.solcover.js

@@ -1,4 +1,10 @@
-const skipFiles = ['lib', 'test', 'acl/ACLSyntaxSugar.sol']
+const skipFiles = [
+  'lib',
+  'test',
+  'acl/ACLSyntaxSugar.sol',
+  'common/DepositableStorage.sol', // Used in tests that send ETH
+  'common/UnstructuredStorage.sol' // Used in tests that send ETH
+]
 
 module.exports = {
     norpc: true,

contracts/common/DepositableDelegateProxy.sol

@@ -1,15 +1,17 @@
 pragma solidity 0.4.24;
 
 import "./DelegateProxy.sol";
+import "./DepositableStorage.sol";
 
 
-contract DepositableDelegateProxy is DelegateProxy {
+contract DepositableDelegateProxy is DepositableStorage, DelegateProxy {
     event ProxyDeposit(address sender, uint256 value);
 
     function () external payable {
         // send / transfer
         if (gasleft() < FWD_GAS_LIMIT) {
             require(msg.value > 0 && msg.data.length == 0);
+            require(isDepositable());
             emit ProxyDeposit(msg.sender, msg.value);
         } else { // all calls except for send or transfer
             address target = implementation();

contracts/common/DepositableStorage.sol

@@ -0,0 +1,19 @@
+pragma solidity 0.4.24;
+
+import "./UnstructuredStorage.sol";
+
+
+contract DepositableStorage {
+    using UnstructuredStorage for bytes32;
+
+    // keccak256("aragonOS.depositableStorage.depositable")
+    bytes32 internal constant DEPOSITABLE_POSITION = 0x665fd576fbbe6f247aff98f5c94a561e3f71ec2d3c988d56f12d342396c50cea;
+
+    function isDepositable() public view returns (bool) {
+        return DEPOSITABLE_POSITION.getStorageBool();
+    }
+
+    function setDepositable(bool _depositable) internal {
+        DEPOSITABLE_POSITION.setStorageBool(_depositable);
+    }
+}

contracts/common/UnstructuredStorage.sol

@@ -6,6 +6,10 @@ pragma solidity ^0.4.24;
 
 
 library UnstructuredStorage {
+    function getStorageBool(bytes32 position) internal view returns (bool data) {
+        assembly { data := sload(position) }
+    }
+
     function getStorageAddress(bytes32 position) internal view returns (address data) {
         assembly { data := sload(position) }
     }
@@ -18,6 +22,10 @@ library UnstructuredStorage {
         assembly { data := sload(position) }
     }
 
+    function setStorageBool(bytes32 position, bool data) internal {
+        assembly { sstore(position, data) }
+    }
+
     function setStorageAddress(bytes32 position, address data) internal {
         assembly { sstore(position, data) }
     }

contracts/factory/DAOFactory.sol

@@ -63,7 +63,7 @@ contract DAOFactory {
         }
 
         emit DeployDAO(address(dao));
-        
+
         return dao;
     }
 }

contracts/test/mocks/AppStubPinnedStorage.sol → contracts/test/mocks/AppProxyPinnedStorageMock.sol

@@ -20,7 +20,7 @@ contract KernelPinnedStorageMock is Kernel, FakeAppConstants {
 // Testing this contract is a bit of a pain... we can't overload anything to make the contract check
 // pass in the constructor, so we're forced to initialize this with a mocked Kernel that already
 // sets a contract for the fake app.
-contract AppStubPinnedStorage is AppProxyPinned, FakeAppConstants {
+contract AppProxyPinnedStorageMock is AppProxyPinned, FakeAppConstants {
     constructor(KernelPinnedStorageMock _mockKernel)
         AppProxyPinned(IKernel(_mockKernel), FAKE_APP_ID, new bytes(0))
         public // solium-disable-line visibility-first

contracts/test/mocks/AppStubStorage.sol → contracts/test/mocks/AppStorageMock.sol

@@ -1,14 +1,9 @@
 pragma solidity 0.4.24;
 
-import "../../apps/UnsafeAragonApp.sol";
+import "../../apps/AppStorage.sol";
 
 
-// Need to use UnsafeAragonApp to allow initialization
-contract AppStubStorage is UnsafeAragonApp {
-    function initialize() onlyInit public {
-        initialized();
-    }
-
+contract AppStorageMock is AppStorage {
     function setKernelExt(IKernel _kernel) public {
         setKernel(_kernel);
     }
@@ -24,8 +19,4 @@ contract AppStubStorage is UnsafeAragonApp {
     function getAppIdPosition() public pure returns (bytes32) {
         return APP_ID_POSITION;
     }
-
-    function getInitializationBlockPosition() public pure returns (bytes32) {
-        return INITIALIZATION_BLOCK_POSITION;
-    }
 }

contracts/test/mocks/AppStub.sol

@@ -5,12 +5,12 @@ import "../../apps/UnsafeAragonApp.sol";
 import "../../kernel/IKernel.sol";
 
 
-contract AppSt {
+contract AppStubStorage {
     uint a;
     string public stringTest;
 }
 
-contract AppStub is AragonApp, AppSt {
+contract AppStub is AragonApp, AppStubStorage {
     bytes32 constant public ROLE = keccak256("ROLE");
 
     function initialize() onlyInit public {
@@ -35,7 +35,7 @@ contract AppStub is AragonApp, AppSt {
     }
 }
 
-contract AppStub2 is AragonApp, AppSt {
+contract AppStub2 is AragonApp, AppStubStorage {
     function getValue() public constant returns (uint) {
         return a * 2;
     }

contracts/test/mocks/AppStubConditionalRecovery.sol

@@ -1,11 +1,17 @@
 pragma solidity 0.4.24;
 
 import "../../apps/AragonApp.sol";
+import "../../common/DepositableStorage.sol";
 
 
-contract AppStubConditionalRecovery is AragonApp {
-	function allowRecoverability(address token) public view returns (bool) {
-		// Doesn't allow to recover ether
-		return token != address(0);
-	}
+contract AppStubConditionalRecovery is AragonApp, DepositableStorage {
+    function initialize() onlyInit public {
+        initialized();
+        setDepositable(true);
+    }
+
+    function allowRecoverability(address token) public view returns (bool) {
+        // Doesn't allow to recover ether
+        return token != address(0);
+    }
 }

contracts/test/mocks/AppStubDepositable.sol

@@ -0,0 +1,26 @@
+pragma solidity 0.4.24;
+
+import "../../apps/AragonApp.sol";
+import "../../apps/UnsafeAragonApp.sol";
+import "../../common/DepositableStorage.sol";
+
+
+contract AppStubDepositable is AragonApp, DepositableStorage {
+    function () external payable {
+        require(isDepositable());
+    }
+
+    function initialize() onlyInit public {
+        initialized();
+    }
+
+    function enableDeposits() external {
+        setDepositable(true);
+    }
+}
+
+contract UnsafeAppStubDepositable is AppStubDepositable, UnsafeAragonApp {
+    constructor(IKernel _kernel) public {
+        setKernel(_kernel);
+    }
+}

contracts/test/mocks/DepositableStorageMock.sol

@@ -0,0 +1,14 @@
+pragma solidity 0.4.24;
+
+import "../../common/DepositableStorage.sol";
+
+
+contract DepositableStorageMock is DepositableStorage {
+    function setDepositableExt(bool _depositable) public {
+        setDepositable(_depositable);
+    }
+
+    function getDepositablePosition() public pure returns (bytes32) {
+        return DEPOSITABLE_POSITION;
+    }
+}

contracts/test/mocks/InitializableStorageMock.sol

@@ -0,0 +1,14 @@
+pragma solidity 0.4.24;
+
+import "../../common/Initializable.sol";
+
+
+contract InitializableStorageMock is Initializable {
+    function initialize() onlyInit public {
+        initialized();
+    }
+
+    function getInitializationBlockPosition() public pure returns (bytes32) {
+        return INITIALIZATION_BLOCK_POSITION;
+    }
+}

contracts/test/mocks/KeccakConstants.sol

@@ -50,6 +50,7 @@ contract KeccakConstants is APMNamehash {
 
     // Unstructured storage
     bytes32 public constant initializationBlockPosition = keccak256(abi.encodePacked("aragonOS.initializable.initializationBlock"));
+    bytes32 public constant depositablePosition = keccak256(abi.encodePacked("aragonOS.depositableStorage.depositable"));
     bytes32 public constant kernelPosition = keccak256(abi.encodePacked("aragonOS.appStorage.kernel"));
     bytes32 public constant appIdPosition = keccak256(abi.encodePacked("aragonOS.appStorage.appId"));
     bytes32 public constant pinnedCodePosition = keccak256(abi.encodePacked("aragonOS.appStorage.pinnedCode"));

contracts/test/mocks/KernelDepositableMock.sol

@@ -0,0 +1,17 @@
+pragma solidity 0.4.24;
+
+import "../../common/DepositableStorage.sol";
+import "../../kernel/Kernel.sol";
+
+contract KernelDepositableMock is Kernel, DepositableStorage {
+    constructor(bool _shouldPetrify) Kernel(_shouldPetrify) public {
+    }
+
+    function () external payable {
+        require(isDepositable());
+    }
+
+    function enableDeposits() external isInitialized {
+        setDepositable(true);
+    }
+}

contracts/test/mocks/VaultMock.sol

@@ -1,11 +1,17 @@
 pragma solidity 0.4.24;
 
-import "../../apps/AragonApp.sol";
+import "../../apps/UnsafeAragonApp.sol";
+import "../../common/DepositableStorage.sol";
 
 
-contract VaultMock is AragonApp {
+contract VaultMock is UnsafeAragonApp, DepositableStorage {
     event LogFund(address sender, uint256 amount);
 
+    function initialize() external {
+        initialized();
+        setDepositable(true);
+    }
+
     function () external payable {
         emit LogFund(msg.sender, msg.value);
     }

test/app_funds.js

@@ -0,0 +1,119 @@
+const { hash } = require('eth-ens-namehash')
+const { assertRevert } = require('./helpers/assertThrow')
+const { getBalance } = require('./helpers/web3')
+const { onlyIf } = require('./helpers/onlyIf')
+
+const ACL = artifacts.require('ACL')
+const Kernel = artifacts.require('Kernel')
+const KernelProxy = artifacts.require('KernelProxy')
+const AppProxyUpgradeable = artifacts.require('AppProxyUpgradeable')
+const AppProxyPinned = artifacts.require('AppProxyPinned')
+
+// Mocks
+const AppStub = artifacts.require('AppStub')
+const UnsafeAppStub = artifacts.require('UnsafeAppStub')
+const AppStubDepositable = artifacts.require('AppStubDepositable')
+const UnsafeAppStubDepositable = artifacts.require('UnsafeAppStubDepositable')
+
+const APP_ID = hash('stub.aragonpm.test')
+const EMPTY_BYTES = '0x'
+const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies
+
+contract('App funds', accounts => {
+  let aclBase, kernelBase
+  let APP_BASES_NAMESPACE
+
+  const permissionsRoot = accounts[0]
+
+  before(async () => {
+    kernelBase = await Kernel.new(true) // petrify immediately
+    aclBase = await ACL.new()
+
+    // Setup constants
+    APP_BASES_NAMESPACE = await kernelBase.APP_BASES_NAMESPACE()
+  })
+
+  const appBases = [
+    {
+      base: AppStub,
+      unsafeBase: UnsafeAppStub,
+    },
+    {
+      base: AppStubDepositable,
+      unsafeBase: UnsafeAppStubDepositable,
+    }
+  ]
+  for ({ base: appBaseType, unsafeBase: unsafeAppBaseType } of appBases) {
+    context(`> ${appBaseType.contractName}`, () => {
+      const onlyAppStubDepositable = onlyIf(() => appBaseType === AppStubDepositable)
+
+      // Test the app itself and when it's behind the proxies to make sure their behaviours are the same
+      const appProxyTypes = ['AppProxyUpgradeable', 'AppProxyPinned']
+      for (const appType of ['App', ...appProxyTypes]) {
+        context(`> ${appType}`, () => {
+          let appBase, app
+
+          before(async () => {
+            if (appProxyTypes.includes(appType)) {
+              // We can reuse the same app base for the proxies
+              appBase = await appBaseType.new()
+            }
+          })
+
+          beforeEach(async () => {
+            const kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address)
+            await kernel.initialize(aclBase.address, permissionsRoot)
+
+            if (appType === 'App') {
+              // Use the unsafe version to use directly without a proxy
+              app = await unsafeAppBaseType.new(kernel.address)
+            } else {
+              // Install app
+              const acl = ACL.at(await kernel.acl())
+              const APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
+              await acl.createPermission(permissionsRoot, kernel.address, APP_MANAGER_ROLE, permissionsRoot)
+              await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address)
+
+              let appProxy
+              if (appType === 'AppProxyUpgradeable') {
+                appProxy = await AppProxyUpgradeable.new(kernel.address, APP_ID, EMPTY_BYTES)
+              } else if (appType === 'AppProxyPinned') {
+                appProxy = await AppProxyPinned.new(kernel.address, APP_ID, EMPTY_BYTES)
+              }
+
+              app = appBaseType.at(appProxy.address)
+            }
+
+            await app.initialize();
+          })
+
+          it('cannot receive ETH', async () => {
+            assert.isTrue(await app.hasInitialized(), 'should have been initialized')
+
+            await assertRevert(async () => {
+              await app.sendTransaction({ value: 1, gas: SEND_ETH_GAS })
+            })
+          })
+
+          onlyAppStubDepositable(() => {
+            it('does not have depositing enabled by default', async () => {
+              assert.isTrue(await app.hasInitialized(), 'should have been initialized')
+              assert.isFalse(await app.isDepositable(), 'should not be depositable')
+            })
+
+            it('can receive ETH after being set to depositable', async () => {
+              const amount = 1
+              const initialBalance = await getBalance(app.address)
+
+              await app.enableDeposits()
+              assert.isTrue(await app.isDepositable(), 'should be depositable')
+
+              await app.sendTransaction({ value: 1, gas: SEND_ETH_GAS })
+              assert.equal((await getBalance(app.address)).valueOf(), initialBalance.plus(amount))
+            })
+          })
+        })
+      }
+    })
+  }
+})