Add IAragonApp interface (#597)

* Add IAragonApp

* Add Aragon App interface id

Refactor ERC-165 related inheritance.

* IAragonApp interface: move it up to AragonApp (out of AppStorage)

* disputable: move interface id helpers to proper mocks

* disputable: Move supportsInterface from interface to implementation

* Update contracts/apps/disputable/IDisputable.sol

Co-authored-by: Brett Sun <[email protected]>

* IAragonApp interface: move constant from implementation to interface

* IAragonApp interface: add tests for AragonApp

* disputable: move supportsInterface from interface to implementation

* disputable: Fix supportsInterface after merge in mock contract

Co-authored-by: Brett Sun <[email protected]>

contracts/apps/AppStorage.sol

@@ -4,11 +4,12 @@
 
 pragma solidity ^0.4.24;
 
+import "./IAragonApp.sol";
 import "../common/UnstructuredStorage.sol";
 import "../kernel/IKernel.sol";
 
 
-contract AppStorage {
+contract AppStorage is IAragonApp {
     using UnstructuredStorage for bytes32;
 
     /* Hardcoded constants to save gas

contracts/apps/AragonApp.sol

@@ -11,14 +11,15 @@ import "../common/ConversionHelpers.sol";
 import "../common/ReentrancyGuard.sol";
 import "../common/VaultRecoverable.sol";
 import "../evmscript/EVMScriptRunner.sol";
+import "../lib/standards/ERC165.sol";
 
 
 // Contracts inheriting from AragonApp are, by default, immediately petrified upon deployment so
 // that they can never be initialized.
 // Unless overriden, this behaviour enforces those contracts to be usable only behind an AppProxy.
 // ReentrancyGuard, EVMScriptRunner, and ACLSyntaxSugar are not directly used by this contract, but
 // are included so that they are automatically usable by subclassing contracts
-contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, ReentrancyGuard, EVMScriptRunner, ACLSyntaxSugar {
+contract AragonApp is ERC165, AppStorage, Autopetrified, VaultRecoverable, ReentrancyGuard, EVMScriptRunner, ACLSyntaxSugar {
     string private constant ERROR_AUTH_FAILED = "APP_AUTH_FAILED";
 
     modifier auth(bytes32 _role) {
@@ -65,4 +66,13 @@ contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, ReentrancyGua
         // Funds recovery via a vault is only available when used with a kernel
         return kernel().getRecoveryVault(); // if kernel is not set, it will revert
     }
+
+    /**
+    * @dev Query if a contract implements a certain interface
+    * @param _interfaceId The interface identifier being queried, as specified in ERC-165
+    * @return True if the contract implements the requested interface and if its not 0xffffffff, false otherwise
+    */
+    function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
+        return super.supportsInterface(_interfaceId) || _interfaceId == ARAGON_APP_INTERFACE_ID;
+    }
 }

contracts/apps/IAragonApp.sol

@@ -0,0 +1,16 @@
+/*
+ * SPDX-License-Identifier:    MIT
+ */
+
+pragma solidity ^0.4.24;
+
+import "../kernel/IKernel.sol";
+
+
+contract IAragonApp {
+    // Includes appId and kernel methods:
+    bytes4 internal constant ARAGON_APP_INTERFACE_ID = bytes4(0x54053e6c);
+
+    function kernel() public view returns (IKernel);
+    function appId() public view returns (bytes32);
+}

contracts/apps/disputable/DisputableAragonApp.sol

@@ -94,6 +94,15 @@ contract DisputableAragonApp is IDisputable, AragonApp {
         return _getAgreement();
     }
 
+    /**
+    * @dev Query if a contract implements a certain interface
+    * @param _interfaceId The interface identifier being queried, as specified in ERC-165
+    * @return True if the contract implements the requested interface and if its not 0xffffffff, false otherwise
+    */
+    function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
+        return super.supportsInterface(_interfaceId) || _interfaceId == DISPUTABLE_INTERFACE_ID;
+    }
+
     /**
     * @dev Internal implementation of the `onDisputableActionChallenged` hook
     * @param _disputableActionId Identifier of the action to be challenged

contracts/apps/disputable/IDisputable.sol

@@ -10,8 +10,9 @@ import "../../lib/standards/ERC165.sol";
 
 
 contract IDisputable is ERC165 {
-    bytes4 internal constant ERC165_INTERFACE_ID = bytes4(0x01ffc9a7);
-    bytes4 internal constant DISPUTABLE_INTERFACE_ID = bytes4(0x737c65f9);
+    // Includes setAgreement, onDisputableActionChallenged, onDisputableActionAllowed,
+    // onDisputableActionRejected, onDisputableActionVoided, getAgreement, canChallenge, and canClose methods:
+    bytes4 internal constant DISPUTABLE_INTERFACE_ID = bytes4(0xf3d3bb51);
 
     event AgreementSet(IAgreement indexed agreement);
 
@@ -30,15 +31,4 @@ contract IDisputable is ERC165 {
     function canChallenge(uint256 _disputableActionId) external view returns (bool);
 
     function canClose(uint256 _disputableActionId) external view returns (bool);
-
-    /**
-    * @dev Query if a contract implements a certain interface
-    * @param _interfaceId The interface identifier being queried, as specified in ERC-165
-    * @return True if the contract implements the requested interface and if its not 0xffffffff, false otherwise
-    */
-    function supportsInterface(bytes4 _interfaceId) external pure returns (bool) {
-        return _interfaceId == DISPUTABLE_INTERFACE_ID || _interfaceId == ERC165_INTERFACE_ID;
-    }
-
-    function appId() public view returns (bytes32);
 }

contracts/lib/arbitration/IArbitrable.sol

@@ -56,7 +56,7 @@ contract IArbitrable is ERC165 {
     * @param _interfaceId The interface identifier being queried, as specified in ERC-165
     * @return True if this contract supports the given interface, false otherwise
     */
-    function supportsInterface(bytes4 _interfaceId) external pure returns (bool) {
+    function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
         return _interfaceId == ARBITRABLE_INTERFACE_ID || _interfaceId == ERC165_INTERFACE_ID;
     }
 }

contracts/lib/standards/ERC165.sol

@@ -5,11 +5,16 @@
 pragma solidity ^0.4.24;
 
 
-interface ERC165 {
+contract ERC165 {
+    // Includes supportsInterface method:
+    bytes4 internal constant ERC165_INTERFACE_ID = bytes4(0x01ffc9a7);
+
     /**
     * @dev Query if a contract implements a certain interface
     * @param _interfaceId The interface identifier being queried, as specified in ERC-165
     * @return True if the contract implements the requested interface and if its not 0xffffffff, false otherwise
     */
-    function supportsInterface(bytes4 _interfaceId) external pure returns (bool);
+    function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
+        return _interfaceId == ERC165_INTERFACE_ID;
+    }
 }

contracts/test/mocks/apps/AragonAppMock.sol

@@ -0,0 +1,18 @@
+pragma solidity 0.4.24;
+
+import "../../../apps/AragonApp.sol";
+
+
+contract AragonAppMock is AragonApp {
+    bytes4 public constant ARAGON_APP_INTERFACE = ARAGON_APP_INTERFACE_ID;
+
+    function initialize() external {
+        initialized();
+    }
+
+    function interfaceID() external pure returns (bytes4) {
+        IAragonApp iAragonApp;
+        return iAragonApp.kernel.selector ^
+            iAragonApp.appId.selector;
+    }
+}

contracts/test/mocks/apps/disputable/AgreementMock.sol

@@ -121,7 +121,7 @@ contract AgreementMock is IAgreement {
         // do nothing
     }
 
-    function supportsInterface(bytes4 _interfaceId) external pure returns (bool) {
+    function supportsInterface(bytes4 _interfaceId) public pure returns (bool) {
         // do nothing
     }
 

contracts/test/mocks/apps/disputable/DisputableAppMock.sol

@@ -4,7 +4,6 @@ import "../../../../apps/disputable/DisputableAragonApp.sol";
 
 
 contract DisputableAppMock is DisputableAragonApp {
-    bytes4 public constant ERC165_INTERFACE = ERC165_INTERFACE_ID;
     bytes4 public constant DISPUTABLE_INTERFACE = DISPUTABLE_INTERFACE_ID;
 
     event DisputableChallenged(uint256 indexed id);
@@ -41,13 +40,7 @@ contract DisputableAppMock is DisputableAragonApp {
             iDisputable.onDisputableActionVoided.selector ^
             iDisputable.getAgreement.selector ^
             iDisputable.canChallenge.selector ^
-            iDisputable.canClose.selector ^
-            iDisputable.appId.selector;
-    }
-
-    function erc165interfaceID() external pure returns (bytes4) {
-        ERC165 erc165;
-        return erc165.supportsInterface.selector;
+            iDisputable.canClose.selector;
     }
 
     /**

contracts/test/mocks/lib/standards/ERC165Mock.sol

@@ -0,0 +1,13 @@
+pragma solidity 0.4.24;
+
+import "../../../../lib/standards/ERC165.sol";
+
+
+contract ERC165Mock is ERC165 {
+    bytes4 public constant ERC165_INTERFACE = ERC165_INTERFACE_ID;
+
+    function interfaceID() external pure returns (bytes4) {
+        ERC165 erc165;
+        return erc165.supportsInterface.selector;
+    }
+}

test/contracts/apps/app_interface.js

@@ -0,0 +1,56 @@
+const { getEventArgument } = require('../../helpers/events')
+const { getNewProxyAddress } = require('../../helpers/events')
+
+const ACL = artifacts.require('ACL')
+const Kernel = artifacts.require('Kernel')
+const DAOFactory = artifacts.require('DAOFactory')
+const AragonApp = artifacts.require('AragonAppMock')
+const ERC165 = artifacts.require('ERC165Mock')
+const EVMScriptRegistryFactory = artifacts.require('EVMScriptRegistryFactory')
+
+contract('AragonApp', ([_, owner, agreement, anotherAgreement, someone]) => {
+  let aragonApp
+
+  const ARAGON_APP_INTERFACE = '0x54053e6c'
+  const ERC165_INTERFACE = '0x01ffc9a7'
+
+  before('deploy DAO and install aragon app', async () => {
+    const kernelBase = await Kernel.new(true)
+    const aclBase = await ACL.new()
+    const registryFactory = await EVMScriptRegistryFactory.new()
+    const daoFact = await DAOFactory.new(kernelBase.address, aclBase.address, registryFactory.address)
+
+    const receiptDao = await daoFact.newDAO(owner)
+    dao = await Kernel.at(getEventArgument(receiptDao, 'DeployDAO', 'dao'))
+    acl = await ACL.at(await dao.acl())
+    const aragonAppBase = await AragonApp.new()
+
+    const APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
+    await acl.createPermission(owner, dao.address, APP_MANAGER_ROLE, owner, { from: owner })
+    const initializeData = aragonAppBase.contract.initialize.getData()
+    const receiptInstance = await dao.newAppInstance('0x1234', aragonAppBase.address, initializeData, false, { from: owner })
+    aragonApp = await AragonApp.at(getNewProxyAddress(receiptInstance))
+  })
+
+  describe('supportsInterface', () => {
+    it('supports ERC165', async () => {
+      const erc165 = await ERC165.new()
+      assert.isTrue(await aragonApp.supportsInterface(ERC165_INTERFACE), 'does not support ERC165')
+
+      assert.equal(await erc165.interfaceID(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
+      assert.equal(await erc165.ERC165_INTERFACE(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
+    })
+
+    it('supports Aragon App interface', async () => {
+      const aragonApp = await AragonApp.new()
+      assert.isTrue(await aragonApp.supportsInterface(ARAGON_APP_INTERFACE), 'does not support Aragon App interface')
+
+      assert.equal(await aragonApp.interfaceID(), ARAGON_APP_INTERFACE, 'Aragon App interface ID does not match')
+      assert.equal(await aragonApp.ARAGON_APP_INTERFACE(), ARAGON_APP_INTERFACE, 'Aragon App interface ID does not match')
+    })
+
+    it('does not support 0xffffffff', async () => {
+      assert.isFalse(await aragonApp.supportsInterface('0xffffffff'), 'should not support 0xffffffff')
+    })
+  })
+})

test/contracts/apps/disputable/disputable_app.js

@@ -8,13 +8,16 @@ const Kernel = artifacts.require('Kernel')
 const DAOFactory = artifacts.require('DAOFactory')
 const AgreementMock = artifacts.require('AgreementMock')
 const DisputableApp = artifacts.require('DisputableAppMock')
+const AragonApp = artifacts.require('AragonAppMock')
+const ERC165 = artifacts.require('ERC165Mock')
 const EVMScriptRegistryFactory = artifacts.require('EVMScriptRegistryFactory')
 
 contract('DisputableApp', ([_, owner, agreement, anotherAgreement, someone]) => {
   let disputable, disputableBase, dao, acl
 
   const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'
-  const DISPUTABLE_INTERFACE = '0x737c65f9'
+  const DISPUTABLE_INTERFACE = '0xf3d3bb51'
+  const ARAGON_APP_INTERFACE = '0x54053e6c'
   const ERC165_INTERFACE = '0x01ffc9a7'
 
   before('deploy DAO', async () => {
@@ -43,17 +46,26 @@ contract('DisputableApp', ([_, owner, agreement, anotherAgreement, someone]) =>
 
   describe('supportsInterface', () => {
     it('supports ERC165', async () => {
+      const erc165 = await ERC165.new()
       assert.isTrue(await disputable.supportsInterface(ERC165_INTERFACE), 'does not support ERC165')
 
-      assert.equal(await disputable.ERC165_INTERFACE(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
-      assert.equal(await disputable.erc165interfaceID(), await disputable.ERC165_INTERFACE(), 'ERC165 interface ID does not match')
+      assert.equal(await erc165.interfaceID(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
+      assert.equal(await erc165.ERC165_INTERFACE(), ERC165_INTERFACE, 'ERC165 interface ID does not match')
+    })
+
+    it('supports Aragon App interface', async () => {
+      const aragonApp = await AragonApp.new()
+      assert.isTrue(await disputable.supportsInterface(ARAGON_APP_INTERFACE), 'does not support Aragon App interface')
+
+      assert.equal(await aragonApp.interfaceID(), ARAGON_APP_INTERFACE, 'Aragon App interface ID does not match')
+      assert.equal(await aragonApp.ARAGON_APP_INTERFACE(), ARAGON_APP_INTERFACE, 'Aragon App interface ID does not match')
     })
 
     it('supports IDisputable', async () => {
       assert.isTrue(await disputable.supportsInterface(DISPUTABLE_INTERFACE), 'does not support IDisputable')
 
-      assert.equal(await disputable.interfaceID(), DISPUTABLE_INTERFACE)
-      assert.equal(await disputable.DISPUTABLE_INTERFACE(), await disputable.interfaceID(), 'IDisputable interface ID does not match')
+      assert.equal(await disputable.interfaceID(), DISPUTABLE_INTERFACE, 'IDisputable interface ID does not match')
+      assert.equal(await disputable.DISPUTABLE_INTERFACE(), DISPUTABLE_INTERFACE, 'IDisputable interface ID does not match')
     })
 
     it('does not support 0xffffffff', async () => {