Adapting existing membership tests

packages/contracts/src/MainVotingPlugin.sol

@@ -50,7 +50,7 @@ contract MainVotingPlugin is MajorityVotingBase {
     ) external initializer {
         __MajorityVotingBase_init(_dao, _votingSettings);
 
-        editorAdded(_initialEditor);
+        _editorAdded(_initialEditor);
     }
 
     /// @notice Checks if this or the parent contract supports an interface by its ID.
@@ -64,26 +64,14 @@ contract MainVotingPlugin is MajorityVotingBase {
     /// @notice The function proposeNewEditor creates an action to call this function after an editor is added.
     /// @param _editor The address of the new editor
     /// @dev This function is also used during the plugin initialization.
-    function editorAdded(address _editor) public auth(UPDATE_ADDRESSES_PERMISSION_ID) {
-        if (!isEditor(_editor)) {
-            revert NotAnEditorYet();
-        }
-
-        editorCount++;
-        emit EditorAdded({editor: _editor});
+    function editorAdded(address _editor) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
+        _editorAdded(_editor);
     }
 
     /// @notice The function proposeRemoveEditor creates an action to call this function after an editor is removed.
     /// @param _editor The addresses of the members to be removed.
     function editorRemoved(address _editor) external auth(UPDATE_ADDRESSES_PERMISSION_ID) {
-        if (isEditor(_editor)) {
-            revert StillAnEditor();
-        } else if (editorCount <= 1) {
-            revert NoEditorsLeft();
-        }
-
-        editorCount--;
-        emit EditorRemoved({editor: _editor});
+        _editorRemoved(_editor);
     }
 
     /// @inheritdoc MajorityVotingBase
@@ -92,12 +80,16 @@ contract MainVotingPlugin is MajorityVotingBase {
         return editorCount;
     }
 
+    /// @notice Returns whether the given address holds membership permission on the main voting plugin
     function isMember(address _account) public view returns (bool) {
-        return dao().hasPermission(_account, address(this), MEMBER_PERMISSION_ID, bytes(""));
+        return
+            dao().hasPermission(address(this), _account, MEMBER_PERMISSION_ID, bytes("")) ||
+            isEditor(_account);
     }
 
+    /// @notice Returns whether the given address holds editor permission on the main voting plugin
     function isEditor(address _account) public view returns (bool) {
-        return dao().hasPermission(_account, address(this), EDITOR_PERMISSION_ID, bytes(""));
+        return dao().hasPermission(address(this), _account, EDITOR_PERMISSION_ID, bytes(""));
     }
 
     /// @inheritdoc MajorityVotingBase
@@ -237,6 +229,26 @@ contract MainVotingPlugin is MajorityVotingBase {
         return true;
     }
 
+    function _editorAdded(address _editor) internal {
+        if (!isEditor(_editor)) {
+            revert NotAnEditorYet();
+        }
+
+        editorCount++;
+        emit EditorAdded({editor: _editor});
+    }
+
+    function _editorRemoved(address _editor) internal {
+        if (isEditor(_editor)) {
+            revert StillAnEditor();
+        } else if (editorCount <= 1) {
+            revert NoEditorsLeft();
+        }
+
+        editorCount--;
+        emit EditorRemoved({editor: _editor});
+    }
+
     /// @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

packages/contracts/src/MemberAccessVotingPlugin.sol

@@ -374,17 +374,19 @@ contract MemberAccessVotingPlugin is IMultisig, PluginUUPSUpgradeable, ProposalU
         _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 permission on the main voting plugin?
+        // Does the address hold the member or editor permission on the main voting plugin?
         return
             dao().hasPermission(
                 address(multisigSettings.mainVotingPlugin),
                 _account,
                 MEMBER_PERMISSION_ID,
                 bytes("")
-            );
+            ) || isEditor(_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

packages/contracts/test/unit-testing/common.ts

@@ -14,9 +14,28 @@ export const EXECUTE_PERMISSION_ID = ethers.utils.id("EXECUTE_PERMISSION");
 export const UPDATE_MULTISIG_SETTINGS_PERMISSION_ID = ethers.utils.id(
   "UPDATE_MULTISIG_SETTINGS_PERMISSION",
 );
+export const UPDATE_VOTING_SETTINGS_PERMISSION_ID = ethers.utils.id(
+  "UPDATE_VOTING_SETTINGS_PERMISSION",
+);
+export const UPDATE_ADDRESSES_PERMISSION_ID = ethers.utils.id(
+  "UPDATE_ADDRESSES_PERMISSION",
+);
 export const ROOT_PERMISSION_ID = ethers.utils.id("ROOT_PERMISSION");
 
 export const ADDRESS_ZERO = ethers.constants.AddressZero;
 export const ADDRESS_ONE = `0x${"0".repeat(39)}1`;
 export const ADDRESS_TWO = `0x${"0".repeat(39)}2`;
 export const NO_CONDITION = ADDRESS_ZERO;
+
+// MAIN VOTING PLUGIN
+
+const RATIO_BASE = 10 ** 6;
+const EARLY_EXECUTION_MODE = 1;
+
+export const defaultMainVotingSettings = {
+  minDuration: 60 * 60, // 1 second
+  minParticipation: 0.1 * RATIO_BASE,
+  supportThreshold: 0.5 * RATIO_BASE,
+  minProposerVotingPower: 0,
+  votingMode: EARLY_EXECUTION_MODE,
+};

packages/contracts/test/unit-testing/main-voting-plugin.ts

@@ -1,24 +1,28 @@
 import {
   DAO,
+  MainVotingPlugin,
+  MainVotingPlugin__factory,
   MemberAccessVotingPlugin,
   MemberAccessVotingPlugin__factory,
   SpacePlugin,
   SpacePlugin__factory,
-  SpaceVotingPlugin,
-  SpaceVotingPlugin__factory,
 } from "../../typechain";
 import { deployWithProxy } from "../../utils/helpers";
 import { deployTestDao } from "../helpers/test-dao";
 import {
+  ADDRESS_ONE,
   ADDRESS_TWO,
-  CONTENT_PERMISSION_ID,
+  ADDRESS_ZERO,
   EDITOR_PERMISSION_ID,
+  EXECUTE_PERMISSION_ID,
   MEMBER_PERMISSION_ID,
-  SUBSPACE_PERMISSION_ID,
+  ROOT_PERMISSION_ID,
+  UPDATE_MULTISIG_SETTINGS_PERMISSION_ID,
 } from "./common";
 import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
 import { expect } from "chai";
 import { ethers } from "hardhat";
+import { defaultMainVotingSettings } from "./common";
 
 export type InitData = { contentUri: string };
 export const defaultInitData: InitData = {
@@ -31,7 +35,7 @@ describe("Default Main Voting plugin", function () {
   let charlie: SignerWithAddress;
   let dao: DAO;
   let memberAccessPlugin: MemberAccessVotingPlugin;
-  let spaceVotingPlugin: SpaceVotingPlugin;
+  let mainVotingPlugin: MainVotingPlugin;
   let spacePlugin: SpacePlugin;
   let defaultInput: InitData;
 
@@ -46,41 +50,53 @@ describe("Default Main Voting plugin", function () {
     memberAccessPlugin = await deployWithProxy<MemberAccessVotingPlugin>(
       new MemberAccessVotingPlugin__factory(alice),
     );
-    spaceVotingPlugin = await deployWithProxy<SpaceVotingPlugin>(
-      new SpaceVotingPlugin__factory(alice),
+    mainVotingPlugin = await deployWithProxy<MainVotingPlugin>(
+      new MainVotingPlugin__factory(alice),
     );
     spacePlugin = await deployWithProxy<SpacePlugin>(
       new SpacePlugin__factory(alice),
     );
 
     await memberAccessPlugin.initialize(dao.address, {
       proposalDuration: 60 * 60 * 24 * 5,
-      mainVotingPlugin: spaceVotingPlugin.address,
+      mainVotingPlugin: mainVotingPlugin.address,
     });
-    await spaceVotingPlugin.initialize(dao.address);
+    await mainVotingPlugin.initialize(
+      dao.address,
+      defaultMainVotingSettings,
+      alice.address,
+    );
     await spacePlugin.initialize(dao.address, defaultInput.contentUri);
 
     // Alice is an editor
     await dao.grant(
-      memberAccessPlugin.address,
-      alice.address,
-      EDITOR_PERMISSION_ID,
-    );
-    await dao.grant(
-      spaceVotingPlugin.address,
+      mainVotingPlugin.address,
       alice.address,
       EDITOR_PERMISSION_ID,
     );
     // Bob is a member
     await dao.grant(
-      memberAccessPlugin.address,
+      mainVotingPlugin.address,
       bob.address,
       MEMBER_PERMISSION_ID,
     );
+    // The DAO is ROOT on itself
     await dao.grant(
-      spaceVotingPlugin.address,
-      bob.address,
-      MEMBER_PERMISSION_ID,
+      dao.address,
+      dao.address,
+      ROOT_PERMISSION_ID,
+    );
+    // The plugin can execute on the DAO
+    await dao.grant(
+      dao.address,
+      memberAccessPlugin.address,
+      EXECUTE_PERMISSION_ID,
+    );
+    // The DAO can update the plugin settings
+    await dao.grant(
+      memberAccessPlugin.address,
+      dao.address,
+      UPDATE_MULTISIG_SETTINGS_PERMISSION_ID,
     );
   });
 
@@ -89,11 +105,15 @@ describe("Default Main Voting plugin", function () {
       await expect(
         memberAccessPlugin.initialize(dao.address, {
           proposalDuration: 60 * 60 * 24 * 5,
-          mainVotingPlugin: spaceVotingPlugin.address,
+          mainVotingPlugin: mainVotingPlugin.address,
         }),
       ).to.be.revertedWith("Initializable: contract is already initialized");
       await expect(
-        spaceVotingPlugin.initialize(dao.address),
+        mainVotingPlugin.initialize(
+          dao.address,
+          defaultMainVotingSettings,
+          alice.address,
+        ),
       ).to.be.revertedWith("Initializable: contract is already initialized");
       await expect(
         spacePlugin.initialize(dao.address, defaultInput.contentUri),
@@ -130,6 +150,59 @@ describe("Default Main Voting plugin", function () {
   it("Only members can create proposals");
   it("Only editors can vote on proposals");
 
+  it("isMember() returns true when appropriate", async () => {
+    expect(await memberAccessPlugin.isMember(ADDRESS_ZERO)).to.eq(false);
+    expect(await memberAccessPlugin.isMember(ADDRESS_ONE)).to.eq(false);
+    expect(await memberAccessPlugin.isMember(ADDRESS_TWO)).to.eq(false);
+
+    expect(await memberAccessPlugin.isMember(alice.address)).to.eq(true);
+    expect(await memberAccessPlugin.isMember(bob.address)).to.eq(true);
+
+    expect(await memberAccessPlugin.isMember(charlie.address)).to.eq(false);
+
+    await dao.grant(
+      mainVotingPlugin.address,
+      charlie.address,
+      MEMBER_PERMISSION_ID,
+    );
+
+    expect(await memberAccessPlugin.isMember(charlie.address)).to.eq(true);
+
+    await dao.revoke(
+      mainVotingPlugin.address,
+      charlie.address,
+      MEMBER_PERMISSION_ID,
+    );
+
+    expect(await memberAccessPlugin.isMember(charlie.address)).to.eq(true);
+
+    await dao.grant(
+      mainVotingPlugin.address,
+      charlie.address,
+      EDITOR_PERMISSION_ID,
+    );
+
+    expect(await memberAccessPlugin.isMember(charlie.address)).to.eq(true);
+  });
+
+  it("isEditor() returns true when appropriate", async () => {
+    expect(await memberAccessPlugin.isEditor(ADDRESS_ZERO)).to.eq(false);
+    expect(await memberAccessPlugin.isEditor(ADDRESS_ONE)).to.eq(false);
+    expect(await memberAccessPlugin.isEditor(ADDRESS_TWO)).to.eq(false);
+
+    expect(await memberAccessPlugin.isEditor(alice.address)).to.eq(true);
+    expect(await memberAccessPlugin.isEditor(bob.address)).to.eq(false);
+    expect(await memberAccessPlugin.isEditor(charlie.address)).to.eq(false);
+
+    await dao.grant(
+      mainVotingPlugin.address,
+      charlie.address,
+      EDITOR_PERMISSION_ID,
+    );
+
+    expect(await memberAccessPlugin.isEditor(charlie.address)).to.eq(true);
+  });
+
   describe("One editor", () => {
     it("Proposals take immediate effect when created by the only editor");
   });
@@ -139,8 +212,6 @@ describe("Default Main Voting plugin", function () {
     it("A minimum support threshold is required");
   });
 
-  it("Only editors can approve");
-  it("Only editors can reject");
   it("Proposals require editor approval when created by a member");
   it("Approved proposals can be executed by anyone");
   it("Rejected proposals cannot be executed");