From a4d774fe9fa2a45ab4a95b56939fa515d80c18f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 2 Jul 2024 11:44:52 +0200 Subject: [PATCH 1/3] Allowing personal space editors to bahave as members --- .../src/personal/PersonalSpaceAdminPlugin.sol | 25 ++++++------ .../PersonalSpaceAdminPluginSetup.sol | 4 +- .../personal-space-admin-plugin.ts | 40 ++++++++++--------- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol b/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol index 8dc60fd..65974e6 100644 --- a/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol +++ b/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol @@ -30,6 +30,16 @@ contract PersonalSpaceAdminPlugin is PluginCloneable, ProposalUpgradeable, IEdit this.submitRemoveEditor.selector ^ this.leaveSpace.selector; + /// @notice Raised when a wallet who is not an editor or a member attempts to do something + error NotAMember(address caller); + + modifier onlyMembers() { + if (!isMember(msg.sender)) { + revert NotAMember(msg.sender); + } + _; + } + /// @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). @@ -86,10 +96,7 @@ contract PersonalSpaceAdminPlugin is PluginCloneable, ProposalUpgradeable, IEdit /// @notice Creates and executes a proposal that makes the DAO emit new content on the given space. /// @param _contentUri The URI of the IPFS content to publish /// @param _spacePlugin The address of the space plugin where changes will be executed - function submitEdits( - string memory _contentUri, - address _spacePlugin - ) public auth(MEMBER_PERMISSION_ID) { + function submitEdits(string memory _contentUri, address _spacePlugin) public onlyMembers { IDAO.Action[] memory _actions = new IDAO.Action[](1); _actions[0].to = _spacePlugin; @@ -105,10 +112,7 @@ contract PersonalSpaceAdminPlugin is PluginCloneable, ProposalUpgradeable, IEdit /// @notice Creates and executes a proposal that makes the DAO accept the given DAO as a subspace. /// @param _subspaceDao The address of the DAO that holds the new subspace /// @param _spacePlugin The address of the space plugin where changes will be executed - function submitAcceptSubspace( - IDAO _subspaceDao, - address _spacePlugin - ) public auth(MEMBER_PERMISSION_ID) { + function submitAcceptSubspace(IDAO _subspaceDao, address _spacePlugin) public onlyMembers { IDAO.Action[] memory _actions = new IDAO.Action[](1); _actions[0].to = _spacePlugin; _actions[0].data = abi.encodeCall(SpacePlugin.acceptSubspace, (address(_subspaceDao))); @@ -123,10 +127,7 @@ contract PersonalSpaceAdminPlugin is PluginCloneable, ProposalUpgradeable, IEdit /// @notice Creates and executes a proposal that makes the DAO remove the given DAO as a subspace. /// @param _subspaceDao The address of the DAO that holds the subspace to remove /// @param _spacePlugin The address of the space plugin where changes will be executed - function submitRemoveSubspace( - IDAO _subspaceDao, - address _spacePlugin - ) public auth(MEMBER_PERMISSION_ID) { + function submitRemoveSubspace(IDAO _subspaceDao, address _spacePlugin) public onlyMembers { IDAO.Action[] memory _actions = new IDAO.Action[](1); _actions[0].to = _spacePlugin; _actions[0].data = abi.encodeCall(SpacePlugin.removeSubspace, (address(_subspaceDao))); diff --git a/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol b/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol index 6abc10c..a43922d 100644 --- a/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol +++ b/packages/contracts/src/personal/PersonalSpaceAdminPluginSetup.sol @@ -51,7 +51,7 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { PermissionLib.MultiTargetPermission[] memory permissions = new PermissionLib.MultiTargetPermission[](2); - // Grant `ADMIN_EXECUTE_PERMISSION` of the plugin to the editor. + // Grant `EDITOR_PERMISSION` of the plugin to the editor. permissions[0] = PermissionLib.MultiTargetPermission( PermissionLib.Operation.Grant, plugin, @@ -73,7 +73,7 @@ contract PersonalSpaceAdminPluginSetup is PluginSetup { } /// @inheritdoc IPluginSetup - /// @dev Currently, there is no reliable way to revoke the `ADMIN_EXECUTE_PERMISSION_ID` from all addresses it has been granted to. Accordingly, only the `EXECUTE_PERMISSION_ID` is revoked for this uninstallation. + /// @dev There is no reliable way to revoke `EDITOR_PERMISSION_ID` from all addresses it has been granted to. Removing `EXECUTE_PERMISSION_ID` only, as being an editor or a member is useless without EXECUTE. function prepareUninstallation( address _dao, SetupPayload calldata _payload diff --git a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts index 58a7634..1825aa5 100644 --- a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts +++ b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts @@ -255,25 +255,27 @@ describe('Personal Space Admin Plugin', function () { ).to.emit(personalSpaceVotingPlugin, 'ProposalCreated'); }); - it('Only members can call content proposal wrappers', async () => { - await expect( - personalSpaceVotingPlugin - .connect(bob) - .submitEdits('ipfs://', spacePlugin.address) - ).to.not.be.reverted; - await expect( - personalSpaceVotingPlugin - .connect(bob) - .submitAcceptSubspace(ADDRESS_TWO, spacePlugin.address) - ).to.not.be.reverted; - await expect( - personalSpaceVotingPlugin - .connect(bob) - .submitRemoveSubspace(ADDRESS_THREE, spacePlugin.address) - ).to.not.be.reverted; - expect(await personalSpaceVotingPlugin.proposalCount()).to.equal( - BigNumber.from(3) - ); + it('Only members or editors can call content proposal wrappers', async () => { + for (const account of [alice, bob]) { + await expect( + personalSpaceVotingPlugin + .connect(account) + .submitEdits('ipfs://', spacePlugin.address) + ).to.not.be.reverted; + await expect( + personalSpaceVotingPlugin + .connect(account) + .submitAcceptSubspace(ADDRESS_TWO, spacePlugin.address) + ).to.not.be.reverted; + await expect( + personalSpaceVotingPlugin + .connect(account) + .submitRemoveSubspace(ADDRESS_THREE, spacePlugin.address) + ).to.not.be.reverted; + expect(await personalSpaceVotingPlugin.proposalCount()).to.equal( + BigNumber.from(3) + ); + } // Non members await expect( From 61e432b9a58236bae91efbb0b34df9812f02b007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 2 Jul 2024 11:52:55 +0200 Subject: [PATCH 2/3] Minor fix --- .../test/unit-testing/personal-space-admin-plugin.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts index 1825aa5..1ef8e1d 100644 --- a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts +++ b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts @@ -272,10 +272,10 @@ describe('Personal Space Admin Plugin', function () { .connect(account) .submitRemoveSubspace(ADDRESS_THREE, spacePlugin.address) ).to.not.be.reverted; - expect(await personalSpaceVotingPlugin.proposalCount()).to.equal( - BigNumber.from(3) - ); } + expect(await personalSpaceVotingPlugin.proposalCount()).to.equal( + BigNumber.from(6) + ); // Non members await expect( From bce58471e17800b86b78a3ef73b0a594768d2050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 2 Jul 2024 12:24:51 +0200 Subject: [PATCH 3/3] Minor edit --- .../personal-space-admin-plugin.ts | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts index 1ef8e1d..620782d 100644 --- a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts +++ b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts @@ -283,46 +283,22 @@ describe('Personal Space Admin Plugin', function () { .connect(carol) .submitEdits('ipfs://', spacePlugin.address) ) - .to.be.revertedWithCustomError( - personalSpaceVotingPlugin, - 'DaoUnauthorized' - ) - .withArgs( - dao.address, - personalSpaceVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); + .to.be.revertedWithCustomError(personalSpaceVotingPlugin, 'NotAMember') + .withArgs(carol.address); await expect( personalSpaceVotingPlugin .connect(carol) .submitAcceptSubspace(ADDRESS_TWO, spacePlugin.address) ) - .to.be.revertedWithCustomError( - personalSpaceVotingPlugin, - 'DaoUnauthorized' - ) - .withArgs( - dao.address, - personalSpaceVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); + .to.be.revertedWithCustomError(personalSpaceVotingPlugin, 'NotAMember') + .withArgs(carol.address); await expect( personalSpaceVotingPlugin .connect(carol) .submitRemoveSubspace(ADDRESS_TWO, spacePlugin.address) ) - .to.be.revertedWithCustomError( - personalSpaceVotingPlugin, - 'DaoUnauthorized' - ) - .withArgs( - dao.address, - personalSpaceVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); + .to.be.revertedWithCustomError(personalSpaceVotingPlugin, 'NotAMember') + .withArgs(carol.address); }); it('Only editors can call permission proposal wrappers', async () => {