From 2d0a721e87557377ba34dedf922b21b52e4d764e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 30 Jan 2024 19:06:58 +0100 Subject: [PATCH] Plugin testing WIP --- .../MemberAccessExecuteCondition.sol | 1 - .../src/personal/PersonalSpaceAdminPlugin.sol | 2 +- .../member-access-condition.ts | 1 - .../test/unit-testing/main-voting-plugin.ts | 62 ++++------ .../member-access-execute-condition.ts | 58 +++++---- .../test/unit-testing/member-access-plugin.ts | 110 ++++++------------ 6 files changed, 87 insertions(+), 147 deletions(-) diff --git a/packages/contracts/src/conditions/MemberAccessExecuteCondition.sol b/packages/contracts/src/conditions/MemberAccessExecuteCondition.sol index 950ae7e..d50d6f6 100644 --- a/packages/contracts/src/conditions/MemberAccessExecuteCondition.sol +++ b/packages/contracts/src/conditions/MemberAccessExecuteCondition.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.17; import {IDAO} from "@aragon/osx/core/dao/IDAO.sol"; import {PermissionCondition} from "@aragon/osx/core/permission/PermissionCondition.sol"; import {PermissionManager} from "@aragon/osx/core/permission/PermissionManager.sol"; -import {MEMBER_PERMISSION_ID} from "../constants.sol"; import {MainVotingPlugin} from "../governance/MainVotingPlugin.sol"; /// @notice The condition associated with `TestSharedPlugin` diff --git a/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol b/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol index 097e66d..64b1982 100644 --- a/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol +++ b/packages/contracts/src/personal/PersonalSpaceAdminPlugin.sol @@ -5,7 +5,7 @@ import {SafeCastUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/mat import {ProposalUpgradeable} from "@aragon/osx/core/plugin/proposal/ProposalUpgradeable.sol"; import {PluginCloneable} from "@aragon/osx/core/plugin/PluginCloneable.sol"; import {IDAO} from "@aragon/osx/core/dao/IDAO.sol"; -import {EDITOR_PERMISSION_ID, MEMBER_PERMISSION_ID} from "../constants.sol"; +import {EDITOR_PERMISSION_ID} from "../constants.sol"; /// @title PersonalSpaceAdminPlugin /// @author Aragon - 2023 diff --git a/packages/contracts/test/integration-testing/member-access-condition.ts b/packages/contracts/test/integration-testing/member-access-condition.ts index fdecbb1..5f15e8a 100644 --- a/packages/contracts/test/integration-testing/member-access-condition.ts +++ b/packages/contracts/test/integration-testing/member-access-condition.ts @@ -19,7 +19,6 @@ import {installPlugin} from '../helpers/setup'; import {deployTestDao} from '../helpers/test-dao'; import { EXECUTE_PERMISSION_ID, - MEMBER_PERMISSION_ID, ROOT_PERMISSION_ID, UPGRADE_PLUGIN_PERMISSION_ID, ONE_BYTES32, diff --git a/packages/contracts/test/unit-testing/main-voting-plugin.ts b/packages/contracts/test/unit-testing/main-voting-plugin.ts index 7070e87..20a1bb4 100644 --- a/packages/contracts/test/unit-testing/main-voting-plugin.ts +++ b/packages/contracts/test/unit-testing/main-voting-plugin.ts @@ -30,7 +30,6 @@ import { EMPTY_DATA, EXECUTE_PERMISSION_ID, getTime, // MAX_UINT64, - MEMBER_PERMISSION_ID, mineBlock, pctToRatio, ROOT_PERMISSION_ID, @@ -49,6 +48,7 @@ import {toUtf8Bytes} from 'ethers/lib/utils'; import {ethers} from 'hardhat'; type InitData = {contentUri: string}; +const mainVotingPluginInterface = MainVotingPlugin__factory.createInterface(); describe('Main Voting Plugin', function () { let signers: SignerWithAddress[]; @@ -95,23 +95,16 @@ describe('Main Voting Plugin', function () { ADDRESS_ZERO ); - // Alice is already an editor (see initialize) - - await dao.grant( - mainVotingPlugin.address, - bob.address, - MEMBER_PERMISSION_ID - ); - // Bob is a member + // The plugin can execute on the DAO await dao.grant( + dao.address, mainVotingPlugin.address, - bob.address, - MEMBER_PERMISSION_ID + EXECUTE_PERMISSION_ID ); - // The plugin can execute on the DAO + // MemberAccessPlugin can execute on the DAO await dao.grant( dao.address, - mainVotingPlugin.address, + memberAccessPlugin.address, EXECUTE_PERMISSION_ID ); // The DAO can add/remove editors @@ -136,6 +129,11 @@ describe('Main Voting Plugin', function () { await dao.grant(dao.address, dao.address, ROOT_PERMISSION_ID); // Alice can make the DAO execute arbitrary stuff (test) await dao.grant(dao.address, alice.address, EXECUTE_PERMISSION_ID); + + // Alice is already an editor (see initialize) + + // Bob is a member + await memberAccessPlugin.proposeNewMember('0x', bob.address); }); describe('initialize', async () => { @@ -351,20 +349,10 @@ describe('Main Voting Plugin', function () { expect(await memberAccessPlugin.isMember(carol.address)).to.eq(false); - await dao.grant( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); - + await memberAccessPlugin.proposeNewMember('0x', carol.address); expect(await memberAccessPlugin.isMember(carol.address)).to.eq(true); - await dao.revoke( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); - + await memberAccessPlugin.proposeRemoveMember('0x', carol.address); expect(await memberAccessPlugin.isMember(carol.address)).to.eq(false); await makeEditor(carol.address); @@ -426,14 +414,10 @@ describe('Main Voting Plugin', function () { context('Multiple editors', () => { it('Proposals created by a member require editor votes', async () => { let pid = 0; + // Carol member + await memberAccessPlugin.proposeNewMember('0x', carol.address); // Bob editor await proposeNewEditor(bob.address); - // Carol member - await dao.grant( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); await expect(createDummyProposal(carol, false)).to.not.be.reverted; pid++; @@ -1030,7 +1014,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'updateVotingSettings', [ { @@ -1049,7 +1033,7 @@ describe('Main Voting Plugin', function () { ) ) .to.emit(mainVotingPlugin, 'VotingSettingsUpdated') - .withArgs(0, 12345, 23456, 60 * 60 * 3, 0); + .withArgs(0, 12345, 23456, 60 * 60 * 3); }); it('The DAO can add editors', async () => { @@ -1068,7 +1052,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'addEditor', [dave.address] ), @@ -1096,7 +1080,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'removeEditor', [bob.address] ), @@ -1130,7 +1114,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'upgradeTo', [await mainVotingPlugin.implementation()] ), @@ -1138,7 +1122,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'supportsInterface', ['0x12345678'] ), @@ -1176,7 +1160,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'addEditor', [_editor] ), @@ -1200,7 +1184,7 @@ describe('Main Voting Plugin', function () { { to: mainVotingPlugin.address, value: 0, - data: MainVotingPlugin__factory.createInterface().encodeFunctionData( + data: mainVotingPluginInterface.encodeFunctionData( 'removeEditor', [_editor] ), diff --git a/packages/contracts/test/unit-testing/member-access-execute-condition.ts b/packages/contracts/test/unit-testing/member-access-execute-condition.ts index 9758489..558bdf2 100644 --- a/packages/contracts/test/unit-testing/member-access-execute-condition.ts +++ b/packages/contracts/test/unit-testing/member-access-execute-condition.ts @@ -11,12 +11,8 @@ import {deployTestDao} from '../helpers/test-dao'; import { ADDRESS_ONE, ADDRESS_TWO, - ADDRESS_ZERO, - DEPLOYER_PERMISSION_ID, - EDITOR_PERMISSION_ID, - EXECUTE_PERMISSION_ID, MEMBER_PERMISSION_ID, - ROOT_PERMISSION_ID, + EXECUTE_PERMISSION_ID, } from './common'; import {hexlify} from '@ethersproject/bytes'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; @@ -159,33 +155,33 @@ describe('Member Access Condition', function () { ) ).to.eq(true); - // // Invalid - // actions[0].data = mainVotingPluginInterface.encodeFunctionData( - // 'removeEditor', - // [carol.address] - // ); - // expect( - // await memberAccessExecuteCondition.isGranted( - // ADDRESS_ONE, // where (used) - // ADDRESS_TWO, // who (used) - // EXECUTE_PERMISSION_ID, // permission (used) - // daoInterface.encodeFunctionData('execute', [ONE_BYTES32, actions, 0]) - // ) - // ).to.eq(true); + // Invalid + actions[0].data = mainVotingPluginInterface.encodeFunctionData( + 'addEditor', + [carol.address] + ); + expect( + await memberAccessExecuteCondition.isGranted( + ADDRESS_ONE, // where (used) + ADDRESS_TWO, // who (used) + EXECUTE_PERMISSION_ID, // permission (used) + daoInterface.encodeFunctionData('execute', [ONE_BYTES32, actions, 0]) + ) + ).to.eq(true); - // // Invalid - // actions[0].data = mainVotingPluginInterface.encodeFunctionData( - // 'addEditor', - // [carol.address] - // ); - // expect( - // await memberAccessExecuteCondition.isGranted( - // ADDRESS_ONE, // where (used) - // ADDRESS_TWO, // who (used) - // EXECUTE_PERMISSION_ID, // permission (used) - // daoInterface.encodeFunctionData('execute', [ONE_BYTES32, actions, 0]) - // ) - // ).to.eq(true); + // Invalid + actions[0].data = mainVotingPluginInterface.encodeFunctionData( + 'removeEditor', + [carol.address] + ); + expect( + await memberAccessExecuteCondition.isGranted( + ADDRESS_ONE, // where (used) + ADDRESS_TWO, // who (used) + EXECUTE_PERMISSION_ID, // permission (used) + daoInterface.encodeFunctionData('execute', [ONE_BYTES32, actions, 0]) + ) + ).to.eq(true); // Invalid actions[0].data = daoInterface.encodeFunctionData('grant', [ diff --git a/packages/contracts/test/unit-testing/member-access-plugin.ts b/packages/contracts/test/unit-testing/member-access-plugin.ts index 83b40f4..6347977 100644 --- a/packages/contracts/test/unit-testing/member-access-plugin.ts +++ b/packages/contracts/test/unit-testing/member-access-plugin.ts @@ -107,14 +107,6 @@ describe('Member Access Plugin', function () { ADDRESS_ZERO ); - // Alice is an editor (see mainVotingPlugin initialize) - - // Bob is a member - await dao.grant( - mainVotingPlugin.address, - bob.address, - MEMBER_PERMISSION_ID - ); // The plugin can execute on the DAO await dao.grantWithCondition( dao.address, @@ -150,6 +142,11 @@ describe('Member Access Plugin', function () { await dao.grant(dao.address, dao.address, ROOT_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 (see mainVotingPlugin initialize) + + // Bob is a member + memberAccessPlugin.proposeNewMember('0x', bob.address); }); describe('initialize', () => { @@ -265,19 +262,20 @@ describe('Member Access Plugin', function () { hexlify(toUtf8Bytes('ipfs://2345')) ); expect(event!.args.actions.length).to.equal(1); - expect(event!.args.actions[0].to).to.equal(dao.address); + expect(event!.args.actions[0].to).to.equal(mainVotingPlugin.address); expect(event!.args.actions[0].value).to.equal(0); expect(event!.args.actions[0].data).to.equal( - DAO__factory.createInterface().encodeFunctionData('grant', [ - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - ]) + MainVotingPlugin__factory.createInterface().encodeFunctionData( + 'addMember', + [carol.address] + ) ); expect(event!.args.allowFailureMap).to.equal(0); }); it('isMember() returns true when appropriate', async () => { + expect(await mainVotingPlugin.addresslistLength()).to.eq(1); + 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); @@ -287,20 +285,10 @@ describe('Member Access Plugin', function () { expect(await memberAccessPlugin.isMember(carol.address)).to.eq(false); - await dao.grant( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); - + await memberAccessPlugin.proposeNewMember('0x', carol.address); expect(await memberAccessPlugin.isMember(carol.address)).to.eq(true); - await dao.revoke( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID - ); - + await memberAccessPlugin.proposeRemoveMember('0x', carol.address); expect(await memberAccessPlugin.isMember(carol.address)).to.eq(false); await proposeNewEditor(carol.address); @@ -309,6 +297,8 @@ describe('Member Access Plugin', function () { }); it('isEditor() returns true when appropriate', async () => { + expect(await mainVotingPlugin.addresslistLength()).to.eq(1); + 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); @@ -352,14 +342,7 @@ describe('Member Access Plugin', function () { it('Only the editor can reject memberships', async () => { expect(await mainVotingPlugin.addresslistLength()).to.eq(1); - expect( - await dao.hasPermission( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - toUtf8Bytes('') - ) - ).to.eq(false); + expect(await mainVotingPlugin.isMember(carol.address)).to.eq(false); await expect( memberAccessPlugin @@ -367,41 +350,20 @@ describe('Member Access Plugin', function () { .proposeNewMember(toUtf8Bytes('ipfs://1234'), carol.address) ).to.not.be.reverted; - expect( - await dao.hasPermission( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - toUtf8Bytes('') - ) - ).to.eq(false); + expect(await mainVotingPlugin.isMember(carol.address)).to.eq(false); // Reject it (Bob) => fail await expect(memberAccessPlugin.connect(bob).reject(0)).to.be.reverted; // Still not a member - expect( - await dao.hasPermission( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - toUtf8Bytes('') - ) - ).to.eq(false); + expect(await mainVotingPlugin.isMember(carol.address)).to.eq(false); // Reject it (Alice) => success await expect(memberAccessPlugin.connect(alice).reject(0)).to.not.be .reverted; // Carol is not a member - expect( - await dao.hasPermission( - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - toUtf8Bytes('') - ) - ).to.eq(false); + expect(await mainVotingPlugin.isMember(carol.address)).to.eq(false); // Try to approve it (Alice) => fail await expect(memberAccessPlugin.connect(alice).approve(0)).to.be.reverted; @@ -521,7 +483,7 @@ describe('Member Access Plugin', function () { .vote(pidMainVoting, VoteOption.Yes, true); }); - it('Only editors can approve new memberships', async () => { + it('Only editors can approve adding members', async () => { expect(await mainVotingPlugin.addresslistLength()).to.eq(3); // Requesting membership for Dave @@ -597,7 +559,7 @@ describe('Member Access Plugin', function () { expect(await memberAccessPlugin.isMember(ADDRESS_TWO)).to.eq(true); }); - it('Only editors can approve removing memberships', async () => { + it('Only editors can approve removing members', async () => { expect(await mainVotingPlugin.addresslistLength()).to.eq(3); await dao.grant( mainVotingPlugin.address, @@ -999,42 +961,42 @@ describe('Member Access Plugin', function () { // Bob: member it('proposeNewMember should generate the right action list', async () => { + const proposalCount = await memberAccessPlugin.proposalCount(); await expect( memberAccessPlugin .connect(carol) .proposeNewMember(toUtf8Bytes('ipfs://1234'), carol.address) ).to.not.be.reverted; - const proposal = await memberAccessPlugin.getProposal(0); + const proposal = await memberAccessPlugin.getProposal(proposalCount); expect(proposal.actions.length).to.eq(1); - expect(proposal.actions[0].to).to.eq(dao.address); + expect(proposal.actions[0].to).to.eq(mainVotingPlugin.address); expect(proposal.actions[0].value).to.eq(0); expect(proposal.actions[0].data).to.eq( - DAO__factory.createInterface().encodeFunctionData('grant', [ - mainVotingPlugin.address, - carol.address, - MEMBER_PERMISSION_ID, - ]) + MainVotingPlugin__factory.createInterface().encodeFunctionData( + 'addMember', + [carol.address] + ) ); }); it('proposeRemoveMember should generate the right action list', async () => { + const proposalCount = await memberAccessPlugin.proposalCount(); await expect( memberAccessPlugin .connect(bob) .proposeRemoveMember(toUtf8Bytes('ipfs://1234'), bob.address) ).to.not.be.reverted; - const proposal = await memberAccessPlugin.getProposal(0); + const proposal = await memberAccessPlugin.getProposal(proposalCount); expect(proposal.actions.length).to.eq(1); - expect(proposal.actions[0].to).to.eq(dao.address); + expect(proposal.actions[0].to).to.eq(mainVotingPlugin.address); expect(proposal.actions[0].value).to.eq(0); expect(proposal.actions[0].data).to.eq( - DAO__factory.createInterface().encodeFunctionData('revoke', [ - mainVotingPlugin.address, - bob.address, - MEMBER_PERMISSION_ID, - ]) + MainVotingPlugin__factory.createInterface().encodeFunctionData( + 'removeMember', + [bob.address] + ) ); });