From e8efa2242d94c10b8707137acb5b6a4ec8cbea09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 13 Aug 2024 15:47:17 +0200 Subject: [PATCH] Adding new tests (WIP) --- README.md | 14 +- packages/contracts-ethers/README.md | 2 +- .../src/personal/PersonalAdminPlugin.sol | 8 +- .../src/personal/PersonalMemberAddHelper.sol | 3 +- .../src/standard/StdGovernancePlugin.sol | 8 +- .../src/standard/StdMemberAddHelper.sol | 10 +- ...ition.ts => execute-selector-condition.ts} | 8 +- .../execute-selector-condition.ts | 4 +- .../personal-member-add-helper.ts | 1000 +++++++++++++++++ .../unit-testing/std-governance-plugin.ts | 2 +- .../unit-testing/std-member-add-helper.ts | 64 +- 11 files changed, 1064 insertions(+), 59 deletions(-) rename packages/contracts/test/integration-testing/{member-add-condition.ts => execute-selector-condition.ts} (95%) create mode 100644 packages/contracts/test/unit-testing/personal-member-add-helper.ts diff --git a/README.md b/README.md index 119ce6f..b709802 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ function grantWithCondition( ); ``` -See the `ExecuteSelectorCondition` contract. It restricts what the [StdMemberAddHelper](#main-member-add-helper) can execute on the DAO. +See the `ExecuteSelectorCondition` contract. It restricts what the [StdMemberAddHelper](#standard-member-add-helper) can execute on the DAO. [Learn more about OSx permissions](https://devs.aragon.org/docs/osx/how-it-works/core/permissions/) @@ -173,10 +173,10 @@ Spaces: Governance settings: -- `UPDATE_VOTING_SETTINGS_PERMISSION_ID` is required to change the settings of the [StdGovernancePlugin](#main-voting-plugin) -- `UPDATE_ADDRESSES_PERMISSION_ID` is required to add or remove members or editors on the [StdGovernancePlugin](#main-voting-plugin) +- `UPDATE_VOTING_SETTINGS_PERMISSION_ID` is required to change the settings of the [StdGovernancePlugin](#standard-governance-plugin) +- `UPDATE_ADDRESSES_PERMISSION_ID` is required to add or remove members or editors on the [StdGovernancePlugin](#standard-governance-plugin) - Typically called by the DAO via proposal -- `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` is required to change the settings of the [StdMemberAddHelper](#main-member-add-helper) +- `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` is required to change the settings of the [StdMemberAddHelper](#standard-member-add-helper) - Typically called by the DAO via proposal Permission management: @@ -221,7 +221,7 @@ It uses the generated typechain artifacts, which contain the interfaces for the ## Adding members and editors -On Spaces with the standard governance, a [StdMemberAddHelper](#main-member-add-helper) and a [StdGovernancePlugin](#main-voting-plugin) will be installed. +On Spaces with the standard governance, a [StdMemberAddHelper](#standard-member-add-helper) and a [StdGovernancePlugin](#standard-governance-plugin) will be installed. ### Members @@ -302,7 +302,7 @@ event SubspaceRemoved(address dao, address subspaceDao); - The DAO can upgrade the plugin - See [Plugin upgrader](#plugin-upgrader) (optional) -### Main Member Add Helper +### Standard Member Add Helper Provides a simple way for any address to request membership on a space. It is a adapted version of Aragon's [Multisig plugin](https://github.com/aragon/osx/blob/develop/packages/contracts/src/plugins/governance/multisig/Multisig.sol). It creates a proposal to `addMember()` on the standard governance plugin and Editors can approve or reject it. Once approved, the member create proposals on the standard governance plugin. @@ -396,7 +396,7 @@ event ProposalExecuted(uint256 indexed proposalId); ### Standard Governance plugin -It's the main governance plugin for standard spaces, where all proposals are voted by editors. It is a adapted version of Aragon's [AddresslistVoting plugin](https://github.com/aragon/osx/blob/develop/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol). Only members (or editors) can create proposals and they can only be executed after a qualified majority has voted for it. +It's the governance plugin for standard spaces, where all proposals are voted by editors. It is a adapted version of Aragon's [AddresslistVoting plugin](https://github.com/aragon/osx/blob/develop/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol). Only members (or editors) can create proposals and they can only be executed after a qualified majority has voted for it. It acts as the source of truth about who is an editor, on Spaces with standard governance. diff --git a/packages/contracts-ethers/README.md b/packages/contracts-ethers/README.md index edfa7ff..2834ef1 100644 --- a/packages/contracts-ethers/README.md +++ b/packages/contracts-ethers/README.md @@ -16,7 +16,7 @@ import { } from ""; // Use it -const mainVotingPluinInstance = StdGovernancePluginFactory__factory.connect(...); +const stdGovernancePluinInstance = StdGovernancePluginFactory__factory.connect(...); ``` ## Development diff --git a/packages/contracts/src/personal/PersonalAdminPlugin.sol b/packages/contracts/src/personal/PersonalAdminPlugin.sol index b256a88..bab675b 100644 --- a/packages/contracts/src/personal/PersonalAdminPlugin.sol +++ b/packages/contracts/src/personal/PersonalAdminPlugin.sol @@ -163,6 +163,10 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors, /// @param _newMember The address to grant member permission to /// @dev Called by the DAO via the PersonalMemberAddHelper. Not by members or editors. function addMember(address _newMember) public auth(ADD_MEMBER_PERMISSION_ID) { + if (dao().hasPermission(address(this), _newMember, MEMBER_PERMISSION_ID, bytes(""))) { + revert AlreadyAMember(_newMember); + } + IDAO.Action[] memory _actions = new IDAO.Action[](1); _actions[0].to = address(dao()); _actions[0].data = abi.encodeCall( @@ -258,7 +262,7 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors, emit EditorRemoved(address(dao()), _editor); } - /// @notice Creates a proposal on PersonalMemberAddHelper to add a new member. + /// @notice Creates a proposal on the PersonalMemberAddHelper to add a new member. If approved, `addMember` will be called back. /// @param _metadataContentUri 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 helper, not to this contract. @@ -266,7 +270,7 @@ contract PersonalAdminPlugin is PluginCloneable, ProposalUpgradeable, IEditors, bytes calldata _metadataContentUri, address _proposedMember ) public returns (uint256 proposalId) { - if (isMember(_proposedMember)) { + if (dao().hasPermission(address(this), _proposedMember, MEMBER_PERMISSION_ID, bytes(""))) { revert AlreadyAMember(_proposedMember); } diff --git a/packages/contracts/src/personal/PersonalMemberAddHelper.sol b/packages/contracts/src/personal/PersonalMemberAddHelper.sol index 3c3cb97..597e04d 100644 --- a/packages/contracts/src/personal/PersonalMemberAddHelper.sol +++ b/packages/contracts/src/personal/PersonalMemberAddHelper.sol @@ -216,6 +216,7 @@ contract PersonalMemberAddHelper is PluginCloneable, ProposalUpgradeable { emit Approved({proposalId: _proposalId, editor: _approver}); + // Only one approval is required: Execute _execute(_proposalId); } @@ -287,7 +288,7 @@ contract PersonalMemberAddHelper is PluginCloneable, ProposalUpgradeable { Proposal storage proposal_ = proposals[_proposalId]; if (!_isProposalOpen(proposal_)) { - // The proposal was executed already + // The proposal is executed, unstarted or expired return false; } else if (!proposal_.destinationPlugin.isEditor(_account)) { // The approver has no voting power. diff --git a/packages/contracts/src/standard/StdGovernancePlugin.sol b/packages/contracts/src/standard/StdGovernancePlugin.sol index 1c03f4f..ae0fc48 100644 --- a/packages/contracts/src/standard/StdGovernancePlugin.sol +++ b/packages/contracts/src/standard/StdGovernancePlugin.sol @@ -10,11 +10,11 @@ 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 {StdMemberAddHelper, MAIN_MEMBER_ADD_INTERFACE_ID} from "./StdMemberAddHelper.sol"; +import {StdMemberAddHelper, STD_MEMBER_ADD_INTERFACE_ID} from "./StdMemberAddHelper.sol"; import {SpacePlugin} from "../space/SpacePlugin.sol"; // The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract. -bytes4 constant MAIN_SPACE_VOTING_INTERFACE_ID = StdGovernancePlugin.initialize.selector ^ +bytes4 constant STD_GOVERNANCE_PLUGIN_INTERFACE_ID = StdGovernancePlugin.initialize.selector ^ StdGovernancePlugin.createProposal.selector ^ StdGovernancePlugin.proposeEdits.selector ^ StdGovernancePlugin.proposeAcceptSubspace.selector ^ @@ -114,7 +114,7 @@ contract StdGovernancePlugin is Addresslist, MajorityVotingBase, IEditors, IMemb _addAddresses(_initialEditors); emit EditorsAdded(_initialEditors); - if (!_stdMemberAddHelper.supportsInterface(MAIN_MEMBER_ADD_INTERFACE_ID)) { + if (!_stdMemberAddHelper.supportsInterface(STD_MEMBER_ADD_INTERFACE_ID)) { revert InvalidInterface(address(_stdMemberAddHelper)); } stdMemberAddHelper = _stdMemberAddHelper; @@ -125,7 +125,7 @@ contract StdGovernancePlugin is Addresslist, MajorityVotingBase, IEditors, IMemb /// @return Returns `true` if the interface is supported. function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) { return - _interfaceId == MAIN_SPACE_VOTING_INTERFACE_ID || + _interfaceId == STD_GOVERNANCE_PLUGIN_INTERFACE_ID || _interfaceId == type(Addresslist).interfaceId || _interfaceId == type(MajorityVotingBase).interfaceId || _interfaceId == type(IMembers).interfaceId || diff --git a/packages/contracts/src/standard/StdMemberAddHelper.sol b/packages/contracts/src/standard/StdMemberAddHelper.sol index adc145a..d828803 100644 --- a/packages/contracts/src/standard/StdMemberAddHelper.sol +++ b/packages/contracts/src/standard/StdMemberAddHelper.sol @@ -11,9 +11,9 @@ import {ProposalUpgradeable} from "@aragon/osx/core/plugin/proposal/ProposalUpgr import {Addresslist} from "./base/Addresslist.sol"; import {IMultisig} from "./base/IMultisig.sol"; import {IEditors} from "../base/IEditors.sol"; -import {StdGovernancePlugin, MAIN_SPACE_VOTING_INTERFACE_ID} from "./StdGovernancePlugin.sol"; +import {StdGovernancePlugin, STD_GOVERNANCE_PLUGIN_INTERFACE_ID} from "./StdGovernancePlugin.sol"; -bytes4 constant MAIN_MEMBER_ADD_INTERFACE_ID = StdMemberAddHelper.initialize.selector ^ +bytes4 constant STD_MEMBER_ADD_INTERFACE_ID = StdMemberAddHelper.initialize.selector ^ StdMemberAddHelper.updateMultisigSettings.selector ^ StdMemberAddHelper.proposeAddMember.selector ^ StdMemberAddHelper.getProposal.selector; @@ -138,7 +138,7 @@ contract StdMemberAddHelper is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade bytes4 _interfaceId ) public view virtual override(PluginUUPSUpgradeable, ProposalUpgradeable) returns (bool) { return - _interfaceId == MAIN_MEMBER_ADD_INTERFACE_ID || + _interfaceId == STD_MEMBER_ADD_INTERFACE_ID || _interfaceId == type(IMultisig).interfaceId || super.supportsInterface(_interfaceId); } @@ -163,7 +163,9 @@ contract StdMemberAddHelper is IMultisig, PluginUUPSUpgradeable, ProposalUpgrade ) public auth(PROPOSER_PERMISSION_ID) returns (uint256 proposalId) { // Check that the caller supports the `addMember` function if ( - !StdGovernancePlugin(msg.sender).supportsInterface(MAIN_SPACE_VOTING_INTERFACE_ID) || + !StdGovernancePlugin(msg.sender).supportsInterface( + STD_GOVERNANCE_PLUGIN_INTERFACE_ID + ) || !StdGovernancePlugin(msg.sender).supportsInterface(type(IEditors).interfaceId) || !StdGovernancePlugin(msg.sender).supportsInterface(type(Addresslist).interfaceId) ) { diff --git a/packages/contracts/test/integration-testing/member-add-condition.ts b/packages/contracts/test/integration-testing/execute-selector-condition.ts similarity index 95% rename from packages/contracts/test/integration-testing/member-add-condition.ts rename to packages/contracts/test/integration-testing/execute-selector-condition.ts index e7b694a..8f1a4de 100644 --- a/packages/contracts/test/integration-testing/member-add-condition.ts +++ b/packages/contracts/test/integration-testing/execute-selector-condition.ts @@ -35,7 +35,7 @@ const pluginSettings: MajorityVotingBase.VotingSettingsStruct = { }; const memberAddProposalDuration = 60 * 60 * 24; -describe('Member Add Condition E2E', () => { +describe('Execute selector condition E2E', () => { let deployer: SignerWithAddress; let pluginUpgrader: SignerWithAddress; let alice: SignerWithAddress; @@ -46,7 +46,7 @@ describe('Member Add Condition E2E', () => { let pluginSetupRef: PluginSetupRefStruct; let pluginSetup: TestStdGovernanceSetup; - let gpsFactory: TestStdGovernanceSetup__factory; + let sgsFactory: TestStdGovernanceSetup__factory; let stdGovernancePlugin: StdGovernancePlugin; // let stdMemberAddHelper: StdMemberAddHelper; @@ -88,8 +88,8 @@ describe('Member Add Condition E2E', () => { ); // Deploy PluginSetup build 1 - gpsFactory = new TestStdGovernanceSetup__factory().connect(deployer); - pluginSetup = await gpsFactory.deploy(psp.address); + sgsFactory = new TestStdGovernanceSetup__factory().connect(deployer); + pluginSetup = await sgsFactory.deploy(psp.address); // Publish build 1 tx = await pluginRepo.createVersion(1, pluginSetup.address, '0x00', '0x00'); diff --git a/packages/contracts/test/unit-testing/execute-selector-condition.ts b/packages/contracts/test/unit-testing/execute-selector-condition.ts index 0c98ad2..c3ee1eb 100644 --- a/packages/contracts/test/unit-testing/execute-selector-condition.ts +++ b/packages/contracts/test/unit-testing/execute-selector-condition.ts @@ -20,13 +20,11 @@ import {ethers, network} from 'hardhat'; const SOME_CONTRACT_ADDRESS = '0x' + '1234567890'.repeat(4); const ONE_BYTES32 = '0x0000000000000000000000000000000000000000000000000000000000000001'; -const PLUGIN_ADDR_1 = ADDRESS_ONE; -const PLUGIN_ADDR_2 = ADDRESS_TWO; const daoInterface = DAO__factory.createInterface(); const stdGovernancePluginInterface = StdGovernancePlugin__factory.createInterface(); -describe('Member Add Condition', function () { +describe('Execute selector Condition', function () { const pspAddress = getPluginSetupProcessorAddress(network.name, true); let alice: SignerWithAddress; diff --git a/packages/contracts/test/unit-testing/personal-member-add-helper.ts b/packages/contracts/test/unit-testing/personal-member-add-helper.ts new file mode 100644 index 0000000..8f5d561 --- /dev/null +++ b/packages/contracts/test/unit-testing/personal-member-add-helper.ts @@ -0,0 +1,1000 @@ +import { + DAO, + IDAO, + IERC165Upgradeable__factory, + IPlugin__factory, + IProposal__factory, + PersonalAdminPlugin, + PersonalAdminPlugin__factory, + PersonalMemberAddHelper, + PersonalMemberAddHelper__factory, + ExecuteSelectorCondition, + ExecuteSelectorCondition__factory, + TestCloneFactory__factory, + TestCloneFactory, +} from '../../typechain'; +import { + ApprovedEvent, + ProposalCreatedEvent, +} from '../../typechain/src/personal/PersonalMemberAddHelper'; +import {findEvent} from '../../utils/helpers'; +import {getInterfaceID} from '../../utils/interfaces'; +import {deployTestDao} from '../helpers/test-dao'; +import { + ADDRESS_ONE, + ADDRESS_TWO, + ADDRESS_ZERO, + EDITOR_PERMISSION_ID, + EMPTY_DATA, + EXECUTE_PERMISSION_ID, + MEMBER_PERMISSION_ID, + mineBlock, + PROPOSER_PERMISSION_ID, + ROOT_PERMISSION_ID, + ADD_MEMBER_PERMISSION_ID, + UPDATE_SETTINGS_PERMISSION_ID, + ZERO_BYTES32, +} from './common'; +import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; +import {expect} from 'chai'; +import {BigNumber} from 'ethers'; +import {hexlify, toUtf8Bytes} from 'ethers/lib/utils'; +import {ethers} from 'hardhat'; + +export type InitData = {contentUri: string}; +export const defaultInitData: InitData = { + contentUri: 'ipfs://', +}; + +export const memberAddInterface = new ethers.utils.Interface([ + 'function initialize(address,tuple(uint64))', + 'function updateSettings(tuple(uint64))', + 'function proposeAddMember(bytes,address,address)', + 'function getProposal(uint256)', +]); +const personalAdminPluginInterface = + PersonalAdminPlugin__factory.createInterface(); + +describe('Personal Member Add Plugin', function () { + let signers: SignerWithAddress[]; + let alice: SignerWithAddress; + let bob: SignerWithAddress; + let carol: SignerWithAddress; + let dave: SignerWithAddress; + let dao: DAO; + let personalMemberAddHelper: PersonalMemberAddHelper; + let executeSelectorCondition: ExecuteSelectorCondition; + let personalAdminPlugin: PersonalAdminPlugin; + let testCloneFactory: TestCloneFactory; + let pid: BigNumber; + + before(async () => { + signers = await ethers.getSigners(); + [alice, bob, carol, dave] = signers; + dao = await deployTestDao(alice); + + const TestCloneFactory = new TestCloneFactory__factory(alice); + testCloneFactory = await TestCloneFactory.deploy(); + }); + + beforeEach(async () => { + // Personal admin (plugin) + const PersonalAdminPluginFactory = new PersonalAdminPlugin__factory(alice); + const nonce = await ethers.provider.getTransactionCount( + testCloneFactory.address + ); + const anticipatedPluginAddress = ethers.utils.getContractAddress({ + from: testCloneFactory.address, + nonce, + }); + await testCloneFactory.clonePersonalAdminPlugin(); + personalAdminPlugin = PersonalAdminPluginFactory.attach( + anticipatedPluginAddress + ); + + // Personal member add (helper) + const PersonalMemberAddFactory = new PersonalMemberAddHelper__factory( + alice + ); + const anticipatedHelperAddress = ethers.utils.getContractAddress({ + from: testCloneFactory.address, + nonce: nonce + 1, + }); + await testCloneFactory.clonePersonalMemberAddHelper(); + personalMemberAddHelper = PersonalMemberAddFactory.attach( + anticipatedHelperAddress + ); + + await initializePAP(anticipatedHelperAddress); + await initializePMAH(); + + executeSelectorCondition = await new ExecuteSelectorCondition__factory( + alice + ).deploy( + personalAdminPlugin.address, + personalAdminPluginInterface.getSighash('addMember') + ); + + // The helper can execute on the DAO + await dao.grantWithCondition( + dao.address, + personalMemberAddHelper.address, + EXECUTE_PERMISSION_ID, + executeSelectorCondition.address + ); + // The plugin can also execute on the DAO + await dao.grant( + dao.address, + personalAdminPlugin.address, + EXECUTE_PERMISSION_ID + ); + // The DAO can manage editors on the standard governance plugin + await dao.grant( + personalAdminPlugin.address, + dao.address, + ADD_MEMBER_PERMISSION_ID + ); + // The DAO can update the plugin settings + await dao.grant( + personalMemberAddHelper.address, + dao.address, + UPDATE_SETTINGS_PERMISSION_ID + ); + // The DAO is ROOT on itself + await dao.grant(dao.address, dao.address, ROOT_PERMISSION_ID); + // The plugin can propose members on the helper + await dao.grant( + personalMemberAddHelper.address, + personalAdminPlugin.address, + PROPOSER_PERMISSION_ID + ); + // Alice can make the DAO execute arbitrary stuff (test) + await dao.grant(dao.address, alice.address, EXECUTE_PERMISSION_ID); + + // Alice is an editor + await dao.grant( + personalAdminPlugin.address, + alice.address, + EDITOR_PERMISSION_ID + ); + + // Bob is a member + await mineBlock(); + await personalAdminPlugin.proposeAddMember('0x', bob.address); + }); + + function initializePAP(helperAddr: string) { + return personalAdminPlugin.initialize( + dao.address, + alice.address, // this just emits an event, not granting the permission here + helperAddr + ); + } + + function initializePMAH() { + return personalMemberAddHelper.initialize(dao.address, { + proposalDuration: 60 * 60 * 24 * 5, + }); + } + + describe('initialize', () => { + it('reverts if trying to re-initialize', async () => { + await expect( + personalMemberAddHelper.initialize(dao.address, { + proposalDuration: 60 * 60 * 24 * 5, + }) + ).to.be.revertedWith('Initializable: contract is already initialized'); + }); + }); + + describe('Before approving', () => { + it('Only addresses with PROPOSER_PERMISSION_ID can propose members', async () => { + // ok + await expect( + personalAdminPlugin.proposeAddMember( + toUtf8Bytes('ipfs://'), + carol.address + ) + ).to.not.be.reverted; + + await dao.revoke( + personalMemberAddHelper.address, + personalAdminPlugin.address, + PROPOSER_PERMISSION_ID + ); + + // Now it fails + await expect( + personalAdminPlugin.proposeAddMember( + toUtf8Bytes('ipfs://'), + dave.address + ) + ).to.be.reverted; + }); + + it('Only callers implementing the right interface can propose members', async () => { + // From a compatible plugin + await expect( + personalAdminPlugin.proposeAddMember( + toUtf8Bytes('ipfs://'), + carol.address + ) + ).to.not.be.reverted; + + await dao.grant( + personalMemberAddHelper.address, + alice.address, + PROPOSER_PERMISSION_ID + ); + + // Fail despite the permission + await expect( + personalMemberAddHelper.proposeAddMember( + toUtf8Bytes('ipfs://'), + dave.address, + alice.address + ) + ).to.be.reverted; + }); + + it('Allows any address to request membership via the PersonalAdminPlugin', async () => { + // Random + expect(await personalAdminPlugin.isMember(carol.address)).to.be.false; + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(carol) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), carol.address) + ).to.not.be.reverted; + + let proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + expect(proposal.actions.length).to.eq(1); + expect(proposal.failsafeActionMap).to.eq(0); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false); + + // Member + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(bob) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_ONE) + ).to.not.be.reverted; + + proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + expect(proposal.actions.length).to.eq(1); + expect(proposal.failsafeActionMap).to.eq(0); + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + + // Editor + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + await expect( + personalAdminPlugin + .connect(alice) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_TWO) + ).to.not.be.reverted; + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(true); + + pid = await personalMemberAddHelper.proposalCount(); + proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(true); + expect(proposal.actions.length).to.eq(1); + expect(proposal.failsafeActionMap).to.eq(0); + // Auto executed + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(true); + }); + + it('Editors should be members too', async () => { + expect(await personalAdminPlugin.isMember(alice.address)).to.eq(true); + + expect(await personalAdminPlugin.isMember(bob.address)).to.eq(true); + await makeEditor(bob.address); + expect(await personalAdminPlugin.isMember(bob.address)).to.eq(true); + }); + + it('Emits an event when membership is requested', async () => { + pid = await personalMemberAddHelper.proposalCount(); + + const tx = await personalAdminPlugin + .connect(carol) + .proposeAddMember(toUtf8Bytes('ipfs://2345'), carol.address); + + await expect(tx).to.emit(personalMemberAddHelper, 'ProposalCreated'); + + const event = await findEvent( + tx, + 'ProposalCreated' + ); + + expect(!!event).to.eq(true); + expect(event!.args.proposalId).to.equal(pid); + expect(event!.args.creator).to.equal(carol.address); + expect(event!.args.metadata).to.equal( + hexlify(toUtf8Bytes('ipfs://2345')) + ); + expect(event!.args.actions.length).to.equal(1); + expect(event!.args.actions[0].to).to.equal(personalAdminPlugin.address); + expect(event!.args.actions[0].value).to.equal(0); + expect(event!.args.actions[0].data).to.equal( + personalAdminPluginInterface.encodeFunctionData('addMember', [ + carol.address, + ]) + ); + expect(event!.args.allowFailureMap).to.equal(0); + }); + + it('isMember() returns true when appropriate', async () => { + expect(await personalAdminPlugin.isMember(ADDRESS_ZERO)).to.eq(false); + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + + expect(await personalAdminPlugin.isMember(alice.address)).to.eq(true); + expect(await personalAdminPlugin.isMember(bob.address)).to.eq(true); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false); + + await personalAdminPlugin.proposeAddMember('0x', carol.address); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(true); + + await dao.revoke( + personalAdminPlugin.address, + carol.address, + MEMBER_PERMISSION_ID + ); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false); + + await makeEditor(carol.address); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(true); + + await removeEditor(carol.address); + expect(await personalAdminPlugin.isMember(carol.address)).to.eq(false); + }); + + it('isEditor() returns true when appropriate', async () => { + expect(await personalAdminPlugin.isEditor(ADDRESS_ZERO)).to.eq(false); + expect(await personalAdminPlugin.isEditor(ADDRESS_ONE)).to.eq(false); + expect(await personalAdminPlugin.isEditor(ADDRESS_TWO)).to.eq(false); + + expect(await personalAdminPlugin.isEditor(alice.address)).to.eq(true); + expect(await personalAdminPlugin.isEditor(bob.address)).to.eq(false); + expect(await personalAdminPlugin.isEditor(carol.address)).to.eq(false); + + await makeEditor(carol.address); + expect(await personalAdminPlugin.isEditor(carol.address)).to.eq(true); + + await removeEditor(carol.address); + expect(await personalAdminPlugin.isEditor(carol.address)).to.eq(false); + }); + }); + + describe('Membership approval', () => { + // Alice: editor + // Bob: editor + // Carol: editor + + beforeEach(async () => { + await makeEditor(bob.address); + await makeEditor(carol.address); + }); + + it('Only editors can approve adding members', async () => { + // Requesting membership for Dave + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Dave cannot approve (fail) + await expect(personalMemberAddHelper.connect(dave).approve(pid)).to.be + .reverted; + + // Dave is still not a member + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Approve it (Alice) + await expect(personalMemberAddHelper.connect(alice).approve(pid)).to.not + .be.reverted; + + // Dave is now a member + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(true); + + // Now requesting for 0x1 + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_ONE) + ).to.not.be.reverted; + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + + // Dave cannot approve (fail) + await expect(personalMemberAddHelper.connect(dave).approve(pid)).to.be + .reverted; + + // ADDRESS_ONE is still not a member + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + + // Approve it (Bob) + await expect(personalMemberAddHelper.connect(bob).approve(pid)).to.not.be + .reverted; + + // ADDRESS_ONE is now a member + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(true); + + // Now requesting for 0x2 + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_TWO) + ).to.not.be.reverted; + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + + // Dave cannot approve (fail) + await expect(personalMemberAddHelper.connect(dave).approve(pid)).to.be + .reverted; + + // ADDRESS_TWO is still not a member + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + + // Approve it (Carol) + await expect(personalMemberAddHelper.connect(carol).approve(pid)).to.not + .be.reverted; + + // ADDRESS_TWO is now a member + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(true); + }); + + it('Proposals should be unsettled when created by a non-editor', async () => { + // Proposed by a random wallet + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + + let proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + await personalAdminPlugin.proposeAddMember('0x', dave.address); + + // Proposed by a (now) member + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_ONE) + ).to.not.be.reverted; + + expect((await personalMemberAddHelper.getProposal(pid)).executed).to.eq( + false + ); + expect(await personalAdminPlugin.isMember(ADDRESS_ONE)).to.eq(false); + + // Proposed by an editor + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(alice) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_TWO) + ).to.not.be.reverted; + + proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + expect(await personalAdminPlugin.isMember(ADDRESS_TWO)).to.eq(false); + }); + + it('Only editors can reject new membership proposals', async () => { + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Reject it (Dave) => fail + await expect(personalMemberAddHelper.connect(dave).reject(pid)).to.be + .reverted; + + // Still not a member + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Reject it (Bob) => success + await expect(personalMemberAddHelper.connect(bob).reject(pid)).to.not.be + .reverted; + + // Still not a member + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Try to approve it (bob) => fail + await expect(personalMemberAddHelper.connect(bob).approve(pid)).to.be + .reverted; + + expect((await personalMemberAddHelper.getProposal(pid)).executed).to.eq( + false + ); + }); + + it("Proposals created by a non-editor need an editor's approval", async () => { + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + + const proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Dave cannot + await expect(personalMemberAddHelper.connect(dave).approve(pid)).to.be + .reverted; + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + // Alice can + await expect(personalMemberAddHelper.connect(alice).approve(pid)).to.not + .be.reverted; + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(true); + }); + + it('Proposals created by an editor are automatically executed', async () => { + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(false); + + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(alice) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(true); + + const proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + }); + + it('Memberships approved by an editor are immediately executed', async () => { + // Dave proposes himself + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), dave.address) + ).to.not.be.reverted; + + let proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + + // Approve it (Dave) => fail + await expect(personalMemberAddHelper.connect(dave).approve(pid)).to.be + .reverted; + + proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(false); + + // Approve it (Alice) => succeed + await expect(personalMemberAddHelper.connect(alice).approve(pid)).to.not + .be.reverted; + + proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(true); + + // Now Dave is a member + expect(await personalAdminPlugin.isMember(dave.address)).to.eq(true); + }); + }); + + describe('Proposal data', () => { + // Alice: editor + // Bob: member + + it('proposeNewMember should generate the right action list', async () => { + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(carol) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), carol.address) + ).to.not.be.reverted; + + const proposal = await personalMemberAddHelper.getProposal(pid); + expect(proposal.actions.length).to.eq(1); + expect(proposal.actions[0].to).to.eq(personalAdminPlugin.address); + expect(proposal.actions[0].value).to.eq(0); + expect(proposal.actions[0].data).to.eq( + personalAdminPluginInterface.encodeFunctionData('addMember', [ + carol.address, + ]) + ); + }); + + it('Proposing an existing member fails', async () => { + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ) + .to.be.revertedWithCustomError(personalAdminPlugin, 'AlreadyAMember') + .withArgs(alice.address); + + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), bob.address) + ) + .to.be.revertedWithCustomError(personalAdminPlugin, 'AlreadyAMember') + .withArgs(bob.address); + + // More + await expect( + personalAdminPlugin + .connect(carol) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; + + await expect( + personalAdminPlugin + .connect(bob) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; + + await expect( + personalAdminPlugin + .connect(alice) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; + }); + + it('Attempting to approve twice fails', async () => { + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), carol.address) + ).to.not.be.reverted; + + await expect(personalMemberAddHelper.approve(pid)).to.not.be.reverted; + await expect(personalMemberAddHelper.approve(pid)).to.be.reverted; + }); + + it('Attempting to reject twice fails', async () => { + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), carol.address) + ).to.not.be.reverted; + + await expect(personalMemberAddHelper.reject(pid)).to.not.be.reverted; + await expect(personalMemberAddHelper.reject(pid)).to.be.reverted; + }); + + it('Rejected proposals cannot be approved', async () => { + pid = await personalMemberAddHelper.proposalCount(); + await expect( + personalAdminPlugin + .connect(dave) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), carol.address) + ).to.not.be.reverted; + + await expect(personalMemberAddHelper.reject(pid)).to.not.be.reverted; + await expect(personalMemberAddHelper.approve(pid)).to.be.reverted; + }); + + it('Only the DAO can call the plugin to update the settings', async () => { + // Nobody else can + await expect( + personalMemberAddHelper.connect(alice).updateSettings({ + proposalDuration: 60 * 60 * 24 * 5, + }) + ).to.be.reverted; + await expect( + personalMemberAddHelper.connect(bob).updateSettings({ + proposalDuration: 60 * 60 * 24 * 5, + }) + ).to.be.reverted; + await expect( + personalMemberAddHelper.connect(carol).updateSettings({ + proposalDuration: 60 * 60 * 24 * 5, + }) + ).to.be.reverted; + await expect( + personalMemberAddHelper.connect(dave).updateSettings({ + proposalDuration: 60 * 60 * 24 * 5, + }) + ).to.be.reverted; + + // The DAO can + const actions: IDAO.ActionStruct[] = [ + { + to: personalMemberAddHelper.address, + value: 0, + data: PersonalMemberAddHelper__factory.createInterface().encodeFunctionData( + 'updateSettings', + [ + { + proposalDuration: 60 * 60 * 24 * 5, + }, + ] + ), + }, + ]; + + await expect(dao.execute(ZERO_BYTES32, actions, 0)).to.not.be.reverted; + }); + }); + + describe('Other tests', () => { + describe('initialize', () => { + it('should emit `SettingsUpdated` during initialization', async () => { + const nonce = await ethers.provider.getTransactionCount( + testCloneFactory.address + ); + const PersonalMemberAddFactory = new PersonalMemberAddHelper__factory( + alice + ); + const anticipatedHelperAddress = ethers.utils.getContractAddress({ + from: testCloneFactory.address, + nonce, + }); + await testCloneFactory.clonePersonalMemberAddHelper(); + personalMemberAddHelper = PersonalMemberAddFactory.attach( + anticipatedHelperAddress + ); + const settings: PersonalMemberAddHelper.SettingsStruct = { + proposalDuration: 60 * 60 * 24 * 5, + }; + + await expect(personalMemberAddHelper.initialize(dao.address, settings)) + .to.emit(personalMemberAddHelper, 'SettingsUpdated') + .withArgs(60 * 60 * 24 * 5); + }); + }); + + describe('plugin interface: ', () => { + it('does not support the empty interface', async () => { + expect(await personalMemberAddHelper.supportsInterface('0xffffffff')).to + .be.false; + }); + + it('supports the `IERC165Upgradeable` interface', async () => { + const iface = IERC165Upgradeable__factory.createInterface(); + expect( + await personalMemberAddHelper.supportsInterface(getInterfaceID(iface)) + ).to.be.true; + }); + + it('supports the `IPlugin` interface', async () => { + const iface = IPlugin__factory.createInterface(); + expect( + await personalMemberAddHelper.supportsInterface(getInterfaceID(iface)) + ).to.be.true; + }); + + it('supports the `IProposal` interface', async () => { + const iface = IProposal__factory.createInterface(); + expect( + await personalMemberAddHelper.supportsInterface(getInterfaceID(iface)) + ).to.be.true; + }); + }); + + describe('updateSettings:', () => { + it('should emit `SettingsUpdated` when `updateMutlsigSettings` gets called', async () => { + await dao.grant( + personalMemberAddHelper.address, + alice.address, + await personalMemberAddHelper.UPDATE_SETTINGS_PERMISSION_ID() + ); + const settings = { + proposalDuration: 60 * 60 * 24 * 5, + }; + + await expect(personalMemberAddHelper.updateSettings(settings)) + .to.emit(personalMemberAddHelper, 'SettingsUpdated') + .withArgs(60 * 60 * 24 * 5); + }); + }); + + describe('createProposal:', () => { + it('increments the proposal counter', async () => { + const pc = await personalMemberAddHelper.proposalCount(); + + await expect( + personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address) + ).not.to.be.reverted; + + expect(await personalMemberAddHelper.proposalCount()).to.equal( + pc.add(1) + ); + }); + + it('creates unique proposal IDs for each proposal', async () => { + const proposalId0 = + await personalAdminPlugin.callStatic.proposeAddMember( + EMPTY_DATA, + carol.address + ); + // create a new proposal for the proposalCounter to be incremented + await expect( + personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address) + ).not.to.be.reverted; + + const proposalId1 = + await personalAdminPlugin.callStatic.proposeAddMember( + EMPTY_DATA, + dave.address + ); + + expect(proposalId0).to.equal(1); + expect(proposalId1).to.equal(2); + + expect(proposalId0).to.not.equal(proposalId1); + }); + + it('emits the `ProposalCreated` event', async () => { + await expect( + personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address) + ).to.emit(personalMemberAddHelper, 'ProposalCreated'); + }); + + it('reverts if the settings have been changed in the same block', async () => { + await dao.grant( + personalMemberAddHelper.address, + dao.address, + await personalMemberAddHelper.UPDATE_SETTINGS_PERMISSION_ID() + ); + + await ethers.provider.send('evm_setAutomine', [false]); + + await dao.execute( + ZERO_BYTES32, + [ + { + to: personalMemberAddHelper.address, + value: 0, + data: personalMemberAddHelper.interface.encodeFunctionData( + 'updateSettings', + [ + { + proposalDuration: 60 * 60 * 24 * 5, + }, + ] + ), + }, + ], + 0 + ); + await expect( + personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address) + ).to.revertedWithCustomError( + personalMemberAddHelper, + 'ProposalCreationForbiddenOnSameBlock' + ); + + await ethers.provider.send('evm_setAutomine', [true]); + }); + }); + + describe('canApprove:', () => { + beforeEach(async () => { + await makeEditor(bob.address); // have 2 editors + await mineBlock(); + + expect(await personalAdminPlugin.isEditor(alice.address)).to.be.true; + expect(await personalAdminPlugin.isEditor(bob.address)).to.be.true; + expect(await personalAdminPlugin.isEditor(carol.address)).to.be.false; + + // Alice approves + pid = await personalMemberAddHelper.proposalCount(); + await personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address); + }); + + it('returns `false` if the proposal is already executed', async () => { + expect((await personalMemberAddHelper.getProposal(pid)).executed).to.be + .false; + await personalMemberAddHelper.connect(bob).approve(pid); + + expect((await personalMemberAddHelper.getProposal(pid)).executed).to.be + .true; + expect( + await personalMemberAddHelper.canApprove(pid, signers[3].address) + ).to.be.false; + }); + + it('returns `false` if the approver is not an editor', async () => { + expect(await personalAdminPlugin.isEditor(signers[9].address)).to.be + .false; + + expect( + await personalMemberAddHelper.canApprove(pid, signers[9].address) + ).to.be.false; + }); + + it('returns `false` if the approver has already approved', async () => { + expect(await personalMemberAddHelper.canApprove(pid, bob.address)).to.be + .true; + await personalMemberAddHelper.connect(bob).approve(pid); + expect(await personalMemberAddHelper.canApprove(pid, bob.address)).to.be + .false; + }); + + it('returns `true` if the approver is listed', async () => { + expect(await personalMemberAddHelper.canApprove(pid, bob.address)).to.be + .true; + }); + + it('returns `false` if the proposal is settled', async () => { + pid = await personalMemberAddHelper.proposalCount(); + await personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address); + + expect(await personalMemberAddHelper.canApprove(pid, bob.address)).to.be + .true; + + await personalMemberAddHelper.connect(bob).approve(pid); + + expect(await personalMemberAddHelper.canApprove(pid, bob.address)).to.be + .false; + }); + }); + + describe('approve:', () => { + beforeEach(async () => { + await makeEditor(bob.address); // have 2 editors + await mineBlock(); + + // Alice approves + pid = await personalMemberAddHelper.proposalCount(); + await personalAdminPlugin.proposeAddMember(EMPTY_DATA, carol.address); + }); + + it('reverts when approving multiple times', async () => { + await personalMemberAddHelper.connect(bob).approve(pid); + + // Try to vote again + await expect(personalMemberAddHelper.connect(bob).approve(pid)) + .to.be.revertedWithCustomError( + personalMemberAddHelper, + 'ApprovalCastForbidden' + ) + .withArgs(pid, bob.address); + }); + + it('approves with the msg.sender address', async () => { + const tx = await personalMemberAddHelper.connect(bob).approve(pid); + + const event = await findEvent(tx, 'Approved'); + expect(event!.args.proposalId).to.eq(pid); + expect(event!.args.editor).to.eq(bob.address); + }); + }); + }); + + // Helpers + + const makeEditor = (newEditor: string) => { + return dao + .connect(alice) + .grant(personalAdminPlugin.address, newEditor, EDITOR_PERMISSION_ID) + .then(tx => tx.wait()); + }; + const removeEditor = (editor: string) => { + return dao + .connect(alice) + .revoke(personalAdminPlugin.address, editor, EDITOR_PERMISSION_ID) + .then(tx => tx.wait()); + }; +}); diff --git a/packages/contracts/test/unit-testing/std-governance-plugin.ts b/packages/contracts/test/unit-testing/std-governance-plugin.ts index 70851ea..daeb4be 100644 --- a/packages/contracts/test/unit-testing/std-governance-plugin.ts +++ b/packages/contracts/test/unit-testing/std-governance-plugin.ts @@ -1709,7 +1709,7 @@ describe('Standard Governance Plugin', function () { }); context('Joining a space via StdMemberAddHelper', () => { - it('Proposing new members via MainMemberAdd plugin grants membership', async () => { + it('Proposing new members via StdMemberAddHelper grants membership', async () => { expect(await stdGovernancePlugin.isMember(carol.address)).to.be.false; await stdGovernancePlugin.proposeAddMember( toUtf8Bytes('ipfs://'), diff --git a/packages/contracts/test/unit-testing/std-member-add-helper.ts b/packages/contracts/test/unit-testing/std-member-add-helper.ts index d801c06..43322f1 100644 --- a/packages/contracts/test/unit-testing/std-member-add-helper.ts +++ b/packages/contracts/test/unit-testing/std-member-add-helper.ts @@ -239,7 +239,6 @@ describe('Member Add Plugin', function () { expect(proposal.actions.length).to.eq(1); expect(proposal.failsafeActionMap).to.eq(0); expect(await stdGovernancePlugin.isMember(carol.address)).to.eq(false); - expect(await stdGovernancePlugin.isMember(carol.address)).to.eq(false); // Member pid = await stdMemberAddHelper.proposalCount(); @@ -256,29 +255,33 @@ describe('Member Add Plugin', function () { expect(proposal.actions.length).to.eq(1); expect(proposal.failsafeActionMap).to.eq(0); expect(await stdGovernancePlugin.isMember(ADDRESS_ONE)).to.eq(false); - expect(await stdGovernancePlugin.isMember(ADDRESS_ONE)).to.eq(false); // Editor + pid = await stdMemberAddHelper.proposalCount(); + expect(await stdGovernancePlugin.isMember(ADDRESS_TWO)).to.eq(false); await expect( stdGovernancePlugin .connect(alice) .proposeAddMember(toUtf8Bytes('ipfs://1234'), ADDRESS_TWO) ).to.not.be.reverted; + expect(await stdGovernancePlugin.isMember(ADDRESS_TWO)).to.eq(true); - proposal = await stdMemberAddHelper.getProposal(1); - expect(proposal.executed).to.eq(false); - expect(proposal.approvals).to.eq(0); + proposal = await stdMemberAddHelper.getProposal(pid); + expect(proposal.executed).to.eq(true); + expect(proposal.approvals).to.eq(1); expect(proposal.parameters.minApprovals).to.eq(1); expect(proposal.actions.length).to.eq(1); expect(proposal.failsafeActionMap).to.eq(0); // Auto executed expect(await stdGovernancePlugin.isMember(ADDRESS_TWO)).to.eq(true); - expect(await stdGovernancePlugin.isMember(ADDRESS_TWO)).to.eq(true); }); it('Editors should be members too', async () => { expect(await stdGovernancePlugin.isMember(alice.address)).to.eq(true); - expect(await stdGovernancePlugin.isMember(alice.address)).to.eq(true); + + expect(await stdGovernancePlugin.isMember(bob.address)).to.eq(true); + await stdGovernancePlugin.proposeAddEditor('0x', bob.address); + expect(await stdGovernancePlugin.isMember(bob.address)).to.eq(true); }); it('Emits an event when membership is requested', async () => { @@ -507,13 +510,11 @@ describe('Member Add Plugin', function () { // Carol: editor beforeEach(async () => { - let pidMainVoting = 0; + let pid = 0; await proposeNewEditor(bob.address); await proposeNewEditor(carol.address); - pidMainVoting = 1; - await stdGovernancePlugin - .connect(bob) - .vote(pidMainVoting, VoteOption.Yes, true); + pid = 1; + await stdGovernancePlugin.connect(bob).vote(pid, VoteOption.Yes, true); }); it('Only editors can approve adding members', async () => { @@ -825,6 +826,25 @@ describe('Member Add Plugin', function () { ) .to.be.revertedWithCustomError(stdGovernancePlugin, 'AlreadyAMember') .withArgs(bob.address); + + // More + await expect( + stdGovernancePlugin + .connect(carol) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; + + await expect( + stdGovernancePlugin + .connect(bob) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; + + await expect( + stdGovernancePlugin + .connect(alice) + .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) + ).to.be.reverted; }); it('Attempting to approve twice fails', async () => { @@ -851,26 +871,6 @@ describe('Member Add Plugin', function () { await expect(stdMemberAddHelper.reject(pid)).to.be.reverted; }); - it('Attempting to propose adding an existing member fails', async () => { - await expect( - stdGovernancePlugin - .connect(carol) - .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) - ).to.be.reverted; - - await expect( - stdGovernancePlugin - .connect(bob) - .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) - ).to.be.reverted; - - await expect( - stdGovernancePlugin - .connect(alice) - .proposeAddMember(toUtf8Bytes('ipfs://1234'), alice.address) - ).to.be.reverted; - }); - it('Rejected proposals cannot be approved', async () => { pid = await stdMemberAddHelper.proposalCount(); await expect(