From 1b09a30646e9128fe9271876a59de314c0cb4ab4 Mon Sep 17 00:00:00 2001 From: Michael Bumann Date: Wed, 3 Apr 2019 10:06:00 +0200 Subject: [PATCH 1/2] Fix contributor oracle permission auth It seems that the entity is either defined by the permission or if we want to use the oracle the permission must be defined for any_entity. In that case the oracle does not get the msg.sender as who/entity thus we will use tx.origin in that case. --- apps/contributor/contracts/Contributor.sol | 47 +++++++++++++--------- contracts/KreditsKit.sol | 9 +++-- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/apps/contributor/contracts/Contributor.sol b/apps/contributor/contracts/Contributor.sol index 4e92d4e..2866613 100644 --- a/apps/contributor/contracts/Contributor.sol +++ b/apps/contributor/contracts/Contributor.sol @@ -14,20 +14,20 @@ contract Contributor is AragonApp { bool exists; } - mapping (address => uint) public contributorIds; - mapping (uint => Contributor) public contributors; + mapping (address => uint256) public contributorIds; + mapping (uint256 => Contributor) public contributors; uint256 public contributorsCount; // ensure alphabetic order enum Apps { Contribution, Contributor, Proposal, Token } bytes32[4] public appIds; - event ContributorProfileUpdated(uint id, bytes32 oldIpfsHash, bytes32 newIpfsHash); - event ContributorAccountUpdated(uint id, address oldAccount, address newAccount); - event ContributorAdded(uint id, address account); + event ContributorProfileUpdated(uint256 id, bytes32 oldIpfsHash, bytes32 newIpfsHash); + event ContributorAccountUpdated(uint256 id, address oldAccount, address newAccount); + event ContributorAdded(uint256 id, address account); function initialize(address root,bytes32[4] _appIds) public onlyInit { - uint _id = contributorsCount + 1; + uint256 _id = contributorsCount + 1; Contributor storage c = contributors[_id]; c.exists = true; c.isCore = true; @@ -40,8 +40,8 @@ contract Contributor is AragonApp { initialized(); } - function coreContributorsCount() view public returns (uint) { - uint count = 0; + function coreContributorsCount() view public returns (uint256) { + uint256 count = 0; for (uint256 i = 1; i <= contributorsCount; i++) { if (contributors[i].isCore) { count += 1; @@ -50,14 +50,14 @@ contract Contributor is AragonApp { return count; } - function updateContributorAccount(uint id, address oldAccount, address newAccount) public auth(MANAGE_CONTRIBUTORS_ROLE) { + function updateContributorAccount(uint256 id, address oldAccount, address newAccount) public auth(MANAGE_CONTRIBUTORS_ROLE) { contributorIds[oldAccount] = 0; contributorIds[newAccount] = id; contributors[id].account = newAccount; ContributorAccountUpdated(id, oldAccount, newAccount); } - function updateContributorIpfsHash(uint id, bytes32 ipfsHash, uint8 hashFunction, uint8 hashSize) public isInitialized auth(MANAGE_CONTRIBUTORS_ROLE) { + function updateContributorIpfsHash(uint256 id, bytes32 ipfsHash, uint8 hashFunction, uint8 hashSize) public isInitialized auth(MANAGE_CONTRIBUTORS_ROLE) { Contributor storage c = contributors[id]; bytes32 oldIpfsHash = c.ipfsHash; c.ipfsHash = ipfsHash; @@ -69,7 +69,7 @@ contract Contributor is AragonApp { function addContributor(address account, bytes32 ipfsHash, uint8 hashFunction, uint8 hashSize, bool isCore) public isInitialized auth(MANAGE_CONTRIBUTORS_ROLE) { require(!addressExists(account)); - uint _id = contributorsCount + 1; + uint256 _id = contributorsCount + 1; assert(!contributors[_id].exists); // this can not be acually Contributor storage c = contributors[_id]; c.exists = true; @@ -84,11 +84,11 @@ contract Contributor is AragonApp { emit ContributorAdded(_id, account); } - function isCore(uint id) view public returns (bool) { + function isCore(uint256 id) view public returns (bool) { return contributors[id].isCore; } - function exists(uint id) view public returns (bool) { + function exists(uint256 id) view public returns (bool) { return contributors[id].exists; } @@ -100,20 +100,20 @@ contract Contributor is AragonApp { return getContributorByAddress(account).exists; } - function getContributorIdByAddress(address account) view public returns (uint) { + function getContributorIdByAddress(address account) view public returns (uint256) { return contributorIds[account]; } - function getContributorAddressById(uint id) view public returns (address) { + function getContributorAddressById(uint256 id) view public returns (address) { return contributors[id].account; } function getContributorByAddress(address account) internal view returns (Contributor) { - uint id = contributorIds[account]; + uint256 id = contributorIds[account]; return contributors[id]; } - function getContributorById(uint _id) public view returns (uint id, address account, bytes32 ipfsHash, uint8 hashFunction, uint8 hashSize, bool isCore, bool exists ) { + function getContributorById(uint256 _id) public view returns (uint256 id, address account, bytes32 ipfsHash, uint8 hashFunction, uint8 hashSize, bool isCore, bool exists ) { id = _id; Contributor storage c = contributors[_id]; account = c.account; @@ -124,7 +124,16 @@ contract Contributor is AragonApp { exists = c.exists; } - function canPerform(address _who, address _where, bytes32 _what, uint256[] _how) public view returns (bool) { - return addressExists(_who); + function canPerform(address _who, address _where, bytes32 _what, uint256[] memory _how) public returns (bool) { + address sender = _who; + if (sender == address(-1)) { + sender = tx.origin; + } + // _what == keccak256('VOTE_PROPOSAL_ROLE') + if (_what == 0xd61216798314d2fc33e42ff2021d66707b1e38517d3f7166798a9d3a196a9c96) { + return contributorIds[sender] != uint256(0); + } + + return addressIsCore(sender); } } diff --git a/contracts/KreditsKit.sol b/contracts/KreditsKit.sol index e2e9c81..4f554bd 100644 --- a/contracts/KreditsKit.sol +++ b/contracts/KreditsKit.sol @@ -50,15 +50,16 @@ contract KreditsKit is KitBase { uint256[] memory params = new uint256[](1); params[0] = uint256(203) << 248 | uint256(1) << 240 | uint240(contributor); - acl.grantPermissionP(root, contribution, contribution.ADD_CONTRIBUTION_ROLE(), params); - acl.grantPermissionP(root, contribution, contribution.VETO_CONTRIBUTION_ROLE(), params); + acl.grantPermissionP(acl.ANY_ENTITY(), contribution, contribution.ADD_CONTRIBUTION_ROLE(), params); + acl.grantPermissionP(acl.ANY_ENTITY(), contribution, contribution.VETO_CONTRIBUTION_ROLE(), params); //acl.setPermissionManager(this, proposal, proposal.VOTE_PROPOSAL_ROLE(); acl.createPermission(root, proposal, proposal.VOTE_PROPOSAL_ROLE(), this); - acl.grantPermissionP(root, proposal, proposal.VOTE_PROPOSAL_ROLE(), params); + acl.grantPermissionP(acl.ANY_ENTITY(), proposal, proposal.VOTE_PROPOSAL_ROLE(), params); acl.createPermission(root, proposal, proposal.ADD_PROPOSAL_ROLE(), this); - acl.grantPermissionP(root, proposal, proposal.ADD_PROPOSAL_ROLE(), params); + //acl.grantPermissionP(address(-1), proposal, proposal.ADD_PROPOSAL_ROLE(), params); + acl.grantPermission(acl.ANY_ENTITY(), proposal, proposal.ADD_PROPOSAL_ROLE()); acl.setPermissionManager(root, proposal, proposal.VOTE_PROPOSAL_ROLE()); acl.setPermissionManager(root, proposal, proposal.ADD_PROPOSAL_ROLE()); -- 2.25.1 From ef69fedb8ba8fa05d164424b68577df2aa9d0ad5 Mon Sep 17 00:00:00 2001 From: Michael Bumann Date: Wed, 3 Apr 2019 10:16:44 +0200 Subject: [PATCH 2/2] Use namehash from ethers.utils no need for the additional dependency --- lib/contracts/kernel.js | 2 +- package.json | 1 - scripts/deploy-kit.js | 8 +++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/contracts/kernel.js b/lib/contracts/kernel.js index 84ac26f..a5cb473 100644 --- a/lib/contracts/kernel.js +++ b/lib/contracts/kernel.js @@ -1,4 +1,4 @@ -const namehash = require('eth-ens-namehash').hash; +const namehash = require('ethers').utils.namehash; const Base = require('./base'); const KERNEL_APP_ADDR_NAMESPACE = '0xd6f028ca0e8edb4a8c9757ca4fdccab25fa1e0317da1188108f7d2dee14902fb'; diff --git a/package.json b/package.json index d453548..46411cc 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,6 @@ "solc": "^0.4.25" }, "dependencies": { - "eth-ens-namehash": "^2.0.8", "ethers": "^4.0.27", "ipfs-api": "^19.0.0", "rsvp": "^4.8.2" diff --git a/scripts/deploy-kit.js b/scripts/deploy-kit.js index fb8d0ce..03588c3 100644 --- a/scripts/deploy-kit.js +++ b/scripts/deploy-kit.js @@ -3,7 +3,7 @@ const deployDAOFactory = require('@aragon/os/scripts/deploy-daofactory.js') const fs = require('fs'); const path = require('path'); const argv = require('yargs').argv -const namehash = require('eth-ens-namehash').hash +const namehash = require('ethers').utils.namehash; const fileInject = require('./helpers/file_inject.js') const getNetworkId = require('./helpers/networkid.js') @@ -28,11 +28,17 @@ module.exports = async function(callback) { console.log(`Using ENS at: ${ensAddr}`); let daoFactory + try { if (daoFactoryAddress) { daoFactory = DAOFactory.at(daoFactoryAddress) } else { daoFactory = (await deployDAOFactory(null, { artifacts, verbose: false })).daoFactory } + } catch(e) { + console.log(e); + callback(e); + return; + } console.log(`Using DAOFactory at: ${daoFactory.address}`) const apps = fs.readdirSync('./apps') -- 2.25.1