add DONOR_ROLE

contracts/LPFactory.sol

@@ -43,6 +43,7 @@ contract LPFactory is DAOFactory {
         bytes32 hatchCallerRole = v.ESCAPE_HATCH_CALLER_ROLE();
         bytes32 authPaymentRole = v.AUTHORIZE_PAYMENT_ROLE();
         bytes32 pledgeAdminRole = lp.PLEDGE_ADMIN_ROLE();
+        bytes32 donorRole = lp.DONOR_ROLE();
         bytes32 pluginManagerRole = lp.PLUGIN_MANAGER_ROLE();
 
         acl.createPermission(_root, address(v), hatchCallerRole, _root);
@@ -50,6 +51,7 @@ contract LPFactory is DAOFactory {
         acl.createPermission(_root, address(lp), pluginManagerRole, _root);
         acl.createPermission(address(lp), address(v), authPaymentRole, _root);
         acl.createPermission(0x0, address(lp), pledgeAdminRole, address(lp));
+        acl.createPermission(0x0, address(lp), donorRole, address(lp));
         // TODO: set pledgeAdminRole manager to 0x0? maybe it doesn't matter b/c it can be recreated by _root anyways
 
         acl.grantPermission(_root, address(kernel), appManagerRole);

contracts/LPVault.sol

@@ -25,6 +25,7 @@ pragma solidity ^0.4.18;
 ///  to allow for an optional escapeHatch to be implemented in case of issues;
 ///  future versions of this contract will be enabled for tokens
 import "./EscapableApp.sol";
+import "./LiquidPledgingACLHelpers.sol";
 import "giveth-common-contracts/contracts/ERC20.sol";
 
 /// @dev `LiquidPledging` is a basic interface to allow the `LPVault` contract
@@ -36,7 +37,7 @@ contract ILiquidPledging {
 
 /// @dev `LPVault` is a higher level contract built off of the `Escapable`
 ///  contract that holds funds for the liquid pledging system.
-contract LPVault is EscapableApp {
+contract LPVault is EscapableApp, LiquidPledgingACLHelpers {
 
     bytes32 constant public CONFIRM_PAYMENT_ROLE = keccak256("CONFIRM_PAYMENT_ROLE");
     bytes32 constant public CANCEL_PAYMENT_ROLE = keccak256("CANCEL_PAYMENT_ROLE");
@@ -107,7 +108,7 @@ contract LPVault is EscapableApp {
     /// @param _automatic If true, payments will confirm instantly, if false
     ///  the training wheels are put on and the owner must manually approve 
     ///  every payment
-    function setAutopay(bool _automatic) external authP(SET_AUTOPAY_ROLE, _arr(_automatic)) {
+    function setAutopay(bool _automatic) external authP(SET_AUTOPAY_ROLE, arr(_automatic)) {
         autoPay = _automatic;
         AutoPaySet(autoPay);
     }
@@ -225,13 +226,4 @@ contract LPVault is EscapableApp {
 
         CancelPayment(_idPayment, p.ref);
     }
-
-    function _arr(bool a) internal pure returns (uint256[] r) {
-        r = new uint256[](1);
-        uint _a;
-        assembly {
-            _a := a // forced casting
-        }
-        r[0] = _a;
-    }
 }

contracts/LiquidPledging.sol

@@ -27,6 +27,7 @@ import "./LiquidPledgingBase.sol";
 ///  to allow for expanded functionality.
 contract LiquidPledging is LiquidPledgingBase {
 
+
     function addGiverAndDonate(uint64 idReceiver, address token, uint amount)
         public
     {
@@ -38,7 +39,7 @@ contract LiquidPledging is LiquidPledgingBase {
     {
         require(donorAddress != 0);
         // default to a 3 day (259200 seconds) commitTime
-        uint64 idGiver = _addGiver(donorAddress, "", "", 259200, ILiquidPledgingPlugin(0));
+        uint64 idGiver = addGiver(donorAddress, "", "", 259200, ILiquidPledgingPlugin(0));
         donate(idGiver, idReceiver, token, amount);
     }
 
@@ -51,7 +52,7 @@ contract LiquidPledging is LiquidPledgingBase {
     /// @param idReceiver The Admin receiving the donation; can be any Admin:
     ///  the Giver themselves, another Giver, a Delegate or a Project
     function donate(uint64 idGiver, uint64 idReceiver, address token, uint amount)
-        public
+        public authP(DONOR_ROLE, arr(idGiver, idReceiver, token, amount, msg.sender))
     {
         require(idGiver > 0); // prevent burning donations. idReceiver is checked in _transfer
         require(amount > 0);

contracts/LiquidPledgingACLHelpers.sol

@@ -0,0 +1,21 @@
+pragma solidity ^0.4.18;
+
+contract LiquidPledgingACLHelpers {
+    function arr(uint64 a, uint64 b, address c, uint d, address e) internal pure returns(uint[] r) {
+        r = new uint[](4);
+        r[0] = uint(a);
+        r[1] = uint(b);
+        r[2] = uint(c);
+        r[3] = d;
+        r[4] = uint(e);
+    }
+
+    function arr(bool a) internal pure returns (uint[] r) {
+        r = new uint[](1);
+        uint _a;
+        assembly {
+            _a := a // forced casting
+        }
+        r[0] = _a;
+    }
+}

contracts/LiquidPledgingPlugins.sol

@@ -21,8 +21,9 @@ pragma solidity ^0.4.18;
 
 import "@aragon/os/contracts/apps/AragonApp.sol";
 import "./LiquidPledgingStorage.sol";
+import "./LiquidPledgingACLHelpers.sol";
 
-contract LiquidPledgingPlugins is AragonApp, LiquidPledgingStorage {
+contract LiquidPledgingPlugins is AragonApp, LiquidPledgingStorage, LiquidPledgingACLHelpers {
 
     bytes32 constant public PLUGIN_MANAGER_ROLE = keccak256("PLUGIN_MANAGER_ROLE");
 
@@ -40,7 +41,7 @@ contract LiquidPledgingPlugins is AragonApp, LiquidPledgingStorage {
         pluginWhitelist[contractHash] = false;
     }
 
-    function useWhitelist(bool useWhitelist) external authP(PLUGIN_MANAGER_ROLE, _arr(useWhitelist)) {
+    function useWhitelist(bool useWhitelist) external authP(PLUGIN_MANAGER_ROLE, arr(useWhitelist)) {
         whitelistDisabled = !useWhitelist;
     }
 
@@ -68,13 +69,4 @@ contract LiquidPledgingPlugins is AragonApp, LiquidPledgingStorage {
         }
         return keccak256(o_code);
     }
-
-    function _arr(bool a) internal pure returns (uint[] r) {
-        r = new uint[](1);
-        uint _a;
-        assembly {
-            _a := a // forced casting
-        }
-        r[0] = _a;
-    }
 }

contracts/PledgeAdmins.sol

@@ -24,7 +24,11 @@ import "@aragon/os/contracts/acl/ACL.sol";
 
 contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
 
+    // NOTE: PLEDGE_ADMIN_ROLE assumes that the 1st param passed to the authP modifier
+    // is the idAdmin. This is critical to prevent unauthorized access
     bytes32 constant public PLEDGE_ADMIN_ROLE = keccak256("PLEDGE_ADMIN_ROLE");
+    bytes32 constant public DONOR_ROLE = keccak256("DONOR_ROLE");
+    address constant public ANY_ENTITY = address(-1);
 
     // Limits inserted to prevent large loops that could prevent canceling
     uint constant MAX_SUBPROJECT_LEVEL = 20;
@@ -57,7 +61,7 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
         ILiquidPledgingPlugin plugin
     ) public returns (uint64 idGiver)
     {
-        return _addGiver(
+        return addGiver(
             msg.sender,
             name,
             url,
@@ -66,7 +70,8 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
         );
     }
 
-    function _addGiver(
+    // TODO: is there an issue w/ allowing anyone to create a giver on behalf of another addy?
+    function addGiver(
         address addr,
         string name,
         string url,
@@ -91,6 +96,7 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
                 url)
         );
 
+        // TODO: do we want to grant permission accept donations as a giver? maybe from self only?
         _grantPledgeAdminPermission(msg.sender, idGiver);
         if (address(plugin) != 0) {
             _grantPledgeAdminPermission(address(plugin), idGiver);
@@ -159,6 +165,7 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
                 url)
         );
 
+        _grantAnyDonorPermission(idDelegate);
         _grantPledgeAdminPermission(msg.sender, idDelegate);
         if (address(plugin) != 0) {
             _grantPledgeAdminPermission(address(plugin), idDelegate);
@@ -239,6 +246,7 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
                 url)
         );
 
+        _grantAnyDonorPermission(idProject);
         _grantPledgeAdminPermission(projectAdmin, idProject);
         if (address(plugin) != 0) {
             _grantPledgeAdminPermission(address(plugin), idProject);
@@ -277,6 +285,32 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
         ProjectUpdated(idProject);
     }
 
+    function grantPledgeAdminPermission(uint64 idAdmin, address _who, uint[] _params) public authP(PLEDGE_ADMIN_ROLE, arr(uint(idAdmin), 0, 1)) {
+        uint[] memory params;
+
+        // if params are passed, we need to add a AND logic statement as the first param
+        // which limits the permission to the given idAdmin. This is to prevent granting
+        // PledgeAdminPermission to a idAdmin that msg.sender is not an admin to
+        // idAdmin (on msg call) == idAdmin AND _params
+        if (_params.length > 0) {
+            params = new uint[](_params.length + 2);
+            // paramId: 204 (LOGIC) op: AND(8) val: 1 (param index) & 2 (param index)
+            params[0] = uint(bytes32(204 << 8 * 31) | bytes32(8 << 8 * 30) | bytes32(2 << 8 * 4) | bytes32(1));
+            // paramId: 0 op: EQ(1) val: idAdmin
+            params[1] = uint(bytes32(1 << 8 * 30) | idAdmin);
+
+            for (uint64 i = 0; i < _params.length; i++) {
+                params[i + 2] = _params[i];
+            }
+        } else {
+            params = new uint[](1);
+            // paramId: 0 op: EQ(1) val: idAdmin
+            params[0] = uint(bytes32(1 << 8 * 30) | idAdmin);
+        }
+
+        ACL(kernel.acl()).grantPermissionP(_who, address(this), PLEDGE_ADMIN_ROLE, _params); 
+    }
+
 
 /////////////////////////////
 // Public constant functions
@@ -374,13 +408,27 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
         return _getProjectLevel(parent) + 1;
     }
 
-    function _grantPledgeAdminPermission(address _who, uint64 idPledge) internal {
+    function _grantPledgeAdminPermission(address _who, uint64 idAdmin) internal {
         bytes32 id;
-        assembly { id := idPledge }
+        assembly { id := idAdmin }
 
         uint[] memory params = new uint[](1);
+        // paramId: 0 op: EQ(1) val: idAdmin
         params[0] = uint(bytes32(1 << 8 * 30) | id);
 
+        // grant _who the PLEDGE_ADMIN_ROLE for idAdmin
         ACL(kernel.acl()).grantPermissionP(_who, address(this), PLEDGE_ADMIN_ROLE, params); 
     }
+
+    function _grantAnyDonorPermission(uint64 idAdmin) internal {
+        bytes32 id;
+        assembly { id := idAdmin }
+
+        uint[] memory params = new uint[](1);
+        // paramId: 1 op: EQ(1) val: idAdmin
+        params[0] = uint(bytes32(1 << 8 * 31) | bytes32(1 << 8 * 30) | id);
+
+        // grant ANY_ENTITY permission to donate to idAdmin
+        ACL(kernel.acl()).grantPermissionP(ANY_ENTITY, address(this), DONOR_ROLE, params); 
+    }
 }

test/NormalOperation.js

@@ -68,7 +68,7 @@ describe('LiquidPledging test', function () {
 
   it('Should deploy LiquidPledging contract', async () => {
     const baseVault = await contracts.LPVault.new(web3);
-    const baseLP = await contracts.LiquidPledgingMock.new(web3);
+    const baseLP = await contracts.LiquidPledgingMock.new(web3, {gas: 6700000});
     lpFactory = await contracts.LPFactory.new(web3, baseVault.$address, baseLP.$address);
 
     const r = await lpFactory.newLP(accounts[0], escapeHatchDestination);
@@ -100,6 +100,11 @@ describe('LiquidPledging test', function () {
   });
   it('Should create a giver', async () => {
     await liquidPledging.addGiver('Giver1', 'URLGiver1', 86400, 0, { from: giver1, gas: 1000000 });
+
+    // grant permission to giver1 to receive donations. This is not done by default in liquidPledging,
+    // but the test cases use this.
+    await acl.grantPermissionP(await liquidPledging.ANY_ENTITY(), liquidPledging.$address, await liquidPledging.DONOR_ROLE(), ["0x101000000000000000000000000000000000000000000000000000000000000"], {$extraGas: 200000});
+
     const nAdmins = await liquidPledging.numberOfPledgeAdmins();
     assert.equal(nAdmins, 1);
     const res = await liquidPledging.getPledgeAdmin(1);