Merge 52f5380 into 69aafc1

aragon · Jul 27, 2018 · 58d8019 · 58d8019
2 parents 69aafc1 + 52f5380
commit 58d8019
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 6 deletions.
diff --git a/contracts/acl/ACL.sol b/contracts/acl/ACL.sol
@@ -41,6 +41,7 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     // Hardcoded constant to save gas
     //bytes32 constant public EMPTY_PARAM_HASH = keccak256(uint256(0));
     bytes32 constant public EMPTY_PARAM_HASH = 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563;
+    bytes32 constant public NO_PERMISSION = bytes32(0);
     address constant public ANY_ENTITY = address(-1);
 
     modifier onlyPermissionManager(address _app, bytes32 _role) {
@@ -49,6 +50,7 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     }
 
     event SetPermission(address indexed entity, address indexed app, bytes32 indexed role, bool allowed);
+    event SetPermissionParams(address indexed entity, address indexed app, bytes32 indexed role, bytes32 paramsHash);
     event ChangePermissionManager(address indexed app, bytes32 indexed role, address indexed manager);
 
     /**
@@ -118,7 +120,7 @@ contract ACL is IACL, AragonApp, ACLHelpers {
         external
         onlyPermissionManager(_app, _role)
     {
-        _setPermission(_entity, _app, _role, bytes32(0));
+        _setPermission(_entity, _app, _role, NO_PERMISSION);
     }
 
     /**
@@ -207,12 +209,12 @@ contract ACL is IACL, AragonApp, ACLHelpers {
 
     function hasPermission(address _who, address _where, bytes32 _what, uint256[] memory _how) public view returns (bool) {
         bytes32 whoParams = permissions[permissionHash(_who, _where, _what)];
-        if (whoParams != bytes32(0) && evalParams(whoParams, _who, _where, _what, _how)) {
+        if (whoParams != NO_PERMISSION && evalParams(whoParams, _who, _where, _what, _how)) {
             return true;
         }
 
         bytes32 anyParams = permissions[permissionHash(ANY_ENTITY, _where, _what)];
-        if (anyParams != bytes32(0) && evalParams(anyParams, ANY_ENTITY, _where, _what, _how)) {
+        if (anyParams != NO_PERMISSION && evalParams(anyParams, ANY_ENTITY, _where, _what, _how)) {
             return true;
         }
 
@@ -255,8 +257,13 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     */
     function _setPermission(address _entity, address _app, bytes32 _role, bytes32 _paramsHash) internal {
         permissions[permissionHash(_entity, _app, _role)] = _paramsHash;
+        bool hasPermission = _paramsHash != NO_PERMISSION;
+        bool hasParams = hasPermission && _paramsHash != EMPTY_PARAM_HASH;
 
-        SetPermission(_entity, _app, _role, _paramsHash != bytes32(0));
+        SetPermission(_entity, _app, _role, hasPermission);
+        if (hasParams) {
+            SetPermissionParams(_entity, _app, _role, _paramsHash);
+        }
     }
 
     function _saveParams(uint256[] _encodedParams) internal returns (bytes32) {

diff --git a/package.json b/package.json
@@ -39,7 +39,8 @@
     "solidity-coverage": "^0.3.5",
     "solium": "^1.1.8",
     "truffle": "4.0.5",
-    "truffle-config": "^1.0.4"
+    "truffle-config": "^1.0.4",
+    "web3-utils": "1.0.0-beta.33"
   },
   "dependencies": {
     "homedir": "^0.6.0",

diff --git a/test/kernel_acl.js b/test/kernel_acl.js
@@ -2,6 +2,7 @@ const { assertRevert } = require('./helpers/assertThrow')
 const Kernel = artifacts.require('Kernel')
 const KernelProxy = artifacts.require('KernelProxy')
 const { getBlockNumber } = require('./helpers/web3')
+const { soliditySha3 } = require('web3-utils')
 const assertEvent = require('./helpers/assertEvent')
 
 const DAOFactory = artifacts.require('DAOFactory')
@@ -91,6 +92,7 @@ contract('Kernel ACL', accounts => {
         beforeEach(async () => {
             const receipt = await acl.createPermission(granted, app, role, granted, { from: permissionsRoot })
             assertEvent(receipt, 'SetPermission')
+            assertEvent(receipt, 'SetPermissionParams', 0) // should not have emitted this
             assertEvent(receipt, 'ChangePermissionManager')
         })
 
@@ -112,6 +114,12 @@ contract('Kernel ACL', accounts => {
             assert.equal(returnedParam[1].valueOf(), parseInt(op, 10), 'param op should match')
             assert.equal(returnedParam[2].valueOf(), parseInt(value, 10), 'param value should match')
 
+            // Assert that the right events have been emitted with the right args
+            assertEvent(r1, 'SetPermission')
+            assertEvent(r1, 'SetPermissionParams')
+            const setParamsHash = r1.logs.filter(l => l.event == 'SetPermissionParams')[0].args.paramsHash
+            assert.equal(setParamsHash, soliditySha3(param))
+
             // grants again without re-saving params
             const r2 = await acl.grantPermissionP(accounts[4], app, role, [param], { from: granted })
 
@@ -130,7 +138,10 @@ contract('Kernel ACL', accounts => {
         it('can grant a public permission', async () => {
             const anyEntity = "0xffffffffffffffffffffffffffffffffffffffff"
 
-            await acl.grantPermission(anyEntity, app, role, { from: granted })
+            const receipt = await acl.grantPermission(anyEntity, app, role, { from: granted })
+            assertEvent(receipt, 'SetPermission')
+            assertEvent(receipt, 'SetPermissionParams', 0) // should not have emitted this
+
             // many entities can succesfully perform action
             // acl is used here just to provide an address which is a contract
             await kernel.setApp('0x121212', '0x00', acl.address, { from: accounts[4] })