Skip to content

Commit

Permalink
Using a consistent mapping for membership tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
@brickpop
brickpop committed Aug 13, 2024
1 parent e8efa22 commit c8cc8ae
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 114 deletions.
3 changes: 0 additions & 3 deletions packages/contracts/src/constants.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

pragma solidity ^0.8.8;

// The ID of the permission required to contribute content to Personal Space proposals.
bytes32 constant MEMBER_PERMISSION_ID = keccak256("MEMBER_PERMISSION");

// The ID of the permission required to approve proposals or manage a Personal Space plugin.
bytes32 constant EDITOR_PERMISSION_ID = keccak256("EDITOR_PERMISSION");

Expand Down
69 changes: 22 additions & 47 deletions packages/contracts/src/personal/PersonalAdminPlugin.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {SpacePlugin} from "../space/SpacePlugin.sol";
import {IMembers} from "../base/IMembers.sol";
import {IEditors} from "../base/IEditors.sol";
import {PersonalMemberAddHelper} from "./PersonalMemberAddHelper.sol";
import {EDITOR_PERMISSION_ID, MEMBER_PERMISSION_ID} from "../constants.sol";
import {EDITOR_PERMISSION_ID} from "../constants.sol";

/// @title PersonalAdminPlugin
/// @author Aragon - 2023
Expand All @@ -35,6 +35,12 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors,
/// @notice The ID of the permission required to call the `addMember` function.
bytes32 public constant ADD_MEMBER_PERMISSION_ID = keccak256("ADD_MEMBER_PERMISSION");

/// @notice The address of the plugin where new memberships are approved, using a different set of rules.
PersonalMemberAddHelper public personalMemberAddHelper;

/// @notice Whether an address is considered as a space member (not editor)
mapping(address => bool) internal members;

/// @notice Thrown when attempting propose membership for an existing member.
error AlreadyAMember(address _member);

Expand All @@ -48,9 +54,6 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors,
_;
}

/// @notice The address of the plugin where new memberships are approved, using a different set of rules.
PersonalMemberAddHelper public personalMemberAddHelper;

/// @notice Initializes the contract.
/// @param _dao The associated DAO.
/// @dev This method is required to support [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167).
Expand Down Expand Up @@ -78,19 +81,17 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors,
super.supportsInterface(_interfaceId);
}

/// @notice Returns whether the given address holds membership/editor permission on the plugin
function isMember(address _account) public view returns (bool) {
return
dao().hasPermission(address(this), _account, MEMBER_PERMISSION_ID, bytes("")) ||
isEditor(_account);
}

/// @notice Returns whether the given address holds editor permission on the plugin
function isEditor(address _account) public view returns (bool) {
// Does the address hold the permission on the plugin?
return dao().hasPermission(address(this), _account, EDITOR_PERMISSION_ID, bytes(""));
}

/// @notice Returns whether the given address holds membership/editor permission on the plugin
function isMember(address _account) public view returns (bool) {
return members[_account] || isEditor(_account);
}

/// @notice Creates and executes a new proposal.
/// @param _metadata The metadata of the proposal.
/// @param _actions The actions to be executed.
Expand Down Expand Up @@ -159,62 +160,36 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors,
// The event will be emitted by the space plugin
}

/// @notice Creates and executes a proposal that makes the DAO grant membership permission to the given address.
/// @param _newMember The address to grant member permission to
/// @notice Sets the given address as a member of the space
/// @param _newMember The address to define as a member
/// @dev Called by the DAO via the PersonalMemberAddHelper. Not by members or editors.
function addMember(address _newMember) public auth(ADD_MEMBER_PERMISSION_ID) {
if (dao().hasPermission(address(this), _newMember, MEMBER_PERMISSION_ID, bytes(""))) {
if (members[_newMember]) {
revert AlreadyAMember(_newMember);
}

IDAO.Action[] memory _actions = new IDAO.Action[](1);
_actions[0].to = address(dao());
_actions[0].data = abi.encodeCall(
PermissionManager.grant,
(address(this), _newMember, MEMBER_PERMISSION_ID)
);

uint256 _proposalId = _createProposal(msg.sender, _actions);

dao().execute(bytes32(_proposalId), _actions, 0);

members[_newMember] = true;
emit MemberAdded(address(dao()), _newMember);
}

/// @notice Creates and executes a proposal that makes the DAO revoke membership permission from the given address
/// @param _member The address that will no longer be a member
function submitRemoveMember(address _member) public auth(EDITOR_PERMISSION_ID) {
IDAO.Action[] memory _actions = new IDAO.Action[](1);
_actions[0].to = address(dao());
_actions[0].data = abi.encodeCall(
PermissionManager.revoke,
(address(this), _member, MEMBER_PERMISSION_ID)
);

uint256 _proposalId = _createProposal(msg.sender, _actions);

dao().execute(bytes32(_proposalId), _actions, 0);
if (!members[_member]) return;

members[_member] = false;
emit MemberRemoved(address(dao()), _member);
}

/// @notice Creates and executes a proposal that makes the DAO revoke any permission from the sender address
function leaveSpace() external {
IDAO.Action[] memory _actions;
if (dao().hasPermission(address(this), msg.sender, MEMBER_PERMISSION_ID, bytes(""))) {
_actions = new IDAO.Action[](1);
_actions[0].to = address(dao());
_actions[0].data = abi.encodeCall(
PermissionManager.revoke,
(address(this), msg.sender, MEMBER_PERMISSION_ID)
);

uint256 _proposalId = _createProposal(msg.sender, _actions);
dao().execute(bytes32(_proposalId), _actions, 0);
if (members[msg.sender]) {
members[msg.sender] = false;
emit MemberLeft(address(dao()), msg.sender);
}

if (isEditor(msg.sender)) {
IDAO.Action[] memory _actions;
_actions = new IDAO.Action[](1);
_actions[0].to = address(dao());
_actions[0].data = abi.encodeCall(
Expand Down Expand Up @@ -270,7 +245,7 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors,
bytes calldata _metadataContentUri,
address _proposedMember
) public returns (uint256 proposalId) {
if (dao().hasPermission(address(this), _proposedMember, MEMBER_PERMISSION_ID, bytes(""))) {
if (members[_proposedMember]) {
revert AlreadyAMember(_proposedMember);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/contracts/src/standard/StdGovernancePlugin.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ contract StdGovernancePlugin is Addresslist, MajorityVotingBase, IEditors, IMemb
bytes calldata _metadataContentUri,
address _proposedMember
) public returns (uint256 proposalId) {
if (isMember(_proposedMember)) {
if (members[_proposedMember]) {
revert AlreadyAMember(_proposedMember);
}

Expand All @@ -384,7 +384,7 @@ contract StdGovernancePlugin is Addresslist, MajorityVotingBase, IEditors, IMemb
) public returns (uint256 proposalId) {
if (!isEditor(msg.sender)) {
revert Unauthorized();
} else if (!isMember(_member)) {
} else if (!members[_member]) {
revert AlreadyNotAMember(_member);
}

Expand Down
1 change: 0 additions & 1 deletion packages/contracts/test/unit-testing/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const ONE_BYTES32 =

export const DEPLOYER_PERMISSION_ID = ethers.utils.id('DEPLOYER_PERMISSION');
export const EDITOR_PERMISSION_ID = ethers.utils.id('EDITOR_PERMISSION');
export const MEMBER_PERMISSION_ID = ethers.utils.id('MEMBER_PERMISSION');

export const CONTENT_PERMISSION_ID = ethers.utils.id('CONTENT_PERMISSION');
export const SUBSPACE_PERMISSION_ID = ethers.utils.id('SUBSPACE_PERMISSION');
Expand Down
32 changes: 20 additions & 12 deletions packages/contracts/test/unit-testing/personal-admin-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
ADDRESS_TWO,
ADDRESS_ZERO,
CONTENT_PERMISSION_ID,
MEMBER_PERMISSION_ID,
EDITOR_PERMISSION_ID,
EXECUTE_PERMISSION_ID,
SUBSPACE_PERMISSION_ID,
Expand Down Expand Up @@ -131,11 +130,7 @@ describe('Personal Admin Plugin', function () {
EDITOR_PERMISSION_ID
);
// Bob is a member
await dao.grant(
personalAdminPlugin.address,
bob.address,
MEMBER_PERMISSION_ID
);
await makeMember(bob.address);
// The plugin can execute on the DAO
await dao.grant(
dao.address,
Expand Down Expand Up @@ -216,13 +211,11 @@ describe('Personal Admin Plugin', function () {
expect(await personalAdminPlugin.isMember(bob.address)).to.eq(true);
expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false);

await dao.grant(
personalAdminPlugin.address,
carol.address,
MEMBER_PERMISSION_ID
);

await makeMember(carol.address);
expect(await personalAdminPlugin.isMember(carol.address)).to.eq(true);

await pullMember(carol.address);
expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false);
});

it('isEditor() returns true when appropriate', async () => {
Expand Down Expand Up @@ -675,4 +668,19 @@ describe('Personal Admin Plugin', function () {
});
});
});

// Helpers

function makeMember(account: string) {
return personalAdminPlugin
.connect(alice)
.proposeAddMember('0x', account)
.then(tx => tx.wait());
}
function pullMember(account: string) {
return personalAdminPlugin
.connect(alice)
.submitRemoveMember(account)
.then(tx => tx.wait());
}
});
Loading

0 comments on commit c8cc8ae

Please sign in to comment.