From da5d8e82d5538b8ba90e74e53b3562cdd5df4f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Thu, 8 Aug 2024 17:53:24 +0200 Subject: [PATCH] Adapted plugin setup and renaming helper contracts to avoid confusion --- .../src/governance/GovernancePluginsSetup.sol | 65 +++++++++---------- ...cessPlugin.sol => MainMemberAddHelper.sol} | 12 ++-- .../src/governance/MainVotingPlugin.sol | 14 ++-- ...Helper.sol => PersonalMemberAddHelper.sol} | 59 ++++++++--------- .../PersonalSpaceAdminPluginSetup.sol | 50 +++++++++++--- .../src/test/TestGovernancePluginsSetup.sol | 48 +++++++------- 6 files changed, 137 insertions(+), 111 deletions(-) rename packages/contracts/src/governance/{MemberAccessPlugin.sol => MainMemberAddHelper.sol} (97%) rename packages/contracts/src/personal/{PersonalAccessHelper.sol => PersonalMemberAddHelper.sol} (87%) diff --git a/packages/contracts/src/governance/GovernancePluginsSetup.sol b/packages/contracts/src/governance/GovernancePluginsSetup.sol index a33e0bb..09114f0 100644 --- a/packages/contracts/src/governance/GovernancePluginsSetup.sol +++ b/packages/contracts/src/governance/GovernancePluginsSetup.sol @@ -7,7 +7,7 @@ import {DAO} from "@aragon/osx/core/dao/DAO.sol"; import {IDAO} from "@aragon/osx/core/dao/IDAO.sol"; import {PluginSetup, IPluginSetup} from "@aragon/osx/framework/plugin/setup/PluginSetup.sol"; import {PluginSetupProcessor} from "@aragon/osx/framework/plugin/setup/PluginSetupProcessor.sol"; -import {MemberAccessPlugin} from "./MemberAccessPlugin.sol"; +import {MainMemberAddHelper} from "./MainMemberAddHelper.sol"; import {MemberAccessExecuteCondition} from "../conditions/MemberAccessExecuteCondition.sol"; import {OnlyPluginUpgraderCondition} from "../conditions/OnlyPluginUpgraderCondition.sol"; import {MainVotingPlugin} from "./MainVotingPlugin.sol"; @@ -16,8 +16,8 @@ import {MajorityVotingBase} from "./base/MajorityVotingBase.sol"; /// @title GovernancePluginsSetup /// @dev Release 1, Build 1 contract GovernancePluginsSetup is PluginSetup { - address private immutable mainVotingPluginImplementation; - address public immutable memberAccessPluginImplementation; + address private immutable pluginImplementation; + address public immutable helperImplementation; address private immutable pluginSetupProcessor; /// @notice Thrown when the array of helpers does not have the correct size @@ -27,8 +27,8 @@ contract GovernancePluginsSetup is PluginSetup { /// @param pluginSetupProcessorAddress The address of the PluginSetupProcessor contract deployed by Aragon on that chain constructor(PluginSetupProcessor pluginSetupProcessorAddress) { pluginSetupProcessor = address(pluginSetupProcessorAddress); - mainVotingPluginImplementation = address(new MainVotingPlugin()); - memberAccessPluginImplementation = address(new MemberAccessPlugin()); + pluginImplementation = address(new MainVotingPlugin()); + helperImplementation = address(new MainMemberAddHelper()); } /// @inheritdoc IPluginSetup @@ -46,13 +46,13 @@ contract GovernancePluginsSetup is PluginSetup { ) = decodeInstallationParams(_data); // Deploy the member access plugin - address _memberAccessPlugin = createERC1967Proxy( - memberAccessPluginImplementation, + address _ainMemberAccessHelper = createERC1967Proxy( + helperImplementation, abi.encodeCall( - MemberAccessPlugin.initialize, + MainMemberAddHelper.initialize, ( IDAO(_dao), - MemberAccessPlugin.MultisigSettings({ + MainMemberAddHelper.MultisigSettings({ proposalDuration: _memberAccessProposalDuration }) ) @@ -61,14 +61,14 @@ contract GovernancePluginsSetup is PluginSetup { // Deploy the main voting plugin mainVotingPlugin = createERC1967Proxy( - mainVotingPluginImplementation, + pluginImplementation, abi.encodeCall( MainVotingPlugin.initialize, ( IDAO(_dao), _votingSettings, _initialEditors, - MemberAccessPlugin(_memberAccessPlugin) + MainMemberAddHelper(_ainMemberAccessHelper) ) ) ); @@ -98,7 +98,7 @@ contract GovernancePluginsSetup is PluginSetup { where: mainVotingPlugin, who: _dao, condition: PermissionLib.NO_CONDITION, - permissionId: MainVotingPlugin(mainVotingPluginImplementation) + permissionId: MainVotingPlugin(pluginImplementation) .UPDATE_VOTING_SETTINGS_PERMISSION_ID() }); // The DAO can manage the list of addresses @@ -107,24 +107,23 @@ contract GovernancePluginsSetup is PluginSetup { where: mainVotingPlugin, who: _dao, condition: PermissionLib.NO_CONDITION, - permissionId: MainVotingPlugin(mainVotingPluginImplementation) - .UPDATE_ADDRESSES_PERMISSION_ID() + permissionId: MainVotingPlugin(pluginImplementation).UPDATE_ADDRESSES_PERMISSION_ID() }); - // The MainVotingPlugin can create membership proposals on the MemberAccessPlugin + // The MainVotingPlugin can create membership proposals on the MainMemberAddHelper permissions[3] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, - where: _memberAccessPlugin, + where: _ainMemberAccessHelper, who: mainVotingPlugin, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + permissionId: MainMemberAddHelper(_ainMemberAccessHelper).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, + who: _ainMemberAccessHelper, // Conditional execution condition: _memberAccessExecuteCondition, permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() @@ -132,10 +131,10 @@ contract GovernancePluginsSetup is PluginSetup { // The DAO needs to be able to update the member access plugin settings permissions[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, - where: _memberAccessPlugin, + where: _ainMemberAccessHelper, who: _dao, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(memberAccessPluginImplementation) + permissionId: MainMemberAddHelper(helperImplementation) .UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() }); @@ -147,7 +146,7 @@ contract GovernancePluginsSetup is PluginSetup { // pluginUpgrader can make the DAO execute grant/revoke address[] memory _targetPluginAddresses = new address[](2); _targetPluginAddresses[0] = mainVotingPlugin; - _targetPluginAddresses[1] = _memberAccessPlugin; + _targetPluginAddresses[1] = _ainMemberAccessHelper; OnlyPluginUpgraderCondition _onlyPluginUpgraderCondition = new OnlyPluginUpgraderCondition( DAO(payable(_dao)), PluginSetupProcessor(pluginSetupProcessor), @@ -164,7 +163,7 @@ contract GovernancePluginsSetup is PluginSetup { preparedSetupData.permissions = permissions; preparedSetupData.helpers = new address[](1); - preparedSetupData.helpers[0] = _memberAccessPlugin; + preparedSetupData.helpers[0] = _ainMemberAccessHelper; } /// @inheritdoc IPluginSetup @@ -178,7 +177,7 @@ contract GovernancePluginsSetup is PluginSetup { // Decode incoming params address _pluginUpgrader = decodeUninstallationParams(_payload.data); - address _memberAccessPlugin = _payload.currentHelpers[0]; + address _ainMemberAccessHelper = _payload.currentHelpers[0]; permissionChanges = new PermissionLib.MultiTargetPermission[]( _pluginUpgrader == address(0x0) ? 6 : 7 @@ -200,7 +199,7 @@ contract GovernancePluginsSetup is PluginSetup { where: _payload.plugin, who: _dao, condition: address(0), - permissionId: MainVotingPlugin(mainVotingPluginImplementation) + permissionId: MainVotingPlugin(pluginImplementation) .UPDATE_VOTING_SETTINGS_PERMISSION_ID() }); // The DAO can no longer manage the list of addresses @@ -209,37 +208,35 @@ contract GovernancePluginsSetup is PluginSetup { where: _payload.plugin, who: _dao, condition: address(0), - permissionId: MainVotingPlugin(mainVotingPluginImplementation) - .UPDATE_ADDRESSES_PERMISSION_ID() + permissionId: MainVotingPlugin(pluginImplementation).UPDATE_ADDRESSES_PERMISSION_ID() }); // Plugin permissions - // The MainVotingPlugin can no longer propose on the MemberAccessPlugin + // The MainVotingPlugin can no longer propose on the MainMemberAddHelper permissionChanges[3] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, - where: _memberAccessPlugin, + where: _ainMemberAccessHelper, who: _payload.plugin, condition: address(0), - permissionId: MemberAccessPlugin(memberAccessPluginImplementation) - .PROPOSER_PERMISSION_ID() + permissionId: MainMemberAddHelper(helperImplementation).PROPOSER_PERMISSION_ID() }); // The plugin can no longer execute on the DAO permissionChanges[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, - who: _memberAccessPlugin, + who: _ainMemberAccessHelper, condition: address(0), permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO can no longer update the plugin settings permissionChanges[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, - where: _memberAccessPlugin, + where: _ainMemberAccessHelper, who: _dao, condition: address(0), - permissionId: MemberAccessPlugin(memberAccessPluginImplementation) + permissionId: MainMemberAddHelper(helperImplementation) .UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() }); @@ -258,7 +255,7 @@ contract GovernancePluginsSetup is PluginSetup { /// @inheritdoc IPluginSetup function implementation() external view returns (address) { - return mainVotingPluginImplementation; + return pluginImplementation; } /// @notice Encodes the given installation parameters into a byte array diff --git a/packages/contracts/src/governance/MemberAccessPlugin.sol b/packages/contracts/src/governance/MainMemberAddHelper.sol similarity index 97% rename from packages/contracts/src/governance/MemberAccessPlugin.sol rename to packages/contracts/src/governance/MainMemberAddHelper.sol index 22908a9..3569d34 100644 --- a/packages/contracts/src/governance/MemberAccessPlugin.sol +++ b/packages/contracts/src/governance/MainMemberAddHelper.sol @@ -13,15 +13,15 @@ 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.proposeAddMember.selector ^ - MemberAccessPlugin.getProposal.selector; +bytes4 constant MAIN_MEMBER_ADD_INTERFACE_ID = MainMemberAddHelper.initialize.selector ^ + MainMemberAddHelper.updateMultisigSettings.selector ^ + MainMemberAddHelper.proposeAddMember.selector ^ + MainMemberAddHelper.getProposal.selector; /// @title Member access plugin (Multisig) - Release 1, Build 1 /// @author Aragon - 2023 /// @notice The on-chain multisig governance plugin in which a proposal passes if X out of Y approvals are met. -contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable { +contract MainMemberAddHelper is IMultisig, PluginUUPSUpgradeable, ProposalUpgradeable { using SafeCastUpgradeable for uint256; /// @notice The ID of the permission required to call the `addAddresses` functions. @@ -138,7 +138,7 @@ contract MemberAccessPlugin is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade bytes4 _interfaceId ) public view virtual override(PluginUUPSUpgradeable, ProposalUpgradeable) returns (bool) { return - _interfaceId == MEMBER_ACCESS_INTERFACE_ID || + _interfaceId == MAIN_MEMBER_ADD_INTERFACE_ID || _interfaceId == type(IMultisig).interfaceId || super.supportsInterface(_interfaceId); } diff --git a/packages/contracts/src/governance/MainVotingPlugin.sol b/packages/contracts/src/governance/MainVotingPlugin.sol index 5167979..0eaee46 100644 --- a/packages/contracts/src/governance/MainVotingPlugin.sol +++ b/packages/contracts/src/governance/MainVotingPlugin.sol @@ -10,7 +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, MEMBER_ACCESS_INTERFACE_ID} from "./MemberAccessPlugin.sol"; +import {MainMemberAddHelper, MAIN_MEMBER_ADD_INTERFACE_ID} from "./MainMemberAddHelper.sol"; import {SpacePlugin} from "../space/SpacePlugin.sol"; // The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract. @@ -48,7 +48,7 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers 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; + MainMemberAddHelper public mainMemberAddHelper; /// @notice Emitted when the creator cancels a proposal event ProposalCanceled(uint256 proposalId); @@ -107,17 +107,17 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers IDAO _dao, VotingSettings calldata _votingSettings, address[] calldata _initialEditors, - MemberAccessPlugin _memberAccessPlugin + MainMemberAddHelper _mainMemberAddHelper ) external initializer { __MajorityVotingBase_init(_dao, _votingSettings); _addAddresses(_initialEditors); emit EditorsAdded(_initialEditors); - if (!_memberAccessPlugin.supportsInterface(MEMBER_ACCESS_INTERFACE_ID)) { - revert InvalidInterface(address(_memberAccessPlugin)); + if (!_mainMemberAddHelper.supportsInterface(MAIN_MEMBER_ADD_INTERFACE_ID)) { + revert InvalidInterface(address(_mainMemberAddHelper)); } - memberAccessPlugin = _memberAccessPlugin; + mainMemberAddHelper = _mainMemberAddHelper; } /// @notice Checks if this or the parent contract supports an interface by its ID. @@ -371,7 +371,7 @@ contract MainVotingPlugin is Addresslist, MajorityVotingBase, IEditors, IMembers /// @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(_metadataContentUri, _proposedMember, msg.sender); + mainMemberAddHelper.proposeAddMember(_metadataContentUri, _proposedMember, msg.sender); } /// @notice Creates a proposal to remove an existing member. diff --git a/packages/contracts/src/personal/PersonalAccessHelper.sol b/packages/contracts/src/personal/PersonalMemberAddHelper.sol similarity index 87% rename from packages/contracts/src/personal/PersonalAccessHelper.sol rename to packages/contracts/src/personal/PersonalMemberAddHelper.sol index f2cbb72..2699e45 100644 --- a/packages/contracts/src/personal/PersonalAccessHelper.sol +++ b/packages/contracts/src/personal/PersonalMemberAddHelper.sol @@ -11,20 +11,19 @@ import {PluginCloneable} from "@aragon/osx/core/plugin/PluginCloneable.sol"; import {IEditors} from "../base/IEditors.sol"; import {PersonalSpaceAdminPlugin} from "./PersonalSpaceAdminPlugin.sol"; -bytes4 constant PERSONAL_ACCESS_INTERFACE_ID = PersonalAccessPlugin.initialize.selector ^ - PersonalAccessPlugin.updatePluginSettings.selector ^ - PersonalAccessPlugin.proposeAddMember.selector ^ - PersonalAccessPlugin.getProposal.selector; +bytes4 constant PERSONAL_MEMBER_ADD_INTERFACE_ID = PersonalMemberAddHelper.initialize.selector ^ + PersonalMemberAddHelper.updateSettings.selector ^ + PersonalMemberAddHelper.proposeAddMember.selector ^ + PersonalMemberAddHelper.getProposal.selector; /// @title Personal access plugin (SingleApproval) - Release 1, Build 1 /// @author Aragon - 2024 /// @notice The on-chain governance plugin in which a proposal passes when approved by the first editor -contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { +contract PersonalMemberAddHelper is PluginCloneable, ProposalUpgradeable { using SafeCastUpgradeable for uint256; /// @notice The ID of the permission required to call the `addAddresses` functions. - bytes32 public constant UPDATE_PLUGIN_SETTINGS_PERMISSION_ID = - keccak256("UPDATE_PLUGIN_SETTINGS_PERMISSION"); + bytes32 public constant UPDATE_SETTINGS_PERMISSION_ID = keccak256("UPDATE_SETTINGS_PERMISSION"); /// @notice The ID of the permission required to create new membership proposals. bytes32 public constant PROPOSER_PERMISSION_ID = keccak256("PROPOSER_PERMISSION"); @@ -55,7 +54,7 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { /// @notice A container for the plugin settings. /// @param proposalDuration The amount of time before a non-approved proposal expires. - struct PluginSettings { + struct Settings { uint64 proposalDuration; } @@ -63,11 +62,11 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { mapping(uint256 => Proposal) internal proposals; /// @notice The current plugin settings. - PluginSettings public pluginSettings; + Settings public settings; /// @notice Keeps track at which block number the plugin settings have been changed the last time. /// @dev This variable prevents a proposal from being created in the same block in which the plugin settings change. - uint64 public lastPluginSettingsChange; + uint64 public lastSettingsChange; /// @notice Thrown when creating a proposal at the same block that the settings were changed. error ProposalCreationForbiddenOnSameBlock(); @@ -99,19 +98,16 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { /// @notice Emitted when the plugin settings are set. /// @param proposalDuration The amount of time before a non-approved proposal expires. - event PluginSettingsUpdated(uint64 proposalDuration); + event SettingsUpdated(uint64 proposalDuration); /// @notice Initializes Release 1, Build 1. /// @dev This method is required to support [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822). /// @param _dao The IDAO interface of the associated DAO. - /// @param _pluginSettings The multisig settings. - function initialize( - IDAO _dao, - PluginSettings calldata _pluginSettings - ) external virtual initializer { + /// @param _Settings The multisig settings. + function initialize(IDAO _dao, Settings calldata _Settings) external virtual initializer { __PluginCloneable_init(_dao); - _updatePluginSettings(_pluginSettings); + _updateSettings(_Settings); } /// @notice Checks if this or the parent contract supports an interface by its ID. @@ -121,15 +117,16 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { bytes4 _interfaceId ) public view virtual override(PluginCloneable, ProposalUpgradeable) returns (bool) { return - _interfaceId == PERSONAL_ACCESS_INTERFACE_ID || super.supportsInterface(_interfaceId); + _interfaceId == PERSONAL_MEMBER_ADD_INTERFACE_ID || + super.supportsInterface(_interfaceId); } /// @notice Updates the plugin settings. - /// @param _pluginSettings The new settings. - function updatePluginSettings( - PluginSettings calldata _pluginSettings - ) external auth(UPDATE_PLUGIN_SETTINGS_PERMISSION_ID) { - _updatePluginSettings(_pluginSettings); + /// @param _Settings The new settings. + function updateSettings( + Settings calldata _Settings + ) external auth(UPDATE_SETTINGS_PERMISSION_ID) { + _updateSettings(_Settings); } /// @notice Creates a proposal to add a new member. @@ -153,7 +150,7 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { _actions[0] = IDAO.Action({ to: address(msg.sender), // We are called by the PersonalSpaceAdminPlugin value: 0, - data: abi.encodeCall(PersonalSpaceAdminPlugin.submitNewMember, (_proposedMember)) + data: abi.encodeCall(PersonalSpaceAdminPlugin.addMember, (_proposedMember)) }); // Create proposal @@ -164,12 +161,12 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { // 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 (lastPluginSettingsChange > snapshotBlock) { + if (lastSettingsChange > snapshotBlock) { revert ProposalCreationForbiddenOnSameBlock(); } uint64 _startDate = block.timestamp.toUint64(); - uint64 _endDate = _startDate + pluginSettings.proposalDuration; + uint64 _endDate = _startDate + settings.proposalDuration; proposalId = _createProposalId(); @@ -309,11 +306,11 @@ contract PersonalAccessPlugin is PluginCloneable, ProposalUpgradeable { } /// @notice Internal function to update the plugin settings. - /// @param _pluginSettings The new settings. - function _updatePluginSettings(PluginSettings calldata _pluginSettings) internal { - pluginSettings = _pluginSettings; - lastPluginSettingsChange = block.number.toUint64(); + /// @param _Settings The new settings. + function _updateSettings(Settings calldata _Settings) internal { + settings = _Settings; + lastSettingsChange = block.number.toUint64(); - emit PluginSettingsUpdated({proposalDuration: _pluginSettings.proposalDuration}); + emit SettingsUpdated({proposalDuration: _Settings.proposalDuration}); } } diff --git a/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol b/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol index a43922d..47fd047 100644 --- a/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol +++ b/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol @@ -9,8 +9,11 @@ import {IDAO} from "@aragon/osx/core/dao/IDAO.sol"; import {DAO} from "@aragon/osx/core/dao/DAO.sol"; import {PermissionLib} from "@aragon/osx/core/permission/PermissionLib.sol"; import {PersonalSpaceAdminPlugin} from "./PersonalSpaceAdminPlugin.sol"; +import {PersonalMemberAddHelper} from "./PersonalMemberAddHelper.sol"; import {EDITOR_PERMISSION_ID} from "../constants.sol"; +uint64 constant MEMBER_ADD_PROPOSAL_DURATION = 7 days; + /// @title PersonalSpaceAdminPluginSetup /// @author Aragon - 2023 /// @notice The setup contract of the `PersonalSpaceAdminPlugin` plugin. @@ -18,15 +21,17 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { using Clones for address; /// @notice The address of the `PersonalSpaceAdminPlugin` plugin logic contract to be cloned. - address private immutable implementation_; + address private immutable pluginImplementation; + address public immutable helperImplementation; /// @notice Thrown if the editor address is zero. /// @param editor The initial editor address. error EditorAddressInvalid(address editor); - /// @notice The constructor setting the `PersonalSpaceAdminPlugin` implementation contract to clone from. + /// @notice The constructor setting the `PersonalSpaceAdminPlugin` and `PersonalMemberAddHelper` implementation contract to clone from. constructor() { - implementation_ = address(new PersonalSpaceAdminPlugin()); + pluginImplementation = address(new PersonalSpaceAdminPlugin()); + helperImplementation = address(new PersonalMemberAddHelper()); } /// @inheritdoc IPluginSetup @@ -41,15 +46,21 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { revert EditorAddressInvalid({editor: editor}); } - // Clone plugin contract. - plugin = implementation_.clone(); + // Clone the contracts + plugin = pluginImplementation.clone(); + address helper = helperImplementation.clone(); - // Initialize cloned plugin contract. + // Initialize the cloned contracts PersonalSpaceAdminPlugin(plugin).initialize(IDAO(_dao), editor); + PersonalMemberAddHelper.Settings memory _helperSettings = PersonalMemberAddHelper.Settings({ + proposalDuration: MEMBER_ADD_PROPOSAL_DURATION + }); + PersonalMemberAddHelper(helper).initialize(IDAO(_dao), _helperSettings); + // Prepare permissions PermissionLib.MultiTargetPermission[] - memory permissions = new PermissionLib.MultiTargetPermission[](2); + memory permissions = new PermissionLib.MultiTargetPermission[](4); // Grant `EDITOR_PERMISSION` of the plugin to the editor. permissions[0] = PermissionLib.MultiTargetPermission( @@ -60,8 +71,26 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { EDITOR_PERMISSION_ID ); - // Grant `EXECUTE_PERMISSION` on the DAO to the plugin. + // Grant `PROPOSER_PERMISSION` on the helper to the plugin. permissions[1] = PermissionLib.MultiTargetPermission( + PermissionLib.Operation.Grant, + helper, + plugin, + PermissionLib.NO_CONDITION, + PersonalMemberAddHelper(helper).PROPOSER_PERMISSION_ID() + ); + + // Grant `UPDATE_PLUGIN_SETTINGS_PERMISSION` on the helper to the plugin. + permissions[2] = PermissionLib.MultiTargetPermission( + PermissionLib.Operation.Grant, + helper, + _dao, + PermissionLib.NO_CONDITION, + PersonalMemberAddHelper(helper).UPDATE_SETTINGS_PERMISSION_ID() + ); + + // Grant `EXECUTE_PERMISSION` on the DAO to the plugin. + permissions[3] = PermissionLib.MultiTargetPermission( PermissionLib.Operation.Grant, _dao, plugin, @@ -70,6 +99,9 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { ); preparedSetupData.permissions = permissions; + + preparedSetupData.helpers = new address[](1); + preparedSetupData.helpers[0] = helper; } /// @inheritdoc IPluginSetup @@ -93,7 +125,7 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { /// @inheritdoc IPluginSetup function implementation() external view returns (address) { - return implementation_; + return pluginImplementation; } /// @notice Encodes the given installation parameters into a byte array diff --git a/packages/contracts/src/test/TestGovernancePluginsSetup.sol b/packages/contracts/src/test/TestGovernancePluginsSetup.sol index 8734c6e..56a2b68 100644 --- a/packages/contracts/src/test/TestGovernancePluginsSetup.sol +++ b/packages/contracts/src/test/TestGovernancePluginsSetup.sol @@ -10,13 +10,13 @@ import {PluginSetupProcessor} from "@aragon/osx/framework/plugin/setup/PluginSet import {MemberAccessExecuteCondition} from "../conditions/MemberAccessExecuteCondition.sol"; import {OnlyPluginUpgraderCondition} from "../conditions/OnlyPluginUpgraderCondition.sol"; import {MainVotingPlugin} from "../governance/MainVotingPlugin.sol"; -import {MemberAccessPlugin} from "../governance/MemberAccessPlugin.sol"; +import {MainMemberAddHelper} from "../governance/MainMemberAddHelper.sol"; import {MajorityVotingBase} from "../governance/base/MajorityVotingBase.sol"; // Not ideal, but to test this E2E, the contract needs to be cloned contract TestGovernancePluginsSetup is PluginSetup { address private immutable mainVotingPluginImplementationAddr; - address public immutable memberAccessPluginImplementationAddr; + address public immutable mainMemberAddHelperImplementationAddr; address private immutable pluginSetupProcessor; /// @notice Thrown when the array of helpers does not have the correct size @@ -27,7 +27,7 @@ contract TestGovernancePluginsSetup is PluginSetup { constructor(PluginSetupProcessor pluginSetupProcessorAddress) { pluginSetupProcessor = address(pluginSetupProcessorAddress); mainVotingPluginImplementationAddr = address(new MainVotingPlugin()); - memberAccessPluginImplementationAddr = address(new MemberAccessPlugin()); + mainMemberAddHelperImplementationAddr = address(new MainMemberAddHelper()); } /// @inheritdoc IPluginSetup @@ -45,12 +45,12 @@ contract TestGovernancePluginsSetup is PluginSetup { ) = decodeInstallationParams(_data); // Deploy the member access plugin - MemberAccessPlugin.MultisigSettings memory _multisigSettings; + MainMemberAddHelper.MultisigSettings memory _multisigSettings; _multisigSettings.proposalDuration = _memberAccessProposalDuration; - address _memberAccessPlugin = createERC1967Proxy( - memberAccessPluginImplementation(), - abi.encodeCall(MemberAccessPlugin.initialize, (IDAO(_dao), _multisigSettings)) + address _mainMemberAddHelper = createERC1967Proxy( + mainMemberAddHelperImplementation(), + abi.encodeCall(MainMemberAddHelper.initialize, (IDAO(_dao), _multisigSettings)) ); // Deploy the main voting plugin @@ -62,7 +62,7 @@ contract TestGovernancePluginsSetup is PluginSetup { IDAO(_dao), _votingSettings, _initialEditors, - MemberAccessPlugin(_memberAccessPlugin) + MainMemberAddHelper(_mainMemberAddHelper) ) ) ); @@ -108,27 +108,27 @@ contract TestGovernancePluginsSetup is PluginSetup { // The member access plugin needs to execute on the DAO permissions[3] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, - where: _memberAccessPlugin, + where: _mainMemberAddHelper, who: mainVotingPlugin, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + permissionId: MainMemberAddHelper(_mainMemberAddHelper).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, + who: _mainMemberAddHelper, condition: _memberAccessExecuteCondition, permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO needs to be able to update the member access plugin settings permissions[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Grant, - where: _memberAccessPlugin, + where: _mainMemberAddHelper, who: _dao, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(_memberAccessPlugin) + permissionId: MainMemberAddHelper(_mainMemberAddHelper) .UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() }); @@ -140,7 +140,7 @@ contract TestGovernancePluginsSetup is PluginSetup { // pluginUpgrader can make the DAO execute grant/revoke address[] memory _targetPluginAddresses = new address[](2); _targetPluginAddresses[0] = mainVotingPlugin; - _targetPluginAddresses[1] = _memberAccessPlugin; + _targetPluginAddresses[1] = _mainMemberAddHelper; OnlyPluginUpgraderCondition _onlyPluginUpgraderCondition = new OnlyPluginUpgraderCondition( DAO(payable(_dao)), PluginSetupProcessor(pluginSetupProcessor), @@ -157,7 +157,7 @@ contract TestGovernancePluginsSetup is PluginSetup { preparedSetupData.permissions = permissions; preparedSetupData.helpers = new address[](1); - preparedSetupData.helpers[0] = _memberAccessPlugin; + preparedSetupData.helpers[0] = _mainMemberAddHelper; } /// @notice WARNING: This test function is meant to revert when performed by the pluginUpgrader @@ -203,7 +203,7 @@ contract TestGovernancePluginsSetup is PluginSetup { // Decode incoming params address _pluginUpgrader = decodeUninstallationParams(_payload.data); - address _memberAccessPlugin = _payload.currentHelpers[0]; + address _mainMemberAddHelper = _payload.currentHelpers[0]; permissionChanges = new PermissionLib.MultiTargetPermission[]( _pluginUpgrader == address(0x0) ? 6 : 7 @@ -243,27 +243,27 @@ contract TestGovernancePluginsSetup is PluginSetup { // The member access plugin needs to execute on the DAO permissionChanges[3] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, - where: _memberAccessPlugin, + where: _mainMemberAddHelper, who: _payload.plugin, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(_memberAccessPlugin).PROPOSER_PERMISSION_ID() + permissionId: MainMemberAddHelper(_mainMemberAddHelper).PROPOSER_PERMISSION_ID() }); // The plugin can no longer execute on the DAO permissionChanges[4] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, where: _dao, - who: _memberAccessPlugin, + who: _mainMemberAddHelper, condition: PermissionLib.NO_CONDITION, permissionId: DAO(payable(_dao)).EXECUTE_PERMISSION_ID() }); // The DAO can no longer update the plugin settings permissionChanges[5] = PermissionLib.MultiTargetPermission({ operation: PermissionLib.Operation.Revoke, - where: _memberAccessPlugin, + where: _mainMemberAddHelper, who: _dao, condition: PermissionLib.NO_CONDITION, - permissionId: MemberAccessPlugin(memberAccessPluginImplementation()) + permissionId: MainMemberAddHelper(mainMemberAddHelperImplementation()) .UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() }); @@ -285,9 +285,9 @@ contract TestGovernancePluginsSetup is PluginSetup { return mainVotingPluginImplementationAddr; } - /// @notice Returns the address of the MemberAccessPlugin implementation - function memberAccessPluginImplementation() public view returns (address) { - return memberAccessPluginImplementationAddr; + /// @notice Returns the address of the MainMemberAddHelper implementation + function mainMemberAddHelperImplementation() public view returns (address) { + return mainMemberAddHelperImplementationAddr; } /// @notice Encodes the given installation parameters into a byte array