remove PLEDGE_ADMIN_ROLE and DONOR_ROLE
w/ current ACL implementation, these roles will not work for us. ACL limits only 1 set of params for a given (_who, _entity, _role), but a single address may control multiple PledgeAdmins.
This commit is contained in:
perissology
parent
d481898ab1
commit
ab0f8aa62f
|
|
@ -42,16 +42,12 @@ contract LPFactory is DAOFactory {
|
|||
bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();
|
||||
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);
|
||||
acl.createPermission(_root, address(lp), hatchCallerRole, _root);
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -52,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 authP(DONOR_ROLE, arr(idGiver, idReceiver, token, amount, msg.sender))
|
||||
public
|
||||
{
|
||||
require(idGiver > 0); // prevent burning donations. idReceiver is checked in _transfer
|
||||
require(amount > 0);
|
||||
|
|
@ -95,8 +95,9 @@ contract LiquidPledging is LiquidPledgingBase {
|
|||
uint64 idPledge,
|
||||
uint amount,
|
||||
uint64 idReceiver
|
||||
) authP(PLEDGE_ADMIN_ROLE, arr(uint(idSender), amount)) public
|
||||
) public
|
||||
{
|
||||
checkAdminOwner(idSender);
|
||||
_transfer(idSender, idPledge, amount, idReceiver);
|
||||
}
|
||||
|
||||
|
|
@ -110,9 +111,7 @@ contract LiquidPledging is LiquidPledgingBase {
|
|||
|
||||
Pledge storage p = _findPledge(idPledge);
|
||||
require(p.pledgeState == PledgeState.Pledged);
|
||||
|
||||
PledgeAdmin storage owner = _findAdmin(p.owner);
|
||||
require(canPerform(msg.sender, PLEDGE_ADMIN_ROLE, arr(uint(p.owner))));
|
||||
checkAdminOwner(p.owner);
|
||||
|
||||
uint64 idNewPledge = _findOrCreatePledge(
|
||||
p.owner,
|
||||
|
|
@ -126,6 +125,7 @@ contract LiquidPledging is LiquidPledgingBase {
|
|||
|
||||
_doTransfer(idPledge, idNewPledge, amount);
|
||||
|
||||
PledgeAdmin storage owner = _findAdmin(p.owner);
|
||||
vault.authorizePayment(bytes32(idNewPledge), owner.addr, p.token, amount);
|
||||
}
|
||||
|
||||
|
|
@ -178,8 +178,9 @@ contract LiquidPledging is LiquidPledgingBase {
|
|||
|
||||
/// @notice Changes the `project.canceled` flag to `true`; cannot be undone
|
||||
/// @param idProject Id of the project that is to be canceled
|
||||
function cancelProject(uint64 idProject) authP(PLEDGE_ADMIN_ROLE, arr(uint(idProject))) public {
|
||||
function cancelProject(uint64 idProject) public {
|
||||
PledgeAdmin storage project = _findAdmin(idProject);
|
||||
checkAdminOwner(idProject);
|
||||
project.canceled = true;
|
||||
|
||||
CancelProject(idProject);
|
||||
|
|
@ -195,8 +196,7 @@ contract LiquidPledging is LiquidPledgingBase {
|
|||
|
||||
Pledge storage p = _findPledge(idPledge);
|
||||
require(p.oldPledge != 0);
|
||||
|
||||
require(canPerform(msg.sender, PLEDGE_ADMIN_ROLE, arr(uint(p.owner))));
|
||||
checkAdminOwner(p.owner);
|
||||
|
||||
uint64 oldPledge = _getOldestPledgeNotCanceled(p.oldPledge);
|
||||
_doTransfer(idPledge, oldPledge, amount);
|
||||
|
|
|
|||
|
|
@ -151,6 +151,14 @@ contract LiquidPledgingBase is EscapableApp, LiquidPledgingStorage, PledgeAdmins
|
|||
// Internal methods
|
||||
////////////////////
|
||||
|
||||
/// @notice A check to see if the msg.sender is the owner or the
|
||||
/// plugin contract for a specific Admin
|
||||
/// @param idAdmin The id of the admin being checked
|
||||
function checkAdminOwner(uint64 idAdmin) internal constant {
|
||||
PledgeAdmin a = _findAdmin(idAdmin);
|
||||
require(msg.sender == address(a.plugin) || msg.sender == a.addr);
|
||||
}
|
||||
|
||||
function _transfer(
|
||||
uint64 idSender,
|
||||
uint64 idPledge,
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ contract LiquidPledgingPlugins is AragonApp, LiquidPledgingStorage, LiquidPledgi
|
|||
pluginWhitelist[contractHash] = false;
|
||||
}
|
||||
|
||||
function useWhitelist(bool useWhitelist) external authP(PLUGIN_MANAGER_ROLE, arr(useWhitelist)) {
|
||||
function useWhitelist(bool useWhitelist) external auth(PLUGIN_MANAGER_ROLE) {
|
||||
whitelistDisabled = !useWhitelist;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -21,11 +21,6 @@ contract LiquidPledgingStorage {
|
|||
/// and can own pledges and act as delegates
|
||||
struct PledgeAdmin {
|
||||
PledgeAdminType adminType; // Giver, Delegate or Project
|
||||
// TODO: this is interesting...
|
||||
// it is possible to revoke permission for this addr to be able to
|
||||
// manage this pledgeAdmin. So we are storing the addr, which may or may not
|
||||
// have permission to manage this PledgeAdmin. Maybe we need a custom
|
||||
// ACL which provides a reverse lookup of addys granted a permission
|
||||
address addr; // Account or contract address for admin
|
||||
uint64 commitTime; // In seconds, used for time Givers' & Delegates' have to veto
|
||||
uint64 parentProject; // Only for projects
|
||||
|
|
|
|||
|
|
@ -20,16 +20,9 @@ pragma solidity ^0.4.18;
|
|||
*/
|
||||
import "./LiquidPledgingPlugins.sol";
|
||||
import "@aragon/os/contracts/apps/AragonApp.sol";
|
||||
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;
|
||||
uint constant MAX_INTERPROJECT_LEVEL = 20;
|
||||
|
|
@ -87,7 +80,7 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
admins.push(
|
||||
PledgeAdmin(
|
||||
PledgeAdminType.Giver,
|
||||
addr, // TODO: is this needed? Yes, until aragon has an easy way to see who has permissions
|
||||
addr,
|
||||
commitTime,
|
||||
0,
|
||||
false,
|
||||
|
|
@ -96,12 +89,6 @@ 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);
|
||||
}
|
||||
|
||||
GiverAdded(idGiver);
|
||||
}
|
||||
|
||||
|
|
@ -120,9 +107,10 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
string newName,
|
||||
string newUrl,
|
||||
uint64 newCommitTime
|
||||
) authP(PLEDGE_ADMIN_ROLE, arr(uint(idGiver))) public
|
||||
) public
|
||||
{
|
||||
PledgeAdmin storage giver = _findAdmin(idGiver);
|
||||
require(msg.sender == giver.addr);
|
||||
require(giver.adminType == PledgeAdminType.Giver); // Must be a Giver
|
||||
giver.addr = newAddr;
|
||||
giver.name = newName;
|
||||
|
|
@ -165,12 +153,6 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
url)
|
||||
);
|
||||
|
||||
_grantAnyDonorPermission(idDelegate);
|
||||
_grantPledgeAdminPermission(msg.sender, idDelegate);
|
||||
if (address(plugin) != 0) {
|
||||
_grantPledgeAdminPermission(address(plugin), idDelegate);
|
||||
}
|
||||
|
||||
DelegateAdded(idDelegate);
|
||||
}
|
||||
|
||||
|
|
@ -191,9 +173,10 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
string newName,
|
||||
string newUrl,
|
||||
uint64 newCommitTime
|
||||
) authP(PLEDGE_ADMIN_ROLE, arr(uint(idDelegate))) public
|
||||
) public
|
||||
{
|
||||
PledgeAdmin storage delegate = _findAdmin(idDelegate);
|
||||
require(msg.sender == delegate.addr);
|
||||
require(delegate.adminType == PledgeAdminType.Delegate);
|
||||
delegate.addr = newAddr;
|
||||
delegate.name = newName;
|
||||
|
|
@ -246,12 +229,6 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
url)
|
||||
);
|
||||
|
||||
_grantAnyDonorPermission(idProject);
|
||||
_grantPledgeAdminPermission(projectAdmin, idProject);
|
||||
if (address(plugin) != 0) {
|
||||
_grantPledgeAdminPermission(address(plugin), idProject);
|
||||
}
|
||||
|
||||
ProjectAdded(idProject);
|
||||
}
|
||||
|
||||
|
|
@ -271,10 +248,11 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
string newName,
|
||||
string newUrl,
|
||||
uint64 newCommitTime
|
||||
) authP(PLEDGE_ADMIN_ROLE, arr(uint(idProject))) public
|
||||
) public
|
||||
{
|
||||
PledgeAdmin storage project = _findAdmin(idProject);
|
||||
|
||||
require(msg.sender == project.addr);
|
||||
require(project.adminType == PledgeAdminType.Project);
|
||||
|
||||
project.addr = newAddr;
|
||||
|
|
@ -285,33 +263,6 @@ 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
|
||||
/////////////////////////////
|
||||
|
|
@ -407,28 +358,4 @@ contract PledgeAdmins is AragonApp, LiquidPledgingPlugins {
|
|||
PledgeAdmin storage parent = _findAdmin(a.parentProject);
|
||||
return _getProjectLevel(parent) + 1;
|
||||
}
|
||||
|
||||
function _grantPledgeAdminPermission(address _who, uint64 idAdmin) internal {
|
||||
bytes32 id;
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
@ -101,10 +101,6 @@ 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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue