Skip to content

Commit

Permalink
Internal suggestions - contract changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@brickpop
brickpop committed Jan 26, 2024
1 parent 195fd4a commit cd25fae
Show file tree
Hide file tree
Showing 6 changed files with 670 additions and 65 deletions.
2 changes: 1 addition & 1 deletion packages/contracts/src/governance/GovernancePluginsSetup.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {MemberAccessPlugin} from "./MemberAccessPlugin.sol";
import {MemberAccessExecuteCondition} from "../conditions/MemberAccessExecuteCondition.sol";
import {OnlyPluginUpgraderCondition} from "../conditions/OnlyPluginUpgraderCondition.sol";
import {MainVotingPlugin} from "./MainVotingPlugin.sol";
import {MajorityVotingBase} from "@aragon/osx/plugins/governance/majority-voting/MajorityVotingBase.sol";
import {MajorityVotingBase} from "./base/MajorityVotingBase.sol";

/// @title GovernancePluginsSetup
/// @dev Release 1, Build 1
Expand Down
62 changes: 43 additions & 19 deletions packages/contracts/src/governance/MainVotingPlugin.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {IMembership} from "@aragon/osx/core/plugin/membership/IMembership.sol";
import {Addresslist} from "@aragon/osx/plugins/utils/Addresslist.sol";
import {RATIO_BASE, _applyRatioCeiled} from "@aragon/osx/plugins/utils/Ratio.sol";
import {IMajorityVoting} from "@aragon/osx/plugins/governance/majority-voting/IMajorityVoting.sol";
import {MajorityVotingBase} from "@aragon/osx/plugins/governance/majority-voting/MajorityVotingBase.sol";
import {MEMBER_PERMISSION_ID} from "../constants.sol";
import {MajorityVotingBase} from "./base/MajorityVotingBase.sol";

// The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract.
bytes4 constant MAIN_SPACE_VOTING_INTERFACE_ID = MainVotingPlugin.initialize.selector ^
Expand All @@ -29,8 +28,12 @@ contract MainVotingPlugin is IMembership, Addresslist, MajorityVotingBase {
bytes32 public constant UPDATE_ADDRESSES_PERMISSION_ID =
keccak256("UPDATE_ADDRESSES_PERMISSION");

/// @notice Who created each proposal
mapping(uint256 => address) internal proposalCreators;

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

/// @notice Emitted when the creator cancels a proposal
event ProposalCanceled(uint256 proposalId);

Expand All @@ -43,6 +46,12 @@ contract MainVotingPlugin is IMembership, Addresslist, MajorityVotingBase {
/// @notice Raised when a wallet who is not an editor or a member attempts to do something
error NotAMember(address caller);

/// @notice Raised when an address is defined as a space member
event NewSpaceMember(address account);

/// @notice Raised when an address is removed as a space member
event RemovedSpaceMember(address account);

/// @notice Raised when someone who didn't create a proposal attempts to cancel it
error OnlyCreatorCanCancel();

Expand Down Expand Up @@ -82,28 +91,47 @@ contract MainVotingPlugin is IMembership, Addresslist, MajorityVotingBase {
super.supportsInterface(_interfaceId);
}

/// @notice Adds new members to the address list.
/// @param _members The addresses of members to be added. NOTE: Only one member can be added at a time.
/// @notice Adds new editors to the address list.
/// @param _editors The addresses of the editors to be added. NOTE: Only one member can be added at a time.
/// @dev This function is used during the plugin initialization.
function addAddresses(
address[] calldata _members
address[] calldata _editors
) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
if (_members.length > 1) revert OnlyOneEditorPerCall(_members.length);
if (_editors.length > 1) revert OnlyOneEditorPerCall(_editors.length);

_addAddresses(_members);
emit MembersAdded({members: _members});
_addAddresses(_editors);
emit MembersAdded({members: _editors});
}

/// @notice Removes existing members from the address list.
/// @param _members The addresses of the members to be removed. NOTE: Only one member can be removed at a time.
/// @notice Removes existing editors from the address list.
/// @param _editors The addresses of the editors to be removed. NOTE: Only one member can be removed at a time.
function removeAddresses(
address[] calldata _members
address[] calldata _editors
) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
if (_members.length > 1) revert OnlyOneEditorPerCall(_members.length);
if (_editors.length > 1) revert OnlyOneEditorPerCall(_editors.length);
else if (addresslistLength() <= 1) revert NoEditorsLeft();

_removeAddresses(_members);
emit MembersRemoved({members: _members});
_removeAddresses(_editors);
emit MembersRemoved({members: _editors});
}

/// @notice Defines the given address as a new space member that can create proposals.
/// @param _account The address of the space member to be added.
/// @dev This function is used during the plugin initialization.
function addSpaceMember(address _account) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
if (members[_account]) return;

members[_account] = true;
emit NewSpaceMember({account: _account});
}

/// @notice Removes the given address as a proposal creator.
/// @param _account The address of the space member to be removed.
function removeSpaceMember(address _account) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
if (!members[_account]) return;

members[_account] = false;
emit RemovedSpaceMember({account: _account});
}

/// @inheritdoc MajorityVotingBase
Expand All @@ -113,9 +141,7 @@ contract MainVotingPlugin is IMembership, Addresslist, MajorityVotingBase {

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

/// @notice Returns whether the given address is currently listed as an editor
Expand All @@ -128,8 +154,6 @@ contract MainVotingPlugin is IMembership, Addresslist, MajorityVotingBase {
bytes calldata _metadata,
IDAO.Action[] calldata _actions,
uint256 _allowFailureMap,
uint64,
uint64,
VoteOption _voteOption,
bool _tryEarlyExecution
) external override onlyMembers returns (uint256 proposalId) {
Expand Down
52 changes: 8 additions & 44 deletions packages/contracts/src/governance/MemberAccessPlugin.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
import {PermissionManager} from "@aragon/osx/core/permission/PermissionManager.sol";
import {PluginUUPSUpgradeable} from "@aragon/osx/core/plugin/PluginUUPSUpgradeable.sol";
import {ProposalUpgradeable} from "@aragon/osx/core/plugin/proposal/ProposalUpgradeable.sol";
import {IMultisig} from "@aragon/osx/plugins/governance/multisig/IMultisig.sol";
import {IMultisig} from "./base/IMultisig.sol";
import {MainVotingPlugin, MAIN_SPACE_VOTING_INTERFACE_ID} from "./MainVotingPlugin.sol";
import {MEMBER_PERMISSION_ID} from "../constants.sol";

bytes4 constant MEMBER_ACCESS_INTERFACE_ID = MemberAccessPlugin.initialize.selector ^
MemberAccessPlugin.updateMultisigSettings.selector ^
Expand Down Expand Up @@ -151,20 +150,6 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade
super.supportsInterface(_interfaceId);
}

/// @notice This function is kept for compatibility with the multisig base class, but will not produce any effect.
function addAddresses(
address[] calldata
) external view auth(UPDATE_MULTISIG_SETTINGS_PERMISSION_ID) {
revert AddresslistDisabled();
}

/// @notice This function is kept for compatibility with the multisig base class, but will not produce any effect.
function removeAddresses(
address[] calldata
) external view auth(UPDATE_MULTISIG_SETTINGS_PERMISSION_ID) {
revert AddresslistDisabled();
}

/// @notice Updates the plugin settings.
/// @param _multisigSettings The new settings.
function updateMultisigSettings(
Expand Down Expand Up @@ -229,7 +214,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade
}

// If the creator is an editor, we assume that the editor approves
approve(proposalId, false);
approve(proposalId);
} else {
proposal_.parameters.minApprovals = MIN_APPROVALS_WHEN_CREATED_BY_NON_EDITOR;
}
Expand All @@ -251,16 +236,9 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade
IDAO.Action[] memory _actions = new IDAO.Action[](1);

_actions[0] = IDAO.Action({
to: address(dao()),
to: address(multisigSettings.mainVotingPlugin),
value: 0,
data: abi.encodeCall(
PermissionManager.grant,
(
address(multisigSettings.mainVotingPlugin), // where
_proposedMember, // who
MEMBER_PERMISSION_ID // permission ID
)
)
data: abi.encodeCall(MainVotingPlugin.addSpaceMember, (_proposedMember))
});

return createProposal(_metadata, _actions);
Expand All @@ -282,24 +260,17 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade
IDAO.Action[] memory _actions = new IDAO.Action[](1);

_actions[0] = IDAO.Action({
to: address(dao()),
to: address(multisigSettings.mainVotingPlugin),
value: 0,
data: abi.encodeCall(
PermissionManager.revoke,
(
address(multisigSettings.mainVotingPlugin), // where
_proposedMember, // who
MEMBER_PERMISSION_ID // permission ID
)
)
data: abi.encodeCall(MainVotingPlugin.removeSpaceMember, (_proposedMember))
});

return createProposal(_metadata, _actions);
}

/// @inheritdoc IMultisig
/// @dev The second parameter is left empty to keep compatibility with the existing multisig interface
function approve(uint256 _proposalId, bool) public {
function approve(uint256 _proposalId) public {
address sender = _msgSender();
if (!_canApprove(_proposalId, sender)) {
revert ApprovalCastForbidden(_proposalId, sender);
Expand Down Expand Up @@ -393,14 +364,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade
/// @notice Returns whether the given address holds membership permission on the main voting plugin
function isMember(address _account) public view returns (bool) {
// Does the address hold the member or editor permission on the main voting plugin?
return
isEditor(_account) ||
dao().hasPermission(
address(multisigSettings.mainVotingPlugin),
_account,
MEMBER_PERMISSION_ID,
bytes("")
);
return multisigSettings.mainVotingPlugin.isMember(_account);
}

/// @notice Returns whether the given address holds editor permission on the main voting plugin
Expand Down
38 changes: 38 additions & 0 deletions packages/contracts/src/governance/base/IMultisig.sol
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity 0.8.17;

import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";

/// @title IMultisig
/// @author Aragon Association - 2023
/// @notice An interface for an on-chain multisig governance plugin in which a proposal passes if X out of Y approvals are met.
interface IMultisig {
/// @notice Approves and, optionally, executes the proposal.
/// @param _proposalId The ID of the proposal.
function approve(uint256 _proposalId) external;

/// @notice Checks if an account can participate on a proposal vote. This can be because the vote
/// - was executed, or
/// - the voter is not listed.
/// @param _proposalId The proposal Id.
/// @param _account The address of the user to check.
/// @return Returns true if the account is allowed to vote.
/// @dev The function assumes the queried proposal exists.
function canApprove(uint256 _proposalId, address _account) external view returns (bool);

/// @notice Checks if a proposal can be executed.
/// @param _proposalId The ID of the proposal to be checked.
/// @return True if the proposal can be executed, false otherwise.
function canExecute(uint256 _proposalId) external view returns (bool);

/// @notice Returns whether the account has approved the proposal. Note, that this does not check if the account is listed.
/// @param _proposalId The ID of the proposal.
/// @param _account The account address to be checked.
/// @return The vote option cast by a voter for a certain proposal.
function hasApproved(uint256 _proposalId, address _account) external view returns (bool);

/// @notice Executes a proposal.
/// @param _proposalId The ID of the proposal to be executed.
function execute(uint256 _proposalId) external;
}
Loading

0 comments on commit cd25fae

Please sign in to comment.