diff --git a/contracts/apps/AppStorage.sol b/contracts/apps/AppStorage.sol index 07f601d64..f31ca5cab 100644 --- a/contracts/apps/AppStorage.sol +++ b/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 diff --git a/contracts/apps/AragonApp.sol b/contracts/apps/AragonApp.sol index b31cce7e6..a26f4307f 100644 --- a/contracts/apps/AragonApp.sol +++ b/contracts/apps/AragonApp.sol @@ -11,6 +11,7 @@ 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 @@ -18,7 +19,7 @@ import "../evmscript/EVMScriptRunner.sol"; // 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; + } } diff --git a/contracts/apps/IAragonApp.sol b/contracts/apps/IAragonApp.sol new file mode 100644 index 000000000..508f47688 --- /dev/null +++ b/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); +} diff --git a/contracts/apps/disputable/DisputableAragonApp.sol b/contracts/apps/disputable/DisputableAragonApp.sol index cf13062dd..71def37fe 100644 --- a/contracts/apps/disputable/DisputableAragonApp.sol +++ b/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 diff --git a/contracts/apps/disputable/IDisputable.sol b/contracts/apps/disputable/IDisputable.sol index f015feb67..b26684a94 100644 --- a/contracts/apps/disputable/IDisputable.sol +++ b/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); } diff --git a/contracts/lib/arbitration/IArbitrable.sol b/contracts/lib/arbitration/IArbitrable.sol index 979f7d9a9..6ca896e85 100644 --- a/contracts/lib/arbitration/IArbitrable.sol +++ b/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; } } diff --git a/contracts/lib/standards/ERC165.sol b/contracts/lib/standards/ERC165.sol index 8fd948fc1..a2fc698c0 100644 --- a/contracts/lib/standards/ERC165.sol +++ b/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; + } } diff --git a/contracts/test/mocks/apps/AragonAppMock.sol b/contracts/test/mocks/apps/AragonAppMock.sol new file mode 100644 index 000000000..e25a21617 --- /dev/null +++ b/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; + } +} diff --git a/contracts/test/mocks/apps/disputable/AgreementMock.sol b/contracts/test/mocks/apps/disputable/AgreementMock.sol index 2047acc02..2796c84c4 100644 --- a/contracts/test/mocks/apps/disputable/AgreementMock.sol +++ b/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 } diff --git a/contracts/test/mocks/apps/disputable/DisputableAppMock.sol b/contracts/test/mocks/apps/disputable/DisputableAppMock.sol index 9a96655d9..29d33c2ee 100644 --- a/contracts/test/mocks/apps/disputable/DisputableAppMock.sol +++ b/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; } /** diff --git a/contracts/test/mocks/lib/standards/ERC165Mock.sol b/contracts/test/mocks/lib/standards/ERC165Mock.sol new file mode 100644 index 000000000..ea61e5925 --- /dev/null +++ b/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; + } +} diff --git a/test/contracts/apps/app_interface.js b/test/contracts/apps/app_interface.js new file mode 100644 index 000000000..d2be5aee3 --- /dev/null +++ b/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') + }) + }) +}) diff --git a/test/contracts/apps/disputable/disputable_app.js b/test/contracts/apps/disputable/disputable_app.js index 5cbd0b6fa..841da1e14 100644 --- a/test/contracts/apps/disputable/disputable_app.js +++ b/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 () => {