diff --git a/packages/contracts/src/governance/GovernancePluginsSetup.sol b/packages/contracts/src/governance/GovernancePluginsSetup.sol index 48630bb..639c811 100644 --- a/packages/contracts/src/governance/GovernancePluginsSetup.sol +++ b/packages/contracts/src/governance/GovernancePluginsSetup.sol @@ -45,25 +45,34 @@ contract GovernancePluginsSetup is PluginSetup { address _pluginUpgrader ) = decodeInstallationParams(_data); + // Deploy the member access plugin + address _memberAccessPlugin = createERC1967Proxy( + memberAccessPluginImplementation, + abi.encodeCall( + MemberAccessPlugin.initialize, + ( + IDAO(_dao), + MemberAccessPlugin.MultisigSettings({ + proposalDuration: _memberAccessProposalDuration + }) + ) + ) + ); + // Deploy the main voting plugin mainVotingPlugin = createERC1967Proxy( mainVotingPluginImplementation, abi.encodeCall( MainVotingPlugin.initialize, - (IDAO(_dao), _votingSettings, _initialEditors) + ( + IDAO(_dao), + _votingSettings, + _initialEditors, + MemberAccessPlugin(_memberAccessPlugin) + ) ) ); - // Deploy the member access plugin - MemberAccessPlugin.MultisigSettings memory _multisigSettings; - _multisigSettings.proposalDuration = _memberAccessProposalDuration; - _multisigSettings.mainVotingPlugin = MainVotingPlugin(mainVotingPlugin); - - address _memberAccessPlugin = createERC1967Proxy( - memberAccessPluginImplementation, - abi.encodeCall(MemberAccessPlugin.initialize, (IDAO(_dao), _multisigSettings)) - ); - // Condition contract (member access plugin execute) address _memberAccessExecuteCondition = address( new MemberAccessExecuteCondition(mainVotingPlugin) @@ -72,7 +81,7 @@ contract GovernancePluginsSetup is PluginSetup { // List the requested permissions PermissionLib.MultiTargetPermission[] memory permissions = new PermissionLib.MultiTargetPermission[]( - _pluginUpgrader == address(0x0) ? 5 : 6 + _pluginUpgrader == address(0x0) ? 6 : 7 ); // The main voting plugin can execute on the DAO @@ -102,16 +111,26 @@ contract GovernancePluginsSetup is PluginSetup { .UPDATE_ADDRESSES_PERMISSION_ID() }); - // The member access plugin needs to execute on the DAO + // The MainVotingPlugin can create membership proposals on the MemberAccessPlugin permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Grant, + where: _memberAccessPlugin, + who: mainVotingPlugin, + condition: PermissionLib.NO_CONDITION, + permissionId: MemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + }); + + // The member access plugin needs to execute on the DAO + permissions[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.GrantWithCondition, where: _dao, who: _memberAccessPlugin, + // Conditional execution condition: _memberAccessExecuteCondition, permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO needs to be able to update the member access plugin settings - permissions[4] = PermissionLib.MultiTargetPermission({ + permissions[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, where: _memberAccessPlugin, who: _dao, @@ -134,7 +153,7 @@ contract GovernancePluginsSetup is PluginSetup { PluginSetupProcessor(pluginSetupProcessor), _targetPluginAddresses ); - permissions[5] = PermissionLib.MultiTargetPermission({ + permissions[6] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.GrantWithCondition, where: _dao, who: _pluginUpgrader, @@ -162,7 +181,7 @@ contract GovernancePluginsSetup is PluginSetup { address _memberAccessPlugin = _payload.currentHelpers[0]; permissionChanges = new PermissionLib.MultiTargetPermission[]( - _pluginUpgrader == address(0x0) ? 5 : 6 + _pluginUpgrader == address(0x0) ? 6 : 7 ); // Main voting plugin permissions @@ -194,10 +213,19 @@ contract GovernancePluginsSetup is PluginSetup { .UPDATE_ADDRESSES_PERMISSION_ID() }); - // Member access plugin permissions + // Plugin permissions - // The plugin can no longer execute on the DAO + // The MainVotingPlugin can no longer propose on the MemberAccessPlugin permissionChanges[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _memberAccessPlugin, + who: _payload.plugin, + condition: address(0), + permissionId: MemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + }); + + // The plugin can no longer execute on the DAO + permissionChanges[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, who: _memberAccessPlugin, @@ -205,7 +233,7 @@ contract GovernancePluginsSetup is PluginSetup { permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO can no longer update the plugin settings - permissionChanges[4] = PermissionLib.MultiTargetPermission({ + permissionChanges[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _memberAccessPlugin, who: _dao, @@ -217,7 +245,7 @@ contract GovernancePluginsSetup is PluginSetup { if (_pluginUpgrader != address(0x0)) { // pluginUpgrader can no longer make the DAO execute applyUpdate // pluginUpgrader can no longer make the DAO execute grant/revoke - permissionChanges[5] = PermissionLib.MultiTargetPermission({ + permissionChanges[6] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, who: _pluginUpgrader, diff --git a/packages/contracts/src/governance/MainVotingPlugin.sol b/packages/contracts/src/governance/MainVotingPlugin.sol index 3317491..88f5bc9 100644 --- a/packages/contracts/src/governance/MainVotingPlugin.sol +++ b/packages/contracts/src/governance/MainVotingPlugin.sol @@ -10,6 +10,7 @@ import {MajorityVotingBase} from "./base/MajorityVotingBase.sol"; import {IMembers} from "../base/IMembers.sol"; import {IEditors} from "../base/IEditors.sol"; import {Addresslist} from "./base/Addresslist.sol"; +import {MemberAccessPlugin} from "./MemberAccessPlugin.sol"; import {SpacePlugin} from "../space/SpacePlugin.sol"; // The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract. @@ -18,6 +19,7 @@ bytes4 constant MAIN_SPACE_VOTING_INTERFACE_ID = MainVotingPlugin.initialize.sel MainVotingPlugin.proposeEdits.selector ^ MainVotingPlugin.proposeAcceptSubspace.selector ^ MainVotingPlugin.proposeRemoveSubspace.selector ^ + MainVotingPlugin.proposeAddMember.selector ^ MainVotingPlugin.proposeRemoveMember.selector ^ MainVotingPlugin.proposeAddEditor.selector ^ MainVotingPlugin.proposeRemoveEditor.selector ^ @@ -45,6 +47,9 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers /// @notice Whether an address is considered as a space member (not editor) mapping(address => bool) internal members; + /// @notice The address of the plugin where new memberships are approved, using a different set of rules. + MemberAccessPlugin public memberAccessPlugin; + /// @notice Emitted when the creator cancels a proposal event ProposalCanceled(uint256 proposalId); @@ -98,12 +103,15 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers function initialize( IDAO _dao, VotingSettings calldata _votingSettings, - address[] calldata _initialEditors + address[] calldata _initialEditors, + MemberAccessPlugin _memberAccessPlugin ) external initializer { __MajorityVotingBase_init(_dao, _votingSettings); _addAddresses(_initialEditors); emit EditorsAdded(_initialEditors); + + memberAccessPlugin = _memberAccessPlugin; } /// @notice Checks if this or the parent contract supports an interface by its ID. @@ -339,19 +347,18 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers /// @notice Creates a proposal to add a new member. /// @param _metadata The metadata of the proposal. /// @param _proposedMember The address of the member who may eveutnally be added. + /// @return proposalId NOTE: The proposal ID will belong to the Multisig plugin, not to this contract. function proposeAddMember( bytes calldata _metadata, address _proposedMember - ) public onlyMembers returns (uint256 proposalId) { + ) public returns (uint256 proposalId) { if (isMember(_proposedMember)) { revert AlreadyAMember(_proposedMember); } - proposalId = _proposeWrappedAction( - _metadata, - address(this), - abi.encodeCall(MainVotingPlugin.addMember, (_proposedMember)) - ); + /// @dev Creating the actual proposal on a separate plugin because the approval rules differ. + /// @dev Keeping all wrappers on the MainVoting plugin, even if one type of approvals are handled on the MemberAccess plugin. + return memberAccessPlugin.proposeAddMember(_metadata, _proposedMember, msg.sender); } /// @notice Creates a proposal to remove an existing member. @@ -554,5 +561,5 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers /// @dev This empty reserved space is put in place to allow future versions to add new /// variables without shifting down storage in the inheritance chain. /// https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps - uint256[48] private __gap; + uint256[47] private __gap; } diff --git a/packages/contracts/src/governance/MemberAccessPlugin.sol b/packages/contracts/src/governance/MemberAccessPlugin.sol index bfb3527..8d07fbb 100644 --- a/packages/contracts/src/governance/MemberAccessPlugin.sol +++ b/packages/contracts/src/governance/MemberAccessPlugin.sol @@ -8,12 +8,14 @@ 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 {Addresslist} from "./base/Addresslist.sol"; import {IMultisig} from "./base/IMultisig.sol"; +import {IEditors} from "../base/IEditors.sol"; import {MainVotingPlugin, MAIN_SPACE_VOTING_INTERFACE_ID} from "./MainVotingPlugin.sol"; bytes4 constant MEMBER_ACCESS_INTERFACE_ID = MemberAccessPlugin.initialize.selector ^ MemberAccessPlugin.updateMultisigSettings.selector ^ - MemberAccessPlugin.proposeNewMember.selector ^ + MemberAccessPlugin.proposeAddMember.selector ^ MemberAccessPlugin.getProposal.selector; /// @title Member access plugin (Multisig) - Release 1, Build 1 @@ -26,6 +28,9 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade bytes32 public constant UPDATE_MULTISIG_SETTINGS_PERMISSION_ID = keccak256("UPDATE_MULTISIG_SETTINGS_PERMISSION"); + /// @notice The ID of the permission required to create new membership proposals. + bytes32 public constant PROPOSER_PERMISSION_ID = keccak256("PROPOSER_PERMISSION"); + /// @notice The minimum total amount of approvals required for proposals created by a non-editor uint16 internal constant MIN_APPROVALS_WHEN_CREATED_BY_NON_EDITOR = uint16(1); @@ -48,6 +53,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade ProposalParameters parameters; mapping(address => bool) approvers; IDAO.Action[] actions; + MainVotingPlugin mainVotingPlugin; uint256 failsafeActionMap; } @@ -68,7 +74,6 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade /// @param mainVotingPlugin The address of the main voting plugin. Used to apply permissions for it. struct MultisigSettings { uint64 proposalDuration; - MainVotingPlugin mainVotingPlugin; } /// @notice A mapping between proposal IDs and proposal information. @@ -81,9 +86,8 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade /// @dev This variable prevents a proposal from being created in the same block in which the multisig settings change. uint64 public lastMultisigSettingsChange; - /// @notice Thrown when a sender is not allowed to create a proposal. - /// @param sender The sender address. - error ProposalCreationForbidden(address sender); + /// @notice Thrown when creating a proposal at the same block that the settings were changed. + error ProposalCreationForbiddenOnSameBlock(); /// @notice Thrown if an approver is not allowed to cast an approve. This can be because the proposal /// - is not open, @@ -97,14 +101,8 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade /// @param proposalId The ID of the proposal. error ProposalExecutionForbidden(uint256 proposalId); - /// @notice Thrown when attempting to use addAddresses. - error AddresslistDisabled(); - - /// @notice Thrown when attempting to use an invalid contract. - error InvalidContract(); - - /// @notice Thrown when attempting request membership for a current member. - error AlreadyMember(address _member); + /// @notice Thrown when called from an incompatible contract. + error InvalidCallerInterface(); /// @notice Emitted when a proposal is approved by an editor. /// @param proposalId The ID of the proposal. @@ -118,8 +116,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade /// @notice Emitted when the plugin settings are set. /// @param proposalDuration The amount of time before a non-approved proposal expires. - /// @param mainVotingPlugin The address of the main voting plugin for the space. Used to apply permissions for it. - event MultisigSettingsUpdated(uint64 proposalDuration, address mainVotingPlugin); + event MultisigSettingsUpdated(uint64 proposalDuration); /// @notice Initializes Release 1, Build 1. /// @dev This method is required to support [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822). @@ -154,14 +151,35 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade _updateMultisigSettings(_multisigSettings); } - /// @notice Creates a new multisig proposal wrapped by proposeNewMember. + /// @notice Creates a proposal to add a new member. /// @param _metadata The metadata of the proposal. - /// @param _actions A list of actions wrapped by proposeNewMember. + /// @param _proposedMember The address of the member who may eveutnally be added. + /// @param _proposer The address to use as the proposal creator. /// @return proposalId The ID of the proposal. - function createProposal( + function proposeAddMember( bytes calldata _metadata, - IDAO.Action[] memory _actions - ) internal returns (uint256 proposalId) { + address _proposedMember, + address _proposer + ) external auth(PROPOSER_PERMISSION_ID) returns (uint256 proposalId) { + // Check that the caller supports the `addMember` function + if ( + !MainVotingPlugin(msg.sender).supportsInterface(MAIN_SPACE_VOTING_INTERFACE_ID) || + !MainVotingPlugin(msg.sender).supportsInterface(type(IEditors).interfaceId) || + !MainVotingPlugin(msg.sender).supportsInterface(type(Addresslist).interfaceId) + ) { + revert InvalidCallerInterface(); + } + + // Build the list of actions + IDAO.Action[] memory _actions = new IDAO.Action[](1); + + _actions[0] = IDAO.Action({ + to: address(msg.sender), // We are called by the MainVotingPlugin + value: 0, + data: abi.encodeCall(MainVotingPlugin.addMember, (_proposedMember)) + }); + + // Create proposal uint64 snapshotBlock; unchecked { snapshotBlock = block.number.toUint64() - 1; // The snapshot block must be mined already to protect the transaction against backrunning transactions causing census changes. @@ -170,7 +188,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade // Revert if the settings have been changed in the same block as this proposal should be created in. // This prevents a malicious party from voting with previous addresses and the new settings. if (lastMultisigSettingsChange > snapshotBlock) { - revert ProposalCreationForbidden(msg.sender); + revert ProposalCreationForbiddenOnSameBlock(); } uint64 _startDate = block.timestamp.toUint64(); @@ -180,7 +198,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade emit ProposalCreated({ proposalId: proposalId, - creator: msg.sender, + creator: _proposer, metadata: _metadata, startDate: _startDate, endDate: _endDate, @@ -195,6 +213,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade proposal_.parameters.startDate = _startDate; proposal_.parameters.endDate = _endDate; + proposal_.mainVotingPlugin = MainVotingPlugin(msg.sender); for (uint256 i; i < _actions.length; ) { proposal_.actions.push(_actions[i]); unchecked { @@ -202,8 +221,12 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade } } - if (isEditor(msg.sender)) { - if (multisigSettings.mainVotingPlugin.addresslistLength() < 2) { + // Another editor needs to approve. Set the minApprovals accordingly + /// @dev The _proposer parameter is technically trusted. + /// @dev However, this function is protected by PROPOSER_PERMISSION_ID and only the MainVoting plugin is granted this permission. + /// @dev See GovernancePluginsSetup.sol + if (MainVotingPlugin(msg.sender).isEditor(_proposer)) { + if (MainVotingPlugin(msg.sender).addresslistLength() < 2) { proposal_.parameters.minApprovals = MIN_APPROVALS_WHEN_CREATED_BY_SINGLE_EDITOR; } else { proposal_.parameters.minApprovals = MIN_APPROVALS_WHEN_CREATED_BY_EDITOR_OF_MANY; @@ -216,30 +239,6 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade } } - /// @notice Creates a proposal to add a new member. - /// @param _metadata The metadata of the proposal. - /// @param _proposedMember The address of the member who may eveutnally be added. - /// @return proposalId The ID of the proposal. - function proposeNewMember( - bytes calldata _metadata, - address _proposedMember - ) external returns (uint256 proposalId) { - if (isMember(_proposedMember)) { - revert AlreadyMember(_proposedMember); - } - - // Build the list of actions - IDAO.Action[] memory _actions = new IDAO.Action[](1); - - _actions[0] = IDAO.Action({ - to: address(multisigSettings.mainVotingPlugin), - value: 0, - data: abi.encodeCall(MainVotingPlugin.addMember, (_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) public { @@ -331,18 +330,6 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade _execute(_proposalId); } - /// @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 multisigSettings.mainVotingPlugin.isMember(_account); - } - - /// @notice Returns whether the given address holds editor permission on the main voting plugin - function isEditor(address _account) public view returns (bool) { - // Does the address hold the permission on the main voting plugin? - return multisigSettings.mainVotingPlugin.isEditor(_account); - } - /// @notice Internal function to execute a vote. It assumes the queried proposal exists. /// @param _proposalId The ID of the proposal. function _execute(uint256 _proposalId) internal { @@ -368,7 +355,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade if (!_isProposalOpen(proposal_)) { // The proposal was executed already return false; - } else if (!isEditor(_account)) { + } else if (!proposal_.mainVotingPlugin.isEditor(_account)) { // The approver has no voting power. return false; } else if (proposal_.approvers[_account]) { @@ -407,21 +394,10 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade /// @notice Internal function to update the plugin settings. /// @param _multisigSettings The new settings. function _updateMultisigSettings(MultisigSettings calldata _multisigSettings) internal { - if ( - !MainVotingPlugin(_multisigSettings.mainVotingPlugin).supportsInterface( - MAIN_SPACE_VOTING_INTERFACE_ID - ) - ) { - revert InvalidContract(); - } - multisigSettings = _multisigSettings; lastMultisigSettingsChange = block.number.toUint64(); - emit MultisigSettingsUpdated({ - proposalDuration: _multisigSettings.proposalDuration, - mainVotingPlugin: address(_multisigSettings.mainVotingPlugin) - }); + emit MultisigSettingsUpdated({proposalDuration: _multisigSettings.proposalDuration}); } /// @dev This empty reserved space is put in place to allow future versions to add new diff --git a/packages/contracts/src/test/TestGovernancePluginsSetup.sol b/packages/contracts/src/test/TestGovernancePluginsSetup.sol index 1ae3e18..2b247c5 100644 --- a/packages/contracts/src/test/TestGovernancePluginsSetup.sol +++ b/packages/contracts/src/test/TestGovernancePluginsSetup.sol @@ -44,25 +44,29 @@ contract TestGovernancePluginsSetup is PluginSetup { address _pluginUpgrader ) = decodeInstallationParams(_data); - // Deploy the main voting plugin - mainVotingPlugin = createERC1967Proxy( - mainVotingPluginImplementationAddr, - abi.encodeCall( - MainVotingPlugin.initialize, - (IDAO(_dao), _votingSettings, _initialEditors) - ) - ); - // Deploy the member access plugin TestMemberAccessPlugin.MultisigSettings memory _multisigSettings; _multisigSettings.proposalDuration = _memberAccessProposalDuration; - _multisigSettings.mainVotingPlugin = MainVotingPlugin(mainVotingPlugin); address _memberAccessPlugin = createERC1967Proxy( memberAccessPluginImplementation(), abi.encodeCall(TestMemberAccessPlugin.initialize, (IDAO(_dao), _multisigSettings)) ); + // Deploy the main voting plugin + mainVotingPlugin = createERC1967Proxy( + mainVotingPluginImplementationAddr, + abi.encodeCall( + MainVotingPlugin.initialize, + ( + IDAO(_dao), + _votingSettings, + _initialEditors, + TestMemberAccessPlugin(_memberAccessPlugin) + ) + ) + ); + // Condition contract (member access plugin execute) address _memberAccessExecuteCondition = address( new MemberAccessExecuteCondition(mainVotingPlugin) @@ -71,7 +75,7 @@ contract TestGovernancePluginsSetup is PluginSetup { // List the requested permissions PermissionLib.MultiTargetPermission[] memory permissions = new PermissionLib.MultiTargetPermission[]( - _pluginUpgrader == address(0x0) ? 5 : 6 + _pluginUpgrader == address(0x0) ? 6 : 7 ); // The main voting plugin can execute on the DAO @@ -103,6 +107,15 @@ contract TestGovernancePluginsSetup is PluginSetup { // The member access plugin needs to execute on the DAO permissions[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.GrantWithCondition, + where: _memberAccessPlugin, + who: mainVotingPlugin, + condition: PermissionLib.NO_CONDITION, + permissionId: TestMemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + }); + + // The member access plugin needs to execute on the DAO + permissions[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.GrantWithCondition, where: _dao, who: _memberAccessPlugin, @@ -110,7 +123,7 @@ contract TestGovernancePluginsSetup is PluginSetup { permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO needs to be able to update the member access plugin settings - permissions[4] = PermissionLib.MultiTargetPermission({ + permissions[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, where: _memberAccessPlugin, who: _dao, @@ -133,7 +146,7 @@ contract TestGovernancePluginsSetup is PluginSetup { PluginSetupProcessor(pluginSetupProcessor), _targetPluginAddresses ); - permissions[5] = PermissionLib.MultiTargetPermission({ + permissions[6] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.GrantWithCondition, where: _dao, who: _pluginUpgrader, @@ -193,7 +206,7 @@ contract TestGovernancePluginsSetup is PluginSetup { address _memberAccessPlugin = _payload.currentHelpers[0]; permissionChanges = new PermissionLib.MultiTargetPermission[]( - _pluginUpgrader == address(0x0) ? 5 : 6 + _pluginUpgrader == address(0x0) ? 6 : 7 ); // Main voting plugin permissions @@ -227,8 +240,17 @@ contract TestGovernancePluginsSetup is PluginSetup { // Member access plugin permissions - // The plugin can no longer execute on the DAO + // The member access plugin needs to execute on the DAO permissionChanges[3] = PermissionLib.MultiTargetPermission({ + operation: PermissionLib.Operation.Revoke, + where: _memberAccessPlugin, + who: _payload.plugin, + condition: PermissionLib.NO_CONDITION, + permissionId: TestMemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + }); + + // The plugin can no longer execute on the DAO + permissionChanges[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, who: _memberAccessPlugin, @@ -236,7 +258,7 @@ contract TestGovernancePluginsSetup is PluginSetup { permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO can no longer update the plugin settings - permissionChanges[4] = PermissionLib.MultiTargetPermission({ + permissionChanges[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _memberAccessPlugin, who: _dao, @@ -248,7 +270,7 @@ contract TestGovernancePluginsSetup is PluginSetup { if (_pluginUpgrader != address(0x0)) { // pluginUpgrader can no longer make the DAO execute applyUpdate // pluginUpgrader can no longer make the DAO execute grant/revoke - permissionChanges[5] = PermissionLib.MultiTargetPermission({ + permissionChanges[6] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, who: _pluginUpgrader, diff --git a/packages/contracts/src/test/TestMemberAccessPlugin.sol b/packages/contracts/src/test/TestMemberAccessPlugin.sol index 5bcdf0d..ade26f9 100644 --- a/packages/contracts/src/test/TestMemberAccessPlugin.sol +++ b/packages/contracts/src/test/TestMemberAccessPlugin.sol @@ -7,14 +7,6 @@ import {MemberAccessPlugin} from "../governance/MemberAccessPlugin.sol"; /// @notice A clone of the MemberAccessPlugin contract, just to test contract TestMemberAccessPlugin is MemberAccessPlugin { - function createArbitraryProposal( - bytes calldata _metadata, - IDAO.Action[] memory _actions - ) public returns (uint256 proposalId) { - // Exposing createProposal for E2E testing - return createProposal(_metadata, _actions); - } - function initialize( IDAO _dao, MultisigSettings calldata _multisigSettings