From c456d0779dda9abf2761fd94518456e3b5544f57 Mon Sep 17 00:00:00 2001 From: Michael Heuer <20623991+Michael-A-Heuer@users.noreply.github.com> Date: Thu, 7 Mar 2024 17:36:05 +0100 Subject: [PATCH] refactor: tests (#2) * refactor: plugin tests * refactor: separated upgradeability tests * refactor: remaining tests * refactor: remove storage.ts * refactor: improved tests and comments * doc: more comment improvements * fix: wrong variable name in test * fix: linter error * refactor: use typechain struct * fix: wrong versioning numbers * test: added missing test * doc: remove todo --- packages/contracts/hardhat.config.ts | 2 +- packages/contracts/src/mocks/Migration.sol | 4 +- .../test/10_unit-testing/11_plugin.ts | 2544 +++++++++++------ .../test/10_unit-testing/12_plugin-setup.ts | 609 ++-- .../22_setup-processing.ts | 213 +- .../20_integration-testing/test-helpers.ts | 120 +- .../31_upgradeability.ts | 135 + packages/contracts/test/multisig-constants.ts | 5 - .../test/test-utils/protocol-version.ts | 11 - packages/contracts/test/test-utils/storage.ts | 18 - .../test/test-utils/typechain-versions.ts | 4 +- .../test/test-utils/uups-upgradeable.ts | 44 +- 12 files changed, 2366 insertions(+), 1343 deletions(-) create mode 100644 packages/contracts/test/30_regression-testing/31_upgradeability.ts delete mode 100644 packages/contracts/test/test-utils/protocol-version.ts delete mode 100644 packages/contracts/test/test-utils/storage.ts diff --git a/packages/contracts/hardhat.config.ts b/packages/contracts/hardhat.config.ts index 28e7f77a..749b7d34 100644 --- a/packages/contracts/hardhat.config.ts +++ b/packages/contracts/hardhat.config.ts @@ -105,7 +105,7 @@ const config: HardhatUserConfig = { throwOnTransactionFailures: true, throwOnCallFailures: true, blockGasLimit: BigNumber.from(10).pow(6).mul(300).toNumber(), // 300 million, really high to test some things that are only possible with a higher block gas limit - gasPrice: BigNumber.from(10).pow(9).mul(150).toNumber(), // 150 gwei + gasPrice: BigNumber.from(10).pow(9).mul(300).toNumber(), // 300 gwei accounts: getHardhatNetworkAccountsConfig( Object.keys(namedAccounts).length ), diff --git a/packages/contracts/src/mocks/Migration.sol b/packages/contracts/src/mocks/Migration.sol index 937651a2..faac7b6f 100644 --- a/packages/contracts/src/mocks/Migration.sol +++ b/packages/contracts/src/mocks/Migration.sol @@ -18,8 +18,8 @@ pragma solidity ^0.8.8; import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; // Regression Testing -import {Multisig as Multisig_v1_0_0} from "@aragon/osx-v1.0.0/plugins/governance/multisig/Multisig.sol"; -import {Multisig as Multisig_v1_3_0} from "@aragon/osx-v1.3.0/plugins/governance/multisig/Multisig.sol"; +import {Multisig as Multisig_v1_1} from "@aragon/osx-v1.0.0/plugins/governance/multisig/Multisig.sol"; +import {Multisig as Multisig_v1_2} from "@aragon/osx-v1.3.0/plugins/governance/multisig/Multisig.sol"; import {ProxyFactory} from "@aragon/osx-commons-contracts/src/utils/deployment/ProxyFactory.sol"; diff --git a/packages/contracts/test/10_unit-testing/11_plugin.ts b/packages/contracts/test/10_unit-testing/11_plugin.ts index 8bbdd861..037dbb5a 100644 --- a/packages/contracts/test/10_unit-testing/11_plugin.ts +++ b/packages/contracts/test/10_unit-testing/11_plugin.ts @@ -19,643 +19,905 @@ import { import { MULTISIG_EVENTS, MULTISIG_INTERFACE, - MultisigSettings, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, } from '../multisig-constants'; -import { - Multisig_V1_0_0__factory, - Multisig_V1_3_0__factory, - Multisig__factory, - Multisig, -} from '../test-utils/typechain-versions'; -import { - deployAndUpgradeFromToCheck, - deployAndUpgradeSelfCheck, - getProtocolVersion, -} from '../test-utils/uups-upgradeable'; +import {Multisig__factory, Multisig} from '../test-utils/typechain-versions'; import { getInterfaceId, proposalIdToBytes32, IDAO_EVENTS, - PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS, IMEMBERSHIP_EVENTS, IPROPOSAL_EVENTS, findEvent, findEventTopicLog, TIME, + DAO_PERMISSIONS, } from '@aragon/osx-commons-sdk'; -import {DAO, DAO__factory} from '@aragon/osx-ethers'; -import {time} from '@nomicfoundation/hardhat-network-helpers'; +import {DAO, DAOStructs, DAO__factory} from '@aragon/osx-ethers'; +import {loadFixture, time} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; -import {BigNumber, Contract, ContractFactory} from 'ethers'; +import {BigNumber} from 'ethers'; import {ethers} from 'hardhat'; -export async function approveWithSigners( - multisigContract: Contract, - proposalId: number, - signers: SignerWithAddress[], - signerIds: number[] -) { - const promises = signerIds.map(i => - multisigContract.connect(signers[i]).approve(proposalId, false) +type FixtureResult = { + deployer: SignerWithAddress; + alice: SignerWithAddress; + bob: SignerWithAddress; + carol: SignerWithAddress; + dave: SignerWithAddress; + eve: SignerWithAddress; + initializedPlugin: Multisig; + uninitializedPlugin: Multisig; + defaultInitData: { + members: string[]; + settings: Multisig.MultisigSettingsStruct; + }; + dao: DAO; + dummyActions: DAOStructs.ActionStruct[]; + dummyMetadata: string; +}; + +async function fixture(): Promise { + const [deployer, alice, bob, carol, dave, eve] = await ethers.getSigners(); + + // Deploy a DAO proxy. + const dummyMetadata = ethers.utils.hexlify( + ethers.utils.toUtf8Bytes('0x123456789') ); + const dao = await createDaoProxy(deployer, dummyMetadata); - await Promise.all(promises); -} - -describe('Multisig', function () { - let signers: SignerWithAddress[]; - let multisig: Multisig; - let dao: DAO; - let dummyActions: any; - let dummyMetadata: string; - let startDate: number; - let endDate: number; - let multisigSettings: MultisigSettings; - - const id = 0; - - before(async () => { - signers = await ethers.getSigners(); - dummyActions = [ - { - to: signers[0].address, - data: '0x00000000', - value: 0, - }, - ]; - dummyMetadata = ethers.utils.hexlify( - ethers.utils.toUtf8Bytes('0x123456789') - ); + // Deploy a plugin proxy factory containing the multisig implementation. + const pluginImplementation = await new Multisig__factory(deployer).deploy(); + const proxyFactory = await new ProxyFactory__factory(deployer).deploy( + pluginImplementation.address + ); - dao = await createDaoProxy(signers[0], dummyMetadata); - }); + // Deploy an initialized plugin proxy. + const defaultInitData = { + members: [alice.address, bob.address, carol.address], + settings: { + onlyListed: true, + minApprovals: 2, + }, + }; + const pluginInitdata = pluginImplementation.interface.encodeFunctionData( + 'initialize', + [dao.address, defaultInitData.members, defaultInitData.settings] + ); + const deploymentTx1 = await proxyFactory.deployUUPSProxy(pluginInitdata); + const proxyCreatedEvent1 = await findEvent( + deploymentTx1, + proxyFactory.interface.getEvent('ProxyCreated').name + ); + const initializedPlugin = Multisig__factory.connect( + proxyCreatedEvent1.args.proxy, + deployer + ); - beforeEach(async function () { - startDate = (await time.latest()) + 1000; - endDate = startDate + 1000; + // Deploy an initialized plugin proxy. + const deploymentTx2 = await proxyFactory.deployUUPSProxy([]); + const proxyCreatedEvent2 = await findEvent( + deploymentTx2, + proxyFactory.interface.getEvent('ProxyCreated').name + ); + const uninitializedPlugin = Multisig__factory.connect( + proxyCreatedEvent2.args.proxy, + deployer + ); - multisigSettings = { - minApprovals: 3, - onlyListed: true, - }; - - const multisigImplementation = await new Multisig__factory( - signers[0] - ).deploy(); - const multisigProxyFactory = await new ProxyFactory__factory( - signers[0] - ).deploy(multisigImplementation.address); - - const tx = await multisigProxyFactory.deployUUPSProxy([]); - const event = await findEvent( - tx, - multisigProxyFactory.interface.getEvent('ProxyCreated').name - ); - multisig = Multisig__factory.connect(event.args.proxy, signers[0]); - - await dao.grant( - dao.address, - multisig.address, - ethers.utils.id('EXECUTE_PERMISSION') - ); - await dao.grant( - multisig.address, - signers[0].address, - ethers.utils.id('UPDATE_MULTISIG_SETTINGS_PERMISSION') - ); - }); + // Provide a dummy action array. + const dummyActions: DAOStructs.ActionStruct[] = [ + { + to: deployer.address, + data: '0x1234', + value: 0, + }, + ]; + + return { + deployer, + alice, + bob, + carol, + dave, + eve, + initializedPlugin, + uninitializedPlugin, + defaultInitData, + dao, + dummyActions, + dummyMetadata, + }; +} +describe('Multisig', function () { describe('initialize', async () => { it('reverts if trying to re-initialize', async () => { - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings + const {dao, initializedPlugin, defaultInitData} = await loadFixture( + fixture ); + // Try to reinitialize the initialized plugin. await expect( - multisig.initialize( + initializedPlugin.initialize( dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings + defaultInitData.members, + defaultInitData.settings ) ).to.be.revertedWith('Initializable: contract is already initialized'); }); it('adds the initial addresses to the address list', async () => { - expect(await multisig.addresslistLength()).to.equal(0); + const { + dao, + uninitializedPlugin: plugin, + defaultInitData, + } = await loadFixture(fixture); + + // Check that the uninitialized plugin has no members. + expect(await plugin.addresslistLength()).to.equal(0); - multisigSettings.minApprovals = 2; - await multisig.initialize( + // Initialize the plugin. + await plugin.initialize( dao.address, - signers.slice(0, 2).map(s => s.address), - multisigSettings + defaultInitData.members, + defaultInitData.settings ); - expect(await multisig.addresslistLength()).to.equal(2); - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(true); - }); - - it('should set the `minApprovals`', async () => { - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings + // Check that all members from the init data have been listed as members. + expect(await plugin.addresslistLength()).to.equal( + defaultInitData.members.length ); - expect((await multisig.multisigSettings()).minApprovals).to.be.eq( - multisigSettings.minApprovals + const promises = defaultInitData.members.map(member => + plugin.isListed(member) ); + (await Promise.all(promises)).forEach(isListedResult => { + expect(isListedResult).to.be.true; + }); }); - it('should set `onlyListed`', async () => { - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings - ); - expect((await multisig.multisigSettings()).onlyListed).to.be.eq( - multisigSettings.onlyListed + it('sets the `minApprovals`', async () => { + const {initializedPlugin, defaultInitData} = await loadFixture(fixture); + expect( + (await initializedPlugin.multisigSettings()).minApprovals + ).to.be.eq(defaultInitData.settings.minApprovals); + }); + + it('sets `onlyListed`', async () => { + const {initializedPlugin, defaultInitData} = await loadFixture(fixture); + expect((await initializedPlugin.multisigSettings()).onlyListed).to.be.eq( + defaultInitData.settings.onlyListed ); }); - it('should emit `MultisigSettingsUpdated` during initialization', async () => { + it('emits `MultisigSettingsUpdated` during initialization', async () => { + const {uninitializedPlugin, defaultInitData, dao} = await loadFixture( + fixture + ); await expect( - multisig.initialize( + uninitializedPlugin.initialize( dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings + defaultInitData.members, + defaultInitData.settings ) ) - .to.emit(multisig, MULTISIG_EVENTS.MultisigSettingsUpdated) - .withArgs(multisigSettings.onlyListed, multisigSettings.minApprovals); - }); - - it('should revert if members list is longer than uint16 max', async () => { - const members = new Array(65536).fill(signers[1].address); - await expect( - multisig.initialize(dao.address, members, multisigSettings, { - gasLimit: BigNumber.from(10).pow(8).toNumber(), - }) - ) - .to.revertedWithCustomError(multisig, 'AddresslistLengthOutOfBounds') - .withArgs(65535, members.length); - }); - }); - - describe('Upgrades', () => { - let legacyContractFactory: ContractFactory; - let currentContractFactory: ContractFactory; - let initArgs: any; - - before(() => { - currentContractFactory = new Multisig__factory(signers[0]); - }); - - beforeEach(() => { - initArgs = { - dao: dao.address, - members: [signers[0].address, signers[1].address, signers[2].address], - multisigSettings: multisigSettings, - }; - }); - - it('upgrades to a new implementation', async () => { - await deployAndUpgradeSelfCheck( - signers[0], - signers[1], - initArgs, - 'initialize', - currentContractFactory, - PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - dao - ); - }); - - it('upgrades from v1.0.0', async () => { - legacyContractFactory = new Multisig_V1_0_0__factory(signers[0]); - - const {fromImplementation, toImplementation} = - await deployAndUpgradeFromToCheck( - signers[0], - signers[1], - initArgs, - 'initialize', - legacyContractFactory, - currentContractFactory, - PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - dao + .to.emit(uninitializedPlugin, MULTISIG_EVENTS.MultisigSettingsUpdated) + .withArgs( + defaultInitData.settings.onlyListed, + defaultInitData.settings.minApprovals ); - expect(toImplementation).to.not.equal(fromImplementation); // The build did change - - const fromProtocolVersion = await getProtocolVersion( - legacyContractFactory.attach(fromImplementation) - ); - const toProtocolVersion = await getProtocolVersion( - currentContractFactory.attach(toImplementation) - ); - - expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); - expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); - expect(toProtocolVersion).to.deep.equal([1, 4, 0]); // TODO Check this automatically }); - it('from v1.3.0', async () => { - legacyContractFactory = new Multisig_V1_3_0__factory(signers[0]); - - const {fromImplementation, toImplementation} = - await deployAndUpgradeFromToCheck( - signers[0], - signers[1], - initArgs, - 'initialize', - legacyContractFactory, - currentContractFactory, - PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, - dao - ); - expect(toImplementation).to.not.equal(fromImplementation); + it('reverts if the member list is longer than uint16 max', async () => { + const {uninitializedPlugin, alice, defaultInitData, dao} = + await loadFixture(fixture); - const fromProtocolVersion = await getProtocolVersion( - legacyContractFactory.attach(fromImplementation) - ); - const toProtocolVersion = await getProtocolVersion( - currentContractFactory.attach(toImplementation) + // Create a member list causing an overflow during initialization. + const uint16MaxValue = 2 ** 16 - 1; // = 65535 + const overflowingMemberList = new Array(uint16MaxValue + 1).fill( + alice.address ); - expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); - expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); - expect(toProtocolVersion).to.deep.equal([1, 4, 0]); // TODO Check this automatically + // Try to initialize the plugin with a list of new members causing an overflow. + await expect( + uninitializedPlugin.initialize( + dao.address, + overflowingMemberList, + defaultInitData.settings, + { + gasLimit: BigNumber.from(10).pow(8).toNumber(), + } + ) + ) + .to.revertedWithCustomError( + uninitializedPlugin, + 'AddresslistLengthOutOfBounds' + ) + .withArgs(uint16MaxValue, overflowingMemberList.length); }); }); describe('ERC-165', async () => { it('does not support the empty interface', async () => { - expect(await multisig.supportsInterface('0xffffffff')).to.be.false; + const {initializedPlugin: plugin} = await loadFixture(fixture); + expect(await plugin.supportsInterface('0xffffffff')).to.be.false; }); it('supports the `IERC165Upgradeable` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IERC165Upgradeable__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IPlugin` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IPlugin__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IProtocolVersion` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IProtocolVersion__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IProposal` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IProposal__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IMembership` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IMembership__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `Addresslist` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = Addresslist__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IMultisig` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const iface = IMultisig__factory.createInterface(); - expect(await multisig.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `Multisig` interface', async () => { + const {initializedPlugin: plugin} = await loadFixture(fixture); const interfaceId = getInterfaceId(MULTISIG_INTERFACE); - expect(await multisig.supportsInterface(interfaceId)).to.be.true; + expect(await plugin.supportsInterface(interfaceId)).to.be.true; }); }); describe('updateMultisigSettings', async () => { - beforeEach(async () => { - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings - ); + it('reverts if the caller misses the `UPDATE_MULTISIG_SETTINGS_PERMISSION` permission', async () => { + const { + alice, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Check that Alice hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` permission on the Multisig plugin. + expect( + await dao.hasPermission( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, + [] + ) + ).to.be.false; + + // Expect Alice's `updateMultisigSettings` call to be reverted because she hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` + // permission on the Multisig plugin. + const newSettings: Multisig.MultisigSettingsStruct = { + onlyListed: false, + minApprovals: 1, + }; + await expect(plugin.connect(alice).updateMultisigSettings(newSettings)) + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); }); - it('should not allow to set minApprovals larger than the address list length', async () => { - const addresslistLength = (await multisig.addresslistLength()).toNumber(); + it('reverts when setting `minApprovals` to a value greater than the address list length', async () => { + const { + alice, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); - multisigSettings.minApprovals = addresslistLength + 1; + // Create settings where `minApprovals` is greater than the address list length + const addresslistLength = (await plugin.addresslistLength()).toNumber(); + const badSettings: Multisig.MultisigSettingsStruct = { + onlyListed: true, + minApprovals: addresslistLength + 1, + }; - await expect(multisig.updateMultisigSettings(multisigSettings)) - .to.be.revertedWithCustomError(multisig, 'MinApprovalsOutOfBounds') - .withArgs(addresslistLength, multisigSettings.minApprovals); + // Try to update the multisig settings + await expect(plugin.connect(alice).updateMultisigSettings(badSettings)) + .to.be.revertedWithCustomError(plugin, 'MinApprovalsOutOfBounds') + .withArgs(addresslistLength, badSettings.minApprovals); }); - it('should not allow to set `minApprovals` to 0', async () => { - multisigSettings.minApprovals = 0; - await expect(multisig.updateMultisigSettings(multisigSettings)) - .to.be.revertedWithCustomError(multisig, 'MinApprovalsOutOfBounds') + it('reverts when setting `minApprovals` to 0', async () => { + const { + alice, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + + // Try as Alice to update the settings with `minApprovals` being 0. + const badSettings: Multisig.MultisigSettingsStruct = { + onlyListed: true, + minApprovals: 0, + }; + await expect(plugin.connect(alice).updateMultisigSettings(badSettings)) + .to.be.revertedWithCustomError(plugin, 'MinApprovalsOutOfBounds') .withArgs(1, 0); }); - it('should emit `MultisigSettingsUpdated` when `updateMutlsigSettings` gets called', async () => { - await expect(multisig.updateMultisigSettings(multisigSettings)) - .to.emit(multisig, MULTISIG_EVENTS.MultisigSettingsUpdated) - .withArgs(multisigSettings.onlyListed, multisigSettings.minApprovals); + it('emits `MultisigSettingsUpdated` when `updateMultisigSettings` gets called', async () => { + const { + alice, + initializedPlugin: plugin, + defaultInitData, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + + // Update the settings as Alice. + await expect( + plugin.connect(alice).updateMultisigSettings(defaultInitData.settings) + ) + .to.emit(plugin, MULTISIG_EVENTS.MultisigSettingsUpdated) + .withArgs( + defaultInitData.settings.onlyListed, + defaultInitData.settings.minApprovals + ); }); }); describe('isListed', async () => { - it('should return false, if a user is not listed', async () => { - multisigSettings.minApprovals = 1; - await multisig.initialize( - dao.address, - [signers[0].address], - multisigSettings - ); + it('returns false, if a user is not listed', async () => { + const {dave, initializedPlugin: plugin} = await loadFixture(fixture); + expect(await plugin.isListed(dave.address)).to.equal(false); + }); - expect(await multisig.isListed(signers[9].address)).to.equal(false); + it('returns true, if a user is listed', async () => { + const {alice, initializedPlugin: plugin} = await loadFixture(fixture); + expect(await plugin.isListed(alice.address)).to.equal(true); }); }); describe('isMember', async () => { - it('should return false, if user is not listed', async () => { - expect(await multisig.isMember(signers[0].address)).to.be.false; + it('returns false, if user is not a member', async () => { + const {dave, initializedPlugin: plugin} = await loadFixture(fixture); + expect(await plugin.isMember(dave.address)).to.be.false; }); - it('should return true if user is in the latest list', async () => { - multisigSettings.minApprovals = 1; - await multisig.initialize( - dao.address, - [signers[0].address], - multisigSettings - ); - expect(await multisig.isMember(signers[0].address)).to.be.true; + it('returns true if user a user is a member', async () => { + const {alice, initializedPlugin: plugin} = await loadFixture(fixture); + expect(await plugin.isMember(alice.address)).to.be.true; }); }); describe('addAddresses', async () => { - it('should add new members to the address list and emit the `MembersAdded` event', async () => { - multisigSettings.minApprovals = 1; - await multisig.initialize( - dao.address, - [signers[0].address], - multisigSettings + it('reverts if the caller misses the `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` permission', async () => { + const { + alice, + dave, + eve, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Check that the Alice hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` permission on the Multisig plugin. + expect( + await dao.hasPermission( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, + [] + ) + ).to.be.false; + + // Expect Alice's `addAddresses` call to be reverted because she hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` + // permission on the Multisig plugin. + await expect( + plugin.connect(alice).addAddresses([dave.address, eve.address]) + ) + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + }); + + it('reverts if the member list would become longer than uint16 max', async () => { + const { + initializedPlugin: plugin, + alice, + dave, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + + const currentMemberCount = ( + await plugin.callStatic.addresslistLength() + ).toNumber(); + + // Create list of new members causing an overflow. + const uint16MaxValue = 2 ** 16 - 1; // = 65535 + const overflowingNewMemberList = new Array( + uint16MaxValue - currentMemberCount + 1 + ).fill(dave.address); + + // Try to add a list of new members causing an overflow as Alice. + await expect(plugin.connect(alice).addAddresses(overflowingNewMemberList)) + .to.revertedWithCustomError(plugin, 'AddresslistLengthOutOfBounds') + .withArgs(uint16MaxValue, uint16MaxValue + 1); + }); + + it('adds new members to the address list and emit the `MembersAdded` event', async () => { + const { + alice, + dave, + eve, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(false); + // Check that Dave and Eve are not listed yet. + expect(await plugin.isListed(dave.address)).to.equal(false); + expect(await plugin.isListed(eve.address)).to.equal(false); - // add a new member - await expect(multisig.addAddresses([signers[1].address])) - .to.emit(multisig, IMEMBERSHIP_EVENTS.MembersAdded) - .withArgs([signers[1].address]); + // Call `addAddresses` as Alice to add Dave and Eve. + await expect( + plugin.connect(alice).addAddresses([dave.address, eve.address]) + ) + .to.emit(plugin, IMEMBERSHIP_EVENTS.MembersAdded) + .withArgs([dave.address, eve.address]); - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(true); + // Check that Dave and Eve are listed now. + expect(await plugin.isListed(dave.address)).to.equal(true); + expect(await plugin.isListed(eve.address)).to.equal(true); }); }); describe('removeAddresses', async () => { - it('should remove users from the address list and emit the `MembersRemoved` event', async () => { - multisigSettings.minApprovals = 1; - await multisig.initialize( - dao.address, - signers.slice(0, 2).map(s => s.address), - multisigSettings + it('reverts if the caller misses the `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` permission', async () => { + const { + alice, + bob, + carol, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Check that Alice hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` permission on the Multisig plugin. + expect( + await dao.hasPermission( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, + [] + ) + ).to.be.false; + + // Expect Alice's `removeAddresses` call to be reverted because she hasn't `UPDATE_MULTISIG_SETTINGS_PERMISSION_ID` + // permission on the Multisig plugin. + await expect( + plugin.connect(alice).removeAddresses([bob.address, carol.address]) + ) + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + }); + + it('removes users from the address list and emit the `MembersRemoved` event', async () => { + const { + alice, + bob, + carol, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(true); + // Check that Alice, Bob, and Carol are listed. + expect(await plugin.isListed(alice.address)).to.equal(true); + expect(await plugin.isListed(bob.address)).to.equal(true); + expect(await plugin.isListed(carol.address)).to.equal(true); - // remove an existing member - await expect(multisig.removeAddresses([signers[1].address])) - .to.emit(multisig, IMEMBERSHIP_EVENTS.MembersRemoved) - .withArgs([signers[1].address]); + // Call `removeAddresses` as Alice to remove Bob. + await expect(plugin.connect(alice).removeAddresses([bob.address])) + .to.emit(plugin, IMEMBERSHIP_EVENTS.MembersRemoved) + .withArgs([bob.address]); - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(false); + // Check that Bob is removed while Alice and Carol remains listed. + expect(await plugin.isListed(alice.address)).to.equal(true); + expect(await plugin.isListed(bob.address)).to.equal(false); + expect(await plugin.isListed(carol.address)).to.equal(true); }); it('reverts if the address list would become empty', async () => { - multisigSettings.minApprovals = 1; - await multisig.initialize( - dao.address, - [signers[0].address], - multisigSettings + const { + alice, + initializedPlugin: plugin, + defaultInitData, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); - await expect(multisig.removeAddresses([signers[0].address])) - .to.be.revertedWithCustomError(multisig, 'MinApprovalsOutOfBounds') - .withArgs( - (await multisig.addresslistLength()).sub(1), - multisigSettings.minApprovals - ); + // Try to remove all members. + await expect( + plugin.connect(alice).removeAddresses(defaultInitData.members) + ) + .to.be.revertedWithCustomError(plugin, 'MinApprovalsOutOfBounds') + .withArgs(0, defaultInitData.settings.minApprovals); }); it('reverts if the address list would become shorter than the current minimum approval parameter requires', async () => { - multisigSettings.minApprovals = 2; - await multisig.initialize( - dao.address, - signers.slice(0, 3).map(s => s.address), - multisigSettings + const { + alice, + carol, + initializedPlugin: plugin, + defaultInitData, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); - await expect(multisig.removeAddresses([signers[1].address])).not.to.be - .reverted; + // Initially, there are 3 members and `minApprovals` is 2. + // Remove one member, which is ok. + await expect(plugin.connect(alice).removeAddresses([carol.address])).not + .to.be.reverted; - await expect(multisig.removeAddresses([signers[2].address])) - .to.be.revertedWithCustomError(multisig, 'MinApprovalsOutOfBounds') + // Try to remove another member, which will revert. + await expect(plugin.connect(alice).removeAddresses([alice.address])) + .to.be.revertedWithCustomError(plugin, 'MinApprovalsOutOfBounds') .withArgs( - (await multisig.addresslistLength()).sub(1), - multisigSettings.minApprovals + (await plugin.addresslistLength()).sub(1), + defaultInitData.settings.minApprovals ); }); }); describe('createProposal', async () => { - beforeEach(async () => { - multisigSettings.minApprovals = 1; - }); + it('increments the proposal count', async () => { + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); - it('increments the proposal counter', async () => { - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings - ); + // Check that the proposal count is 0. + expect(await plugin.proposalCount()).to.equal(0); - expect(await multisig.proposalCount()).to.equal(0); + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; - await expect( - multisig.createProposal( + await plugin + .connect(alice) + .createProposal( dummyMetadata, dummyActions, 0, false, false, 0, - startDate - ) - ).not.to.be.reverted; + endDate + ); + // Check that the proposal count is 1. + expect(await plugin.proposalCount()).to.equal(1); + + // Create another proposal as Alice. + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); - expect(await multisig.proposalCount()).to.equal(1); + // Check that the proposal count is 2. + expect(await plugin.proposalCount()).to.equal(2); }); it('creates unique proposal IDs for each proposal', async () => { - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings - ); - await ethers.provider.send('evm_mine', []); - - const proposalId0 = await multisig.callStatic.createProposal( + const { + alice, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - 0, - endDate - ); - // create a new proposal for the proposalCounter to be incremented - await expect( - multisig.createProposal( + } = await loadFixture(fixture); + + // Make a static call to `createProposal` to get the first proposal ID ahead of time. + const endDate = (await time.latest()) + TIME.HOUR; + const proposalId0 = await plugin + .connect(alice) + .callStatic.createProposal( dummyMetadata, dummyActions, 0, false, false, 0, - startDate - ) - ).not.to.be.reverted; + endDate + ); - const proposalId1 = await multisig.callStatic.createProposal( - dummyMetadata, - dummyActions, - 0, - false, - false, - 0, - endDate - ); + // Create the new proposal as Alice. + await expect( + plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ) + ).not.to.be.reverted; - expect(proposalId0).to.equal(0); // To be removed when proposal ID is generated as a hash. - expect(proposalId1).to.equal(1); // To be removed when proposal ID is generated as a hash. + // Make a static call to `createProposal` to get the next proposal ID ahead of time. + const proposalId1 = await plugin + .connect(alice) + .callStatic.createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); - expect(proposalId0).to.not.equal(proposalId1); + // Check that the proposal IDs are as expected. + expect(proposalId0).to.equal(0); + expect(proposalId1).to.equal(1); }); it('emits the `ProposalCreated` event', async () => { - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings - ); - - const allowFailureMap = 1; + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + } = await loadFixture(fixture); + // Create a proposal as Alice and check the event arguments. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + const expectedProposalId = 0; await expect( - multisig - .connect(signers[0]) + plugin + .connect(alice) .createProposal( dummyMetadata, [], - allowFailureMap, + 0, false, false, startDate, endDate ) ) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalCreated) + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalCreated) .withArgs( - id, - signers[0].address, + expectedProposalId, + alice.address, startDate, endDate, dummyMetadata, [], - allowFailureMap + 0 ); }); it('reverts if the multisig settings have been changed in the same block', async () => { - await multisig.initialize( + const { + alice, + initializedPlugin: plugin, + dao, + } = await loadFixture(fixture); + + // Grant Alice the permission to update the settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + + const newSettings = { + onlyListed: false, + minApprovals: 1, + }; + + /* Make two calls to update the settings in the same block. */ + // Disable auto-mining so that both proposals end up in the same block. + await ethers.provider.send('evm_setAutomine', [false]); + // Update #1 + await plugin.connect(alice).updateMultisigSettings(newSettings); + // Update #2 + await plugin.connect(alice).updateMultisigSettings(newSettings); + // Re-enable auto-mining so that the remaining tests run normally. + await ethers.provider.send('evm_setAutomine', [true]); + }); + + it('reverts if the multisig settings have been changed in the same block via the proposals process', async () => { + const { + alice, + uninitializedPlugin: plugin, + dummyMetadata, + dao, + } = await loadFixture(fixture); + await plugin.initialize(dao.address, [alice.address], { + onlyListed: true, + minApprovals: 1, + }); + + // Grant permissions between the DAO and the plugin. + await dao.grant( + plugin.address, dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); await dao.grant( - multisig.address, dao.address, - await multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID ); + // Create an action calling `updateMultisigSettings`. + const updateMultisigSettingsAction = { + to: plugin.address, + value: 0, + data: plugin.interface.encodeFunctionData('updateMultisigSettings', [ + { + onlyListed: false, + minApprovals: 1, + }, + ]), + }; + + /* Create two proposals to update the settings in the same block. */ + const endDate = (await time.latest()) + TIME.HOUR; + + // Disable auto-mining so that both proposals end up in the same block. await ethers.provider.send('evm_setAutomine', [false]); - await multisig.connect(signers[0]).createProposal( + // Create and execute proposal #1 calling `updateMultisigSettings`. + await plugin.connect(alice).createProposal( dummyMetadata, - [ - { - to: multisig.address, - value: 0, - data: multisig.interface.encodeFunctionData( - 'updateMultisigSettings', - [ - { - onlyListed: false, - minApprovals: 1, - }, - ] - ), - }, - ], + [updateMultisigSettingsAction], 0, - true, - true, + true, // approve + true, // execute 0, endDate ); + + // Try to call update the settings a second time. await expect( - multisig - .connect(signers[0]) - .createProposal(dummyMetadata, [], 0, true, true, 0, endDate) + plugin.connect(alice).createProposal( + dummyMetadata, + [updateMultisigSettingsAction], + 0, + false, // approve + false, // execute + 0, + endDate + ) ) - .to.revertedWithCustomError(multisig, 'ProposalCreationForbidden') - .withArgs(signers[0].address); + .to.revertedWithCustomError(plugin, 'ProposalCreationForbidden') + .withArgs(alice.address); + // Re-enable auto-mining so that the remaining tests run normally. await ethers.provider.send('evm_setAutomine', [true]); }); - context('`onlyListed` is set to `false`', async () => { - beforeEach(async () => { - multisigSettings.onlyListed = false; + describe('`onlyListed` is set to `false`', async () => { + it('creates a proposal when an unlisted accounts is calling', async () => { + const { + alice, + dave, + initializedPlugin: plugin, + dao, + dummyMetadata, + } = await loadFixture(fixture); - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID ); - }); - it('creates a proposal when unlisted accounts are allowed', async () => { + // As Alice, set `onlyListed` to `false`. + await plugin.connect(alice).updateMultisigSettings({ + minApprovals: 2, + onlyListed: false, + }); + + // Create a proposal as Dave (who is not listed). + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + + const expectedProposalId = 0; + await expect( - multisig - .connect(signers[1]) // not listed + plugin + .connect(dave) // Dave is not listed. .createProposal( dummyMetadata, [], @@ -666,10 +928,10 @@ describe('Multisig', function () { endDate ) ) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalCreated) + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalCreated) .withArgs( - id, - signers[1].address, + expectedProposalId, + dave.address, startDate, endDate, dummyMetadata, @@ -679,62 +941,103 @@ describe('Multisig', function () { }); }); - context('`onlyListed` is set to `true`', async () => { - beforeEach(async () => { - multisigSettings.onlyListed = true; - - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings - ); - }); + describe('`onlyListed` is set to `true`', async () => { + it('reverts if the caller is not listed and only listed accounts can create proposals', async () => { + const { + dave, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); - it('reverts if the user is not on the list and only listed accounts can create proposals', async () => { + // Try to create a proposal as Dave (who is not listed), which should revert. + const endDate = (await time.latest()) + TIME.HOUR; await expect( - multisig - .connect(signers[1]) // not listed - .createProposal(dummyMetadata, [], 0, false, false, 0, startDate) + plugin + .connect(dave) // Dave is not listed. + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ) ) - .to.be.revertedWithCustomError(multisig, 'ProposalCreationForbidden') - .withArgs(signers[1].address); - - await expect( - multisig - .connect(signers[0]) - .createProposal(dummyMetadata, [], 0, false, false, 0, startDate) - ).not.to.be.reverted; + .to.be.revertedWithCustomError(plugin, 'ProposalCreationForbidden') + .withArgs(dave.address); }); - it('reverts if `_msgSender` is not listed in the current block although he was listed in the last block', async () => { + it('reverts if caller is not listed in the current block although she was listed in the last block', async () => { + const { + alice, + carol, + dave, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Grant Alice the permission to update settings. + await dao.grant( + plugin.address, + alice.address, + UPDATE_MULTISIG_SETTINGS_PERMISSION_ID + ); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Disable auto-mining so that all subsequent transactions end up in the same block. await ethers.provider.send('evm_setAutomine', [false]); const expectedSnapshotBlockNumber = ( await ethers.provider.getBlock('latest') ).number; - // Transaction 1 & 2: Add signers[1] and remove signers[0] - const tx1 = await multisig.addAddresses([signers[1].address]); - const tx2 = await multisig.removeAddresses([signers[0].address]); - - // Transaction 3: Expect the proposal creation to fail for signers[0] because he was removed as a member in transaction 2. - await expect( - multisig - .connect(signers[0]) - .createProposal(dummyMetadata, [], 0, false, false, 0, startDate) - ) - .to.be.revertedWithCustomError(multisig, 'ProposalCreationForbidden') - .withArgs(signers[0].address); + // Transaction 1 & 2: Add Dave and remove Carol. + const tx1 = await plugin.connect(alice).addAddresses([dave.address]); + const tx2 = await plugin + .connect(alice) + .removeAddresses([carol.address]); - // Transaction 4: Create the proposal as signers[1] - const tx4 = await multisig - .connect(signers[1]) - .createProposal(dummyMetadata, [], 0, false, false, 0, startDate); + // Transaction 3: Expect the proposal creation to fail for Carol because she was removed as a member in transaction 2. - // Check the listed members before the block is mined - expect(await multisig.isListed(signers[0].address)).to.equal(true); - expect(await multisig.isListed(signers[1].address)).to.equal(false); - - // Mine the block + await expect( + plugin + .connect(carol) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ) + ) + .to.be.revertedWithCustomError(plugin, 'ProposalCreationForbidden') + .withArgs(carol.address); + + // Transaction 4: Create the proposal as Dave + const tx4 = await plugin + .connect(dave) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Check the listed members before the block is mined. + expect(await plugin.isListed(carol.address)).to.equal(true); + expect(await plugin.isListed(dave.address)).to.equal(false); + + // Mine the block await ethers.provider.send('evm_mine', []); const minedBlockNumber = (await ethers.provider.getBlock('latest')) .number; @@ -745,475 +1048,1003 @@ describe('Multisig', function () { expect((await tx4.wait()).blockNumber).to.equal(minedBlockNumber); expect(minedBlockNumber).to.equal(expectedSnapshotBlockNumber + 1); - // Expect the listed member to have changed - expect(await multisig.isListed(signers[0].address)).to.equal(false); - expect(await multisig.isListed(signers[1].address)).to.equal(true); + // Expect the listed member to have changed. + expect(await plugin.isListed(carol.address)).to.equal(false); + expect(await plugin.isListed(dave.address)).to.equal(true); - // Check the `ProposalCreatedEvent` for the creator and proposalId + // Check the `ProposalCreatedEvent` for the creator and proposalId. const event = await findEvent( tx4, 'ProposalCreated' ); expect(event.args.proposalId).to.equal(id); - expect(event.args.creator).to.equal(signers[1].address); + expect(event.args.creator).to.equal(dave.address); - // Check that the snapshot block stored in the proposal struct - const proposal = await multisig.getProposal(id); + // Check that the snapshot block stored in the proposal struct. + const proposal = await plugin.getProposal(id); expect(proposal.parameters.snapshotBlock).to.equal( expectedSnapshotBlockNumber ); + // Re-enable auto-mining so that the remaining tests run normally. await ethers.provider.send('evm_setAutomine', [true]); }); + }); - it('creates a proposal successfully and does not approve if not specified', async () => { - await time.setNextBlockTimestamp(startDate); - - await expect( - multisig.createProposal( - dummyMetadata, - [], - 0, - false, - false, - startDate, - endDate - ) - ) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalCreated) - .withArgs( - id, - signers[0].address, - startDate, - endDate, - dummyMetadata, - [], - 0 - ); + it('creates a proposal successfully and does not approve if not specified', async () => { + const { + alice, + bob, + initializedPlugin: plugin, + defaultInitData, + dummyMetadata, + } = await loadFixture(fixture); - const block = await ethers.provider.getBlock('latest'); + // Create a proposal (ID 0) as Alice but don't approve on creation. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + const allowFailureMap = 0; + await time.setNextBlockTimestamp(startDate); + const id = 0; - const proposal = await multisig.getProposal(id); - expect(proposal.executed).to.equal(false); - expect(proposal.parameters.snapshotBlock).to.equal(block.number - 1); - expect(proposal.parameters.minApprovals).to.equal( - multisigSettings.minApprovals - ); - expect(proposal.allowFailureMap).to.equal(0); - expect(proposal.parameters.startDate).to.equal(startDate); - expect(proposal.parameters.endDate).to.equal(endDate); - expect(proposal.approvals).to.equal(0); - expect(proposal.actions.length).to.equal(0); - - expect(await multisig.canApprove(id, signers[0].address)).to.be.true; - expect(await multisig.canApprove(id, signers[1].address)).to.be.false; - }); + const approveProposal = false; - it('creates a proposal successfully and approves if specified', async () => { - const allowFailureMap = 1; + await expect( + plugin.connect(alice).createProposal( + dummyMetadata, + [], + allowFailureMap, + approveProposal, // false + false, + startDate, + endDate + ) + ) + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalCreated) + .withArgs(id, alice.address, startDate, endDate, dummyMetadata, [], 0); - await time.setNextBlockTimestamp(startDate); + const latestBlock = await ethers.provider.getBlock('latest'); - await expect( - multisig.createProposal( - dummyMetadata, - [], - allowFailureMap, - true, - false, - 0, - endDate - ) - ) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalCreated) - .withArgs( - id, - signers[0].address, - startDate, - endDate, - dummyMetadata, - [], - allowFailureMap - ) - .to.emit(multisig, MULTISIG_EVENTS.Approved) - .withArgs(id, signers[0].address); + // Check that the proposal was created as expected and has 0 approvals. + const proposal = await plugin.getProposal(id); + expect(proposal.executed).to.equal(false); + expect(proposal.allowFailureMap).to.equal(0); + expect(proposal.parameters.snapshotBlock).to.equal( + latestBlock.number - 1 + ); + expect(proposal.parameters.minApprovals).to.equal( + defaultInitData.settings.minApprovals + ); + expect(proposal.parameters.startDate).to.equal(startDate); + expect(proposal.parameters.endDate).to.equal(endDate); + expect(proposal.actions.length).to.equal(0); + expect(proposal.approvals).to.equal(0); + + // Check that Alice hasn't approved the proposal yet. + expect(await plugin.canApprove(id, alice.address)).to.be.true; + // Check that, e.g., Bob hasn't approved the proposal yet. + expect(await plugin.canApprove(id, bob.address)).to.be.true; + }); - const block = await ethers.provider.getBlock('latest'); + it('creates a proposal successfully and approves if specified', async () => { + const { + alice, + bob, + initializedPlugin: plugin, + defaultInitData, + dummyMetadata, + } = await loadFixture(fixture); - const proposal = await multisig.getProposal(id); - expect(proposal.executed).to.equal(false); - expect(proposal.allowFailureMap).to.equal(allowFailureMap); - expect(proposal.parameters.snapshotBlock).to.equal(block.number - 1); - expect(proposal.parameters.minApprovals).to.equal( - multisigSettings.minApprovals - ); - expect(proposal.parameters.startDate).to.equal(startDate); - expect(proposal.parameters.endDate).to.equal(endDate); - expect(proposal.approvals).to.equal(1); - }); + // Create a proposal as Alice and approve on creation. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + const allowFailureMap = 1; + const approveProposal = true; - it('increases the proposal count', async () => { - expect(await multisig.proposalCount()).to.equal(0); + await time.setNextBlockTimestamp(startDate); - await multisig.createProposal( + const id = 0; + await expect( + plugin.connect(alice).createProposal( dummyMetadata, - dummyActions, - 0, - true, + [], + allowFailureMap, + approveProposal, // true false, - 0, + startDate, endDate - ); - expect(await multisig.proposalCount()).to.equal(1); - - await multisig.createProposal( + ) + ) + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalCreated) + .withArgs( + id, + alice.address, + startDate, + endDate, dummyMetadata, - dummyActions, - 0, - true, - false, - 0, - endDate - ); - expect(await multisig.proposalCount()).to.equal(2); - }); - }); + [], + allowFailureMap + ) + .to.emit(plugin, MULTISIG_EVENTS.Approved) + .withArgs(id, alice.address); - it('should revert if startDate is < than now', async () => { - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings + const latestBlock = await ethers.provider.getBlock('latest'); + + // Check that the proposal was created as expected and has 1 approval. + const proposal = await plugin.getProposal(id); + expect(proposal.executed).to.equal(false); + expect(proposal.allowFailureMap).to.equal(allowFailureMap); + expect(proposal.parameters.snapshotBlock).to.equal( + latestBlock.number - 1 ); + expect(proposal.parameters.minApprovals).to.equal( + defaultInitData.settings.minApprovals + ); + expect(proposal.parameters.startDate).to.equal(startDate); + expect(proposal.parameters.endDate).to.equal(endDate); + expect(proposal.actions.length).to.equal(0); + expect(proposal.approvals).to.equal(1); + + // Check that Alice has approved the proposal already. + expect(await plugin.canApprove(id, alice.address)).to.be.false; + // Check that, e.g., Bob hasn't approved the proposal yet. + expect(await plugin.canApprove(id, bob.address)).to.be.true; + }); - const currentDate = await time.latest(); - const startDateInThePast = currentDate - 1; - const endDate = 0; // startDate + minDuration + it('reverts if startDate < now', async () => { + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Set the clock to the start date. + const startDate = (await time.latest()) + TIME.MINUTE; + await time.setNextBlockTimestamp(startDate); + // Try to create a proposal as Alice where the start date lies in the past. + const startDateInThePast = startDate - 1; + const endDate = startDate + TIME.HOUR; await expect( - multisig.createProposal( - dummyMetadata, - dummyActions, - 0, - true, - false, - startDateInThePast, - endDate - ) + plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + true, + false, + startDateInThePast, + endDate + ) ) - .to.be.revertedWithCustomError(multisig, 'DateOutOfBounds') - .withArgs( - currentDate + 1, // await takes one second - startDateInThePast - ); + .to.be.revertedWithCustomError(plugin, 'DateOutOfBounds') + .withArgs(startDate, startDateInThePast); }); - it('should revert if endDate is < than startDate', async () => { - await multisig.initialize( - dao.address, - [signers[0].address], // signers[0] is listed - multisigSettings - ); + it('reverts if endDate < startDate', async () => { + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + // Try to create a proposal as Alice where the end date is before the start date const startDate = (await time.latest()) + TIME.MINUTE; const endDate = startDate - 1; // endDate < startDate await expect( - multisig.createProposal( - dummyMetadata, - dummyActions, - 0, - true, - false, - startDate, - endDate - ) + plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + true, + false, + startDate, + endDate + ) ) - .to.be.revertedWithCustomError(multisig, 'DateOutOfBounds') + .to.be.revertedWithCustomError(plugin, 'DateOutOfBounds') .withArgs(startDate, endDate); }); }); context('Approving and executing proposals', async () => { - beforeEach(async () => { - multisigSettings.minApprovals = 3; - await multisig.initialize( - dao.address, - signers.slice(0, 5).map(s => s.address), - multisigSettings - ); - - await multisig.createProposal( - dummyMetadata, - dummyActions, - 0, - false, - false, - 0, - endDate - ); - }); - describe('canApprove', async () => { it('returns `false` if the proposal is already executed', async () => { - await approveWithSigners(multisig, id, signers, [0, 1]); + const { + alice, + bob, + carol, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); - await multisig.connect(signers[2]).approve(id, true); - expect((await multisig.getProposal(id)).executed).to.be.true; + const endDate = (await time.latest()) + TIME.HOUR; - expect(await multisig.canApprove(id, signers[3].address)).to.be.false; + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); + + // Check that Carol can approve. + expect(await plugin.canApprove(id, carol.address)).to.be.true; + + // Approve with Alice. + await plugin.connect(alice).approve(id, false); + // Approve and execute with Bob. + await plugin.connect(bob).approve(id, true); + + // Check that the proposal got executed. + expect((await plugin.getProposal(id)).executed).to.be.true; + + // Check that Carol cannot approve the executed proposal anymore. + expect(await plugin.canApprove(id, carol.address)).to.be.false; }); it('returns `false` if the approver is not listed', async () => { - expect(await multisig.isListed(signers[9].address)).to.be.false; + const { + alice, + dave, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - expect(await multisig.canApprove(id, signers[9].address)).to.be.false; + // Check that Dave who is not listed cannot approve. + expect(await plugin.isListed(dave.address)).to.be.false; + expect(await plugin.canApprove(id, dave.address)).to.be.false; }); it('returns `false` if the approver has already approved', async () => { - await multisig.connect(signers[0]).approve(id, false); - expect(await multisig.canApprove(id, signers[0].address)).to.be.false; + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + await plugin.connect(alice).approve(id, false); + expect(await plugin.canApprove(id, alice.address)).to.be.false; }); it('returns `true` if the approver is listed', async () => { - expect(await multisig.canApprove(id, signers[0].address)).to.be.true; + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + expect(await plugin.canApprove(id, alice.address)).to.be.true; }); it("returns `false` if the proposal hasn't started yet", async () => { - await multisig.createProposal( + const { + alice, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - startDate, - endDate - ); + } = await loadFixture(fixture); + + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + startDate, + endDate + ); + const id = 0; - expect(await multisig.canApprove(1, signers[0].address)).to.be.false; + expect(await plugin.canApprove(id, alice.address)).to.be.false; await time.increaseTo(startDate); - expect(await multisig.canApprove(1, signers[0].address)).to.be.true; + expect(await plugin.canApprove(id, alice.address)).to.be.true; }); it('returns `false` if the proposal has ended', async () => { - await multisig.createProposal( + const { + alice, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - 0, - endDate - ); + } = await loadFixture(fixture); - expect(await multisig.canApprove(1, signers[0].address)).to.be.true; + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + expect(await plugin.canApprove(id, alice.address)).to.be.true; await time.increaseTo(endDate + 1); - expect(await multisig.canApprove(1, signers[0].address)).to.be.false; + expect(await plugin.canApprove(id, alice.address)).to.be.false; }); }); describe('hasApproved', async () => { it("returns `false` if user hasn't approved yet", async () => { - expect(await multisig.hasApproved(id, signers[0].address)).to.be.false; + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + expect(await plugin.hasApproved(id, alice.address)).to.be.false; }); it('returns `true` if user has approved', async () => { - await multisig.approve(id, true); - expect(await multisig.hasApproved(id, signers[0].address)).to.be.true; + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + await plugin.connect(alice).approve(id, false); + expect(await plugin.hasApproved(id, alice.address)).to.be.true; }); }); describe('approve', async () => { it('reverts when approving multiple times', async () => { - await multisig.approve(id, true); + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + await plugin.connect(alice).approve(id, true); + + // Try to vote again + await expect(plugin.connect(alice).approve(id, true)) + .to.be.revertedWithCustomError(plugin, 'ApprovalCastForbidden') + .withArgs(id, alice.address); + }); + + it('reverts if minimal approval is not met yet', async () => { + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + dao, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0) + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - // Try to vote again - await expect(multisig.approve(id, true)) - .to.be.revertedWithCustomError(multisig, 'ApprovalCastForbidden') - .withArgs(id, signers[0].address); - }); + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); - it('reverts if minimal approval is not met yet', async () => { - const proposal = await multisig.getProposal(id); + // Try to execute the proposals although `minApprovals` has not been reached yet. + const proposal = await plugin.getProposal(id); expect(proposal.approvals).to.eq(0); - await expect(multisig.execute(id)) - .to.be.revertedWithCustomError(multisig, 'ProposalExecutionForbidden') + await expect(plugin.connect(alice).execute(id)) + .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden') .withArgs(id); }); - it('approves with the msg.sender address', async () => { - expect((await multisig.getProposal(id)).approvals).to.equal(0); + it('approves with the caller address', async () => { + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + // Create a proposal (with ID 0). + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Check that there are 0 approvals yet. + expect((await plugin.getProposal(id)).approvals).to.equal(0); - const tx = await multisig.connect(signers[0]).approve(id, false); + // Approve the proposal as Alice. + const tx = await plugin.connect(alice).approve(id, false); + // Check the `Approved` event and make sure that Alice is emitted as the approver. const event = await findEvent(tx, 'Approved'); expect(event.args.proposalId).to.eq(id); - expect(event.args.approver).to.not.eq(multisig.address); - expect(event.args.approver).to.eq(signers[0].address); + expect(event.args.approver).to.eq(alice.address); - expect((await multisig.getProposal(id)).approvals).to.equal(1); + // Check that the approval was counted. + expect((await plugin.getProposal(id)).approvals).to.equal(1); }); it("reverts if the proposal hasn't started yet", async () => { - await multisig.createProposal( + const { + alice, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - startDate, - endDate - ); + } = await loadFixture(fixture); - await expect(multisig.approve(1, false)).to.be.revertedWithCustomError( - multisig, - 'ApprovalCastForbidden' - ); + // Create a proposal as Alice that didn't started yet. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + startDate, + endDate + ); + const id = 0; + // Try to approve the proposal as Alice although being before the start date. + await expect(plugin.connect(alice).approve(id, false)) + .to.be.revertedWithCustomError(plugin, 'ApprovalCastForbidden') + .withArgs(0, alice.address); + + // Advance to the start date. await time.increaseTo(startDate); - await expect(multisig.approve(1, false)).not.to.be.reverted; + // Approve as Alice and check that this doesn't revert. + await expect(plugin.connect(alice).approve(id, false)).not.to.be + .reverted; }); it('reverts if the proposal has ended', async () => { - await multisig.createProposal( + const { + alice, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - 0, - endDate - ); + } = await loadFixture(fixture); - await expect(multisig.connect(signers[1]).approve(1, false)).not.to.be - .reverted; + // Create a proposal as Alice that starts now. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + // Advance time after the end date. await time.increaseTo(endDate + 1); - await expect(multisig.approve(1, false)).to.be.revertedWithCustomError( - multisig, - 'ApprovalCastForbidden' - ); + await expect(plugin.connect(alice).approve(id, false)) + .to.be.revertedWithCustomError(plugin, 'ApprovalCastForbidden') + .withArgs(id, alice.address); }); }); describe('canExecute', async () => { it('returns `false` if the proposal has not reached the minimum approval yet', async () => { - const proposal = await multisig.getProposal(id); + const { + alice, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Check that `minApprovals` isn't met yet. + const proposal = await plugin.getProposal(id); expect(proposal.approvals).to.be.lt(proposal.parameters.minApprovals); - expect(await multisig.canExecute(id)).to.be.false; + // Check that the proposal can not be executed. + expect(await plugin.canExecute(id)).to.be.false; }); it('returns `false` if the proposal is already executed', async () => { - await approveWithSigners(multisig, id, signers, [0, 1]); - await multisig.connect(signers[2]).approve(id, true); + const { + alice, + bob, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - expect((await multisig.getProposal(id)).executed).to.be.true; + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); + + // Approve as Alice. + await plugin.connect(alice).approve(id, false); + // Approve and execute as Bob. + await plugin.connect(bob).approve(id, true); + + // Check that the proposal got executed. + expect((await plugin.getProposal(id)).executed).to.be.true; - expect(await multisig.canExecute(id)).to.be.false; + // Check that it cannot be executed again. + expect(await plugin.canExecute(id)).to.be.false; }); it('returns `true` if the proposal can be executed', async () => { - await approveWithSigners(multisig, id, signers, [0, 1, 2]); + const { + alice, + bob, + initializedPlugin: plugin, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - expect((await multisig.getProposal(id)).executed).to.be.false; + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); - expect(await multisig.canExecute(id)).to.be.true; + expect((await plugin.getProposal(id)).executed).to.be.false; + expect(await plugin.canExecute(id)).to.be.true; }); it("returns `false` if the proposal hasn't started yet", async () => { - await multisig.createProposal( + const { + alice, + bob, + carol, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - startDate, - endDate - ); + } = await loadFixture(fixture); + + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + startDate, + endDate + ); + const id = 0; - expect(await multisig.canExecute(1)).to.be.false; + expect(await plugin.canExecute(id)).to.be.false; await time.increaseTo(startDate); - await multisig.connect(signers[0]).approve(1, false); - await multisig.connect(signers[1]).approve(1, false); - await multisig.connect(signers[2]).approve(1, false); + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); + await plugin.connect(carol).approve(id, false); - expect(await multisig.canExecute(1)).to.be.true; + expect(await plugin.canExecute(id)).to.be.true; }); it('returns `false` if the proposal has ended', async () => { - await multisig.createProposal( + const { + alice, + bob, + carol, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - 0, - endDate - ); + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - await multisig.connect(signers[0]).approve(1, false); - await multisig.connect(signers[1]).approve(1, false); - await multisig.connect(signers[2]).approve(1, false); + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); + await plugin.connect(carol).approve(id, false); - expect(await multisig.canExecute(1)).to.be.true; + expect(await plugin.canExecute(id)).to.be.true; await time.increaseTo(endDate + 1); - expect(await multisig.canExecute(1)).to.be.false; + expect(await plugin.canExecute(id)).to.be.false; }); }); describe('execute', async () => { it('reverts if the minimum approval is not met', async () => { - await expect(multisig.execute(id)) - .to.be.revertedWithCustomError(multisig, 'ProposalExecutionForbidden') + const { + alice, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); + + // Check that proposal cannot be executed if the minimum approval is not met yet. + await expect(plugin.execute(id)) + .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden') .withArgs(id); }); it('executes if the minimum approval is met', async () => { - await approveWithSigners(multisig, id, signers, [0, 1, 2]); + const { + alice, + bob, + initializedPlugin: plugin, + defaultInitData, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); - const proposal = await multisig.getProposal(id); + // Approve with Alice and Bob. + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); + // Check that the `minApprovals` threshold is met. + const proposal = await plugin.getProposal(id); expect(proposal.parameters.minApprovals).to.equal( - multisigSettings.minApprovals + defaultInitData.settings.minApprovals + ); + expect(proposal.approvals).to.be.eq( + defaultInitData.settings.minApprovals ); - expect(proposal.approvals).to.be.eq(multisigSettings.minApprovals); - expect(await multisig.canExecute(id)).to.be.true; - await expect(multisig.execute(id)).not.to.be.reverted; + // Check that the proposal can be executed. + expect(await plugin.canExecute(id)).to.be.true; + + // Check that it executes. + await expect(plugin.execute(id)).not.to.be.reverted; }); it('executes if the minimum approval is met and can be called by an unlisted accounts', async () => { - await approveWithSigners(multisig, id, signers, [0, 1, 2]); + const { + alice, + bob, + dave, + initializedPlugin: plugin, + defaultInitData, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + const endDate = (await time.latest()) + TIME.HOUR; + + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); - const proposal = await multisig.getProposal(id); + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); + + const proposal = await plugin.getProposal(id); expect(proposal.parameters.minApprovals).to.equal( - multisigSettings.minApprovals + defaultInitData.settings.minApprovals + ); + expect(proposal.approvals).to.be.eq( + defaultInitData.settings.minApprovals ); - expect(proposal.approvals).to.be.eq(multisigSettings.minApprovals); - expect(await multisig.canExecute(id)).to.be.true; - expect(await multisig.isListed(signers[9].address)).to.be.false; // signers[9] is not listed - await expect(multisig.connect(signers[9]).execute(id)).not.to.be - .reverted; + expect(await plugin.canExecute(id)).to.be.true; + expect(await plugin.isListed(dave.address)).to.be.false; // Dave is not listed + await expect(plugin.connect(dave).execute(id)).not.to.be.reverted; }); it('executes if the minimum approval is met when multisig with the `tryExecution` option', async () => { - await multisig.connect(signers[0]).approve(id, true); + const { + alice, + bob, + carol, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; - expect(await multisig.canExecute(id)).to.equal(false); + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); - // `tryExecution` is turned on but the vote is not decided yet - let tx = await multisig.connect(signers[1]).approve(id, true); + // Approve and try execution as Alice although the `minApprovals` threshold is not met yet. + let tx = await plugin.connect(alice).approve(id, true); await expect( findEventTopicLog( tx, @@ -1224,10 +2055,10 @@ describe('Multisig', function () { `Event "${IDAO_EVENTS.Executed}" could not be found in transaction ${tx.hash}.` ); - expect(await multisig.canExecute(id)).to.equal(false); + expect(await plugin.canExecute(id)).to.equal(false); - // `tryExecution` is turned off and the vote is decided - tx = await multisig.connect(signers[2]).approve(id, false); + // Approve but do not try execution as Bob although the `minApprovals` threshold is reached now. + tx = await plugin.connect(bob).approve(id, false); await expect( findEventTopicLog( tx, @@ -1238,8 +2069,10 @@ describe('Multisig', function () { `Event "${IDAO_EVENTS.Executed}" could not be found in transaction ${tx.hash}.` ); - // `tryEarlyExecution` is turned on and the vote is decided - tx = await multisig.connect(signers[3]).approve(id, true); + // Approve and try execution as Carol while `minApprovals` threshold is reached already. + tx = await plugin.connect(carol).approve(id, true); + + // Check that the proposal got executed by checking the `Executed` event emitted by the DAO. { const event = await findEventTopicLog( tx, @@ -1247,7 +2080,7 @@ describe('Multisig', function () { IDAO_EVENTS.Executed ); - expect(event.args.actor).to.equal(multisig.address); + expect(event.args.actor).to.equal(plugin.address); expect(event.args.callId).to.equal(proposalIdToBytes32(id)); expect(event.args.actions.length).to.equal(1); expect(event.args.actions[0].to).to.equal(dummyActions[0].to); @@ -1255,11 +2088,11 @@ describe('Multisig', function () { expect(event.args.actions[0].data).to.equal(dummyActions[0].data); expect(event.args.execResults).to.deep.equal(['0x']); - const prop = await multisig.getProposal(id); + const prop = await plugin.getProposal(id); expect(prop.executed).to.equal(true); } - // check for the `ProposalExecuted` event in the multisig contract + // Check that the proposal got executed by checking the `ProposalExecuted` event emitted by the plugin. { const event = await findEvent( tx, @@ -1268,73 +2101,184 @@ describe('Multisig', function () { expect(event.args.proposalId).to.equal(id); } - // calling execute again should fail - await expect(multisig.execute(id)) - .to.be.revertedWithCustomError(multisig, 'ProposalExecutionForbidden') + // Try executing it again. + await expect(plugin.execute(id)) + .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden') .withArgs(id); }); it('emits the `ProposalExecuted` and `Executed` events', async () => { - await approveWithSigners(multisig, id, signers, [0, 1, 2]); + const { + alice, + bob, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Grant the plugin `EXECUTE_PERMISSION_ID` on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); + + // Approve the proposal as Alice and Bob. + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); - await expect(multisig.connect(signers[3]).execute(id)) + // Execute the proposal and check that the `Executed` and `ProposalExecuted` event is emitted + // and that the `Approved` event is not emitted. + await expect(plugin.connect(alice).execute(id)) .to.emit(dao, IDAO_EVENTS.Executed) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalExecuted) - .to.not.emit(multisig, MULTISIG_EVENTS.Approved); + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalExecuted) + .to.not.emit(plugin, MULTISIG_EVENTS.Approved); }); it('emits the `Approved`, `ProposalExecuted`, and `Executed` events if execute is called inside the `approve` method', async () => { - await approveWithSigners(multisig, id, signers, [0, 1]); + const { + alice, + bob, + initializedPlugin: plugin, + dao, + dummyMetadata, + dummyActions, + } = await loadFixture(fixture); - await expect(multisig.connect(signers[2]).approve(id, true)) + // Create a proposal as Alice. + const endDate = (await time.latest()) + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID + ); + + // Approve the proposal as Alice. + await plugin.connect(alice).approve(id, false); + + // Approve and execute the proposal as Bob and check that the `Executed`, `ProposalExecuted`, and `Approved` + // event is not emitted. + await expect(plugin.connect(bob).approve(id, true)) .to.emit(dao, IDAO_EVENTS.Executed) - .to.emit(multisig, IPROPOSAL_EVENTS.ProposalExecuted) - .to.emit(multisig, MULTISIG_EVENTS.Approved); + .to.emit(plugin, IPROPOSAL_EVENTS.ProposalExecuted) + .to.emit(plugin, MULTISIG_EVENTS.Approved); }); it("reverts if the proposal hasn't started yet", async () => { - await multisig.createProposal( + const { + alice, + bob, + initializedPlugin: plugin, + dao, dummyMetadata, dummyActions, - 0, - false, - false, - startDate, - endDate - ); + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + startDate, + endDate + ); + const id = 0; - await expect(multisig.execute(1)).to.be.revertedWithCustomError( - multisig, - 'ProposalExecutionForbidden' + // Grant the plugin `EXECUTE_PERMISSION_ID` permission on the DAO. + await dao.grant( + dao.address, + plugin.address, + DAO_PERMISSIONS.EXECUTE_PERMISSION_ID ); + // Try to execute the proposal before the start date. + await expect(plugin.execute(id)) + .to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden') + .withArgs(id); + + // Advance time to the start date. await time.increaseTo(startDate); - await multisig.connect(signers[0]).approve(1, false); - await multisig.connect(signers[1]).approve(1, false); - await multisig.connect(signers[2]).approve(1, false); + // Approve the proposal as Alice and Bob. + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); - await expect(multisig.execute(1)).not.to.be.reverted; + // Execute the proposal. + await expect(plugin.execute(id)).not.to.be.reverted; }); it('reverts if the proposal has ended', async () => { - await multisig.createProposal( + const { + alice, + bob, + initializedPlugin: plugin, dummyMetadata, dummyActions, - 0, - false, - false, - 0, - endDate - ); - await multisig.connect(signers[0]).approve(1, false); - await multisig.connect(signers[1]).approve(1, false); - await multisig.connect(signers[2]).approve(1, false); + } = await loadFixture(fixture); + + // Create a proposal as Alice. + const startDate = (await time.latest()) + TIME.MINUTE; + const endDate = startDate + TIME.HOUR; + await plugin + .connect(alice) + .createProposal( + dummyMetadata, + dummyActions, + 0, + false, + false, + 0, + endDate + ); + const id = 0; + + // Approve the proposal but do not execute yet. + await plugin.connect(alice).approve(id, false); + await plugin.connect(bob).approve(id, false); + + // Advance time after the end date + await time.increase(endDate + 1); - await time.increase(10000); + // Try to execute the proposal after the end date. await expect( - multisig.connect(signers[1]).execute(1) - ).to.be.revertedWithCustomError(multisig, 'ProposalExecutionForbidden'); + plugin.connect(bob).execute(id) + ).to.be.revertedWithCustomError(plugin, 'ProposalExecutionForbidden'); }); }); }); diff --git a/packages/contracts/test/10_unit-testing/12_plugin-setup.ts b/packages/contracts/test/10_unit-testing/12_plugin-setup.ts index 62d87bee..86c0d758 100644 --- a/packages/contracts/test/10_unit-testing/12_plugin-setup.ts +++ b/packages/contracts/test/10_unit-testing/12_plugin-setup.ts @@ -1,89 +1,105 @@ import {createDaoProxy} from '../20_integration-testing/test-helpers'; -import metadata from '../../src/build-metadata.json'; -import { - MultisigSetup, - MultisigSetup__factory, - ProxyFactory__factory, -} from '../../typechain'; -import {ProxyCreatedEvent} from '../../typechain/@aragon/osx-commons-contracts/src/utils/deployment/ProxyFactory'; -import {hashHelpers} from '../../utils/helpers'; +import {METADATA, VERSION} from '../../plugin-settings'; +import {MultisigSetup, MultisigSetup__factory} from '../../typechain'; import { MULTISIG_INTERFACE, - MultisigSettings, UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, } from '../multisig-constants'; import {Multisig__factory, Multisig} from '../test-utils/typechain-versions'; import { getInterfaceId, - findEvent, Operation, DAO_PERMISSIONS, PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS, getNamedTypesFromMetadata, } from '@aragon/osx-commons-sdk'; -import { - DAO, - IPluginRepo__factory, - InterfaceBasedRegistryMock, - InterfaceBasedRegistryMock__factory, - PluginRepo, - PluginRepo__factory, - PluginSetupProcessor, - PluginSetupProcessorEvents, - PluginSetupProcessor__factory, -} from '@aragon/osx-ethers'; +import {DAO} from '@aragon/osx-ethers'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {ethers} from 'hardhat'; const abiCoder = ethers.utils.defaultAbiCoder; const AddressZero = ethers.constants.AddressZero; -const EMPTY_DATA = '0x'; -let defaultMultisigSettings: MultisigSettings; +type FixtureResult = { + deployer: SignerWithAddress; + alice: SignerWithAddress; + bob: SignerWithAddress; + carol: SignerWithAddress; + pluginSetup: MultisigSetup; + defaultMembers: string[]; + defaultMultisigSettings: Multisig.MultisigSettingsStruct; + prepareInstallationInputs: string; + prepareUninstallationInputs: string; + dao: DAO; +}; + +async function fixture(): Promise { + const [deployer, alice, bob, carol] = await ethers.getSigners(); + + // Deploy a DAO proxy. + const dummyMetadata = ethers.utils.hexlify( + ethers.utils.toUtf8Bytes('0x123456789') + ); + const dao = await createDaoProxy(deployer, dummyMetadata); + + // Deploy a plugin setup contract + const pluginSetup = await new MultisigSetup__factory(deployer).deploy(); + + // Provide default multisig settings + const defaultMembers = [alice.address, bob.address, carol.address]; + const defaultMultisigSettings: Multisig.MultisigSettingsStruct = { + onlyListed: true, + minApprovals: 1, + }; + + // Provide installation inputs + const prepareInstallationInputs = ethers.utils.defaultAbiCoder.encode( + getNamedTypesFromMetadata( + METADATA.build.pluginSetup.prepareInstallation.inputs + ), + [defaultMembers, Object.values(defaultMultisigSettings)] + ); + + // Provide uninstallation inputs + const prepareUninstallationInputs = ethers.utils.defaultAbiCoder.encode( + getNamedTypesFromMetadata( + METADATA.build.pluginSetup.prepareUninstallation.inputs + ), + [] + ); + + return { + deployer, + alice, + bob, + carol, + pluginSetup, + defaultMembers, + defaultMultisigSettings, + prepareInstallationInputs, + prepareUninstallationInputs, + dao, + }; +} describe('MultisigSetup', function () { - let signers: SignerWithAddress[]; - let multisigSetup: MultisigSetup; - let MultisigFactory: Multisig__factory; - let implementationAddress: string; - let targetDao: DAO; - let minimum_data: any; - - before(async () => { - signers = await ethers.getSigners(); - targetDao = await createDaoProxy(signers[0], EMPTY_DATA); - - defaultMultisigSettings = { - onlyListed: true, - minApprovals: 1, - }; - - minimum_data = abiCoder.encode( - getNamedTypesFromMetadata( - metadata.pluginSetup.prepareInstallation.inputs - ), - [[signers[0].address], Object.values(defaultMultisigSettings)] - ); - - const MultisigSetup = new MultisigSetup__factory(signers[0]); - multisigSetup = await MultisigSetup.deploy(); - - MultisigFactory = new Multisig__factory(signers[0]); - - implementationAddress = await multisigSetup.implementation(); - }); - it('does not support the empty interface', async () => { - expect(await multisigSetup.supportsInterface('0xffffffff')).to.be.false; + const {pluginSetup} = await loadFixture(fixture); + expect(await pluginSetup.supportsInterface('0xffffffff')).to.be.false; }); - it('creates multisig base with the correct interface', async () => { - const factory = new Multisig__factory(signers[0]); - const multisigContract = factory.attach(implementationAddress); + it('has a multisig implementation supporting the correct interface', async () => { + const {deployer, pluginSetup} = await loadFixture(fixture); + + const factory = new Multisig__factory(deployer); + const multisigImplementation = factory.attach( + await pluginSetup.implementation() + ); expect( - await multisigContract.supportsInterface( + await multisigImplementation.supportsInterface( getInterfaceId(MULTISIG_INTERFACE) ) ).to.be.true; @@ -91,45 +107,59 @@ describe('MultisigSetup', function () { describe('prepareInstallation', async () => { it('fails if data is empty, or not of minimum length', async () => { - await expect( - multisigSetup.prepareInstallation(targetDao.address, EMPTY_DATA) - ).to.be.reverted; + const {pluginSetup, dao, prepareInstallationInputs} = await loadFixture( + fixture + ); - await expect( - multisigSetup.prepareInstallation( - targetDao.address, - minimum_data.substring(0, minimum_data.length - 2) - ) - ).to.be.reverted; + // Try calling `prepareInstallation` without input data. + await expect(pluginSetup.prepareInstallation(dao.address, [])).to.be + .reverted; + // Try calling `prepareInstallation` without input data of wrong length. + const trimmedData = prepareInstallationInputs.substring( + 0, + prepareInstallationInputs.length - 2 + ); + await expect(pluginSetup.prepareInstallation(dao.address, trimmedData)).to + .be.reverted; + + // Check that `prepareInstallation` can be called with the correct input data. await expect( - multisigSetup.prepareInstallation(targetDao.address, minimum_data) + pluginSetup.prepareInstallation(dao.address, prepareInstallationInputs) ).not.to.be.reverted; }); - it('reverts if zero members are provided in `_data`', async () => { - const noMembers: string[] = []; + it('reverts if zero members are provided in the initialization data', async () => { + const {deployer, pluginSetup, dao, defaultMultisigSettings} = + await loadFixture(fixture); + // Create input data containing an empty list of initial members. + const noMembers: string[] = []; const wrongPrepareInstallationData = abiCoder.encode( getNamedTypesFromMetadata( - metadata.pluginSetup.prepareInstallation.inputs + METADATA.build.pluginSetup.prepareInstallation.inputs ), [noMembers, defaultMultisigSettings] ); + // Anticipate the plugin proxy address that will be deployed. const nonce = await ethers.provider.getTransactionCount( - multisigSetup.address + pluginSetup.address ); const anticipatedPluginAddress = ethers.utils.getContractAddress({ - from: multisigSetup.address, + from: pluginSetup.address, nonce, }); + const multisig = Multisig__factory.connect( + anticipatedPluginAddress, + deployer + ); - const multisig = MultisigFactory.attach(anticipatedPluginAddress); - + // Try calling `prepareInstallation`, which will fail during plugin initialization because of the empty initial + // member list. await expect( - multisigSetup.prepareInstallation( - targetDao.address, + pluginSetup.prepareInstallation( + dao.address, wrongPrepareInstallationData ) ) @@ -138,31 +168,39 @@ describe('MultisigSetup', function () { }); it('reverts if the `minApprovals` value in `_data` is zero', async () => { - const multisigSettings: MultisigSettings = { + const {deployer, pluginSetup, dao} = await loadFixture(fixture); + + // Create input data containing a `minApprovals` threshold of 0. + const multisigSettings: Multisig.MultisigSettingsStruct = { onlyListed: true, minApprovals: 0, }; - const members = [signers[0].address]; - + const members = [deployer.address]; const wrongPrepareInstallationData = abiCoder.encode( getNamedTypesFromMetadata( - metadata.pluginSetup.prepareInstallation.inputs + METADATA.build.pluginSetup.prepareInstallation.inputs ), [members, multisigSettings] ); + // Anticipate the plugin proxy address that will be deployed. const nonce = await ethers.provider.getTransactionCount( - multisigSetup.address + pluginSetup.address ); const anticipatedPluginAddress = ethers.utils.getContractAddress({ - from: multisigSetup.address, + from: pluginSetup.address, nonce, }); - const multisig = MultisigFactory.attach(anticipatedPluginAddress); + const multisig = Multisig__factory.connect( + anticipatedPluginAddress, + deployer + ); + // Try calling `prepareInstallation`, which will fail during plugin initialization because of the invalid + // `minApprovals` value. await expect( - multisigSetup.prepareInstallation( - targetDao.address, + pluginSetup.prepareInstallation( + dao.address, wrongPrepareInstallationData ) ) @@ -170,32 +208,41 @@ describe('MultisigSetup', function () { .withArgs(1, 0); }); - it('reverts if the `minApprovals` value in `_data` is greater than the number members', async () => { - const multisigSettings: MultisigSettings = { + it('reverts if the `minApprovals` value in `_data` is greater than the number of members', async () => { + const {deployer, pluginSetup, dao} = await loadFixture(fixture); + + // Create input data containing an initial member list with a length lower that the specified `minApprovals` + // threshold. + const multisigSettings: Multisig.MultisigSettingsStruct = { onlyListed: true, minApprovals: 2, }; - const members = [signers[0].address]; - + const members = [deployer.address]; const wrongPrepareInstallationData = abiCoder.encode( getNamedTypesFromMetadata( - metadata.pluginSetup.prepareInstallation.inputs + METADATA.build.pluginSetup.prepareInstallation.inputs ), [members, multisigSettings] ); + // Anticipate the plugin proxy address that will be deployed. const nonce = await ethers.provider.getTransactionCount( - multisigSetup.address + pluginSetup.address ); const anticipatedPluginAddress = ethers.utils.getContractAddress({ - from: multisigSetup.address, + from: pluginSetup.address, nonce, }); - const multisig = MultisigFactory.attach(anticipatedPluginAddress); + const multisig = Multisig__factory.connect( + anticipatedPluginAddress, + deployer + ); + // Try calling `prepareInstallation`, which will fail during plugin initialization because of the mismatch + // between the `minApprovals` value and the initial member list length. await expect( - multisigSetup.prepareInstallation( - targetDao.address, + pluginSetup.prepareInstallation( + dao.address, wrongPrepareInstallationData ) ) @@ -204,22 +251,29 @@ describe('MultisigSetup', function () { }); it('returns the plugin, helpers, and permissions', async () => { + const {pluginSetup, dao, prepareInstallationInputs} = await loadFixture( + fixture + ); + + // Anticipate the plugin proxy address that will be deployed. const nonce = await ethers.provider.getTransactionCount( - multisigSetup.address + pluginSetup.address ); const anticipatedPluginAddress = ethers.utils.getContractAddress({ - from: multisigSetup.address, + from: pluginSetup.address, nonce, }); + // Make a static call to check that the plugin preparation data being returned is correct. const { plugin, preparedSetupData: {helpers, permissions}, - } = await multisigSetup.callStatic.prepareInstallation( - targetDao.address, - minimum_data + } = await pluginSetup.callStatic.prepareInstallation( + dao.address, + prepareInstallationInputs ); + // Check the return data. expect(plugin).to.be.equal(anticipatedPluginAddress); expect(helpers.length).to.be.equal(0); expect(permissions.length).to.be.equal(3); @@ -227,20 +281,20 @@ describe('MultisigSetup', function () { [ Operation.Grant, plugin, - targetDao.address, + dao.address, AddressZero, UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, ], [ Operation.Grant, plugin, - targetDao.address, + dao.address, AddressZero, PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, ], [ Operation.Grant, - targetDao.address, + dao.address, plugin, AddressZero, DAO_PERMISSIONS.EXECUTE_PERMISSION_ID, @@ -249,36 +303,52 @@ describe('MultisigSetup', function () { }); it('sets up the plugin', async () => { - const daoAddress = targetDao.address; + const { + deployer, + pluginSetup, + dao, + prepareInstallationInputs, + defaultMembers, + defaultMultisigSettings, + } = await loadFixture(fixture); + // Anticipate the plugin proxy address that will be deployed. const nonce = await ethers.provider.getTransactionCount( - multisigSetup.address + pluginSetup.address ); const anticipatedPluginAddress = ethers.utils.getContractAddress({ - from: multisigSetup.address, + from: pluginSetup.address, nonce, }); - await multisigSetup.prepareInstallation(daoAddress, minimum_data); + // Prepare the installation + await pluginSetup.prepareInstallation( + dao.address, + prepareInstallationInputs + ); - const factory = new Multisig__factory(signers[0]); - const multisigContract = factory.attach(anticipatedPluginAddress); + const plugin = Multisig__factory.connect( + anticipatedPluginAddress, + deployer + ); - expect(await multisigContract.dao()).to.eq(daoAddress); - expect(await multisigContract.addresslistLength()).to.be.eq(1); - const settings = await multisigContract.multisigSettings(); - expect(settings.onlyListed).to.be.true; - expect(settings.minApprovals).to.eq(1); + // Check that the plugin is initialized correctly. + expect(await plugin.dao()).to.eq(dao.address); + expect(await plugin.addresslistLength()).to.be.eq(defaultMembers.length); + const settings = await plugin.multisigSettings(); + expect(settings.onlyListed).to.equal(defaultMultisigSettings.onlyListed); + expect(settings.minApprovals).to.eq(defaultMultisigSettings.minApprovals); }); }); describe('prepareUpdate', async () => { it('should return nothing', async () => { - const dao = ethers.Wallet.createRandom().address; - const currentBuild = 1; - const prepareUpdateData = await multisigSetup.callStatic.prepareUpdate( - dao, - currentBuild, + const {pluginSetup, dao} = await loadFixture(fixture); + + // Make a static call to check that the plugin update data being returned is correct. + const prepareUpdateData = await pluginSetup.callStatic.prepareUpdate( + dao.address, + VERSION.build, { currentHelpers: [ ethers.Wallet.createRandom().address, @@ -288,6 +358,7 @@ describe('MultisigSetup', function () { plugin: ethers.Wallet.createRandom().address, } ); + // Check the return data. expect(prepareUpdateData.initData).to.be.eq('0x'); expect(prepareUpdateData.preparedSetupData.permissions).to.be.eql([]); expect(prepareUpdateData.preparedSetupData.helpers).to.be.eql([]); @@ -296,36 +367,44 @@ describe('MultisigSetup', function () { describe('prepareUninstallation', async () => { it('correctly returns permissions', async () => { + const {pluginSetup, dao, prepareUninstallationInputs} = await loadFixture( + fixture + ); + + // Use a random address to prepare an uninstallation. + // Note: Applying this uninstallation would fail because the PSP knows if the plugin was installed at some point. const plugin = ethers.Wallet.createRandom().address; - const permissions = await multisigSetup.callStatic.prepareUninstallation( - targetDao.address, + // Make a static call to check that the plugin uninstallation data being returned is correct. + const permissions = await pluginSetup.callStatic.prepareUninstallation( + dao.address, { plugin, currentHelpers: [], - data: EMPTY_DATA, + data: prepareUninstallationInputs, } ); + // Check the return data. expect(permissions.length).to.be.equal(3); expect(permissions).to.deep.equal([ [ Operation.Revoke, plugin, - targetDao.address, + dao.address, AddressZero, UPDATE_MULTISIG_SETTINGS_PERMISSION_ID, ], [ Operation.Revoke, plugin, - targetDao.address, + dao.address, AddressZero, PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, ], [ Operation.Revoke, - targetDao.address, + dao.address, plugin, AddressZero, DAO_PERMISSIONS.EXECUTE_PERMISSION_ID, @@ -333,280 +412,4 @@ describe('MultisigSetup', function () { ]); }); }); - - describe('Updates', async () => { - let psp: PluginSetupProcessor; - let setup1: MultisigSetup; - let setup2: MultisigSetup; - let dao: DAO; - let managingDAO: DAO; - let owner: SignerWithAddress; - let pluginRepoRegistry: InterfaceBasedRegistryMock; - let pluginRepo: PluginRepo; - - before(async () => { - [owner] = await ethers.getSigners(); - managingDAO = await createDaoProxy(owner, EMPTY_DATA); - - // Create the PluginRepo - const pluginRepoImplementation = await new PluginRepo__factory( - signers[0] - ).deploy(); - const pluginRepoProxyFactory = await new ProxyFactory__factory( - signers[0] - ).deploy(pluginRepoImplementation.address); - const tx = await pluginRepoProxyFactory.deployUUPSProxy([]); - const event = await findEvent( - tx, - pluginRepoProxyFactory.interface.getEvent('ProxyCreated').name - ); - pluginRepo = PluginRepo__factory.connect(event.args.proxy, signers[0]); - - await pluginRepo.initialize(owner.address); - - // Create the PluginRepoRegistry - const pluginRepoRegistryFactory = new InterfaceBasedRegistryMock__factory( - owner - ); - pluginRepoRegistry = await pluginRepoRegistryFactory.deploy(); - await pluginRepoRegistry.initialize( - managingDAO.address, - getInterfaceId(IPluginRepo__factory.createInterface()) - ); - - // Grant the owner full rights on the registry - await managingDAO.grant( - pluginRepoRegistry.address, - owner.address, - await pluginRepoRegistry.REGISTER_PERMISSION_ID() - ); - - // Register the PluginRepo in the registry - await pluginRepoRegistry.register(pluginRepo.address); - - // Create the PluginSetupProcessor - const pspFactory = new PluginSetupProcessor__factory(owner); - psp = await pspFactory.deploy(pluginRepoRegistry.address); - - // Prepare all MultisigSetup' - We can reuse the same for now - const multisigSetupFactory = new MultisigSetup__factory(owner); - setup1 = await multisigSetupFactory.deploy(); - setup2 = await multisigSetupFactory.deploy(); - - // Create the versions in the plugin repo - await expect(pluginRepo.createVersion(1, setup1.address, '0x00', '0x00')) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 1, setup1.address, '0x00'); - await expect(pluginRepo.createVersion(1, setup2.address, '0x00', '0x00')) - .to.emit(pluginRepo, 'VersionCreated') - .withArgs(1, 2, setup2.address, '0x00'); - }); - - describe('Release 1 Build 1', () => { - let plugin: Multisig; - let helpers: string[]; - - before(async () => { - dao = await createDaoProxy(owner, EMPTY_DATA); - // grant the owner full permission for plugins - await dao.applySingleTargetPermissions(psp.address, [ - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_INSTALLATION_PERMISSION_ID(), - }, - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_UPDATE_PERMISSION_ID(), - }, - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_UNINSTALLATION_PERMISSION_ID(), - }, - ]); - // grant the PSP root to apply stuff - await dao.grant( - dao.address, - psp.address, - await dao.ROOT_PERMISSION_ID() - ); - }); - - it('should install', async () => { - const tx = await psp.prepareInstallation(dao.address, { - pluginSetupRef: { - versionTag: { - build: 1, - release: 1, - }, - pluginSetupRepo: pluginRepo.address, - }, - data: ethers.utils.defaultAbiCoder.encode( - ['address[]', '(bool, uint16)'], - [[owner.address], [true, 1]] - ), - }); - const preparedEvent = - await findEvent( - tx, - 'InstallationPrepared' - ); - - await expect( - psp.applyInstallation(dao.address, { - pluginSetupRef: { - versionTag: { - build: 1, - release: 1, - }, - pluginSetupRepo: pluginRepo.address, - }, - helpersHash: hashHelpers( - preparedEvent.args.preparedSetupData.helpers - ), - permissions: preparedEvent.args.preparedSetupData.permissions, - plugin: preparedEvent.args.plugin, - }) - ).to.emit(psp, 'InstallationApplied'); - - plugin = Multisig__factory.connect(preparedEvent.args.plugin, owner); - helpers = preparedEvent.args.preparedSetupData.helpers; - - expect(await plugin.implementation()).to.be.eq( - await setup1.implementation() - ); - }); - - it('should update to Release 1 Build 2', async () => { - // grant psp permission to update the proxy implementation - await dao.grant( - plugin.address, - psp.address, - await plugin.UPGRADE_PLUGIN_PERMISSION_ID() - ); - - const tx = await psp.prepareUpdate(dao.address, { - currentVersionTag: { - release: 1, - build: 1, - }, - newVersionTag: { - release: 1, - build: 2, - }, - pluginSetupRepo: pluginRepo.address, - setupPayload: { - plugin: plugin.address, - currentHelpers: helpers, - data: '0x00', - }, - }); - const preparedEvent = - await findEvent( - tx, - 'UpdatePrepared' - ); - - await expect( - psp.applyUpdate(dao.address, { - plugin: plugin.address, - helpersHash: hashHelpers( - preparedEvent.args.preparedSetupData.helpers - ), - permissions: preparedEvent.args.preparedSetupData.permissions, - initData: preparedEvent.args.initData, - pluginSetupRef: { - versionTag: { - release: 1, - build: 2, - }, - pluginSetupRepo: pluginRepo.address, - }, - }) - ).to.emit(psp, 'UpdateApplied'); - - expect(await plugin.implementation()).to.be.eq( - await setup2.implementation() - ); - }); - }); - - describe('Release 1 Build 2', () => { - before(async () => { - dao = await createDaoProxy(owner, EMPTY_DATA); - // grant the owner full permission for plugins - await dao.applySingleTargetPermissions(psp.address, [ - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_INSTALLATION_PERMISSION_ID(), - }, - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_UPDATE_PERMISSION_ID(), - }, - { - operation: Operation.Grant, - who: owner.address, - permissionId: await psp.APPLY_UNINSTALLATION_PERMISSION_ID(), - }, - ]); - // grant the PSP root to apply stuff - await dao.grant( - dao.address, - psp.address, - await dao.ROOT_PERMISSION_ID() - ); - }); - - it('should install', async () => { - const tx = await psp.prepareInstallation(dao.address, { - pluginSetupRef: { - versionTag: { - release: 1, - build: 2, - }, - pluginSetupRepo: pluginRepo.address, - }, - data: ethers.utils.defaultAbiCoder.encode( - ['address[]', '(bool, uint16)'], - [[owner.address], [true, 1]] - ), - }); - const preparedEvent = - await findEvent( - tx, - 'InstallationPrepared' - ); - - await expect( - psp.applyInstallation(dao.address, { - pluginSetupRef: { - versionTag: { - release: 1, - build: 2, - }, - pluginSetupRepo: pluginRepo.address, - }, - helpersHash: hashHelpers( - preparedEvent.args.preparedSetupData.helpers - ), - permissions: preparedEvent.args.preparedSetupData.permissions, - plugin: preparedEvent.args.plugin, - }) - ).to.emit(psp, 'InstallationApplied'); - - const plugin = Multisig__factory.connect( - preparedEvent.args.plugin, - owner - ); - expect(await plugin.implementation()).to.be.eq( - await setup2.implementation() - ); - }); - }); - }); }); diff --git a/packages/contracts/test/20_integration-testing/22_setup-processing.ts b/packages/contracts/test/20_integration-testing/22_setup-processing.ts index 87eac0ed..3b1fc997 100644 --- a/packages/contracts/test/20_integration-testing/22_setup-processing.ts +++ b/packages/contracts/test/20_integration-testing/22_setup-processing.ts @@ -1,8 +1,13 @@ import {METADATA, VERSION} from '../../plugin-settings'; import {MultisigSetup, Multisig__factory} from '../../typechain'; import {getProductionNetworkName, findPluginRepo} from '../../utils/helpers'; -import {MultisigSettings} from '../multisig-constants'; -import {createDaoProxy, installPLugin, uninstallPLugin} from './test-helpers'; +import {Multisig} from '../test-utils/typechain-versions'; +import { + createDaoProxy, + installPLugin, + uninstallPLugin, + updateFromBuildTest, +} from './test-helpers'; import { getLatestNetworkDeployment, getNetworkNameByAlias, @@ -28,11 +33,92 @@ import env, {deployments, ethers} from 'hardhat'; const productionNetworkName = getProductionNetworkName(env); +type FixtureResult = { + deployer: SignerWithAddress; + alice: SignerWithAddress; + bob: SignerWithAddress; + dao: DAO; + defaultInitData: { + members: string[]; + settings: Multisig.MultisigSettingsStruct; + }; + psp: PluginSetupProcessor; + pluginRepo: PluginRepo; + pluginSetup: MultisigSetup; + pluginSetupRefLatestBuild: PluginSetupProcessorStructs.PluginSetupRefStruct; +}; + +async function fixture(): Promise { + // Deploy all contracts + const tags = ['CreateRepo', 'NewVersion']; + await deployments.fixture(tags); + + const [deployer, alice, bob] = await ethers.getSigners(); + const dummyMetadata = ethers.utils.hexlify( + ethers.utils.toUtf8Bytes('0x123456789') + ); + const dao = await createDaoProxy(deployer, dummyMetadata); + + const network = getNetworkNameByAlias(productionNetworkName); + if (network === null) { + throw new UnsupportedNetworkError(productionNetworkName); + } + const networkDeployments = getLatestNetworkDeployment(network); + if (networkDeployments === null) { + throw `Deployments are not available on network ${network}.`; + } + + // Get the `PluginSetupProcessor` from the network + const psp = PluginSetupProcessor__factory.connect( + networkDeployments.PluginSetupProcessor.address, + deployer + ); + + // Get the deployed `PluginRepo` + const {pluginRepo, ensDomain} = await findPluginRepo(env); + if (pluginRepo === null) { + throw `PluginRepo '${ensDomain}' does not exist yet.`; + } + + const release = 1; + const pluginSetup = MultisigSetup__factory.connect( + (await pluginRepo['getLatestVersion(uint8)'](release)).pluginSetup, + deployer + ); + + const defaultInitData = { + members: [alice.address], + settings: { + onlyListed: true, + minApprovals: 1, + }, + }; + + const pluginSetupRefLatestBuild = { + versionTag: { + release: VERSION.release, + build: VERSION.build, + }, + pluginSetupRepo: pluginRepo.address, + }; + + return { + deployer, + alice, + bob, + psp, + dao, + defaultInitData, + pluginRepo, + pluginSetup, + pluginSetupRefLatestBuild, + }; +} + describe(`PluginSetup processing on network '${productionNetworkName}'`, function () { it('installs & uninstalls the current build', async () => { - const {alice, bob, deployer, psp, dao, pluginSetupRef} = await loadFixture( - fixture - ); + const {alice, bob, deployer, psp, dao, pluginSetupRefLatestBuild} = + await loadFixture(fixture); // Grant deployer all required permissions await dao @@ -54,9 +140,8 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio .grant(dao.address, psp.address, DAO_PERMISSIONS.ROOT_PERMISSION_ID); // Install the current build. - const initialMembers = [alice.address, bob.address]; - const multisigSettings: MultisigSettings = { + const multisigSettings: Multisig.MultisigSettingsStruct = { onlyListed: true, minApprovals: 2, }; @@ -65,7 +150,7 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio deployer, psp, dao, - pluginSetupRef, + pluginSetupRefLatestBuild, ethers.utils.defaultAbiCoder.encode( getNamedTypesFromMetadata( METADATA.build.pluginSetup.prepareInstallation.inputs @@ -88,7 +173,7 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio psp, dao, plugin, - pluginSetupRef, + pluginSetupRefLatestBuild, ethers.utils.defaultAbiCoder.encode( getNamedTypesFromMetadata( METADATA.build.pluginSetup.prepareUninstallation.inputs @@ -98,76 +183,48 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio [] ); }); - it.skip('updates to the current build', async () => { - expect(false).to.be.true; - }); -}); -type FixtureResult = { - deployer: SignerWithAddress; - alice: SignerWithAddress; - bob: SignerWithAddress; - dao: DAO; - psp: PluginSetupProcessor; - pluginRepo: PluginRepo; - pluginSetup: MultisigSetup; - pluginSetupRef: PluginSetupProcessorStructs.PluginSetupRefStruct; -}; - -async function fixture(): Promise { - // Deploy all contracts - const tags = ['CreateRepo', 'NewVersion']; - await deployments.fixture(tags); - - const [deployer, alice, bob] = await ethers.getSigners(); - const dummyMetadata = ethers.utils.hexlify( - ethers.utils.toUtf8Bytes('0x123456789') - ); - const dao = await createDaoProxy(deployer, dummyMetadata); - - const network = getNetworkNameByAlias(productionNetworkName); - if (network === null) { - throw new UnsupportedNetworkError(productionNetworkName); - } - const networkDeployments = getLatestNetworkDeployment(network); - if (networkDeployments === null) { - throw `Deployments are not available on network ${network}.`; - } - - // Get the `PluginSetupProcessor` from the network - const psp = PluginSetupProcessor__factory.connect( - networkDeployments.PluginSetupProcessor.address, - deployer - ); - - // Get the deployed `PluginRepo` - const {pluginRepo, ensDomain} = await findPluginRepo(env); - if (pluginRepo === null) { - throw `PluginRepo '${ensDomain}' does not exist yet.`; - } + it('updates from build 1 to the current build', async () => { + const { + deployer, + psp, + dao, + defaultInitData, + pluginRepo, + pluginSetupRefLatestBuild, + } = await loadFixture(fixture); - const release = 1; - const pluginSetup = MultisigSetup__factory.connect( - (await pluginRepo['getLatestVersion(uint8)'](release)).pluginSetup, - deployer - ); + await updateFromBuildTest( + dao, + deployer, + psp, + pluginRepo, + pluginSetupRefLatestBuild, + 1, + [defaultInitData.members, Object.values(defaultInitData.settings)], + [] + ); + }); - const pluginSetupRef = { - versionTag: { - release: VERSION.release, - build: VERSION.build, - }, - pluginSetupRepo: pluginRepo.address, - }; + it('updates from build 2 to the current build', async () => { + const { + deployer, + psp, + dao, + defaultInitData, + pluginRepo, + pluginSetupRefLatestBuild, + } = await loadFixture(fixture); - return { - deployer, - alice, - bob, - psp, - dao, - pluginRepo, - pluginSetup, - pluginSetupRef, - }; -} + await updateFromBuildTest( + dao, + deployer, + psp, + pluginRepo, + pluginSetupRefLatestBuild, + 2, + [defaultInitData.members, Object.values(defaultInitData.settings)], + [] + ); + }); +}); diff --git a/packages/contracts/test/20_integration-testing/test-helpers.ts b/packages/contracts/test/20_integration-testing/test-helpers.ts index a9bac891..bfbf2046 100644 --- a/packages/contracts/test/20_integration-testing/test-helpers.ts +++ b/packages/contracts/test/20_integration-testing/test-helpers.ts @@ -1,10 +1,18 @@ -import {IPlugin, ProxyFactory__factory} from '../../typechain'; +import {METADATA, VERSION} from '../../plugin-settings'; +import { + IPlugin, + PluginUpgradeableSetup__factory, + ProxyFactory__factory, +} from '../../typechain'; import {ProxyCreatedEvent} from '../../typechain/@aragon/osx-commons-contracts/src/utils/deployment/ProxyFactory'; +import {PluginUUPSUpgradeable__factory} from '../../typechain/factories/@aragon/osx-v1.0.0/core/plugin'; import {hashHelpers} from '../../utils/helpers'; import { DAO_PERMISSIONS, PLUGIN_SETUP_PROCESSOR_PERMISSIONS, + PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS, findEvent, + getNamedTypesFromMetadata, } from '@aragon/osx-commons-sdk'; import { PluginSetupProcessorEvents, @@ -13,6 +21,7 @@ import { DAOStructs, DAO, DAO__factory, + PluginRepo, } from '@aragon/osx-ethers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; @@ -217,6 +226,115 @@ async function checkPermissions( } } +export async function updateFromBuildTest( + dao: DAO, + deployer: SignerWithAddress, + psp: PluginSetupProcessor, + pluginRepo: PluginRepo, + pluginSetupRefLatestBuild: PluginSetupProcessorStructs.PluginSetupRefStruct, + build: number, + installationInputs: any[], + updateInputs: any[] +) { + // Grant deployer all required permissions + await dao + .connect(deployer) + .grant( + psp.address, + deployer.address, + PLUGIN_SETUP_PROCESSOR_PERMISSIONS.APPLY_INSTALLATION_PERMISSION_ID + ); + await dao + .connect(deployer) + .grant( + psp.address, + deployer.address, + PLUGIN_SETUP_PROCESSOR_PERMISSIONS.APPLY_UPDATE_PERMISSION_ID + ); + + await dao + .connect(deployer) + .grant(dao.address, psp.address, DAO_PERMISSIONS.ROOT_PERMISSION_ID); + + // Install build 1. + const pluginSetupRefBuild1 = { + versionTag: { + release: VERSION.release, + build: build, + }, + pluginSetupRepo: pluginRepo.address, + }; + const installationResults = await installPLugin( + deployer, + psp, + dao, + pluginSetupRefBuild1, + ethers.utils.defaultAbiCoder.encode( + getNamedTypesFromMetadata( + METADATA.build.pluginSetup.prepareInstallation.inputs + ), + installationInputs + ) + ); + + // Get the plugin address. + const plugin = PluginUUPSUpgradeable__factory.connect( + installationResults.preparedEvent.args.plugin, + deployer + ); + + // Check that the implementation of the plugin proxy matches the latest build + const implementationBuild1 = await PluginUpgradeableSetup__factory.connect( + ( + await pluginRepo['getVersion((uint8,uint16))']( + pluginSetupRefBuild1.versionTag + ) + ).pluginSetup, + deployer + ).implementation(); + expect(await plugin.implementation()).to.equal(implementationBuild1); + + // Grant the PSP the permission to upgrade the plugin implementation. + await dao + .connect(deployer) + .grant( + plugin.address, + psp.address, + PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID + ); + + // Update build 1 to the latest build + await expect( + updatePlugin( + deployer, + psp, + dao, + plugin, + installationResults.preparedEvent.args.preparedSetupData.helpers, + pluginSetupRefBuild1, + pluginSetupRefLatestBuild, + ethers.utils.defaultAbiCoder.encode( + getNamedTypesFromMetadata( + METADATA.build.pluginSetup.prepareUpdate[1].inputs + ), + updateInputs + ) + ) + ).to.not.be.reverted; + + // Check that the implementation of the plugin proxy matches the latest build + const implementationLatestBuild = + await PluginUpgradeableSetup__factory.connect( + ( + await pluginRepo['getVersion((uint8,uint16))']( + pluginSetupRefLatestBuild.versionTag + ) + ).pluginSetup, + deployer + ).implementation(); + expect(await plugin.implementation()).to.equal(implementationLatestBuild); +} + // TODO Move into OSX commons as part of Task OS-928. export async function createDaoProxy( deployer: SignerWithAddress, diff --git a/packages/contracts/test/30_regression-testing/31_upgradeability.ts b/packages/contracts/test/30_regression-testing/31_upgradeability.ts new file mode 100644 index 00000000..4d57b5c5 --- /dev/null +++ b/packages/contracts/test/30_regression-testing/31_upgradeability.ts @@ -0,0 +1,135 @@ +import {createDaoProxy} from '../20_integration-testing/test-helpers'; +import { + Multisig_V1_1__factory, + Multisig_V1_2__factory, + Multisig__factory, + Multisig, +} from '../test-utils/typechain-versions'; +import { + deployAndUpgradeFromToCheck, + deployAndUpgradeSelfCheck, + getProtocolVersion, +} from '../test-utils/uups-upgradeable'; +import {PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS} from '@aragon/osx-commons-sdk'; +import {DAO} from '@aragon/osx-ethers'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; +import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; +import {expect} from 'chai'; +import {ethers} from 'hardhat'; + +describe('Upgrades', () => { + it('upgrades to a new implementation', async () => { + const {deployer, alice, dao, defaultInitData} = await loadFixture(fixture); + const currentContractFactory = new Multisig__factory(deployer); + + await deployAndUpgradeSelfCheck( + deployer, + alice, + [dao.address, defaultInitData.members, defaultInitData.settings], + 'initialize', + currentContractFactory, + PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + }); + + it('upgrades from v1.1', async () => { + const {deployer, alice, dao, defaultInitData} = await loadFixture(fixture); + const currentContractFactory = new Multisig__factory(deployer); + const legacyContractFactory = new Multisig_V1_1__factory(deployer); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + deployer, + alice, + [dao.address, defaultInitData.members, defaultInitData.settings], + 'initialize', + legacyContractFactory, + currentContractFactory, + PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.not.equal(fromImplementation); // The build did change + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.deep.equal([1, 4, 0]); + }); + + it('from v1.2', async () => { + const {deployer, alice, dao, defaultInitData} = await loadFixture(fixture); + const currentContractFactory = new Multisig__factory(deployer); + const legacyContractFactory = new Multisig_V1_2__factory(deployer); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + deployer, + alice, + [dao.address, defaultInitData.members, defaultInitData.settings], + 'initialize', + legacyContractFactory, + currentContractFactory, + PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.not.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 0, 0]); + expect(toProtocolVersion).to.deep.equal([1, 4, 0]); + }); +}); + +type FixtureResult = { + deployer: SignerWithAddress; + alice: SignerWithAddress; + bob: SignerWithAddress; + carol: SignerWithAddress; + defaultInitData: { + members: string[]; + settings: Multisig.MultisigSettingsStruct; + }; + dao: DAO; +}; + +async function fixture(): Promise { + const [deployer, alice, bob, carol] = await ethers.getSigners(); + + const dummyMetadata = ethers.utils.hexlify( + ethers.utils.toUtf8Bytes('0x123456789') + ); + + const dao = await createDaoProxy(deployer, dummyMetadata); + + // Create an initialized plugin clone + const defaultInitData = { + members: [alice.address, bob.address, carol.address], + settings: { + onlyListed: true, + minApprovals: 2, + }, + }; + + return { + deployer, + alice, + bob, + carol, + dao, + defaultInitData, + }; +} diff --git a/packages/contracts/test/multisig-constants.ts b/packages/contracts/test/multisig-constants.ts index b908bdcf..d9e55641 100644 --- a/packages/contracts/test/multisig-constants.ts +++ b/packages/contracts/test/multisig-constants.ts @@ -15,8 +15,3 @@ export const MULTISIG_INTERFACE = new ethers.utils.Interface([ export const UPDATE_MULTISIG_SETTINGS_PERMISSION_ID = ethers.utils.id( 'UPDATE_MULTISIG_SETTINGS_PERMISSION' ); - -export type MultisigSettings = { - minApprovals: number; - onlyListed: boolean; -}; diff --git a/packages/contracts/test/test-utils/protocol-version.ts b/packages/contracts/test/test-utils/protocol-version.ts deleted file mode 100644 index 5ddbe0c3..00000000 --- a/packages/contracts/test/test-utils/protocol-version.ts +++ /dev/null @@ -1,11 +0,0 @@ -import {version} from '../../package.json'; - -/** - * Returns the NPM version number from the `osx` package.json file - */ -// TODO This will be refactored as part of Task OS-1093. -export function osxContractsVersion(): [number, number, number] { - const trimmedVersion = version.split('-')[0]; - const semver = trimmedVersion.split('.'); - return [Number(semver[0]), Number(semver[1]), Number(semver[2])]; -} diff --git a/packages/contracts/test/test-utils/storage.ts b/packages/contracts/test/test-utils/storage.ts deleted file mode 100644 index 543c0fa1..00000000 --- a/packages/contracts/test/test-utils/storage.ts +++ /dev/null @@ -1,18 +0,0 @@ -import {defaultAbiCoder} from 'ethers/lib/utils'; -import {ethers} from 'hardhat'; - -// See https://eips.ethereum.org/EIPS/eip-1967 -export const ERC1967_IMPLEMENTATION_SLOT = - '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) - -export const OZ_INITIALIZED_SLOT_POSITION = 0; - -export async function readStorage( - contractAddress: string, - location: number | string, - types: string[] -): Promise { - return ethers.provider - .getStorageAt(contractAddress, location) - .then(encoded => defaultAbiCoder.decode(types, encoded)[0]); -} diff --git a/packages/contracts/test/test-utils/typechain-versions.ts b/packages/contracts/test/test-utils/typechain-versions.ts index e9f430bd..34988e62 100644 --- a/packages/contracts/test/test-utils/typechain-versions.ts +++ b/packages/contracts/test/test-utils/typechain-versions.ts @@ -2,8 +2,8 @@ /// The version specified in src is the factory and contract without the version number. /// Import as needed in the test files, and use the correct version of the contract. -export {Multisig__factory as Multisig_V1_0_0__factory} from '../../typechain/factories/@aragon/osx-v1.0.0/plugins/governance/multisig/Multisig__factory'; -export {Multisig__factory as Multisig_V1_3_0__factory} from '../../typechain/factories/@aragon/osx-v1.3.0/plugins/governance/multisig/Multisig__factory'; +export {Multisig__factory as Multisig_V1_1__factory} from '../../typechain/factories/@aragon/osx-v1.0.0/plugins/governance/multisig/Multisig__factory'; +export {Multisig__factory as Multisig_V1_2__factory} from '../../typechain/factories/@aragon/osx-v1.3.0/plugins/governance/multisig/Multisig__factory'; export {Multisig__factory} from '../../typechain/factories/src/Multisig__factory'; export {Multisig} from '../../typechain/src/Multisig'; export {IMultisig} from '../../typechain/src/IMultisig'; diff --git a/packages/contracts/test/test-utils/uups-upgradeable.ts b/packages/contracts/test/test-utils/uups-upgradeable.ts index a3b43293..80183815 100644 --- a/packages/contracts/test/test-utils/uups-upgradeable.ts +++ b/packages/contracts/test/test-utils/uups-upgradeable.ts @@ -1,15 +1,21 @@ -import {readStorage, ERC1967_IMPLEMENTATION_SLOT} from './storage'; import {DAO, PluginRepo} from '@aragon/osx-ethers'; +import {defaultAbiCoder} from '@ethersproject/abi'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {Contract, ContractFactory, errors} from 'ethers'; -import {upgrades} from 'hardhat'; +import {ethers, upgrades} from 'hardhat'; // The protocol version number of contracts not having a `getProtocolVersion()` function because they don't inherit from `ProtocolVersion.sol` yet. export const IMPLICIT_INITIAL_PROTOCOL_VERSION: [number, number, number] = [ 1, 0, 0, ]; +// See https://eips.ethereum.org/EIPS/eip-1967 +export const ERC1967_IMPLEMENTATION_SLOT = + '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; // bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) + +export const OZ_INITIALIZED_SLOT_POSITION = 0; + // Deploys a proxy and a new implementation from the same factory and checks that the upgrade works. export async function deployAndUpgradeSelfCheck( deployer: SignerWithAddress, @@ -70,22 +76,20 @@ export async function deployAndUpgradeSelfCheck( const toImplementation = (await factory.deploy()).address; // Confirm that the two implementations are different - const fromImplementation = await readStorage( - proxy.address, - ERC1967_IMPLEMENTATION_SLOT, - ['address'] - ); + + const fromImplementation = await ethers.provider + .getStorageAt(proxy.address, ERC1967_IMPLEMENTATION_SLOT) + .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); + expect(toImplementation).to.not.equal(fromImplementation); // Upgrade from the old to the new implementation await proxy.connect(upgrader).upgradeTo(toImplementation); // Confirm that the proxy points to the new implementation - const implementationAfterUpgrade = await readStorage( - proxy.address, - ERC1967_IMPLEMENTATION_SLOT, - ['address'] - ); + const implementationAfterUpgrade = await ethers.provider + .getStorageAt(proxy.address, ERC1967_IMPLEMENTATION_SLOT) + .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); expect(implementationAfterUpgrade).to.equal(toImplementation); } @@ -116,11 +120,9 @@ export async function deployAndUpgradeFromToCheck( } ); - const fromImplementation = await readStorage( - proxy.address, - ERC1967_IMPLEMENTATION_SLOT, - ['address'] - ); + const fromImplementation = await ethers.provider + .getStorageAt(proxy.address, ERC1967_IMPLEMENTATION_SLOT) + .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); // Grant the upgrade permission const grantArgs: [string, string, string] = [ @@ -159,11 +161,9 @@ export async function deployAndUpgradeFromToCheck( constructorArgs: [], }); - const toImplementation = await readStorage( - proxy.address, - ERC1967_IMPLEMENTATION_SLOT, - ['address'] - ); + const toImplementation = await ethers.provider + .getStorageAt(proxy.address, ERC1967_IMPLEMENTATION_SLOT) + .then(encoded => defaultAbiCoder.decode(['address'], encoded)[0]); return {proxy, fromImplementation, toImplementation}; }